Which Defects a Diff Reader Can Actually Detect

Review is often described by what it catches, and the descriptions are usually drawn from a recollection of the good catches rather than from the structure of the task. A more useful framing is by what the artifact permits: a reviewer holds the changed lines, a small window of context around them, and whatever knowledge of the surrounding system happens to be in memory. Any defect whose detection requires information outside that set is not detected by reading, whatever the reviewer’s diligence. The taxonomy below separates defects by what they demand of the reader, and the boundary between the categories is the realistic ceiling on review as a defect filter.

Defects contained in the changed lines

The defects that reading reliably finds share one property: the evidence for them is inside the hunk. An off-by-one in a loop bound, a condition that is inverted relative to its comment, a wrong variable used in a branch that was just written, a missing null check on a value the same hunk produced, an assignment where a comparison was intended — each is visible in the changed text plus the two or three lines around it. A reader who compares the code against the stated intent can catch these, and this is the class where review earns its reputation.

Fagan’s inspection work in the 1970s established the underlying mechanism for this class: detection depends on the reader forming an explicit model of intent and checking the artifact against it, and detection rates collapse when the reader is given the artifact without the model. The modern equivalent is a diff with a description that states what the change should do. A diff without that description leaves the reader constructing a model from the code, which means the code is checked against itself and the check is circular.

The same property explains why an unfamiliar language costs so much. In a language the reader does not know well, “what does this line mean” is itself uncertain, so the comparison against intent cannot be made precisely. Reviewing a diff in a language the reviewer does not write is close to proofreading for spelling: only the most superficial defects are accessible.

Defects that need context the diff does not carry

A second class is detectable only by a reader who knows something outside the hunk, and this is where most of the interesting failures live.

Invariant violations. A function that assumed its argument was sorted, called from a new site that does not sort. The changed line is the call; the broken assumption is in a function whose body is not in the diff. Nothing in the hunk is wrong. The defect is in the relation between the hunk and code the reviewer must recall.

Contract mismatches across modules. A new caller that passes a nullable value to an interface that documented its parameter as non-null. The interface’s documentation is not in the diff. Detecting this requires either the reviewer’s memory of the interface or the ability to search for its definition, which is why review tools that permit jumping to definitions get more of this class than tools that show a fixed context window.

Resource lifecycle. A new code path that opens a handle and returns early on an error without closing it. This one is sometimes visible — the early return is in the hunk — but only if the reader also sees that the handle was acquired earlier and tracks it through the branch. The larger the function, the less likely the tracking succeeds, which is the mechanism behind the empirical finding that defects cluster in large functions: not that large functions are written worse, but that they exceed the span over which a reader can hold state.

Concurrency. A shared field written without the lock that protects it, or a check-then-act sequence that is correct single-threaded and racy under interleaving. The hunk shows one thread’s view. The defect exists in the space of interleavings, and reading a single path through the code cannot enumerate them. Reviewers do catch some of these, but they catch them by pattern recognition against a known-bad idiom, not by reasoning about the interleavings — which means unfamiliar locking disciplines are effectively invisible.

Error paths that were never exercised. A handler for an exception that the underlying call cannot actually raise, or a fallback branch that returns a value of the wrong shape. The reader can see the branch; what they cannot see is whether it is reachable, and whether its return type matches what the caller expects across all paths.

Where review’s signal is genuinely weak

Performance and resource-consumption defects form a category of their own. An allocation inside a hot loop, a query issued per row, a string concatenation in a recursion — each is a defect only relative to a load profile the reviewer does not have. The code is correct. Its cost is the problem, and cost is not visible in the source without either measurement or a model of the workload. Review can catch the egregious cases, such as an obviously quadratic loop over a collection whose size is known to be large, but the boundary between “fine” and “too slow” is a property of the deployment, not the diff.

The same is true of security properties that depend on data flow, which is the class analysis tools are built for. A tainted value reaching a sink is a graph property over the whole program. A reviewer looking at the sink sees a call that is correct given its arguments; whether the argument can be attacker-controlled is a question about callers, and callers are not in the hunk. This is precisely the class that automated taint analysis is built for, and it is also the class where a review comment tends to be vague, because the reviewer senses the shape of the problem without being able to construct the path.

Missing functionality is its own case. The diff shows what was added; nothing shows what was omitted. A change that implements two of three required cases reads as complete. The only defences are a specification to check against and a reviewer who knows the domain well enough to notice the absence, and absence is much harder to notice than presence.

What the distribution implies for process

If the detectable classes are concentrated in the local category and the valuable ones are spread across the contextual category, several process conclusions follow, and they are not the ones usually drawn.

The first is that a review’s yield depends heavily on the reviewer’s prior knowledge of the code being changed. A reviewer who has worked in the module can supply the context the diff omits; a reviewer who has not is restricted to the local class. This is the argument for reviewer selection based on familiarity rather than on seniority or rotation, and it also explains why a review by someone unfamiliar produces many style comments and few semantic ones: style is the only thing assessable without context.

The second is that the description of a change is not a courtesy. It is the input that makes the local class detectable at all, because detection is comparison against intent. A change with no stated intent can only be checked for internal consistency, which catches almost nothing.

The third is that review cannot substitute for execution. The classes it cannot reach — reachability, interleaving, cost, data flow — are exactly the classes that tests and analysis tools target, and the division of labour is structural rather than a matter of maturity. A process that removes testing because “review catches it” is removing the only mechanism aimed at the majority of the defect space.

The asymmetry between finding and judging

One further property of reading deserves attention because it shapes review comments more than any other. Detecting that something is wrong is easier than deciding what is right. A reviewer may notice that a retry loop has no backoff without being able to say what backoff is appropriate, because the appropriate value depends on the failure mode of the remote service. The comment that results is either a question or a suggestion, and both are weaker signals than a definite defect.

This asymmetry is why review discussions so often stall on questions of degree rather than of correctness, and why a review process that counts comments as a proxy for defects measures the wrong thing. The classes above are the ones where a definite statement is possible: this path leaks, this branch is unreachable, this index is off by one. Those are the comments worth acting on, and their scarcity relative to total comment volume is a property of the task, not of the reviewers.

What a reviewer’s approval therefore means, and what it does not, is a separate question about the semantics of sign-off.