review-loop

name	review-loop
description	Use when code changes need iterative multi-round review until no Critical or Important issues remain, with isolated independent reviewers, verification of findings, and parallel review after fixes

Review Loop

Iterative code review with isolated subagents. Loops until clean.

Overview

Dispatch independent reviewer subagents → screen and verify findings → fix confirmed issues → parallel dual-path re-review (fix diff + full review) → repeat until no Critical or Important issues remain.

Each reviewer is fully isolated: no prior review conclusions, no conversation history, no knowledge of previous fixes. Only the round number and diff range are provided.

When to Use

• After completing a feature branch, before merge
• After each task in an implementation plan
• When code quality needs thorough, iterative validation
• When a single-shot review (requesting-code-review) isn't sufficient

Don't use for:

• Quick sanity checks — use requesting-code-review instead
• Review of other people's PRs — this is for your own code iteration

Process Flow

digraph review_loop {
    "Determine diff range\nand reviewer count" [shape=box];
    "Dispatch 1~N reviewer\nsubagents (isolated)" [shape=box];
    "Collect results" [shape=box];
    "Screen each issue" [shape=box];
    "Issue clearly valid?" [shape=diamond];
    "Add to fix list" [shape=box];
    "Issue clearly false?" [shape=diamond];
    "Skip with reason" [shape=box];
    "Dispatch verifier\nsubagent" [shape=box];
    "Any Important+\nissues?" [shape=diamond];
    "PASS" [shape=doublecircle];
    "Report round\nsummary to user" [shape=box];
    "Fix issues" [shape=box];
    "Parallel dual-path\nreview" [shape=box];
    "Merge and deduplicate\nresults" [shape=box];
    "Exceeded MAX_ROUNDS?" [shape=diamond];
    "STOP: Pause, ask user" [shape=octagon, style=filled, fillcolor=red, fontcolor=white];

    "Determine diff range\nand reviewer count" -> "Dispatch 1~N reviewer\nsubagents (isolated)";
    "Dispatch 1~N reviewer\nsubagents (isolated)" -> "Collect results";
    "Collect results" -> "Screen each issue";
    "Screen each issue" -> "Issue clearly valid?";
    "Issue clearly valid?" -> "Add to fix list" [label="yes"];
    "Issue clearly valid?" -> "Issue clearly false?" [label="no"];
    "Issue clearly false?" -> "Skip with reason" [label="yes"];
    "Issue clearly false?" -> "Dispatch verifier\nsubagent" [label="uncertain"];
    "Add to fix list" -> "Any Important+\nissues?";
    "Skip with reason" -> "Any Important+\nissues?";
    "Dispatch verifier\nsubagent" -> "Any Important+\nissues?";
    "Any Important+\nissues?" -> "Report round\nsummary to user" [label="yes"];
    "Any Important+\nissues?" -> "PASS" [label="no"];
    "Report round\nsummary to user" -> "Exceeded MAX_ROUNDS?";
    "Exceeded MAX_ROUNDS?" -> "STOP: Pause, ask user" [label="yes"];
    "Exceeded MAX_ROUNDS?" -> "Fix issues" [label="no"];
    "Fix issues" -> "Parallel dual-path\nreview";
    "Parallel dual-path\nreview" -> "Merge and deduplicate\nresults";
    "Merge and deduplicate\nresults" -> "Screen each issue";
}

Parameters

All optional — infer from context:

Parameter	Default	Description
BASE_SHA	Auto-inferred	Review start point
HEAD_SHA	HEAD	Review end point
DESCRIPTION	From git log	Brief change description
PLAN_OR_REQUIREMENTS	From context	Spec or requirements doc
MAX_ROUNDS	5	Max rounds before pausing

Auto-inference:

1. Explicit commit range in conversation → use it
2. Branch has unmerged commits vs main → BASE = merge-base, HEAD = HEAD (includes uncommitted)
3. Only uncommitted changes → review working tree diff
4. Nothing → ask user

Step 1: Determine Reviewer Count

Assess change scope with git diff --stat:

Scope	Reviewers	Example focus split
Small (<100 lines, 1-2 files)	1	Full coverage
Medium (100-500 lines, multi-file)	2	A: logic/security/edge; B: architecture/maintainability
Large (>500 lines, cross-module)	2-3	A: logic/security; B: architecture/patterns; C: spec/tests

Adapt focus areas to code characteristics. Database-heavy → SQL injection/transactions reviewer. UI-heavy → accessibility/UX reviewer.

Step 2: Dispatch Reviewers

For each reviewer, fill reviewer-prompt.md template and dispatch via Agent tool:

• Each reviewer is a separate Agent call — they run in parallel
• Provide only: round number, diff range, description, focus areas, project conventions
• Never provide: prior review results, fix history, controller opinions
• Before dispatching, check for locally installed review skills (e.g. requesting-code-review, code-reviewer agents) and reference their output format conventions to keep reviewer output consistent with project norms

You MUST dispatch subagents. Do NOT review the code yourself. Even for 1 reviewer on a small diff, dispatch a subagent. The controller's job is to coordinate, screen, and fix — not to review. Reviewing your own code (or code you just read) defeats the isolation principle. If you catch yourself thinking "I'll just review it myself, it's small" — that's the exact rationalization this rule prevents.

Step 3: Screen Results

For each issue reported by any reviewer:

1. Clearly valid and necessary → add to fix list
2. Clearly false positive (code doesn't have this problem, or reviewer missed context) → skip, record reason
3. Uncertain → dispatch verifier subagent with verifier-prompt.md:
   • Provide: issue description, file paths, project conventions
   • Do NOT provide: which reviewer, your leaning
   • Verifier returns: Confirmed / Not confirmed / Exists but no fix needed

Multi-reviewer dedup: Same file:line with same issue type → merge, take higher severity.

Step 4: Report Round Summary to User

After screening, report with full issue details:

## Round N Review

### Issue Details

| # | Level | Description | Decision | Reason |
|---|-------|-------------|----------|--------|
| 1 | Critical | ... | Fix | ... |
| 2 | Important | ... | Fix | ... |
| 3 | Important | ... | Rejected | False positive: ... |
| 4 | Minor | ... | Verified → Fix | Verifier confirmed |
| 5 | Minor | ... | Skipped | Non-blocking, cosmetic |

Decision values: Fix / Rejected / Verified → Fix / Verified → Rejected / Skipped

### Summary
- Issues found: X (Critical: a, Important: b, Minor: c)
- Confirmed to fix: Y, Rejected: Z
- Status: continuing / passed / paused at limit

### Convergence Trend (Round 2+)
- Round 1: 5 issues (C:1 I:3 M:1) → Round 2: 2 issues (C:0 I:1 M:1) → ...
- Trend: converging / stable / diverging
- If diverging or stable after 2+ rounds: note possible cause and suggest strategy adjustment

Convergence rules:

• Converging: Important+ count strictly decreasing across rounds → continue
• Stable: Important+ count unchanged for 2 consecutive rounds → flag to user, suggest reviewing fix quality or adjusting strategy
• Diverging: Important+ count increasing → stop and ask user before continuing

If no Important+ issues → report and exit (PASS).

Step 5: Fix Confirmed Issues

Choose strategy based on issue characteristics:

• Simple issues (typo, missing null check, small logic fix) → fix directly
• Complex single issue (architectural change, multi-file refactor) → dispatch 1 fixer subagent
• Multiple independent issues → dispatch N fixer subagents in parallel (one per issue or per related group)

Fixer subagents receive: issue description, file paths, project conventions, concrete fix suggestion from reviewer. They commit their fixes.

Step 6: Dual-Path Parallel Review

After fixes, dispatch two review paths in parallel:

Path A — Fix diff review:

• 1 subagent with fix-reviewer-prompt.md
• Diff range: only the fix commits
• Does NOT know what the original issues were

Path B — Full review (new round):

• 1~N subagents with reviewer-prompt.md (same count logic as Step 1)
• Diff range: original BASE_SHA..HEAD (now includes fixes)
• Fully isolated — no knowledge of any prior round

Both paths return results → merge and deduplicate → back to Step 3.

Step 7: Termination

Pass condition: No Critical or Important issues after screening. Minor issues listed but don't block.

Max rounds (default 5): After MAX_ROUNDS cycles, pause and present to user:

• Remaining Important+ issue list
• Summary of all rounds
• Recommendation: continue, stop, or adjust strategy

Ask user when:

• Fix direction involves architectural trade-offs
• Verifier contradicts reviewer and you can't judge
• Fix scope exceeds current task boundaries

Final Output

## Review Loop Result

### Convergence Summary
- Round 1: X issues (C:a I:b M:c) → Round 2: Y issues (C:d I:e M:f) → ...
- Trend: converging / stable / diverging
- Important+ trajectory: [count per round, e.g. 4 → 2 → 0]

### Result
- Total rounds: N
- Status: Passed / User terminated
- Total issues found: X, Total fixed: Y, Total rejected: Z
- Remaining Minor issues: (list, optional fix)

Isolation Principles

What	Rule
Prior review conclusions	Never passed to next round's reviewers
Prior fix content	Full reviewers don't know; fix reviewer only sees diff range
Conversation history	Not inherited; every subagent starts fresh
Round info	Only round number ("Round N") — no history details
Controller opinions	Never shared with verifiers

Why isolation matters: Without it, later-round reviewers anchor on earlier findings and miss new issues.

Common Mistakes

Mistake	Fix
Passing prior review results to new reviewer	Only provide round number and diff range
Skipping verification for "obvious" issues	If unsure, dispatch verifier — it's cheap
Fixing all issues yourself instead of parallelizing	Multiple independent issues → parallel fixer subagents
Running only full review after fix (no fix-diff review)	Always dual-path: fix diff + full review in parallel
Telling fix reviewer what the original issues were	Causes confirmation bias — let them judge independently
Batching all rounds before reporting to user	Report after EVERY round — user needs visibility
Omitting issues from review output	Reviewers MUST list ALL issues exhaustively
Stopping loop after one clean round without verifying	If fixes were made, dual-path review IS the verification
Reviewing the code yourself instead of dispatching subagent	Controller coordinates — NEVER reviews. Even for small diffs, dispatch a subagent

Integration

Skill	Relationship
requesting-code-review	Complementary — single-shot vs. iterative loop
subagent-driven-development	Can invoke review-loop as review step per task
receiving-code-review	Controller applies its verify-before-act principles
verification-before-completion	Use after review-loop passes for final check