Software Development

Code Review Broke When Our PR Volume Tripled

23 views
code-review-broke-when-our-pr-volume-tripled
Share :

Our pull request volume tripled last year. Our review capacity did not move.

For about four months I read that as a productivity win. More PRs opened, more merged, cycle time flat. On a dashboard, that is a team getting faster.

What was actually happening is that reviews got shallower. A reviewer facing eleven open PRs reads differently than one facing three.

My first instinct was that this was a staffing problem, so I spread review load wider and added a second required approver on critical paths. That made it worse. Two shallow reviews are not better than one careful one. They are slower, and they create shared responsibility, which in practice means nobody feels accountable for the approval.

The real problem was that our review process was built for a world where writing code was the expensive step. That stopped being true, and we kept the process anyway.

The signal was not in the throughput numbers

Throughput looked healthy the entire time. What told the story were the things we were not tracking.

Comments per PR dropped sharply. Large changes started coming back approved in under ten minutes. Nobody reads three hundred lines that fast; they scan it. First-submission approval rate climbed toward a number that should have worried me. And rework, the follow-up commits that fix something within two weeks of merge, went up.

That last one is where the cost lives. The defects did not disappear. They moved downstream, where they are more expensive and arrive as a separate ticket instead of a review comment.

Generated code is harder to review, not easier

I expected more bugs. I got fewer obvious ones and more expensive ones.

The mechanical defects mostly vanished. Null checks, off-by-one errors, unhandled rejections, missing type guards. Modern tools handle that layer well and lint catches the rest.

Duplication across boundaries: A generated utility does not know your repository. We found four separate currency formatting helpers scattered across different directories, each correct on its own, each rounding slightly differently at the fourth decimal. On a financial platform that is not a style issue.

Error handling that looks complete: A try/catch around the right block, a log line, and execution continuing into a state that should have halted. It reads as careful. It is the opposite.

Tests that assert the implementation: Coverage went up while confidence went down, because the tests described what the function did rather than what the feature required. They survived refactors that broke behaviour.

The wrong abstraction, cleanly executed: The most expensive category, because nothing about the diff itself is wrong. The problem sits between the diff and the rest of the system, and you do not see that by reading changed lines alone.

Author confidence stopped being a signal

I used to rely on a simple heuristic. A clear explanation of the tradeoff in the description meant I could review lighter. A thin description meant read closely.

That broke. Descriptions got better on average, because descriptions are easy to generate. And I started hearing an answer I had never heard before when I asked why an approach was chosen: some version of “that is what it produced.”

That is not a knock on the engineer. It is what happens when a tool produces working code and the deadline is Thursday. But it means review is now the first point at which anyone has reasoned about the design, and most review processes were never built to carry that weight.

What we changed

The first fix was a design note, written by a human, before review opens. Three lines in the PR description: the approach taken, an approach rejected and why, and one thing the author is unsure about. It costs about ninety seconds. The “unsure about” line has quietly become the most useful sentence in most of our PRs. If the author can’t write it, the PR isn’t ready, regardless of who or what wrote the code.

The second was a hard cap on reviewable diff size, enforced in CI. This one did not land cleanly. Two of our strongest engineers pushed back hard for the first month, arguing it punished the people shipping the most. I did not have a great counter beyond “give it six weeks.” It took close to that long before the rework numbers came down enough to change their minds, and one of them still splits PRs more reluctantly than I’d like.

The rest followed once those two were in place: reviewers now check for duplication, boundary violations, and failure-path handling instead of reading line by line, and everything mechanical — formatting, types, lint, coverage, bundle size — is enforced in CI before a human ever opens the diff.

What I would do differently

Track review quality before volume becomes a problem. Comments per PR and rework within two weeks of merge are both cheap to measure, and either would have flagged this in week three instead of month four.

Set the size cap before volume grows, not after. Doing it early is a formality. Doing it once people are used to shipping big PRs is a fight, as we found out.

Don’t add reviewers first. More reviewers on an unchanged process just produces more shallow reviews.

And say out loud, early, that review is now the constraint. We spent a quarter with people optimizing their own output while the queue grew behind them.

The question I would ask first

Can every reviewer on your team state, in one sentence, what they are personally responsible for catching?

When we asked, four engineers gave four different answers. That told me more about the state of our reviews than any metric did.

We have not solved this so much as made it visible enough to manage. Review is the limiting factor on my team’s throughput now, and I don’t expect that to change soon.