fix: reviewer agent must verify findings against actual code#37
Merged
fix: reviewer agent must verify findings against actual code#37
Conversation
The reviewer agent and /develop Phase 4 were producing unvalidated
findings — stating issues without reading the code, using "ensure"
and "consider" language that assumes problems exist without checking.
Changes:
- reviewer.md: Add verification protocol requiring file:line citations
and quoted code for every finding. Explicitly ban unverified language
("might", "ensure", "consider"). Add "Verified Clean" section.
- develop.md: Phase 4 now requires re-reading each changed file,
grep-based checks instead of assumptions, and dropping any finding
that can't cite a specific line.
📝 WalkthroughWalkthroughThe changes tighten code review and development processes by enforcing verification-driven practices. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
commands/develop.md (1)
73-75: Align Phase 4 verification rules with post-edit automation output.Line 73-Line 75 require strictly verified reporting, but
scripts/post-edit-check.jsstill emits heuristic findings and advisory wording (Consider: ...), which conflicts with this policy and can cause unverified findings to leak into reviews.Suggested wording adjustment in this section
-3. **Grep for problems** — run `grep` for console.log, TODO, hardcoded secrets, debug statements. Report only what grep finds. -4. **Never report unverified findings** — don't say "ensure X" or "consider Y". Either it's a confirmed problem with a file:line citation, or it's not worth reporting. +3. **Grep for problems** — run `grep` for console.log, TODO, hardcoded secrets, debug statements. Treat matches as leads, then verify in-file before reporting. +4. **Never report unverified findings** — automated checks are advisory only. Report an issue only after confirming it in code with a file:line citation and quoted line.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commands/develop.md` around lines 73 - 75, The develop.md Phase 4 rule requires only strictly verified findings, but scripts/post-edit-check.js currently emits heuristic/advisory messages (e.g., lines prefixed with "Consider: ...") that can leak unverified guidance into reviews; update scripts/post-edit-check.js to stop emitting advisory phrasing and instead output only confirmed findings with precise file:line citations (or add a strict verification mode flag the script honors), remove or reword any "Consider:" tokens, and ensure the report generator/formatter in that script produces the exact verified-only wording required by develop.md.agents/reviewer.md (1)
15-82: Sync this new reviewer contract with public docs to avoid conflicting instructions.This file now mandates evidence-based
file:line+ quoted findings, butskills/pro-workflow/SKILL.md(Line 372-Line 427) andREADME.md(Line 178) still describe the older checklist/severity model. Please update those docs in the same PR (or immediately after) so users don’t follow contradictory reviewer behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agents/reviewer.md` around lines 15 - 82, The reviewer contract in "agents/reviewer.md" introduced an evidence-first verification protocol but the other docs still describe the older checklist/severity model; update the other docs (SKILL.md and README.md) to match the new "Verification Protocol" wording and behavior by replacing the legacy checklist/severity guidance (the block describing checklist/severity model) with the new rules: require quoting exact code, file:line evidence, and the prohibition on speculative language, and ensure examples and output format sections reflect the new evidence-based format and wording used in "Verification Protocol" so all three documents are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agents/reviewer.md`:
- Line 55: The fenced code block opener in agents/reviewer.md uses "```" with no
language tag which triggers MD040; update the block opener to include a language
identifier (for example change "```" to "```text" or "```markdown") so the
fenced code block has a language (e.g., modify the "```" token surrounding the
review snippet to "```text").
---
Nitpick comments:
In `@agents/reviewer.md`:
- Around line 15-82: The reviewer contract in "agents/reviewer.md" introduced an
evidence-first verification protocol but the other docs still describe the older
checklist/severity model; update the other docs (SKILL.md and README.md) to
match the new "Verification Protocol" wording and behavior by replacing the
legacy checklist/severity guidance (the block describing checklist/severity
model) with the new rules: require quoting exact code, file:line evidence, and
the prohibition on speculative language, and ensure examples and output format
sections reflect the new evidence-based format and wording used in "Verification
Protocol" so all three documents are consistent.
In `@commands/develop.md`:
- Around line 73-75: The develop.md Phase 4 rule requires only strictly verified
findings, but scripts/post-edit-check.js currently emits heuristic/advisory
messages (e.g., lines prefixed with "Consider: ...") that can leak unverified
guidance into reviews; update scripts/post-edit-check.js to stop emitting
advisory phrasing and instead output only confirmed findings with precise
file:line citations (or add a strict verification mode flag the script honors),
remove or reword any "Consider:" tokens, and ensure the report
generator/formatter in that script produces the exact verified-only wording
required by develop.md.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13d84399-8696-45d4-ada5-1c86e41647c1
📒 Files selected for processing (2)
agents/reviewer.mdcommands/develop.md
|
|
||
| ## Output Format | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced code block.
Line 55 opens a fenced block without a language, which triggers MD040. Use a language tag (e.g., text) for lint compliance.
Proposed fix
-```
+```text
## Review: [Files/PR]
...</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 55-55: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agents/reviewer.md` at line 55, The fenced code block opener in
agents/reviewer.md uses "```" with no language tag which triggers MD040; update
the block opener to include a language identifier (for example change "```" to
"```text" or "```markdown") so the fenced code block has a language (e.g.,
modify the "```" token surrounding the review snippet to "```text").
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.
Summary
/developPhase 4 requires grep-based checks and drops unverified findingsProblem
When
/developis given a code review task, it provides unvalidated findings. The reviewer assumes issues exist without reading the code, using vague language like "ensure X handles Y" or "consider adding validation" without checking whether validation already exists.Fix
reviewer.md:
develop.md Phase 4:
Test plan
/developwith a code review task and verify all findings cite file:lineSummary by CodeRabbit