feat: add review-sweep skill - #10
Conversation
Fans out finders by failure dimension, refutes every finding with two skeptics, and files the survivors as a parent issue plus one sub-issue per defect for /review-crunch to fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9kGPJFFrUM9XSDJQKmbQU
📝 WalkthroughWalkthroughAdds a user-invocable ChangesReview sweep procedure
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ReviewSweep
participant FinderAgents
participant SkepticAgents
participant IssueTracker
ReviewSweep->>FinderAgents: assign review dimensions
FinderAgents->>ReviewSweep: return evidence-backed findings
ReviewSweep->>SkepticAgents: request refutation verdicts
SkepticAgents->>ReviewSweep: return survivor or refuted status
ReviewSweep->>IssueTracker: file parent and defect issues
ReviewSweep->>IssueTracker: comment the review handoff
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 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 `@skills/review-sweep/SKILL.md`:
- Around line 37-38: Update the “Done when” condition in the review-sweep
instructions to require a completely clean working tree, including untracked
files, before Step 1 proceeds. Use git status --porcelain as the authoritative
check, or explicitly stage/commit all session-owned files; do not rely on git
status showing no tracked changes or git diff --stat.
- Around line 92-94: Clarify the unjudged-finding workflow so self-adjudication
cannot promote findings into survivors without satisfying the two-skeptic gate.
Require rerunning both skeptic seats before promotion, or explicitly keep
unjudged findings excluded from survived/refuted counts while reporting them
separately in the parent.
- Around line 214-224: Update Step 6 to define the rejection path for the fix
skeptic: keep the data-safety ticket open, revert or amend the rejected fix,
then rerun verification and skeptic review. Only instruct the owner to comment
the fixing commit and mutation verdict and close the ticket after the fix is
accepted and the bar passes on the handed-over tree.
- Around line 76-78: Update the retry branch in the parallel mapping around
refutePrompt and agent so the missing skeptic retains its original index i; use
i rather than i + 1 for the seat argument, while keeping the retry suffix only
in the agent label metadata.
🪄 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
Run ID: 3828ed37-60b1-4acb-8d4a-0bd479423dc9
📒 Files selected for processing (1)
skills/review-sweep/SKILL.md
| **Done when:** you can name the base SHA, every changed file, and which subsystems the | ||
| change touches; `git status` shows no tracked changes. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Require a truly clean tree, including untracked files.
The completion condition only checks for tracked changes, while git diff --stat also omits untracked files. An untracked source or test file can therefore be excluded from the review scope. Require git status --porcelain to be empty before Step 1 proceeds, or explicitly stage/commit all session-owned files.
🤖 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 `@skills/review-sweep/SKILL.md` around lines 37 - 38, Update the “Done when”
condition in the review-sweep instructions to require a completely clean working
tree, including untracked files, before Step 1 proceeds. Use git status
--porcelain as the authoritative check, or explicitly stage/commit all
session-owned files; do not rely on git status showing no tracked changes or git
diff --stat.
| const retried = await parallel(vs.map((v, i) => () => | ||
| v ? Promise.resolve(v) | ||
| : agent(refutePrompt(f, i + 1), { label: `refute:${f.key}:${i + 1}:retry`, phase: 'Refute', schema: VERDICT, model: 'opus' }))) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Retry the same skeptic seat, not i + 1.
When seat 0 is empty, this retries it as seat 1, duplicating the other skeptic’s seat identity and weakening the required two-skeptic gate. Preserve the missing seat index; add retry metadata only to the label.
Proposed fix
- : agent(refutePrompt(f, i + 1), { label: `refute:${f.key}:${i + 1}:retry`, phase: 'Refute', schema: VERDICT, model: 'opus' })))
+ : agent(refutePrompt(f, i), { label: `refute:${f.key}:${i}:retry`, phase: 'Refute', schema: VERDICT, model: 'opus' })))📝 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.
| const retried = await parallel(vs.map((v, i) => () => | |
| v ? Promise.resolve(v) | |
| : agent(refutePrompt(f, i + 1), { label: `refute:${f.key}:${i + 1}:retry`, phase: 'Refute', schema: VERDICT, model: 'opus' }))) | |
| const retried = await parallel(vs.map((v, i) => () => | |
| v ? Promise.resolve(v) | |
| : agent(refutePrompt(f, i), { label: `refute:${f.key}:${i}:retry`, phase: 'Refute', schema: VERDICT, model: 'opus' }))) |
🤖 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 `@skills/review-sweep/SKILL.md` around lines 76 - 78, Update the retry branch
in the parallel mapping around refutePrompt and agent so the missing skeptic
retains its original index i; use i rather than i + 1 for the seat argument,
while keeping the retry suffix only in the agent label metadata.
| A finding with `unjudged > 0` is **neither survived nor refuted** — nobody looked at it. | ||
| Set those aside and judge them yourself; count them separately in the parent, because | ||
| folding them into either number overstates the rigour of the funnel it publishes. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not let self-adjudication bypass the two-skeptic gate.
“Judge them yourself” is ambiguous: if that judgment can promote an unjudged finding, the workflow violates its stated rule that unjudged findings do not pass. Either rerun both skeptic seats or explicitly keep these findings excluded from survivors while reporting them separately.
🤖 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 `@skills/review-sweep/SKILL.md` around lines 92 - 94, Clarify the
unjudged-finding workflow so self-adjudication cannot promote findings into
survivors without satisfying the two-skeptic gate. Require rerunning both
skeptic seats before promotion, or explicitly keep unjudged findings excluded
from survived/refuted counts while reporting them separately in the parent.
| - **Spawn one skeptic on the fix itself**, with the same refute framing as Step 3. You | ||
| wrote the code and you wrote the fix; the whole reason this pile exists is that an | ||
| author rationalises. | ||
|
|
||
| Per fix, in one turn: commit, comment the SHA and the evidence on its ticket, close it. | ||
| The same turn — a hot fix landed without its ticket is how six of the first pile's | ||
| findings reached the crunch already fixed, and the crunch paid a triage phase to discover | ||
| it. | ||
|
|
||
| **Done when:** no `data-safety` ticket is open, each closed one names the commit that | ||
| fixed it and its mutation verdict, and the bar passed on the tree you are handing over. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Define the failure path when the fix skeptic rejects the fix.
Step 6 requires a skeptic review, but then unconditionally instructs the owner to comment the SHA and close the ticket. A rejected fix could therefore leave a data-safety issue closed with an unsafe or incomplete change. Keep the ticket open, revert or amend the fix, and rerun verification and skeptic review before closure.
🤖 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 `@skills/review-sweep/SKILL.md` around lines 214 - 224, Update Step 6 to define
the rejection path for the fix skeptic: keep the data-safety ticket open, revert
or amend the rejected fix, then rerun verification and skeptic review. Only
instruct the owner to comment the fixing commit and mutation verdict and close
the ticket after the fix is accepted and the bar passes on the handed-over tree.
1 participant
Problem / Intent
A session that just wrote code is the only one holding the context needed to review it accurately, but it is also the one most likely to rationalise its own defects.
/review-sweepcloses that gap: it reviews hot, then hands every finding to skeptics whose job is to refute it, so only what survives independent attack gets filed.Approach
Adds
skills/review-sweep/SKILL.md, a seven-step user-invocable skill:pipelinewhere each finding is refuted by two skeptics as it arrives; either refutation kills it, and a finding no skeptic answered is counted as unjudged rather than passed.class/cluster/severity, and files a parent issue plus one sub-issue per defect in the anatomy/review-crunchexpects.data-safetyclass in-session, behind a mutation check and a skeptic on the fix itself.Summary by CodeRabbit