It's time to rip off the band-aid and stop performing code reviews
For everyone who just got a little tense reading the headline: I get it. I was there too, not so long ago. At the beginning of this year I spent a month building what was at the time–and might still be–the best AI-aided code review tool in the world. (The screenshot above is from that.)
And now I don't do code review in the classic sense at all.
The World is Changing
In human-driven software development, code review served three primary purposes:
- Knowledge dissemination. It's not good if only one engineer has the understanding required to make changes to a given piece of code without breaking things. Code review is a good way to ensure that at least one other member of your team understands the code, and the "whys" behind it.
- Training. Having a senior engineer review a junior's code helps the junior improve for next time.
- Defect detection and technical debt avoidance. Conventional wisdom is that having someone who didn't write the code review it is one of the least expensive and most effective ways to do QA.
Code review is no longer necessary for the first two, and only partially effective for the third:
- We can get up to speed on what's going on in arbitrary code so fast with AI assistance that the tradeoff that code review makes (hours or days reading each PR before merging–"big review up front", as the agile people might say) no longer makes sense; just derive what you need when you need it in your favorite harness.
- Models don't learn across sessions (although ~every big lab is trying to solve this and I expect they will in a single-digit number of years). And giving feedback to a junior engineer on code his AI wrote is just not valuable enough to justify the time.
- Technical debt is nearly a nothingburger now. If you went in the wrong direction, just... change the code to go in the right direction. This is the YAGNI principle applied at a higher level: don't spend time doing expensive things now that might not even be necessary, when the cost of doing it later is no higher than doing it today. (Actually, with models getting smarter month to month, it will often be lower.)
In other words: the optimists were right. New models really can clean up technical debt created by last-gen models. Yes, they still make mistakes, but fewer with each new release. Fable 5, and to a lesser degree Sol 5.6 and K3, are very, very good at improving code in a way that we didn't see before.
That leaves defect detection.
How do you know if the code is any good, if you haven't read it?
First of all, let's level-set: code review was never amazing at finding defects. It was better than nothing, sure, but the best studies showed about a 35% defect detection rate, and often lower in specialized domains like security.
We're engineers. We automate things. How can we apply that to defects? (If only there were a vast empirical literature about software QA!)
You can write code to verify code
With intelligence-on-demand you have options that you didn't before. I'd go so far as to say that QA is one of the most exciting fields in software engineering right now. We have literally decades of research on how to make our code robust, even really hard-to-reason-about distributed systems code, and most of it was completely impractical in The Before Times because of the prohibitively high level of changes you needed to make to your project to structure or instrument it appropriately.
That problem just doesn't exist anymore. So it's crazy to me that almost nobody has started pulling in these techniques yet.
But code authoring is only part of it
I think many engineers have a bit of a blind spot here because the only time they use inference is in a coding harness or a web chat UI. But you can connect models up to all kinds of problem shapes!
For instance: besides distributed testing, another set of tools that has been around for decades and seen relatively low adoption is static analysis. Coverity has been around since 2002! But we gravitated instead towards simpler, less sophisticated tools like Ruff and Clippy because more ambitious checks were too noisy in real codebases to be useful.
But now you have a tool that can literally wade through the noise and decide what's actually relevant. (You might be tempted to just say, "Code is cheap so let's just fix all warnings everywhere," but this will still do ugly things to your code and make it worse, not better. Judgement is still required.) I've written two different custom analysis pipelines for Bifrost that both feed into GPT to classify which should be ignored, which should be autofixed, and which should be escalated to a human.
What then is the "right" way to spend your "review" budget?
Again, I'm not arguing that you shouldn't be involved with the details. Humans still need to understand architecture, invariants, and intent. What I'm arguing is that manually reading diffs, even condensed diffs, is no longer the best way to achieve or maintain that understanding.
/plan is your friend
I thought at first that maybe with Fable and Sol I don't need to rely so much on /plan, and it's true that they don't need it nearly as much as older models did at a tactical level. But /plan is how you surface and resolve ambiguities in your instructions, and you should only skip it for the most mechanical and straightforward changes. You'll be surprised at what the model thinks is ambiguous, and glad that you didn't let it take its best guess unaided.
This is also the best place to review external commitments that are hard to change: persisted data formats, public APIs, protocols, and so forth.
Talk to the code
Here's an example from this morning of what I'm talking about. I'm building Hel, a harness-of-harnesses, and I realized that once a session grew to hundreds of thousands of events, the worker<->controller protocol was so bloated and slow that it took minutes to reattach.
This is the kind of thing that I would have caught during review 100% of the time. Does that mean that skipping review was a mistake? No, because doing that level of review for every commit would mean that I'd be barely getting started instead of being almost done with 1.0 after four days.
So instead, having discovered the problem, I went into /plan and said,
Let's start over. Given what you know about our goals for the dashboard, what events do we need from the worker and how should we represent those? (raw event stream? aggregated summary every 100ms? something else?)
It replied with a reasonable design, but I saw some things that didn't quite line up, so I replied,
We want to show "unread while you were detached"; I think the code may have drifted from that. Map your proposed structure to the problem/feature it drives so I can check for other drift.
And that surfaced several more things to clean up.
Use human time wisely
Until ~2024, code reviews were a valuable tool to spread knowledge, mentor your team, prevent technical debt, and fix bugs early. But the ease with which we can make changes, and the way we do it, have changed such that manual, routine diff review is simply a poor use of senior engineering time. If I were looking for work as an IC, I would avoid orgs that still practice this as a kind of make-work nearly as offensive as gratuitous meetings.
At most tech companies, human engineering time is by far your most expensive resource. Don't waste that time reviewing diffs manually. Spend that time instead building new systems that catch regressions even better than human eyeballs.