Teaching Claude Code to review its own PRs

I have been running agent-written code through an agent-written review for about five months. The honest summary: the first version was worse than useless. It produced twelve findings per PR, two of which were real, and reading it cost more attention than reading the diff. The version I run today produces two or three findings per PR, and I have merged exactly one bug past it since April.

What changed was not the model. It was the structure around it — splitting review from adjudication, forcing every claim to cite a line it can be checked against, and giving the reviewers the test output instead of asking them to imagine it.

## The shape of the pipeline

The core idea is that reviewing and adjudicating are different jobs, and collapsing them into one context is what makes single-agent review so noisy. A reviewer is rewarded for suspicion: it is looking for what might be wrong, and a model asked to find problems will find problems. A judge is rewarded for skepticism. If the same context window does both, suspicion wins — the findings are already in the transcript, and the model reads its own output as evidence.

So the pipeline is three passes:

1. Two independent reviewers over the same diff, in parallel, in separate contexts. One is Claude Code with repo access; the other is a second CLI with the same diff and the same rule file. Neither sees the other's output.
2. A judge that receives both finding lists plus the raw diff and the test output, and must re-derive each finding from the diff itself before keeping it.
3. A formatter that posts surviving findings as review comments anchored to file:line, and silently drops anything it cannot anchor.

Two reviewers instead of one is not about consensus — I do not require agreement to keep a finding. It is about disagreement as signal. When both reviewers independently land on the same three lines, that is almost always real. When only one does, the judge has to work for it, and roughly half of those die.

The rule file

Every repo gets a REVIEW.md at the root: the standards specific to that codebase that would otherwise be re-litigated on every PR. In the iOS repo it says things like "no force unwraps outside tests" and "any new @Observable needs a re-render test." In the TypeScript agent repo it says "no new Zod schema without a round-trip parse test" and "never widen a LangGraph state channel without updating its reducer." Both reviewers get it verbatim. It is the highest-leverage file in the whole setup, and it is maintained the way you maintain a lint config: you add a rule the first time a mistake ships twice.

## Running it

It is a shell script, not a service. I invoke it as a slash command from inside Claude Code, or directly against a diff target when I want to check a branch before opening a PR at all.

# review the working branch against main, or any explicit diff target
$ ./scripts/review.sh origin/main...HEAD --reviewers=claude,codex --judge=claude

→ collecting diff     37 files, +1,204 / -318
→ loading rules       REVIEW.md (18 rules) + CLAUDE.md (test policy)
→ running tests       pnpm test:all — 412 passed, 0 failed, 6 skipped (94s)
→ reviewer:claude     9 findings  (41s)
→ reviewer:codex      6 findings  (38s)
→ judge               3 kept, 12 rejected (1 dup, 8 unverifiable, 3 out-of-diff)
→ posting             gh pr review --comment → 3 threads on 2 files

# the flag I actually tuned the prompts with: print the judge's reasons, post nothing
$ ./scripts/review.sh origin/main...HEAD --explain-rejections --no-post

--explain-rejections matters more than it sounds. For the first month I tuned entirely by reading what the judge threw away, never what it kept. The rejection log is where you find out your reviewers are spending nine findings a run on style opinions nobody asked for.

The roles themselves live in a small YAML file, so swapping a reviewer or pointing the judge at a cheaper model is a config change rather than a prompt edit:

reviewers:
  - id: claude
    cmd: "claude -p --output-format=json"
    context: [diff, rules, test_output, changed_file_bodies]
    must_cite: true      # every finding needs a file:line inside the diff
  - id: codex
    cmd: "codex exec --json"
    context: [diff, rules, test_output]
    must_cite: true

judge:
  cmd: "claude -p --output-format=json"
  # the judge never sees reviewer *rationale* — only the claim and the
  # citation — so it cannot inherit reasoning it never checked itself
  context: [findings_without_rationale, diff, test_output, rules]
  verdicts:
    keep: "the cited lines, read alone, demonstrate the claimed problem"
    reject: "requires assuming code outside the diff behaves a certain way"
    downgrade: "real but stylistic — emit as a note, never as a blocker"
  max_kept: 5         # a review with 12 comments does not get read; force the ranking

post:
  target: github
  drop_uncitable: true
  block_merge_on: [correctness, security]  # style and naming never block

max_kept: 5 is doing unglamorous but real work. A review with twelve comments is not four times as useful as a review with three; it is less useful, because the reader stops ranking and starts skimming. Forcing the judge to rank means the top finding is the one it actually believes most.

## What it actually catches

The failures it is genuinely good at are the ones that live between two files — exactly the class a human skim misses, because the human is reading one hunk at a time and the problem is in the relationship.

• Migrations without their consumers. A column is renamed, the model is updated, and one serializer in a different directory still reads the old key. This is the most common real catch by a wide margin.
• Tests edited to match the bug. When a diff changes an assertion and the implementation in the same commit, it asks which one moved first. About a third of the time the honest answer is "I made the test agree with the regression."
• Swallowed errors. A catch that logs and returns a default, on a path where the caller reads that default as success. Trivial to see, very easy to skim past.
• N+1s introduced by a refactor. Not the query itself — the moment a previously-batched call gets moved inside a loop that lives three functions away.
• Rule-file violations, reliably. Anything written down in REVIEW.md is caught at close to 100%. That is the boring result, and it is most of the value: the pipeline is a lint rule you can write in English.

The pipeline is not smarter than me about this codebase. It is more patient than me, at 11pm, on the fourth PR of the evening. That turns out to be the axis that matters.

— the whole thesis, honestly

## What it confidently invents

This is the part most write-ups skip, and it is the part that decides whether anyone keeps using the thing. Early on, my rejection log was dominated by four failure modes, and they all share a shape: the model reasons about code it cannot see, then reports the conclusion at the same confidence as an observation.

Dead code that is wired up elsewhere

"This function is never called." It is called — by a route table, a dependency-injection container, an @objc selector, or a fixture that builds handlers by name. The diff shows a definition and no call site, and the model treats absence of evidence as evidence. This was my number-one false positive for weeks.

Impossible null dereferences

Reviewers love flagging user.profile.id as unsafe. In a repo with strictNullChecks on, the type system has already proven profile non-optional, and the reviewer is pattern-matching on shape rather than reading the type. The tell is that it never cites the type declaration. It cannot: it did not look.

CVE-shaped findings in safe code

The most expensive category, because it is the most persuasive. String concatenation anywhere near a database produces a confident SQL-injection finding complete with a plausible exploit string — even when the concatenation is building a LIMIT clause from a validated integer, or the driver is parameterizing underneath. Security findings read as urgent, so they get acted on, so a false one costs a real half hour.

Invented conventions

"This should use the repository pattern, consistent with the rest of the codebase." There is no repository pattern in the codebase. The model inferred a house style from the general population of code it has seen and attributed it to this repo. Anything phrased as "consistent with" and not accompanied by a citation of the thing it is supposedly consistent with is fabricated about half the time.

## The changes that closed the gap

Four changes, in the order I made them, roughly in order of how much they helped.

1. Mandatory citation, mechanically enforced. Every finding must carry a file:line that resolves to a line inside the diff. The check is a regex in the script, not an instruction to the model — an uncitable finding is dropped before the judge ever sees it. This alone killed most of the invented conventions, because "consistent with the rest of the codebase" has nowhere to point.
2. The judge re-derives instead of trusting. It gets the claim and the citation but not the reviewer's rationale, and has to decide whether the cited lines, read alone, demonstrate the problem. Stripping the rationale mattered more than any wording change I made: a confident paragraph of reasoning is contagious, and a judge that reads one will ratify it.
3. Feed in the test output. Both reviewers get the real result of pnpm test:all. A reviewer that can see 412 passing tests stops hypothesizing about runtime failures the suite would have caught, and starts noticing which behaviors have no test at all — which is the finding I wanted in the first place. This is the other half of the argument in the case for boring test infrastructure.
4. Name the false-positive classes explicitly. The reviewer prompt now lists the four categories above by name, with the instruction that a finding in one of them must cite the evidence ruling the benign explanation out: the type declaration, the missing registration, the unparameterized driver call. Naming a failure mode to a model works about as well as naming it to a junior engineer — imperfectly, but far better than not naming it.

## What it costs

About ninety seconds of wall clock per PR, most of it the test run I was paying for anyway. Two reviewer calls and one judge call. The judge is the cheapest of the three, because it gets findings and a diff rather than a repo — stripping the rationale had the pleasant side effect of shrinking its context by roughly 60%.

The cost not measured in tokens is the rule file. REVIEW.md only works if it stays true, and a stale rule produces confident, well-cited, completely unwanted findings forever. Unlike a lint rule, nothing fails loudly when it goes out of date. I re-read mine whenever the pipeline tells me something I disagree with twice.

## Where it still fails

It cannot tell me the feature is wrong. It reviews the diff against its stated intent, and if the intent is mistaken the review is a very thorough endorsement of the mistake. It has never once said "this whole approach is the problem," and I do not expect it to: nothing in its context contains the conversation where that would have been decided.

It is also poor at concurrency. Anything whose bug lives in an interleaving — a race between a cache write and its invalidation, an actor reentrancy hazard — it either misses entirely or flags everywhere, and I have not found a prompt that moves it. That is the one category where I still read the diff line by line myself, and probably should have all along.