The Craft of Acceptance: How to Review AI's Code

· tech

#ai#code-review#leadership

📑 Contents

Preface

The first post said the width of the neck is the bandwidth you have for accepting work responsibly; the last one said a spec decides when something counts as done. This post takes the next step: the work is done and in your hands — how do you check it?

“Reviewing AI’s code” sounds like the same job as reviewing a person’s code, only with more volume. This post doesn’t start with an argument. It starts with an experiment — with me as the subject.

The experiment: a green PR with three bugs in it

The material came from the last post’s experiment: the “Create branch from stash…” feature an AI built for GitCrisp, a real AI-written PR of a little over 160 lines. I had an AI plant three bugs in it at three different levels, with two conditions: every existing test still had to pass — 1,294 tests, all green, exactly like the PRs you get every day — and it couldn’t tell me anything about what it planted or where, not even a hint. Here are the answers, revealed only after the review:

  • B1, a logic boundary: the stash index gets “translated” once, under a confident comment: “get_stashes() lists the oldest stash first, while git counts stash@{0} from the newest, so translate the sidebar index.” That comment is wrong. With a single stash the translation changes nothing, so the tests pass; with two or more, the user clicks the newest one, and the one that gets branched and then dropped is the oldest.
  • B2, a silent regression: one existing line of wiring changes — “Apply” is now connected to “Pop”, so pressing Apply also deletes the stash. The new feature is fine; what broke is the thing next to it that used to work, and the existing tests only check that the signal is emitted, not where it’s connected.
  • B3, architecture: the new flow does import pygit2 in the presentation layer to validate branch names, going around the port. It works perfectly. It’s a “should this be written here” problem.

Then both sides ran at once. The human arm was me, blind — not knowing how many bugs there were or where, I read only the PR description first and wrote down my predictions, then timed myself reading the diff. The AI arm was clean Claude Code with the same instruction, “review this PR, don’t modify files”, under three conditions, three runs each: a bare review with nothing, one with GitCrisp’s CLAUDE.md, and one that also had a review skill (adapted from the review loop in another project of mine, whose core rule is “every finding needs a concrete ‘input/state → wrong result’ scenario; if you can’t write the scenario, it isn’t a finding”).

Author, blind Bare AI +CLAUDE.md +review skill B1 reversed index logic boundary △ "no test pins it" not verified 3/3 3/3 3/3 B2 Apply → Pop silent regression ✓ 3/3 3/3 3/3 B3 layer bypass architecture call ✓ 1/3 2/3 3/3 Unplanted real bug half-done, says "error" ✗ 1/3 1/3 3/3 Cost 20 minutes 0.4–2.2 min, $0.09–0.19 per run How B1 was checked Read the diff 8 of 9 tested pygit2 with a script
One green PR, three planted bugs, one that wasn't planted. Most of what the human and the AI caught overlaps — the cells that don't are what this post is about.

The honest part first: each condition ran three times, and there was exactly one human subject (me). This isn’t a statistic; it’s one controlled observation. Also, the AI running the experiment set up the bare-review arm with a hole the first time — CLAUDE.md had been deleted but was still in the git history, and two of the reviews read it. The numbers in the diagram are from a rerun with CLAUDE.md removed from the history too.

What I caught, and how

In twenty minutes I touched all three, but in different ways.

B2 I read off the page. The diff is almost all additions, and that one line is a change — connect(self._on_stash_apply) became connect(self._on_stash_pop). A PR that adds a feature has no reason to touch Apply’s wiring, and a spot where “in theory nothing should have changed” is where the eye stops.

B3 I predicted. Before reading the diff, I wrote down: “Finishing the implementation and keeping the architecture consistent are two different things” — AI is very good at getting a feature working, but the path it takes to get there doesn’t necessarily stay on the Clean Architecture grid. So when import pygit2 showed up in the presentation layer, I knew that was what I’d been looking for.

B1 is the interesting one, because I did not catch it being wrong. My second prediction was “I’m not sure how a library like pygit2 behaves, so I’ll need some proof.” When I reached the comment saying the oldest stash comes first, I couldn’t find evidence that it was right, or that it was wrong — and honestly, as the reviewer, I didn’t want to go digging for it. So my review comment was: “The comment says the oldest comes first, but no test case pins that behaviour down.”

There’s also a standard I always raise in review and forgot to put in my report this time: don’t use comments to explain behaviour — express behaviour through function and variable names; keep comments for why a workaround exists, never for how something works. By that rule, B1’s comment wouldn’t have survived whether or not it was true — it explains how to translate the index, when the translation should have been pulled out into a function whose name says what it does, with a test pinning its behaviour. The counterexample is in the same file: the original PR’s comment — “libgit2 has no equivalent of git stash branch; shell out so the branch/checkout/apply/drop semantics match git exactly” — explains why the code takes a detour. That kind stays.

What the AI caught, and how

B1 and B2: nine reports, nine hits. And the AI caught B1 in a completely different way: eight of the nine wrote a throwaway script on the spot — create a scratch repo, stash twice, print the order pygit2 returns — and then pointed at the output: “this comment is wrong, I tested it.” The step I didn’t want to take, it took in a minute or two, for ten or twenty cents.

B3 was a different story. The bare review mentioned it once in three runs, the one with CLAUDE.md twice, and only with the review skill — which says outright that “presentation must not reach past the ports into infrastructure” — did it land three times out of three, with two of those marking it blocking. On a judgment call, the AI only sees it reliably when a spec spells it out.

Then there’s the cell I lost. The original AI-written PR already had a bug nobody had planted. When the stash includes untracked files and the working tree happens to contain a file with the same name, git stash branch has already created the new branch and switched to it before failing to restore the stash — and the UI shows a single ERROR line, with no hint that the user is now standing on a different branch. The AI reproduced it with a script afterwards; it’s real. I didn’t see it. All three runs with the review skill did.

The other side needs saying too: on top of all that, every AI report attached three to seven non-blocking comments — some useful (a branch name starting with - gets read by git as an option), some speculative (“index.read() may not refresh HEAD”), and one that got the mechanism wrong (it said a dirty working tree would leave things half-done; when tested afterwards, it failed cleanly). Every one of those has to be weighed by someone.

The division of acceptance: evidence, judgment, guardrails

Laying that matrix out changed the shape of what I think “reviewing AI’s code” means. It isn’t “read more carefully”. It’s routing each doubt to the right way of settling it:

A doubt in the PR Settling it takes evidence or judgment? Cheap evidence → hand it to a machine Library behaviour, index mapping, edge values Throwaway script or AI reviewer — minutes, cents Needs judgment → keep it human Architecture, scope, whether a change belongs Where the neck belongs; to delegate, write a spec No evidence → demand a guardrail Behaviour not pinned by any test Send it back: add a test that fails when wrong Sign-off
The division of acceptance. Doubts don't sort by difficulty; they sort by what it costs to settle them — cheap evidence goes to a machine, judgment stays with a person, and where there's no evidence you demand a guardrail.

Cheap evidence goes to a machine. The order pygit2 returns stashes in is a fact with a right answer, and the answer sits in the output of a ten-line script. Doubts like that used to depend on the reviewer’s experience or willingness — someone with experience remembers, someone willing goes and checks, and with neither it slips through. Now the price is a minute or two and ten or twenty cents. I didn’t take that step, not because I couldn’t, but because a person’s willingness is a scarce resource — and a scarce resource shouldn’t be spent on something ten or twenty cents can buy.

Judgment stays with a person. B3 has no right answer you can test for — import pygit2 works and the tests pass; the question is whether this project should be written that way. That’s exactly where the funnel’s neck should be spending its effort. And the experiment’s other conclusion is that judgment can be partly delegated too — but only once it’s been written down as a spec. One run in three for the bare review; three in three once it was in the review skill. That’s the last post’s “spec as code” — only this time, the one running it was the reviewer.

Where there’s no evidence, demand a guardrail. Look again at my comment on B1: “no test case pins that behaviour down.” It didn’t identify the bug, but it pointed straight at the right fix — add a test with two stashes, and that test goes red the moment it runs. It’s an underrated move for a reviewer: the burden of proof is on the author, not on the reviewer. I don’t have to prove it wrong myself. I only have to refuse to sign where there’s no evidence.

Reflection

A green build is a claim, not evidence

The most cunning thing about this PR is that all 1,294 tests are green — and the new tests aren’t bad: a real-repo test in infrastructure, flow tests in presentation, an e2e journey. It’s just that every one of them uses a single stash, which is precisely the point where the wrong translation and the right one give the same answer.

This is the physical form of what the first post meant by “when a guardrail breaks, it breaks silently”. That’s why, when I review an AI’s PR, I lean harder on the tests than on the feature, and the question isn’t “are there tests?” but “if this code were wrong, would this test go red?” A test that can’t tell wrong from right isn’t a guardrail; it’s decoration. Of the nine AI reports, all three with the review skill listed “single stash only” as its own test gap, two of them as blocking — because the skill has a line asking: “Would the tests fail if the code were wrong? Check what each test actually asserts.”

Saying “I can’t sign this part” is part of the neck’s width

The first post said neck width = acceptance speed × calibration: real neck width includes knowing where you don’t understand, and for those parts slowing down, adding a guardrail, or saying plainly “I can’t sign this part yet.” When I wrote that, it was an argument. This time it was the real thing — that’s exactly what I did with B1. I wasn’t sure how pygit2 behaves, so I didn’t pretend to be. I asked for a test.

The scary version is the reverse: if I’d read “the oldest comes first”, thought “yeah, sounds reasonable”, and let it through, that would have been the authority illusion from the incident post — the AI’s comment looks the most like an answer. That comment was confident, professional in tone, came with a reason — and was wrong. A comment an AI wrote isn’t documentation; it’s a claim waiting to be verified — and often the best move is not to let a how-it-works comment exist at all: what a good name can say needs no comment, and what a name can’t say, a comment only makes look trustworthy.

An AI reviewer isn’t a second pair of eyes; it’s an evidence machine

The usual line is that an AI reviewer’s value is “one more pair of eyes”. Going by this experiment, that metaphor doesn’t fit. Most of what it caught overlapped with what I caught, so it isn’t a different pair of eyes. What’s genuinely different is that it’s willing to pay the cost of verification for every small doubt — spin up a scratch repo, run a script, reproduce a failure. That’s also how the unplanted real bug got found.

But it’s also a machine that produces noise: three to seven non-blocking comments per report, some speculative, one with the mechanism wrong. Taking its output straight as the verdict just swaps the neck for another kind of rubber stamp — I’d only have moved from stamping AI-written code to stamping AI-written reviews. So the way I’ll use it is: let it run first, treat its findings as leads to verify, and spend my own time on two things only — what it marks as blocking, and what it can’t judge at all.

If this post leaves one sentence behind, it’s this: the hard part of reviewing AI’s code isn’t understanding every line, it’s knowing who should settle each doubt, and at what cost. Evidence goes to the machine, judgment stays with you, and where there’s no evidence, you don’t sign.

Earlier in this series: #1 the responsibility funnel · #2 a clock on the neck · #3 the neck on the board · #4 the responsibility premium · #5 spec as code