Add post-run review UX renderer - #18
Merged
Merged
Conversation
WalkthroughCollects campaign artifact paths after writers finish, escapes file/path output for Rich-safe console rendering, adds Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Runner (run_campaign)
participant ArtifactPaths as ArtifactPaths (get_campaign_artifact_paths)
participant UI as UI Renderer (render_post_run_review)
participant Console as Console Output
Runner->>ArtifactPaths: get_campaign_artifact_paths(db_path)
activate ArtifactPaths
ArtifactPaths-->>Runner: artifact_list
deactivate ArtifactPaths
Runner->>UI: render_post_run_review(campaign_id, report_ok, artifacts, diagnostics_path, yolo_mode)
activate UI
alt yolo_mode is True
UI->>Console: Print YOLO-active reminder
end
alt artifacts provided
UI->>Console: Render artifact status block (paths escaped)
end
alt report_ok is True
UI->>Console: Print "Next actions"
end
alt diagnostics_path provided
UI->>Console: Print diagnostics retention message (path escaped)
end
deactivate UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/AUDITS/Post-Run-Review-UX/Post-Run-Review-UX-Blast-Radius-Audit.md`:
- Around line 71-85: The out-of-scope file list in
Post-Run-Review-UX-Blast-Radius-Audit.md is stale because this PR modifies
src/artifact_paths.py and src/acpm_planning.py; update the table or wording in
the document to remove or soften the “must not be modified” claim so it reflects
the current bundle (refer to the table rows for `src/artifact_paths.py` and
`src/acpm_planning.py`), ensuring the docs either exclude those files or add a
qualifying note about exceptions/required separate review to avoid the
self-contradiction.
- Around line 269-275: The heading "Appendix A — Read-Only Commands Run During
Audit" is incorrect because the PowerShell line New-Item -ItemType Directory
-Force ... mutates the workspace; either remove that New-Item line from the
read-only list or rename the heading to something like "Commands Run During
Audit" / "Commands (including workspace changes)" so it accurately reflects
behavior; update the Appendix A heading text and any surrounding explanation
mentioning read-only commands and ensure the snippet with Select-String and
New-Item are placed in the correct section and consistent with docs/** guidance
about factual consistency.
In `@docs/Plans/Post-Run-Review-UX/Post-Run-Review-UX-Validation.md`:
- Around line 17-18: The table references the wrong-cased directory
`docs/Audits/...` while the new file lives under `docs/AUDITS/...`; update the
entry in `Post-Run-Review-UX-Validation.md` to use the exact `docs/AUDITS/...`
path (replace
`docs/Audits/Post-Run-Review-UX/Post-Run-Review-UX-Blast-Radius-Audit.md` with
`docs/AUDITS/Post-Run-Review-UX/Post-Run-Review-UX-Blast-Radius-Audit.md`) and
scan the same file for any other `docs/Audits` occurrences to correct them to
`docs/AUDITS` to avoid case-sensitive filesystem mismatches.
In `@src/runner.py`:
- Around line 2853-2870: The current post-run review uses
get_campaign_artifact_paths(effective_campaign_id, _effective_lab_root,
db_path=_eff_db_path) which can return stale artifacts from previous runs;
change the logic to restrict artifacts to those produced in this invocation by
checking the presence and freshness of canonical artifact rows (e.g.,
campaign_summary_md, metadata_json) in the current run before assigning
_artifact_list, or explicitly mark missing/failed canonical artifacts for
effective_campaign_id before calling ui.render_post_run_review; ensure you
reference get_campaign_artifact_paths, effective_campaign_id,
_effective_lab_root/_eff_db_path, _artifact_list and ui.render_post_run_review
so the review UI only receives artifacts verified as created/updated during this
invocation (or includes explicit failure markers) instead of reusing stale
DB/filesystem entries.
🪄 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: a7425782-8ad0-4b91-be8a-c1d6ed96abb5
⛔ Files ignored due to path filters (2)
test_cli_ux_post_run_review.pyis excluded by none and included by nonetest_cli_ux_yolo_review.pyis excluded by none and included by none
📒 Files selected for processing (6)
docs/AUDITS/Post-Run-Review-UX/Post-Run-Review-UX-Blast-Radius-Audit.mddocs/Plans/Post-Run-Review-UX/Post-Run-Review-UX-Validation.mdsrc/acpm_planning.pysrc/artifact_paths.pysrc/runner.pysrc/ui.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runner.py`:
- Around line 2867-2875: The three branches for _art["artifact_type"]
(ARTIFACT_CAMPAIGN_SUMMARY, ARTIFACT_RUN_REPORTS, ARTIFACT_METADATA) repeat
identical logic; instead map artifact types to their corresponding "ok" flags
(report_ok, v2_ok, meta_ok) and compute a single conditional: if the artifact
type is one of those and its mapped ok flag is False then set _art["db_status"]
= "failed" and _art["exists"] = False. Update the code around the checks of
_art["artifact_type"] to use this map/lookup so the duplicated branches are
consolidated into one place.
🪄 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: aeda5107-e599-49b3-8a3b-8dd7957b1111
⛔ Files ignored due to path filters (1)
test_cli_ux_post_run_review.pyis excluded by none and included by none
📒 Files selected for processing (3)
docs/AUDITS/Post-Run-Review-UX/Post-Run-Review-UX-Blast-Radius-Audit.mddocs/Plans/Post-Run-Review-UX/Post-Run-Review-UX-Validation.mdsrc/runner.py
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/Plans/Post-Run-Review-UX/Post-Run-Review-UX-Validation.md (1)
1-109: 🧹 Nitpick | 🔵 TrivialDocumentation is factually consistent with the source implementation.
The validation note accurately describes:
- The
get_campaign_artifact_pathsimport and read-only discovery call (lines 29-36)- The artifact status override consolidation (lines 71-72)
- The call sequence (artifact writers → discovery → render)
Minor formatting note: The code block at line 78 should specify a language (e.g.,
bashorshell) for consistent rendering. This is a nitpick flagged by markdownlint.🔧 Optional fix for code block language
-``` +```bash # Targeted lint🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/Plans/Post-Run-Review-UX/Post-Run-Review-UX-Validation.md` around lines 1 - 109, The fenced code block under "Validation Commands and Results" (the block that begins with "# Targeted lint") is missing a language tag; update that fenced code block to use a language specifier (e.g., add "bash" so the fence becomes ```bash) to satisfy markdownlint and ensure consistent rendering in render_post_run_review / docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/Plans/Post-Run-Review-UX/Post-Run-Review-UX-Validation.md`:
- Around line 1-109: The fenced code block under "Validation Commands and
Results" (the block that begins with "# Targeted lint") is missing a language
tag; update that fenced code block to use a language specifier (e.g., add "bash"
so the fence becomes ```bash) to satisfy markdownlint and ensure consistent
rendering in render_post_run_review / docs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4f49aa13-fa58-4d1b-a584-a3f8b7b8ea32
📒 Files selected for processing (2)
docs/Plans/Post-Run-Review-UX/Post-Run-Review-UX-Validation.mdsrc/runner.py
This was referenced Apr 26, 2026
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
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.
Summary
Validation
Notes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores