Review as a Gate
Human review reliably catches some classes of defect and reliably misses others; treating it as a general safety net is how the missed classes reach production.
The question, the obvious approach, and why it breaks
Every lesson starts where the work starts: an operational problem, a first attempt that is entirely reasonable, and the way production disagrees with it.
What does code review actually catch, what does it provably not catch, and what should therefore be checked some other way?
Review is the most universally adopted quality gate in software and the least examined. Teams treat "two approvals" as a statement about risk, when it is a statement about attention — and attention is unevenly distributed across defect classes.
Require review on every change. A second pair of eyes catches mistakes, so more reviewers and more required approvals means fewer defects reaching production.
Reviewer attention does not scale with diff size. Past a few hundred changed lines, comments shift from logic to style, because logic review requires holding the whole change in working memory.
- Reviewer attention does not scale with diff size. Past a few hundred changed lines, comments shift from logic to style, because logic review requires holding the whole change in working memory.
- Reviewers cannot see the classes of defect that only exist under production conditions: lock duration on a large table, behaviour under concurrency, cost per request at real traffic, tail latency (Why Local Success Predicts So Little).
- Required approvals from people without context produce rubber stamps. The approval is recorded; the review did not happen.
- Review as a bottleneck lengthens the feedback loop, which pushes people toward larger batches — which makes review less effective. The gate degrades its own inputs.
- A green review on a stale merge base tells you about a code state that no longer exists (Required Checks).
What is actually happening
Underneath the tooling, which is the part that survives a change of tool.
- Review is a human reading a diff with a model of intent. It is strong exactly where a model of intent is the missing ingredient and weak where the missing ingredient is execution.
- Humans are good at: does this do what the ticket says, is this the right place for it, is this name misleading, has an edge case been ignored, does this violate an invariant the reader knows about, does this smell like a security pattern they recognise.
- Humans are bad at: anything requiring execution against real data or real concurrency, anything requiring measurement, anything where the defect is an absence rather than a presence, and anything in a diff too large to hold.
- Automated checks are the mirror image: excellent at execution and measurement, blind to intent. That is why review and CI are complements rather than redundant layers (Required Checks).
- The strongest reviews are ones where the reviewer has context — they own the service, or they wrote the code being changed. CODEOWNERS-style routing is an attempt to make that structural rather than accidental.
What review catches, by defect class
Honest accounting matters here because the alternative is a gate everybody trusts for things it cannot do. The right reading of this table is not "review is weak" — it is "review and execution-based checks cover disjoint sets, so having one does not substitute for the other".
| Defect class | Human review | Automated check | Production verification |
|---|---|---|---|
| Wrong intent — does not do what was asked | Strong | None | Late and expensive |
| Misleading name, wrong abstraction, wrong layer | Strong | Weak | None |
| Missing edge case the reader knows about | Strong | Only if a test exists | Sometimes, as an error |
| Recognisable insecure pattern | Moderate | Strong for known patterns | Only via exploitation |
| Type and contract errors | Weak | Strong | Too late |
| Behaviour under concurrency | Very weak | Weak — hard to test deterministically | Where it actually appears |
| Lock duration on a large table | None — depends on row count | Only via a rehearsal against real volume | Where it appears, as an outage |
| Performance regression | Very weak | Only with a benchmark gate | Canary comparison (Canary Analysis: Compared Against What?) |
| Cost per request | None | Rarely instrumented | The bill, or a canary cost signal |
| Integration wiring against real dependencies | Weak | Moderate with real dependencies | Strong |
| Config correctness for the target environment | Weak — the reviewer sees one environment | Strong with schema validation (Validate at Startup, Fail Clearly) | Immediate and loud |
The size effect
Every experienced reviewer has noticed the same thing: a twenty-line diff gets substantive questions about logic and a two-thousand-line diff gets "LGTM" or a comment about a variable name. The mechanism is that reviewing logic requires holding the change in working memory, and past some size that stops being possible, so attention falls back to what can be judged locally.
The practical consequence is that diff size is a property the author controls and the reviewer cannot compensate for. This is why change size shows up as a delivery lever and not just an aesthetic preference (Change Size: Why Small Changes Are Safer, and When They Are Not).
- 1Small — one concern
The reviewer reconstructs intent and checks it against the code.
fails by Reviewed in isolation from the larger design it belongs to.
evidence Comments are about behaviour and edge cases.
- 2Medium — one feature
The reviewer follows the main path and spot-checks the rest.
fails by Branches off the main path get less attention than the main path.
evidence Comments cluster on the primary flow; error handling gets fewer.
- 3Large — feature plus refactor
The reviewer reads for structure and samples the detail.
fails by The behaviour change hides inside the refactor noise.
evidence Comments become structural and stylistic; logic questions drop off.
- 4Very large — a branch's worth
The reviewer checks that it looks like the described change.
fails by Approval becomes a statement of trust in the author, not a review.
evidence Approval arrives fast, with few or no comments — the diagnostic signal.
Fast approval with no comments on a large diff is not efficiency. It is the observable signature of the gate having stopped functioning.
Deciding what needs a human
Once you accept that review is a specific instrument rather than a general net, the question becomes which changes justify the latency, and what should be routed differently.
A change is ready. What human attention should it get before it can reach production?
when Generated code, dependency version bumps with a passing suite, or content changes with no code path.
cost You are trusting the checks completely; a gap in them ships unnoticed.
when The normal case: a change within a service, by someone who works on it.
cost Latency proportional to that person's availability; concentrates load on owners.
when The change crosses into a service or a directory owned by another team.
cost Cross-team latency, and a real bottleneck if the owning team is stretched.
when Schema migrations and anything destructive — reading is not enough, it must be run against production-scale data (Destructive Migrations).
cost Needs a realistic dataset to rehearse against, which is itself work (Production Data in Lower Environments).
when Authentication, authorisation, secret handling, or a new trust boundary.
cost A specialist queue; worth reserving for changes that genuinely move a boundary rather than every diff that touches an auth file.
How to do it properly
Most important first.
- Keep diffs small enough to be reviewable. This is the single highest-leverage change to review quality and it is a property of the author, not the reviewer (Change Size: Why Small Changes Are Safer, and When They Are Not).
- Route review to people with context, not to whoever is available. An owner-based rule turns "someone approved" into "someone who will be paged for this approved".
- Move every checkable property to an automated check, so review attention is spent on the parts only a human can do (Required Checks).
- Say what the change does in the description, including the rollout shape and the rollback path. A reviewer who has to reconstruct intent from the diff reviews the diff, not the change.
- Review the migration and the config change with more care than the application code — they have a larger blast radius and less test coverage (Destructive Migrations).
- Treat review latency as a delivery metric. A review that takes two days changes how people batch work.
How much can this affect
Every production change has a blast radius. Stated as a scale so it is comparable between changes rather than adjectival — and paired with what actually contains it, because a wide scope with a real containment mechanism is a different situation from a wide scope with none.
Whatever is downstream — canary analysis, progressive rollout, flags and rollback. Review itself contains nothing; it only reduces what arrives.
What can go wrong
- Approval theatre: the process is satisfied, the reading did not happen, and everyone now believes the change was reviewed.
- Review used as a substitute for tests, so the same reasoning is repeated by hand on every change instead of encoded once.
- A single reviewer becoming the bottleneck for a whole service, which is also a single point of knowledge.
- Required approvals used to enforce separation of duties in a way that makes the fastest path to production an emergency bypass (Break-Glass Access).
- Review comments about style consuming the attention budget, so the design problem in the same diff is never raised.
- "Review caught it, so review works." Survivorship. The interesting number is what review does not catch, which by definition nobody sees at review time.
- "More required approvals means safer." Beyond one contextful reviewer, additional approvals mainly add latency and diffuse responsibility.
- "Review is where quality happens." Review is where intent is checked. Quality is distributed across tests, types, canaries, flags and observability (A Successful Deploy Is Not Evidence of a Healthy System).
- "The reviewer is responsible for the defect." The system that let an unreviewable diff through is responsible. Blaming the reader is the "human error" answer wearing a different hat.
Operating it
Evidence is the signal, not the intention. Rollback is sometimes 'you cannot, and that is the point'.
- Approvals are recorded against a specific commit, and the merge base has not moved since.
- Defects found in review are logged by class, so you can see whether review is catching what you think it catches.
- Median time to first review comment is measured, and it is short enough that authors do not start something else.
- Post-incident, review is honestly assessed: could a reader have seen this? For most production incidents the answer is no, and that is information about the gate, not about the reviewer.
- Review has no deployment to roll back; its failure mode is a defect passing through, which is recovered downstream by canary, flag or rollback.
- A bad review *policy* is reversible: required-approval counts and ownership rules are configuration, changeable in minutes.
- What is not reversible is the culture effect. A team that has learned approvals are ceremonial takes a long time to unlearn it.
- Automate: formatting, linting, type checks, dependency policy, secret scanning, migration linting, test execution, coverage on changed lines, and diff-size warnings.
- Automate the routing: owner-based reviewer assignment, so the right person is asked without anyone remembering to ask.
- Keep human: design judgement, naming, whether the change matches its intent, and whether an edge case matters (How to Automate Something).
- Never automate approval of destructive changes. A migration that drops a column should require a person who understands the data (Destructive Migrations).
- Review adds latency between writing and shipping, which is the exact quantity every other lesson in this module wants to reduce.
- Owner-based routing improves quality and concentrates load on the people who own the most.
- Mandatory review on every change treats a one-line copy fix and a schema migration identically, which spends the same attention on very different risk.
Where this applies
This domain is unusually tool- and organisation-dependent. These labels say what each claim is specific to, and what a different platform, provider or organisation does instead.
- GENERALThe split between what human reading catches and what only execution catches holds across languages and stacks. Strong static typing shifts the boundary — the compiler takes some of what review used to do — but does not move review into the execution-dependent classes.
- ORG-SPECIFICWhether review is mandatory, advisory, or a compliance control is a policy choice. In a regulated environment the recorded approval is itself the deliverable and cannot be dropped even where it adds little defect-finding value; elsewhere pairing can substitute for it entirely.
Where the depth lives
This domain teaches delivery and operation, and hands the mechanism off to the domain that owns it.
- — Testing & Reliability Engineering — which classes of confidence come from tests rather than from readers, and what a test suite is actually evidence of.