TheWayWithin Code Review Loop
Capability
Run the canonical working loop from the loops/autoresearch research: a read-only critic scores a diff and raises evidence-backed findings, a read-write fixer addresses ONLY those findings, then the critic re-audits. Repeat until two consecutive clean rounds or a cap. The separation is the whole point — the thing that judges the code cannot edit the code, so it cannot pass the review by loosening it (the Sprint 6a read-only principle, applied to review).
Use Cases
- Reviewing a pull request to a converged, clean state.
- Hardening one changed file or surface before merge.
- Turning a pile of review comments into addressed-and-re-verified findings.
- Any "iterate until clean" task where "clean" is a checkable signal, not a vibe.
When NOT to use this skill
- No checkable signal for "done". If "good" is subjective taste with no test, gate, or rubric, a loop just burns tokens. Do a single review pass instead.
- Green-field authoring. This loop reviews and fixes an existing diff; it does not design features from scratch.
- Structural rewrites. The fixer addresses raised findings on a named surface. A loop will not invent an architecture change that needs a step backwards first.
- Unbounded scope. If every round surfaces a brand-new area, the surface is too big. Narrow it first.
The two roles (never the same agent on the same turn)
| Role | Tools | Model | Job |
|---|---|---|---|
| Critic | Read-only (Read, Grep, Bash for running checks — NO Edit/Write) | A different model to the fixer | Score the diff, raise findings with file:line + evidence. Never edits code. |
| Fixer | Read-write (Edit/Write on the named surface only) | The session default | Address ONLY the raised findings. Never edits the gate, the test, or the critic's rubric. |
Run the critic on a different model to the fixer. Read-only separates the incentives; a
different model separates the blind spots. A critic sharing the generator's weights inherits the
same assumptions and tends to return agreement rather than verification, so a same-model critic
scoring its own family's work is closer to a consistency check than a review. Switch with /model
between roles, set CLAUDE_CODE_SUBAGENT_MODEL, or pass opts.model if you are driving the loop
from a script. If only one model is available the loop still works and is still worth running: note
in the log that critic and fixer shared weights, and treat a clean round as weaker evidence.
Watch for unanimity. When every round comes back clean immediately, that is as likely to be correlated bias as quality. A critic that has never disagreed with its fixer is not yet earning its turn.
Deterministic-first: when a test suite or quality gate exists, that is the critic's score (cheap, objective). Use an LLM critic with a numeric score (0-100 or findings-count) only when no deterministic signal is available. A deterministic score has no blind spots to share, which is why it outranks any cross-model arrangement when one is available.
The loop
CODE-REVIEW LOOP:
baseline:
- Identify the named surface (file/folder/diff). Everything else is off-limits.
- Critic runs deterministic checks (gates/tests) if they exist; records the score.
repeat (until 2 clean rounds OR cap):
1. CRITIC (read-only): produce findings. Each finding = {file:line, severity, evidence,
why-it-matters}. A claim with no evidence is dropped, not raised (default-fail).
If findings == 0 → increment clean-round counter; else reset it to 0.
2. If clean-round counter == 2 → STOP "converged".
3. FIXER (read-write, surface only): address ONLY the raised findings. No opportunistic
edits, no touching gates/tests/rubric. If a fix needs to leave the surface → escalate.
4. Re-run deterministic checks. Append one JSONL line to .loops/review-<surface>.log.
5. CAP CHECK: stop "capped" if rounds ≥ max (default 5) or diff ≥ 1000 lines or tokens ≥
ceiling. Escalate on 3 consecutive rounds with the same unresolved finding.
Log schema (.loops/review-<surface>.log, append-only JSONL, one line per round):
{"round": 2, "findings": 3, "resolved": 3, "score": 96, "diff_lines": 54, "tokens": 3100, "status": "fixing", "note": "null-check + 2 error-path tests"}
Caps (library defaults — overridable)
| Cap | Default | Why |
|---|---|---|
| Max rounds | 5 | Convergence is 2 clean rounds; 5 total bounds a thrashing loop. |
| Max diff / round | 1000 lines | Keeps each round reviewable and the fixer honest. |
| Token ceiling | Logged per run | Loops cost multiples of a single review; the human sees the meter. |
| Clean rounds to converge | 2 | One clean round can be luck; two is the signal. |
Exit Criteria
Run each check and paste the output. "Looks reviewed" is not sufficient.
| # | Check | Verification | Pass condition |
|---|---|---|---|
| 1 | Critic never edited code | git log --author / diff authorship on the surface |
Only the fixer's commits touch code. |
| 2 | Findings are evidence-backed | Read the log's finding entries | Every raised finding cites file:line + evidence; none are bare assertions. |
| 3 | Fixer stayed on surface | git diff --stat |
All changes confined to the named surface. |
| 4 | Gates/tests not edited | git diff -- gates/ '**/*.quality-gates.json' |
Empty. The judge was never touched. |
| 5 | Converged or capped explicitly | Last log line | status is converged or capped, never an open loop. |
| 6 | Token cost recorded | .loops/review-<surface>.log |
Final round logs cumulative tokens. |
If any check fails, do not declare done. Fix and re-run.
Integration Points
- mission-optimize: same
.loops/log discipline and read-only set; optimize ratchets a metric, this loop ratchets review findings to zero. - project/gates/: the deterministic critic score comes from here when gates exist.
- quality-gates-guide / loop-discipline-guide: the read-only and default-fail contract this skill enforces.
Anti-Patterns (Excuse / Rebuttal)
Excuse: "Let the reviewing agent also fix what it finds — one pass, less overhead."
Rebuttal: The moment the judge can edit the code, it can pass the review by editing the code instead of fixing it, or by quietly relaxing what it flags. Keeping critic read-only and fixer read-write is the entire anti-reward-hacking mechanism. The extra turn is cheap; a review that grades its own homework is worthless.
Excuse: "The critic is sure there's a bug; raise it even without a specific line."
Rebuttal: A finding with no file:line and no evidence is a guess, and the fixer will either chase a phantom or "fix" the wrong thing. Default-fail: an unevidenced finding is dropped, not raised. If it is real, find the line.
Excuse: "While we're in here, let's clean up that other file too."
Rebuttal: Opportunistic edits outside the named surface break the loop's bound, balloon the diff, and make the re-audit meaningless. The fixer addresses raised findings on the surface and nothing else. The other file is a separate loop.
Excuse: "It's been five rounds and one finding won't die; just mark it done."
Rebuttal: That is the escalation case, not the done case. A finding that survives three rounds means the fixer can't resolve it within the surface, or the finding is wrong. Escalate to a human with the log; don't paper over it by redefining clean.
References
project/field-manual/loop-discipline-guide.md— the five-gate "loop or not" test and ratchet mechanics.project/gates/README.md— the read-only gate contract (Sprint 6a).- Karpathy autoresearch / Claude agentic loops research cards — the source discipline.