refactor: split LLM workflow update comment assembly - #2608
Merged
Merged
Conversation
📝 WalkthroughWalkthrough
ChangesComment Assembly Refactor and Notes Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/generate_llm_workflow_update_comment.py`:
- Around line 16-44: The markdown escaping in _escape_markdown is incomplete for
notes and paths, so text can still render as code, headings, or list items.
Update _escape_markdown and the note emission flow in
generate_llm_workflow_update_comment to neutralize backticks and other
block-triggering markdown characters, and make sure the note lines built around
the referenced workflow/comment rendering path are fully escaped before output.
Verify the new “markdown-safe notes/workflow text” handling covers inputs like
backticks, headings, bullets, and numbered lists.
In `@tests/scripts/test_generate_llm_workflow_update_comment.py`:
- Around line 40-52: The test for build_comment(include_label=True) is too loose
and can miss unintended wording or spacing changes. Update
test_build_comment_preserves_output_without_notes in
test_generate_llm_workflow_update_comment.py to assert the entire legacy comment
string exactly, using build_comment as the target behavior, instead of checking
only a few substrings like “Workflow updates required” and “Affected
workflows:”.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 509635cb-66ed-4acd-a2db-8e7e0935034e
📒 Files selected for processing (2)
scripts/generate_llm_workflow_update_comment.pytests/scripts/test_generate_llm_workflow_update_comment.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
stranske/Template(auto-detected)stranske/Fine-Art-Archive(auto-detected)stranske/Ready(auto-detected)stranske/Workflows-Integration-Tests(auto-detected)
Comment on lines
+40
to
+52
| def test_build_comment_preserves_output_without_notes() -> None: | ||
| """Verify backward compatibility: output unchanged when notes not provided.""" | ||
| comment = build_comment(include_label=True) | ||
|
|
||
| # Verify structure is preserved | ||
| lines = comment.split("\n") | ||
| assert lines[0] == "Label: needs-human" | ||
| assert "Workflow updates required" in comment | ||
| assert "Affected workflows:" in comment | ||
| assert "- .github/workflows/agents-auto-pilot.yml" in comment | ||
| assert "- .github/workflows/reusable-agents-verifier.yml" in comment | ||
| # Verify no notes section is added | ||
| assert "Notes:" not in comment |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Assert the full legacy string here.
This test only checks a few substrings, so spacing or wording drift in build_comment(include_label=True) would still pass even though the PR objective is to preserve the pre-refactor comment text unchanged. Compare against the exact expected string instead of partial contains checks.
Suggested tightening
def test_build_comment_preserves_output_without_notes() -> None:
"""Verify backward compatibility: output unchanged when notes not provided."""
comment = build_comment(include_label=True)
-
- # Verify structure is preserved
- lines = comment.split("\n")
- assert lines[0] == "Label: needs-human"
- assert "Workflow updates required" in comment
- assert "Affected workflows:" in comment
- assert "- .github/workflows/agents-auto-pilot.yml" in comment
- assert "- .github/workflows/reusable-agents-verifier.yml" in comment
- # Verify no notes section is added
- assert "Notes:" not in comment
+ expected = "\n".join(
+ [
+ "Label: needs-human",
+ "Workflow updates required in .github/workflows/agents-auto-pilot.yml and "
+ ".github/workflows/reusable-agents-verifier.yml. Add pinned installs "
+ "(`pip install -r tools/requirements-llm.txt` and "
+ "`pip install -r .workflows-lib/tools/requirements-llm.txt` for evaluate/compare), "
+ "add actions/cache@v4 pip cache keyed by requirements hash + Python version, "
+ "and remove any floating `pip install langchain*` lines. Workflow edits require "
+ "agent-high-privilege.",
+ "",
+ "Affected workflows:",
+ "- .github/workflows/agents-auto-pilot.yml",
+ "- .github/workflows/reusable-agents-verifier.yml",
+ ]
+ )
+ assert comment == expected📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_build_comment_preserves_output_without_notes() -> None: | |
| """Verify backward compatibility: output unchanged when notes not provided.""" | |
| comment = build_comment(include_label=True) | |
| # Verify structure is preserved | |
| lines = comment.split("\n") | |
| assert lines[0] == "Label: needs-human" | |
| assert "Workflow updates required" in comment | |
| assert "Affected workflows:" in comment | |
| assert "- .github/workflows/agents-auto-pilot.yml" in comment | |
| assert "- .github/workflows/reusable-agents-verifier.yml" in comment | |
| # Verify no notes section is added | |
| assert "Notes:" not in comment | |
| def test_build_comment_preserves_output_without_notes() -> None: | |
| """Verify backward compatibility: output unchanged when notes not provided.""" | |
| comment = build_comment(include_label=True) | |
| expected = "\n".join( | |
| [ | |
| "Label: needs-human", | |
| "Workflow updates required in .github/workflows/agents-auto-pilot.yml and " | |
| ".github/workflows/reusable-agents-verifier.yml. Add pinned installs " | |
| "(`pip install -r tools/requirements-llm.txt` and " | |
| "`pip install -r .workflows-lib/tools/requirements-llm.txt` for evaluate/compare), " | |
| "add actions/cache@v4 pip cache keyed by requirements hash + Python version, " | |
| "and remove any floating `pip install langchain*` lines. Workflow edits require " | |
| "agent-high-privilege.", | |
| "", | |
| "Affected workflows:", | |
| "- .github/workflows/agents-auto-pilot.yml", | |
| "- .github/workflows/reusable-agents-verifier.yml", | |
| ] | |
| ) | |
| assert comment == expected |
🤖 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 `@tests/scripts/test_generate_llm_workflow_update_comment.py` around lines 40 -
52, The test for build_comment(include_label=True) is too loose and can miss
unintended wording or spacing changes. Update
test_build_comment_preserves_output_without_notes in
test_generate_llm_workflow_update_comment.py to assert the entire legacy comment
string exactly, using build_comment as the target behavior, instead of checking
only a few substrings like “Workflow updates required” and “Affected
workflows:”.
Contributor
Automated Status SummaryHead SHA: 132466e
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeRefactor Tasks
Acceptance criteria
|
- Split build_comment() into small pure helper functions: - _escape_markdown(): Escape markdown-sensitive characters - _build_label_line(): Build optional label line - _build_main_body(): Build main instruction text - _build_workflows_section(): Build affected workflows list - _build_notes_section(): Build optional notes section - Add notes parameter to build_comment() for optional notes - Escape markdown-sensitive characters in workflow paths and notes - Add comprehensive tests for: - Empty and multiline notes - Markdown-sensitive characters in repo/workflow names - All helper functions - Backward compatibility Validation: pytest tests/scripts/test_generate_llm_workflow_update_comment.py -q Result: 13 passed in 0.05s Closes #2605 Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
stranske
force-pushed
the
orchestrator/issue-2605
branch
from
June 28, 2026 02:30
8e96612 to
b743379
Compare
Owner
Author
|
Runner dispatch state for autofix on PR #2608. Do not edit. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2605
Automated Status Summary
Scope
Refactor
scripts/generate_llm_workflow_update_comment.pycomment assembly into small pure-helper paths while preserving output text.Tasks
build_comment()output for required inputs and optional notes.tests/scripts/test_generate_llm_workflow_update_comment.pyfor empty notes, multiline notes, and repository/workflow names containing Markdown-sensitive characters.Acceptance criteria
pytest tests/scripts/test_generate_llm_workflow_update_comment.py -qpasses.Head SHA: b743379
Latest Runs: ✅ success — Gate
Required: gate: ✅ success
Summary by CodeRabbit
New Features
Bug Fixes
Tests