Code review in a generation-cheap world

Code review is the step where a second engineer reads a change before it merges, and it had a clear job long before coding agents started writing much of the diff. Ask a team what review is for and they will say finding bugs; when Microsoft studied its own reviewers in 2013, finding bugs turned out to be a small part of what reviews did. The larger part was different: the reviewer learned a part of the system they had not worked in, and the team found out that a change was happening. Now and then a second person suggested a better way to do it. Reviewers in that study needed above all to understand the change, and that was the need the tools served worst. That is still how I think about review: a pull request is where a second engineer builds a picture of what changed and why, and the bugs it catches come out of building that picture.

The common advice on how to review generated code is also sound, and I use it. Put a machine reviewer in front of the human: let it handle style, lint, known security patterns and the simple bugs, and keep human attention for whether the change does what was meant and fits the system around it. On my team the first round of every review is an agent, and it does take a large share of the small comments off the humans.

The advice stops short of the practical part: it says "focus on architecture" and stops there. It does not say what the human reviewer should stop reading, or what the author owes before asking for a review, and it says nothing about which diffs need a human at all. It also does not say why those questions became urgent: producing a diff now takes minutes, and checking one takes as long as it ever did. When a diff takes its author minutes to produce and a reviewer most of an afternoon to understand and check, the work has shifted to the reviewer, and on my team the reviewer is one of the seniors. I first saw this clearly when we let product managers contribute code directly, which seemed like a generous idea at the time. The main codebase started getting large pull requests full of generated code nobody had read, and the seniors spent more time reviewing them and going back and forth than an engineer would have spent building the feature from start to finish. Buying a better reviewer bot would not have fixed that; the cost had to go back to the author, and the sections below are how we rebuilt the review step on my team to do it. How a team talks inside a review, what it takes to approve and how comments are labelled, is its own essay, code review culture; this one is about what the review covers now and what it stops covering.

Make the author the first reviewer

The commonest thing junior engineers send me is a generated diff on the agent's word. Ask why a branch of the logic exists and the answer is that the agent said it works. The shortcut is understandable, since the agent sounds confident and the tests are green, but it is also the whole problem in one PR: the author skipped reading it, so the reviewer reads it for them.

We wrote a rule: the PR author reads and understands every line of the diff, whoever or whatever produced it. Around that rule we built four things:

• a PR template that carries the intent of the change,
• a checklist the author works through before requesting review,
• a rule file the agent reviewer reads,
• a norm document that says all of this in plain words.

The implementation doc did more than all four of those: before any code, the engineer writes down what they intend to build and how, and that doc becomes the thing the diff is later checked against.

A checklist on its own turns into a box-ticking exercise, so seniors keep it honest by asking random questions about the doc and the diff: why does this handler branch here, what happens on this path if the upstream call fails. An author who has read their own diff answers in a sentence, and one who has not learns quickly that reading it is part of the job. Write the rule down before an incident tests it; people agree with "you own what you submit" until a postmortem, and then they want to talk about it again.

Give the machine the first round

We set up the first-round agent reviewer knowing our applications handle accounting and real money, and it still misses. Every time it does, we add a rule to its rule file, either a business rule for the case it missed or a rule about how our framework is meant to be used. The rule file grows every round and the misses on single-file changes get rarer.

Where the machine stops has not changed: when a change spans several files, it loses the thread between them and passes something a human would have caught. That limit is the most useful thing we know about it, because it tells the human reviewer where to look. The habit is the same as with the checklist: we never treat the rule file as enough, and the list of what it missed becomes the next rule.

Read for what the machine misses

On my team the human reviewer reads for four things:

• Does the diff match the implementation doc, or did the agent solve a nearby problem?
• Does the logic hold across files, where data passes from one module to another and the machine reviewer lost the thread?
• Does the change fit the architecture, and is there anything new here that should not exist?
• Is any of it on a money or accounting path? If so, it gets the slowest, most careful review on the team regardless of size.

The human stops re-checking what lint, the test suite and the rule-based reviewer already covered. That is harder than it sounds, because attention slips back to what the machine already covered, in two ways. One is habit: reviewers keep reading for style and run out of attention before they reach the logic. The other starts once the reviewer knows the diff is generated, and their reading slides toward naming and consistency and away from whether the logic is right. Either way the senior spends attention on things the machine already checked. How one person reads a diff for its contracts and structure is a skill of its own, covered in reading generated code; this section is only the team rule about what the reviewer is looking for.

Read every change, some more deeply

You can run review the other way round: merge first and review after, and skip review entirely for small or urgent changes if the author writes down why. The AI-era version says the engineer who drove the agent has all the context and should be the main reviewer, with a peer reviewing only a sample, or more lightly. I do not do either, at least not for large systems or for anything that handles real money. Bugs are expensive there, and unread generated code piles up when it reaches the main branch and other work gets built on top of it. A review is also not independent when the same model that wrote the change checks it. On my team every change gets read by a human, whatever its size.

The other policy works in a smaller codebase where one reviewer can hold the whole system in their head and a mistake is cheap to undo. Where your system sits between those two ends changes how deeply a change gets read and by whom, but it never takes the human out. Read deepest on money paths, and read deeper than the line count suggests when a change touches several repos or several files.

Route outside contributors around the codebase

Back to the product managers, whose goal was a good one: let PMs test ideas fast without waiting for an engineer to be free. There were two obvious answers, ban the contributions or make PMs meet the same author rules as engineers, and we did neither. We built our own sandbox instead: a small SDK inside the desktop app that lets PMs put together their own apps on top of the team's APIs and interfaces, without ever touching the main codebase. Rough generated code is fine there because it never reaches the main codebase, and no review is required.

We changed where the work went instead of how it was reviewed: low-risk work got somewhere else to go, and the seniors stopped spending time on it. Watch for the sandbox quietly turning into production: an experiment that works gets depended on, and somebody has to review it properly months later, all at once. The line between the two has to be real, and anything that crosses it goes through the full review process.

Protect senior attention by reducing what reaches it

Everything above also protects senior attention, because on my team the seniors are the default reviewers and they have their own work: when reviews pile up, changes wait longer to ship and the seniors' own work slips. Reviewing means understanding logic, and reading unfamiliar logic every day wears people down in a way that no dashboard shows until it has already happened.

One more thing worked beyond what is described above: we ran the agent review before push, inside the editor, so the author fixes its findings while the code is still in front of them and the diff reaches the human already clear of the things the machine catches. Two more I would try: a team norm on smaller PRs, because a large generated diff is hard on a tired reviewer, and a walkthrough led by the author for anything so large that reading it cold would take an hour.

None of this is free: the author rules slow the author down and somebody has to own the rule file. The sandbox was harder to justify, because it was engineering work that shipped no feature of its own. A team with small diffs and a system that fits in one head, where mistakes are cheap to undo, can run lighter than this. Asking seniors to review faster does not fix the wait, because the queue is made of what reaches it. I have not tried rotating reviewers, and I do not know yet whether it helps or just spreads the tiredness around.