Skip to content
Pere Villega
Go back

Stop Reviewing Every Line

8 min read

A two-thousand-line pull request arrives late in the afternoon. It is tidy, the tests are green, and the description is extremely confident. Reading every line carefully will take hours. Skimming it will create the appearance of review without much of the substance.

Agent-generated code can make this situation more frequent, but it did not invent the problem. A large enough change has always exceeded our ability to review it properly. Faster generation makes it easier to reach that limit.

We should stop pretending that line-by-line review is the universal answer. We should not stop reviewing.

Review Is a Risk Decision

Ben Shoemaker’s defence of not reading code caused a predictable argument, partly because the slogan is much sharper than the qualification. His stated practice is not to use line-by-line reading as the primary verification method for most product code. He reviews specifications, tests, selected diffs and production signals, then escalates to detailed code reading for particular risks.

That distinction is sensible. A generated translation update and a new authorisation policy do not deserve the same review process. Requiring identical scrutiny wastes attention on the first and provides a dangerously familiar ceremony for the second.

Before opening the diff, classify the change. I would start with something like this:

RiskTypical changesMinimum evidenceHuman attention
LowDocumentation, generated fixtures, mechanical renamesScope check, deterministic validation, clean diffConfirm intent and absence of surprising files
NormalIsolated product behaviour behind an existing interfaceAcceptance criteria, focused and regression tests, runtime demonstration, rollback noteReview tests, interfaces and selected implementation hotspots
HighAuthentication, authorisation, payments, personal data, concurrency, irreversible migrationsIndependent tests, threat or failure analysis, staged rollout and recovery evidenceDomain expert reads and understands the critical path
Prohibited without redesignUnbounded credentials, destructive operations without recovery, unverifiable requirementsNone is sufficientStop; narrow the authority or make the outcome verifiable

This is not a compliance standard. It is a starting vocabulary for making the decision explicit. Each team will classify differently, and a small diff can still be high risk. A one-line permission change may deserve more scrutiny than a thousand generated test cases.

The classification must also be mechanically connected to the workflow. If adding a migration file is high risk, CI can require a migration plan and an owner from the relevant team. If code touches an authorisation package, ownership rules can prevent an unqualified approval. A label that relies on the author remembering it under pressure is documentation, not a guardrail.

Start With the Claim, Not the Syntax

Reviewing from line one of the diff puts implementation before intent. By the time we discover that the change solves the wrong problem, we may already have spent an hour debating names.

I prefer this order:

  1. What user or operational outcome is meant to change?
  2. Which interfaces, invariants and data flows may be affected?
  3. What evidence would distinguish success from a convincing imitation?
  4. What did the checks actually exercise, on which revision?
  5. What remains uncertain, and how do we recover if it fails?
  6. Which portions of the implementation can still hide material risk?

Only then does the implementation diff have enough context. We can inspect the lines that implement a security boundary, an unfamiliar concurrency strategy or an irreversible state transition, while sampling repetitive mechanical code rather than reading every copy. The same context tells us to ask why the change crossed a module boundary that the plan said it would not touch.

This is more demanding than watching green ticks. It requires the reviewer to understand the system above the level of individual statements. Architecture, contracts and failure modes become the map used to choose where detailed reading is worth its cost.

It also changes what a good pull request contains. The author should provide:

An agent can draft this material, but it cannot certify its own truth by writing persuasively. Commands, logs, screenshots, test reports and links to the actual specification should carry the factual load.

Do Not Move the Work to Somebody Else

Risk-based review is not permission to produce enormous speculative changes and ask a colleague to work out whether they are safe.

Simon Willison’s agentic engineering anti-patterns make the author’s responsibility clear: do not submit code you have not reviewed and then transfer the real validation work to other people. He recommends small changes and evidence such as manual test notes, implementation commentary, screenshots or video.

The author’s obligation to review a change before handing it over does not require equal attention to every line. It means the author owns the quality argument. A reviewer should challenge that argument, not construct it from an unexplained diff.

Small changes remain important because they preserve causality. If one pull request changes the database model, public API, user interface and deployment process, a failure has several plausible sources and rollback becomes a negotiation. An agent may find splitting the work tedious. That is a poor reason to spend scarce human attention on a tangled change.

The costs of evidence are real. Preparing a runtime demonstration takes time. Maintaining ownership and classification rules creates administration. Staging and rollback controls consume infrastructure. But those costs scale with the risk we choose to accept. Reading every line also costs time; it simply hides the expense inside a ritual whose quality falls as the diff grows.

Green Is an Input to Review

Tests provide backpressure, not a proof of correctness. A reviewer needs to ask what the tests could not observe.

Did the implementation agent also write the acceptance test? Were snapshots updated in bulk? Did mocks remove the failure mode that matters in production? Was the service actually started? Did the test run on the reviewed revision? Does a clean result depend on retrying a flaky check?

The same scepticism applies to AI review. A second model can find omissions, security issues and surprising scope. It is cheap enough to make a useful first pass. But two agents can share training biases, accept the same false premise or be misled by the same incomplete specification. Agreement is not independence.

Use automated and agent review to narrow the search space: identify dangerous APIs, map changed data flows, compare the diff with the plan, and flag untested branches. Let deterministic tools enforce what they can. Then spend human judgement on the disputed or consequential parts.

I would still require detailed human code reading when:

The last condition matters. “We can regenerate it if it breaks” is only credible when failure is contained and state can be restored. It is not a strategy for corrupting customer data more efficiently.

Keep a Theory of the System

Even correct code can create a different problem: the software changes faster than the team can understand it.

Margaret-Anne Storey calls the accumulated gap between a system’s structure and a team’s shared understanding cognitive debt. Her proposed mitigations include ensuring at least one human understands each generated change, recording why it changed, and holding checkpoints that rebuild shared understanding.

This does not require that one person memorise every function. It requires a coherent theory at the appropriate level: where state lives, which boundaries exist, what the critical invariants are, and why this change belongs where it does. A team that can only ask an agent to explain its own last output has outsourced the map to a disposable context window.

Architecture notes, decision records, executable examples and focused walkthroughs help. So does deleting unnecessary code. The most efficient line to review is often the one the change did not need to add.

There is a tension here. Demanding complete human understanding of every low-risk generated detail restores the bottleneck we were trying to remove. Allowing architecture to emerge from unexamined diffs creates a system nobody can safely change. Risk classification is how we decide where understanding must be deep and where evidence plus a stable interface is enough.

What I Would Reject

Blanket line-by-line review treats attention as infinite, encourages oversized queues, and focuses reviewers on syntax even when the important error is in the requirement.

The opposite policy, blanket “write-only code”, assumes verification is complete, interfaces tell the truth, and failures are recoverable. Those conditions sometimes hold. They are engineering achievements, not defaults.

Review theatre fails for a different reason: a large agent-generated diff, an agent-generated summary, an agent-generated approval, and a human click at the end still leave the underlying claim untested. More fluent text does not create accountability.

The useful target is evidence-based, risk-tiered review. Read the specification broadly and the tests critically. For ordinary changes, inspect the implementation selectively; where the consequence demands it, go deep. Require somebody to own the explanation and the remaining uncertainty.

Stopping equal review of every line is not lowering the standard. Done properly, it is admitting that review has a budget and spending it where a failure would actually hurt.


Share this post on:

Previous Post
The Agent Harness Is the Product
Next Post
Tests Are Backpressure