Conversation
Automated sync from stranske/Workflows Template hash: d0d4ad2bdd25 Changes synced from sync-manifest.yml
📝 WalkthroughWalkthroughUpdates the design system CSS with font-token corrections and accessibility rules (focus-visible, prefers-reduced-motion), hardens Streamlit UI helpers with HTML escaping and case-insensitive error translation, adds a ChangesDesign System Updates
CI Workflow SHA Pin Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Workflow state fingerprint for Agents Gate Followups. Do not edit. |
|
Workflow state fingerprint for Keepalive Loop Reporter. Do not edit. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@design-system/ds_streamlit.py`:
- Around line 115-121: The docstring for the `action` parameter indicates it
accepts markdown, but the implementation HTML-escapes the action parameter
before inserting it into the div element, which prevents markdown syntax from
being parsed. Update the docstring to accurately reflect that the `action`
parameter accepts literal text only (not markdown), or if markdown support is
intended, remove the escape call on the action parameter and add a comment
explaining any security considerations or trade-offs associated with this
change. Ensure the documentation matches the actual behavior that callers will
experience.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b7673469-50a7-457f-9fb7-7ae70ac718af
📒 Files selected for processing (6)
.github/workflows/agents-guard.ymldesign-system/PRESENTATION_PATTERNS.mddesign-system/README.mddesign-system/components.cssdesign-system/ds_streamlit.pydesign-system/tokens.css
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
stranske/Workflows(auto-detected)stranske/Template(auto-detected)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
startup_failurein GitHub Actions workflows with zero jobs indicates GitHub couldn't parse the workflow; check for invalid YAML syntax, conflictingpermissions:blocks onworkflow_callreusable workflows, invalid permission scopes, or circular workflow references
Files:
.github/workflows/agents-guard.yml
.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Reference reusable workflows with
@mainby default unless intentionally pinning to an exact commit SHA for a documented reason
Files:
.github/workflows/agents-guard.yml
{.github/workflows/agents-*.yml,.github/workflows/autofix.yml,.github/codex/**,docs/**}
📄 CodeRabbit inference engine (CLAUDE.md)
Synced workflows, prompts, scripts, and consumer docs are managed through .github/sync-manifest.yml in Workflows and should not be edited locally
Files:
.github/workflows/agents-guard.yml
**/.github/workflows/*.yml
📄 CodeRabbit inference engine (AGENTS.md)
Reference reusable workflows with
@mainunless intentionally pinning to an exact commit SHA for a controlled reason.
Files:
.github/workflows/agents-guard.yml
{**/.github/workflows/agents-*.yml,.github/workflows/autofix.yml,.github/codex/**,docs/ops/CONSUMER_REPO_MAINTENANCE.md,docs/keepalive/Agents.md}
📄 CodeRabbit inference engine (AGENTS.md)
Synced workflows, prompts, scripts, and consumer docs (including
agents-*.yml,autofix.yml,.github/codex/prompts, and synced scripts) are managed through.github/sync-manifest.ymlin Workflows and should not be edited locally; fix issues in Workflows instead.
Files:
.github/workflows/agents-guard.yml
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For Manager-Database repository: use Prefect 2.x and import schedules from
prefect.client.schemas.schedules
Files:
design-system/ds_streamlit.py
🪛 LanguageTool
design-system/README.md
[uncategorized] ~50-~50: The official name of this software platform is spelled with a capital “H”.
Context: ...m/and distributed by Maint 68 through.github/sync-manifest.yml`. Update it here firs...
(GITHUB)
🔀 Multi-repo context stranske/Workflows
Perfect! Now I have a complete picture. Let me document my findings:
Linked repositories findings
Based on my exploration of stranske/Workflows and stranske/Template, I've identified critical breaking changes in the synced ds_streamlit.py module that will affect downstream consumer repositories.
Breaking API Changes in ds_streamlit.py
[::stranske/Workflows::]
1. availability_badge(label) return value — BREAKING CHANGE
- Old behavior (stranske/Template
design-system/ds_streamlit.py): Returns HTML markup<span class='ds-badge'>{label}</span> - New behavior (synced version): Returns plain string
· {str(label).strip()} - Impact: Any Streamlit app using
availability_badge()in tab titles will now display plain text with a bullet separator instead of styled HTML badge. Visual presentation changes from styled badge to plain marker.
2. humanize_id(raw, mapping) ID parsing logic — BREAKING CHANGE
- Old behavior (stranske/Template): Takes the FIRST colon-separated segment:
split(":")[0] - New behavior (synced version): Takes the LAST colon-separated segment:
split(":")[-1] - Impact: IDs with colons (e.g.,
"prefix:item:id123") will now be decoded to"id123"instead of"prefix". This changes the human-readable label displayed to users. Any app relying on the old behavior will show incorrect decoded labels.
3. Security hardening in empty_state() — NON-BREAKING
icon,title,descparameters are now HTML-escaped before interpolation into markdown- File:
./templates/consumer-repo/design-system/ds_streamlit.pylines ~75-85 - Impact: Strengthens XSS protection but should not break existing calls unless code was intentionally passing raw HTML
4. Security hardening in notice() — NON-BREAKING
title,body,actionparameters are now HTML-escaped- File:
./templates/consumer-repo/design-system/ds_streamlit.pylines ~100-108 - Impact: Strengthens XSS protection
5. translate_error() matching logic — BEHAVIOR IMPROVEMENT
- Changed from case-sensitive substring matching (
"financing_mode" in text,"No investable funds") to case-insensitive matching usingtext_lowervariable - File:
./templates/consumer-repo/design-system/ds_streamlit.pylines ~125-140 - Impact: More robust error pattern matching; existing error handling may catch more cases
6. New function added: diagnostics_expander()
- Added contextmanager for P4 (diagnostics) pattern
- File:
./templates/consumer-repo/design-system/ds_streamlit.pylines ~162-167 - Impact: Non-breaking addition; purely optional enhancement
Documentation Alignment
[::stranske/Workflows::]
The PRESENTATION_PATTERNS.md confirms these functions are part of the design-system rollout strategy:
- P5 (availability_badge): "Streamlit:
ds_streamlit.availability_badge(label)in the tab title / disabled control caption" - P6 (humanize_id): "decode internal IDs...into human-readable labels before display"
The patterns are documented as standards that will be "synced fleet-wide" to consumer repositories via .github/sync-manifest.yml.
Key Review Considerations
-
Downstream impact: The change affects all consumer repositories that have been or will be synced from this template. Any Streamlit apps using
availability_badge()orhumanize_id()will experience behavior changes. -
No current internal usages found: Neither
stranske/Templatenorstranske/Workflows/templates/consumer-reposhow active usages of these functions outside their definitions, suggesting they are forward-looking API additions for new consumer implementations. -
Design intent: The changes align with the stated goal of "hardening synced design system helpers" (commit message:
dfe0854 fix: harden synced design system helpers (#2511)), prioritizing security and correctness of ID parsing.
🔇 Additional comments (7)
design-system/tokens.css (1)
29-29: LGTM!Also applies to: 74-74
design-system/components.css (1)
41-41: LGTM!Also applies to: 97-99
design-system/ds_streamlit.py (2)
29-30: LGTM!Also applies to: 93-98, 137-149, 165-171
174-185: 🎯 Functional CorrectnessThis review comment should be removed. The
availability_badge()andhumanize_id()functions are newly synced helpers that Fine-Art-Archive does not call or depend on, so downstream compatibility concerns do not apply to this repository. These functions were added in the initial template sync (commit c95f3cb) and have zero call sites in the codebase.> Likely an incorrect or invalid review comment.design-system/README.md (1)
1-1: LGTM!Also applies to: 41-41, 50-50
design-system/PRESENTATION_PATTERNS.md (1)
17-24: LGTM!Also applies to: 33-33, 43-51, 60-60
.github/workflows/agents-guard.yml (1)
114-114: LGTM!Also applies to: 183-183
| head = f"<strong>{escape(str(title))}</strong><br>" if title else "" | ||
| act = f"<div style='margin-top:6px'>{escape(str(action))}</div>" if action else "" | ||
| safe_body = escape(str(body)) | ||
| st.markdown( | ||
| f"<div class='ds-notice' style='background:{bg};border-color:{color}33'>" | ||
| f"<span class='ic' style='color:{color}'>{ic}</span>" | ||
| f"<div>{head}{body}{act}</div></div>", | ||
| f"<div>{head}{safe_body}{act}</div></div>", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate notice() usages and nearby arguments.
rg -nP --type=py -C3 '\bnotice\s*\('
rg -nP --type=py -C3 '\baction\s*='Repository: stranske/Fine-Art-Archive
Length of output: 163
🌐 Web query:
In Streamlit, when using st.markdown(..., unsafe_allow_html=True), is Markdown syntax inside an injected HTML <div> rendered as Markdown (e.g., links), or displayed literally?
💡 Result:
In Streamlit, when using st.markdown(..., unsafe_allow_html=True), Markdown syntax inside an HTML
Citations:
- 1: st.markdown links not parsed correctly when content enclosed in div streamlit/streamlit#11668
- 2: Fix #11668: parse links inside div in st.markdown streamlit/streamlit#14462
- 3: Rendering of HTML tables in Markdown is whitespace-sensitive streamlit/streamlit#9312
- 4: https://docs.streamlit.io/develop/api-reference/text/st.markdown
- 5: https://github.com/streamlit/agent-skills/blob/main/developing-with-streamlit/skills/using-streamlit-markdown/SKILL.md
- 6: https://discuss.streamlit.io/t/how-to-add-a-div-in-streamlit-without-closing-it-at-the-same-time/80932
- 7: https://discuss.streamlit.io/t/can-i-wrap-st-info-or-other-elements-inside-custom-html/618
🏁 Script executed:
#!/bin/bash
# First, locate and read the notice() function definition
fd -e py "ds_streamlit" | head -5Repository: stranske/Fine-Art-Archive
Length of output: 193
🏁 Script executed:
#!/bin/bash
# Read the notice function with context around line 111-121
sed -n '100,130p' design-system/ds_streamlit.pyRepository: stranske/Fine-Art-Archive
Length of output: 1397
🏁 Script executed on selected repositories:
#!/bin/bash
# Search for notice() usages in related repos
rg -nP --type=py '\bnotice\s*\(' -A2 -B2 || true
Repositories: stranske/Workflows, stranske/Template
Length of output: 416
🏁 Script executed:
#!/bin/bash
# Check if there's any recent commit history or changes context
git log --oneline -n 20 design-system/ds_streamlit.py 2>/dev/null || echo "git log unavailable"Repository: stranske/Fine-Art-Archive
Length of output: 222
🏁 Script executed:
#!/bin/bash
# Check the full notice() function including docstring to confirm markdown claim
sed -n '111,122p' design-system/ds_streamlit.pyRepository: stranske/Fine-Art-Archive
Length of output: 771
Docstring-implementation mismatch: action parameter escapes HTML but is documented as markdown.
Line 111's docstring states action is "optional remediation (markdown)", but line 116 HTML-escapes the action parameter before rendering inside a <div>. In Streamlit, Markdown syntax inside HTML block elements is treated as literal text rather than parsed, so callers cannot pass markdown-formatted remediation text (e.g., links like [click here](url)) as the docstring implies.
Either update the docstring to document that action accepts literal text only, or clarify the intentional security-first tradeoff if the escape is non-negotiable. This change appears intentional for XSS hardening, but the contract with callers needs alignment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design-system/ds_streamlit.py` around lines 115 - 121, The docstring for the
`action` parameter indicates it accepts markdown, but the implementation
HTML-escapes the action parameter before inserting it into the div element,
which prevents markdown syntax from being parsed. Update the docstring to
accurately reflect that the `action` parameter accepts literal text only (not
markdown), or if markdown support is intended, remove the escape call on the
action parameter and add a comment explaining any security considerations or
trade-offs associated with this change. Ensure the documentation matches the
actual behavior that callers will experience.
|
Closing as superseded by newer sync PR #133 from the latest Workflows sync wave. |
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Source SHA:
dfe0854ae9b1ba1c616e4b57fb498f283ea3216fTemplate hash:
d0d4ad2bdd25Sync branch:
sync/workflows-d0d4ad2bdd25Consumer repo:
stranske/Fine-Art-ArchiveManifest:
.github/sync-manifest.ymlSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Style