Why do humans review pull requests?

This is a response to that question, in the context of the age of actually-competent AI agents. If we're having agents help write our tickets, help write our code, help test our code, and then review our PRs - why does a human still need to review it? The answer isn't "because our policy says so" - but what is the answer?

What is expected from a PR review?

Before we had our agents, a good code review was supposed to evaluate:

1. Is the purpose of the PR well-explained? (Do you understand what it's doing, why it's doing it, and why it's doing it the way that it is?)
2. Does the change within the PR match its description? (Is the full scope accomplished? Is there extra here that doesn't need to be here?)
3. Does this change meet standards and conventions? (Style guides, architectural patterns, industry norms, good naming, etc)
4. Does this change cause unreasonable issues? (A PR doesn't need to be bug-free at all times, nor does it always need to account for all edge cases, but - knowing what you know, do you see any problems that would be unacceptable if this change merged?)
5. Is this change maintainable? (Does it have adequate test coverage? Does it even need tests? Does it introduce too much maintenance burden or risk?)
6. Is this change the correct level of complexity? (Is it over-engineered? Is it too simple? Does it feel too complicated for the task it's accomplishing?)
7. Can this change be understood by others in the future? (Is it self-explanatory, or is there adequate accompanying documentation to compensate? If not for your current context and awareness, could you understand this if you saw it again years from now?)

Additionally, the review itself is beholden to many of the same guidelines - in and of itself, it also needs to be understandable. It needs to be the right level of detail - no more, no less.

What can agents reliably do for us?

Agents are good at evaluating patterns and branches. They're good at checking rules, matching patterns, and - increasingly - at evaluating if the PR does what it's supposed to, and what it says it does. That means we can use agents for mutch of the above - most or all of the above.

What's left that an agent can't do?

An agent can't do three things:

1. It can't tell you if you feel that the change is worth the added support burden.
2. It can't tell you how confident you are anyone else on your team can understand the change today, or in a week, or in a year.
3. It can't tell you if a change feels like the right level of complexity.

The third one is big. The differentiator between a junior engineer and a senior engineer is experience - it's the wisdom they've gained from seeing things go well, poorly, right, wrong, up, and down - it's a sixth sense of "this smells wrong". Someday, agents might get good at that, too - but we're not there yet.

There's something else along these lines that this doc hasn't discussed, though.

What is the purpose of a PR?

1. To act as a quality gateway.
2. To make sure norms/patterns/rules are followed.
3. To make at least one other person aware of the workings of a change.

That third one can't be handled by an agent - at least, not in the present state of AI-accelerated software engineering. To deliver software with confidence, we still rely on a team of humans understanding the working parts that build up the product. We may no longer need to have a line-by-line memory, or reach quite a level of architectural consistency that we once did - but, we still need to understand the consequences of proposed change into a system. We need to maintain the ability to rough-size the complexity level of a requested change, to know if even entertaining taking on the work is worth the time. We need to be able to have an idea of where the source of a production issue might be, in order to ask our agents the right questions to figure out an answer.

#3 is why we still need humans to do PR reviews. Not just because some SOC-2-compliant policy says so.

What should a human's PR review look like now?

Assuming that an agent is doing its own review following the guidelines established above, a human reviewer's responsibilities shift towards answering:

1. Is the purpose of the PR well-explained? (Do you understand what it's doing, why it's doing it, and why it's doing it the way that it is?)
2. Do the AI comments make sense? (Is it being overly pedantic? Did it misunderstand? Is it requesting changes that are out of scope?)
3. Is the change worth merging? (Is the increasing maintenance burden worth its benefit? Does it cause more risk than it offers in benefit?)
4. Is this change the right level of complexity? (Is it over-engineered? Is it too simple? Does it feel too complicated for the task it's accomplishing?)
5. Can this change be understood by others in the future? (Is it self-explanatory, or is there adequate accompanying documentation to compensate? If not for your current context and awareness, could you understand this if you saw it again years from now?)

We lose three items and gain one, and some get simpler. Assuming a well-configured and well-supported AI reviewer, we can reasonably trust that it will check standards/patterns/norms, and that edge cases are being reasonably checked. We no longer to need to take a depthy technical eye through the whole change - we can instead skim the code and think more about the comments.

A key to remember, here, is that our agents are just highly-automated tools. Their usefulness needs to be constantly evaluated so that their value can be improved.

<- home