chore(agents): rework auditor/architect prompts and drop impact-reviewer subagent - #93
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR removes the ChangesAgent and workflow consolidation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Auditor
participant Repository
participant FindingsStore
Auditor->>Repository: Establish scope and changed-file manifest
Auditor->>Repository: Inspect files, callers, duplication, and superseded code
Auditor->>FindingsStore: Reconcile and persist verified findings
FindingsStore-->>Auditor: Return finding state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Uh oh!
There was an error while loading. Please reload this page.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@src/prompts/agents/architect.md`:
- Around line 15-16: Resolve the contract mismatch between the architect
prompt’s conclusion in “Conclude” and the Loop-mode behavior documented in
README.md. Either update README.md to state that the architect never invokes
execute-plan and execution is user-directed, or revise the prompt and its
related test to preserve Loop-mode dispatch; keep the documentation, prompt, and
test consistent.
In `@src/prompts/agents/auditor.md`:
- Around line 73-91: Update the “Verification” guidance in the auditor prompt to
explicitly prioritize the final-audit requirement from
auditor-final-audit-addendum over the generic rule allowing reliable earlier
verification evidence to skip checks. Require every top-level verification
command to run against the final state unless an explicit technical reason
prevents it.
- Around line 5-10: Update the review-scope instructions so every invocation
mode runs git status --short and reads the contents of each untracked file,
including commit-hash, branch, and PR modes. Ensure untracked files are included
in both the generated manifest and whole-change/final-audit analysis, while
preserving the existing tracked-change commands and unknown-base handling.
- Line 25: Add crossSection to the documented review-write arguments in the
prompt contract, alongside sectionIndex, so it matches the supported inputs and
sectioned-loop behavior implemented by review-write.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 37d46558-8328-46fa-8a06-9e9c202540c0
📒 Files selected for processing (11)
README.mdsrc/agents/impact-reviewer.tssrc/agents/index.tssrc/agents/types.tssrc/prompts/agents/architect-auto.mdsrc/prompts/agents/architect.mdsrc/prompts/agents/auditor-final-audit-addendum.mdsrc/prompts/agents/auditor-loop-addendum.mdsrc/prompts/agents/auditor.mdsrc/prompts/agents/impact-reviewer.mdtest/agents.test.ts
💤 Files with no reviewable changes (3)
- src/prompts/agents/impact-reviewer.md
- src/agents/impact-reviewer.ts
- src/agents/index.ts
Comment on lines
15
to
16
| 5. **Conclude** — Only after the stored plan is complete and warning-free, end by summarizing the plan in chat — the intention, goal, approach, and key findings; do not emit the full plan. Stop there: do not call `execute-plan`, do not call the `question` tool, and do not ask how to launch. The user decides whether and how to execute. | ||
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align the architect execution contract with the documentation.
Line 15 forbids the architect from calling execute-plan and defers execution to the user. README.md Line 133 still says that Loop mode is an exception where the interactive architect invokes execute-plan. These contracts conflict.
Update README.md to describe the new dispatch path, or change the prompt and its test to preserve the documented Loop behavior.
🤖 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 `@src/prompts/agents/architect.md` around lines 15 - 16, Resolve the contract
mismatch between the architect prompt’s conclusion in “Conclude” and the
Loop-mode behavior documented in README.md. Either update README.md to state
that the architect never invokes execute-plan and execution is user-directed, or
revise the prompt and its related test to preserve Loop-mode dispatch; keep the
documentation, prompt, and test consistent.
Comment on lines
+5
to
+10
| Determine the review scope from the invocation input: | ||
| 1. **Uncommitted changes**: Run `git diff` for unstaged, `git diff --cached` for staged, `git status --short` for untracked files | ||
| 2. **Commit hash**: Run `git show <hash>` | ||
| 3. **Branch name**: Run `git diff <branch>...HEAD` | ||
| 4. **PR URL or number**: Run `gh pr view <input>` and `gh pr diff <input>` | ||
| 1. **Uncommitted changes**: `git diff` for unstaged tracked changes, `git diff --cached` for staged changes, and `git status --short` to list untracked files. Read every untracked file because `git diff` omits them. | ||
| 2. **Commit hash**: `git show <hash>`. | ||
| 3. **Branch**: `git merge-base <base-ref> HEAD` to obtain the merge base, then `git diff <merge-base>` — this covers all committed, staged, and unstaged tracked changes since the branch diverged. Also run `git status --short` and read every untracked file. If the base ref is missing or unknown, do not guess; report that the review scope cannot be established. | ||
| 4. **PR URL or number**: `gh pr view <input>` and `gh pr diff <input>`. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enumerate and read untracked files for every invocation mode.
The uncommitted path lists untracked files but does not say to read them. The commit and PR paths do not run git status --short or read untracked files. src/prompts/agents/auditor-final-audit-addendum.md Line 8 requires the final audit to cover every tracked and untracked worktree change.
A new untracked file can therefore be absent from the manifest and from whole-change analysis. Add one common status-and-read step to all scope modes, or restrict final audits to scopes that include the complete worktree.
🤖 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 `@src/prompts/agents/auditor.md` around lines 5 - 10, Update the review-scope
instructions so every invocation mode runs git status --short and reads the
contents of each untracked file, including commit-hash, branch, and PR modes.
Ensure untracked files are included in both the generated manifest and
whole-change/final-audit analysis, while preserving the existing tracked-change
commands and unknown-base handling.
| 6. **Persist**: Store each new **bug** and **warning** with `review-write`. Do NOT store suggestions. Do not re-store resolved findings. | ||
| When reporting, include any still-open previous findings under a "### Previously Identified Issues" heading before presenting new findings. | ||
| Use `review-write` with: `file`, `line`, `severity` ("bug" or "warning"), `description`, `scenario`, and `status` ("open" by default). Put the required fix, acceptance criterion, and narrow verification in `description`; they are not separate tool arguments. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
rg -n -C 4 '\breview-write\b|\bsectionIndex\b|\bcrossSection\b' src testRepository: chriswritescode-dev/opencode-forge
Length of output: 50391
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'--- src/prompts/agents/auditor.md relevant lines ---\n'
sed -n '1,50p' src/prompts/agents/auditor.md
printf'\n--- src/tools/review.ts review-write args / execute ---\n'
sed -n '35,95p' src/tools/review.ts
printf'\n--- auditor addendum lines ---\n'forfin src/prompts/agents/auditor-loop-addendum.md src/prompts/agents/auditor-final-audit-addendum;doif [ -f"$f" ];thenprintf'\n### %s\n'"$f"
sed -n '15,35p'"$f"fidoneRepository: chriswritescode-dev/opencode-forge
Length of output: 263
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'%s\n''--- src/prompts/agents/auditor.md relevant lines ---'
sed -n '1,50p' src/prompts/agents/auditor.md
printf'%s\n'''printf'%s\n''--- src/tools/review.ts review-write args / execute ---'
sed -n '35,95p' src/tools/review.ts
printf'%s\n'''printf'%s\n''--- auditor addendum lines ---'
sed -n '15,35p' src/prompts/agents/auditor-loop-addendum.md
sed -n '15,35p' src/prompts/agents/auditor-final-audit-addendum.mdRepository: chriswritescode-dev/opencode-forge
Length of output: 12587
Add crossSection to the review-write contract.
review-write supports sectionIndex and crossSection and defaults sectioned loop findings to the current section, so those addendum usages are valid. This line still omits crossSection, which makes the prompt contract disagree with src/tools/review.ts.
🤖 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 `@src/prompts/agents/auditor.md` at line 25, Add crossSection to the documented
review-write arguments in the prompt contract, alongside sectionIndex, so it
matches the supported inputs and sectioned-loop behavior implemented by
review-write.
Comment on lines
73
to
91
| ## Before You Flag Something | ||
| Be certain. If you're going to call something a bug, you need to be confident it actually is one. | ||
| - Focus your review on the changes and code directly related to them | ||
| - If you discover a bug in pre-existing code that affects the correctness of the current changes, report it — do not dismiss it as "out of scope" | ||
| - Don't flag something as a bug if you're unsure — investigate first | ||
| - Don't invent hypothetical problems — if an edge case matters, explain the realistic scenario where it breaks | ||
| - Don't be a zealot about style: verify the code is actually in violation before flagging; some "violations" are acceptable when they're the simplest option; don't flag style preferences unless they clearly violate established project conventions | ||
| - Focus on the changes and code directly related to them. | ||
| - If you discover a bug in pre-existing code that affects the correctness of the current changes, report it — do not dismiss it as "out of scope". | ||
| - Don't flag something as a bug if you're unsure — investigate first. | ||
| - Don't invent hypothetical problems — if an edge case matters, explain the realistic scenario where it breaks. | ||
| - Verify the code is actually in violation before flagging style; some "violations" are acceptable when they're the simplest option. Don't flag style preferences unless they clearly violate established project conventions. | ||
| - If you can't verify something, say "I'm not sure about X" rather than flagging it as a definite issue. | ||
| If you're uncertain about something and can't verify it, say "I'm not sure about X" rather than flagging it as a definite issue. | ||
| ## Verification | ||
| ## Tool Usage | ||
| Run the narrowest relevant validation — not an unconditional full typecheck. | ||
| 1. Determine the repository- or plan-mandated checks (package.json scripts, Makefile, pyproject.toml, or other build config). If the plan lists verification commands, prefer those. | ||
| 2. Run them. When reliable evidence proves a check already passed (e.g., documented coder verification notes), do not re-run redundant full checks. | ||
| 3. Report validation failures only when they are caused by or affect the reviewed change. Failures confined to unrelated files go under Observations, not as blocking findings. | ||
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exclude final integration audits from the generic check-skip rule.
This section permits skipping a full check when reliable earlier evidence shows that it passed. src/prompts/agents/auditor-final-audit-addendum.md Line 12 requires every top-level verification command to run against the final state, except for an explicit technical reason.
Add an explicit final-audit exception or precedence rule. Otherwise an auditor can skip a required final check because coder notes show an earlier pass.
🤖 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 `@src/prompts/agents/auditor.md` around lines 73 - 91, Update the
“Verification” guidance in the auditor prompt to explicitly prioritize the
final-audit requirement from auditor-final-audit-addendum over the generic rule
allowing reliable earlier verification evidence to skip checks. Require every
top-level verification command to run against the final state unless an explicit
technical reason prevents it.
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 freeto 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.
Summary
Removes the dedicated
impact-reviewersubagent and folds whole-change impact analysis directly into the auditor-loop audit workflow. Auditor and architect agent prompts are tightened around the minimal-design ladder and single-source-of-truth planning.Impact-reviewer removal
src/agents/impact-reviewer.tsandsrc/prompts/agents/impact-reviewer.mddeleted; registration andAgentRoleupdated insrc/agents/index.tsandsrc/agents/types.ts.README.mdagents table updated (7 → 6 bundled agents).Auditor prompt rework
review-writefields documented; resolved findings deleted immediately.Architect prompt rework
Tests
test/agents.test.tsupdated: 6-agent count, impact-reviewer tests removed, new auditor-loop impact-analysis assertions.Validation
pnpm build,pnpm typecheck,pnpm lintclean;test/agents.test.ts(33 tests) passes.Summary by CodeRabbit
Improvements
Changes