Review Metrics and the Confounders That Invert Them

Review produces a large amount of event data, and almost all of it is easy to compute and hard to interpret. The reason is structural: every review metric is a function of at least three things — the quality of the review, the shape of the change, and the capacity of the people doing it — and the three are not separable from the numbers alone. A metric that improves while review quality degrades is not a rare pathology; it is the expected outcome of optimising any of the usual quantities. What follows separates the commonly used measures by what they can and cannot distinguish.

Latency is a queueing quantity, not a diligence quantity

The most used measure is time to first response: the interval between a change becoming reviewable and the first comment or approval. It is also the measure most often misread.

The interval is dominated by the reviewer’s queue, not by the reviewer’s reading speed. A change that waits nine hours for a first look and then takes twenty minutes to read has a latency of nine hours and twenty minutes, of which the reading is four percent. Reducing the reading time cannot move the metric; only reducing the wait can.

Little’s law makes the relationship exact in steady state: the average number of changes awaiting review equals the arrival rate times the average wait. A team that reviews twenty changes a day with an average wait of six hours has, by that identity, five changes in flight at any moment. If the arrival rate doubles and the reviewer capacity does not, the wait doubles; the reviewers have not become less diligent, and the metric has moved by a factor of two.

Two consequences follow. First, latency comparisons across teams are comparisons of load relative to capacity, and a team with fast latency may simply be underloaded. Second, latency can be improved without improving review at all, by adding reviewers, by shrinking changes so that reading is cheaper, or by pre-empting the queue for small changes. The last of these is worth naming because it inverts the incentive: if small changes get priority, authors learn to split changes, which is usually good, but they also learn to game the size threshold by moving code into separate changes that are individually small and collectively incoherent, which is bad and invisible in the metric.

The distribution matters as much as the mean. Review latency is heavy-tailed, because a change can sit behind a reviewer’s vacation, a holiday, or a large unrelated change. A mean of four hours with a ninety-fifth percentile of three days describes a system where a fraction of changes experience a wait that the mean conceals entirely, and the authors of those changes experience the process as unpredictable rather than slow. Percentiles, not means, are the interpretable summary.

Comment density measures disagreement, not defects

Comments per hundred lines is a common measure of review thoroughness, and it is a measure of how much the reviewer had to say, which depends on the reviewer’s standards, the code’s novelty, and the project’s comment culture at least as much as on the code’s quality.

A high density can mean the reviewer found many problems. It can also mean the reviewer has strong opinions about formatting, that the code is in an unfamiliar idiom so every line invites a question, or that the project’s norms encourage discussion. A low density can mean the code is clean, or that the reviewer skimmed, or that the reviewer’s comments were made in a side channel and never recorded.

The measure has a specific failure mode under pressure: it is trivially improved by commenting more, and comments about naming and formatting are cheap to produce. A team that sets a target for comment density gets more comments about naming and formatting. This is not a hypothetical; it is what Goodhart’s observation predicts, and the mechanism is that the cheap comments are the ones available without understanding.

The usable variant is comment classification: tag each comment by what it asks for, then look at the distribution. A change that draws four comments about a missing edge case is different from one that draws four about indentation, and the counts are only meaningful after the split. Classification is manual work and does not scale to every change, which is why sampling is the practical approach: classify a few hundred comments periodically and read the distribution rather than tracking it continuously.

Approval count and the missing denominator

Approvals per change is easy to compute and nearly uninterpretable without knowing how many changes were not approved, and why.

A change that is approved on the first revision and one that is approved after four rounds both contribute one approval. The second consumed far more reviewer time and indicates something the first does not. Revision count per change is a better proxy for friction, with the caveat that revisions include rebases and mechanical updates, so it overcounts when the base is moving and undercounts when an author responds to an unrecorded side-channel comment by pushing a new revision with no discussion.

The count also interacts with what an approval asserts: if approvals are carried across revisions without re-review, the count measures attendance rather than examination. A team whose policy requires two approvals and carries them forward across substantial rewrites has a metric that reports two and a process that examined one.

Reviewer load is the denominator that is usually missing. A reviewer handling fifteen changes a day and one handling three are both counted as one approval each, and the time available per change differs by a factor of five. Any aggregate that sums approvals without weighting by the reviewer’s concurrent load is measuring the number of events, not the amount of review.

Time-to-merge and the coupling to everything else

Total time from first commit to merge is the metric most often presented to management, and it is the one most confounded, because it includes every stage: authoring, review wait, review rounds, continuous integration, and the merge queue.

A drop in time-to-merge can come from a faster queue, from a smaller change, from a reviewer who happened to be free, or from authors abandoning changes that were going to be contentious. The last is the dangerous one: a change that is withdrawn before review never contributes to the metric, so a process that discourages ambitious changes improves the metric by removing the changes that would have been slow. Nothing in the data shows the removal.

Decomposition helps here, because it separates the stages. Time from reviewable to first response measures the queue. Time from first response to approval measures the review rounds. Time from approval to merge measures the merge infrastructure. Each of the three is attributable to a different mechanism, and a single end-to-end number is the sum of three quantities that can move independently and in opposite directions.

Defect escape rate is the only outcome measure, and it is lagging

Everything above measures the process. The outcome is whether defects reached the mainline, and the standard measure is the rate at which changes are subsequently reverted, hot-fixed, or found to be the cause of an incident.

This measure has three problems that no amount of tooling removes. It is lagging, so a process change takes weeks to show in it. It is sparse, so the counts per team per month are small and the confidence intervals are wide. And attribution is unreliable, because a defect may be introduced by one change and manifest after three others have built on it, so the change that “caused” the incident is a matter of where the investigation stopped rather than where the defect entered.

The version that is usable is a rolling count of changes that required a follow-up fix within a fixed window, restricted to changes that were reviewed, compared against changes that were not. The comparison is the informative part: it answers whether review is filtering anything, which is the only question the outcome measure can address. It cannot tell which review practices work, because the practices are not randomly assigned across changes.

What can be measured honestly

Three quantities survive the confounders well enough to be worth tracking continuously.

Review latency percentiles. The median and the ninety-fifth percentile of time to first response, read as a queueing property. Interpretable, actionable, and directly connected to reviewer capacity.

Change size distribution. The distribution of lines changed, since size is the strongest single predictor of how much a reviewer can actually examine, and the distribution shows whether the process is producing reviewable units. A median of forty lines and a tail at nine hundred is a different system from a median of two hundred. The distribution also shows whether changes are being split into units a reviewer can actually hold, which no comment count can reveal.

Revision count per change, split by cause. Revisions due to review comments are friction in the review; revisions due to base movement are a property of the branching model; revisions due to failing checks are a property of the verification pipeline. Splitting them turns one noisy number into three interpretable ones.

Everything else is either a proxy for these three or a measure of something the process does not control. The general rule is that a metric is interpretable when the mechanism producing it is understood well enough to predict which way it moves when the underlying quality changes, and uninterpretable when two mechanisms move it in opposite directions. Most review metrics are in the second category, and the discipline is to notice which.