The Shape of the System

The Thousand-Line Trap

Put six reviewers on one enormous change and you do not get six reviews. You get none, and a queue of people each quietly certain someone else did the reading.

reviewing

A developer opened a pull request one Thursday. Twelve hundred lines, give or take, and it touched the payment service and the webhook handler and the cache, all in the one change. The note on it said refactor, said it would shave off some latency. Six people approved it inside two hours. It went out on the Friday. By the Monday the cache was handing back stale data to a thin slice of users, and double-charging some of them too, and this went on quietly until one of those users got fed up and complained somewhere public. The bug was nothing. A missing cache key when one parameter came through empty. That is the sort of thing a reviewer spots straight away, and it was just sitting there in plain view, only it was a long way down the diff. Six people had the file open and none of them had read that far. Each of them had assumed, somewhere below the level you actually think about, that with five other people on the review surely someone else was doing the real reading.

This has a name and it doesn't come out of software at all. Back in 1968 two psychologists, John Darley and Bibb Latané, sat people in a room and had them overhear an emergency they couldn't see, and then watched to see who got up. What they found was not the comforting thing. The more people there were who heard it, the less likely any single one of them was to do anything, and the slower they were when they did. Not because anyone was cruel. It is that responsibility shared out among a lot of people doesn't add up the way you'd hope. It divides. Each person feels a thinner slice of the thing, figures someone better placed will step in, and so in the end nobody moves. Diffusion of responsibility, they called it. And it makes no difference whether the emergency is a stranger gone down on the pavement or a thousand-line diff with a money bug buried in it. One reviewer on a change owns the change. They read it because if they don't, nobody does. Put six on it and that ownership smears out across all of them until there isn't enough left for anyone to feel.

So the speed should have scared everyone rather than putting them at ease. Six approvals in two hours, on a change that moves money through three services, is not six people who thought hard. It is six people having a glance, seeing nothing obviously on fire in their ninety seconds, and clicking the button. And that button means one of two things depending on who you ask. Either I have read this carefully, or I assume this is fine. They look identical. Four letters either way. An LGTM that turns up three minutes after the diff went out isn't an approval at all. It is silence that everyone has quietly agreed to treat as a yes. The bigger the change, the louder that silence gets, because the more there is to wade through, the more sensible it feels to assume the wading was someone else's problem.

And there is a hard physical limit sitting underneath all of this as well. People have measured how well defect-finding holds up as a review gets bigger, and the answer is that it doesn't. Past a few hundred lines, or an hour of attention, whichever comes first, the rate at which a reader catches anything real just falls off a cliff. Comprehension has a ceiling and a big diff goes straight over the top of it. A case-control study of review in the Chromium OS project found the same thing coming at it from the other side: the more files and directories a change touched, the less likely it was that a defect in it got caught. But the size limit is the part most people already half-know, and ignore anyway. The worse half is the social one, the part you can't read off the line count. Adding reviewers to a big change can leave it less reviewed than before, because every extra name on the thing is one more person who now gets to assume the work was done by somebody else.

The fix is almost insultingly obvious and people fight it anyway. Make the change small enough that one person can hold the whole of it in their head at once, and small enough that being the reviewer obviously means being the one who does the reading. A three-hundred-line change has nowhere for responsibility to hide in. Someone is going to read it because there isn't enough of it to assume away. And the pushback is just as easy to predict. Splitting the work up feels slow. The refactor that would have gone out in one satisfying push now has to go as three separate changes over three days, and that feels like ceremony, like friction you've invented for no reason. It is the reverse of that. It is the difference between a change that actually got reviewed and a change that six people waved past while each quietly trusted the others to have done the thing not one of them did.

A review only does anything if someone reads the change, and someone only reads the change if it is plainly theirs to read. A crowd will not give you that. A crowd is the exact place responsibility goes to evaporate. What gives it to you is a change small enough that one tired person can hold all of it, with a single name attached who understands the reading stops at them. The thousand-line pull request is not thorough on account of being big. It is unread on account of being big, and the bigger it gets the more confident everyone is that somebody else has already had a proper look.

In the manifesto, this is tenets (I) and (XXV).

Sources

  • [Cohen 2006] Jason Cohen, "Best Kept Secrets of Peer Code Review" (Cisco MeetingPlace case study). SmartBear Software, 2006. https://static0.smartbear.co/support/media/resources/cc/book/code-review-cisco-case-study.pdf. The 2,500-review, 3.2-million-line study: defect detection holds to roughly 200 to 400 lines and an hour, then falls off; tenets I, XXV.
  • [Darley & Latané 1968] John M. Darley & Bibb Latané, "Bystander Intervention in Emergencies: Diffusion of Responsibility". Journal of Personality and Social Psychology 8(4), 1968. https://doi.org/10.1037/h0025589. Coined "diffusion of responsibility": more witnesses means each is less likely and slower to act; tenet XXV.
  • [Paul et al. 2021] Rajshakhar Paul, Asif Kamal Turzo & Amiangshu Bosu, "Why Security Defects Go Unnoticed during Code Reviews? A Case-Control Study of the Chromium OS Project". ICSE 2021. https://doi.org/10.1109/ICSE43902.2021.00112. The larger the review scope (more files and directories), the lower the chance a defect is caught; tenets I, XXV.

One of a series of field notes on building software for the way minds actually work: tired, distractible, ordinary, and now partly machine. They all lead back to the manifesto behind them, The Shape of the System.