Table of contents
- What review actually finds
- The size of the change decides the quality of the review
- One vocabulary for two reviewers
- The review path
- Part I, reviewing as a human
- Part II, directing a reviewing agent
- A checklist both reviewers can use
- Check what you have learned
- Code Review Quiz
- Measuring whether the process works
- Where this fits with the rest of the practice
- Conclusion
- References
Code review: a guide for humans and agents
The content here is under the Attribution 4.0 International (CC BY 4.0) license
Code review is the most repeated quality activity in a software team and one of the least examined. Most teams can state their branching strategy and their test framework, but few can state what their reviews are for, what a comment is allowed to block, or who is accountable for the decision that follows. When a reviewer wants clarity they get a style preference, and when they want a defect found they get a naming discussion. The process drifts because nobody wrote the rules down.
This guide writes the rules down. It is addressed to the two readers who now occupy the same pull request, the human reviewer and the reviewing agent, and it gives them one vocabulary so a finding from either source carries the same weight and the same consequence.
What review actually finds
Fagan formalised code inspection in 1976 as a structured, meeting-based process with defined roles and an exit criterion, and for two decades the inspection was the reference model (Fagan, 1976). What most teams practise today is lighter, asynchronous and tool-based. The empirical record on that lighter practice is worth stating precisely, because three findings reshape what a reviewer should spend attention on.
Mäntylä and Lassenius classified the defects found in nine industrial C/C++ reviews and 23 student Java reviews, 388 and 371 defects respectively, and found that 75 percent of them did not affect visible functionality. The defects that review catches are mostly evolvability defects, the ones that make software easier or harder to understand and modify, and their conclusion is that this is review’s comparative advantage over execution-based quality assurance, which cannot see them at all (Mäntylä & Lassenius, 2009). If your review checklist is a list of bug-hunting prompts, it is aimed at the 25 percent.
Bacchelli and Bird observed, interviewed and surveyed developers and managers at Microsoft and classified hundreds of review comments. Finding defects was the stated motivation, yet defect comments were rarer than expected, and the reviews still paid for themselves through knowledge transfer, shared awareness and alternative solutions. Their central observation is that code and change understanding is the key aspect of reviewing, and that most tools do not serve that need (Bacchelli & Bird, 2013). Understanding is the bottleneck, so anything that reduces the cost of understanding the change is a review improvement.
Rigby and Bird compared review practices across Android, Chrome, three Microsoft projects and AMD, and found convergence on a lightweight, asynchronous process with small changes, few reviewers and fast feedback. Their knowledge sharing measure is the useful one for a reviewer who wonders why they are on the thread: conducting peer review increases the number of distinct files a developer knows about by 66 to 150 percent depending on the project (Rigby & Bird, 2013). The reviewer is learning the system while checking it, which is a return on the review that never appears in a defect count.
| What review produces | What the evidence shows | What it means for the checklist |
|---|---|---|
| Evolvability defects | 75 percent of defects found in the classified reviews did not affect visible functionality (Mäntylä & Lassenius, 2009) | Weight the checklist towards naming, duplication, coupling and error handling, not only correctness |
| Functional defects | The remaining quarter, mostly on paths tests do not reach | Keep the correctness check narrow and specific, so it stays credible |
| Change understanding | Named the key aspect of reviewing at Microsoft (Bacchelli & Bird, 2013) | Spend effort on intent and verification steps before spending it on style |
| Knowledge transfer | 66 to 150 percent more files known per developer (Rigby & Bird, 2013) | Choose reviewers for the file they should learn, not only for the file they already own |
The size of the change decides the quality of the review
A reviewer has one lever that dominates all others, and it is not reading speed. Bosu, Greiler and Bird classified 1.5 million review comments across five Microsoft projects and found that the more files a change touches, the lower the proportion of comments that are of value to the author. They also found that a reviewer’s usefulness rises sharply over their first year and then plateaus (Bosu et al., 2015). The first finding is the actionable one. A large change does not produce a proportionally larger review. It produces a lower quality review, because the reviewer spends their attention on navigation instead of judgement.
The practice the evidence points at is the same one the Google study describes. In a study covering 9 million reviewed changes, over 35 percent of the changes modified a single file and about 90 percent modified fewer than ten files (Sadowski et al., 2018). Small changes are not a politeness the author extends to the reviewer. They are the working condition under which review finds anything at all.
That gives you a number to argue with. A review budget is a team decision, not a law of nature, so pick one and write it down. Mine is this. A change of more than 400 changed lines, or more than ten files, goes back for a split, unless it is a mechanical change, such as a rename or a dependency bump, that a reviewer can verify by pattern rather than by reading. The budget is a request to split, which is why it sits at the Blocker level in the taxonomy below. Rejecting the shape of a change early is cheaper than reviewing it badly and merging it anyway.
Keeping changes small is a consequence of trunk-based development, where the branch lives for hours rather than weeks. If your team still works on long-lived branches, fix that before optimising the review.
One vocabulary for two reviewers
Two reviewers now read the same diff, and a thread where the human says “this is a problem” and the agent says “possible consideration” is a thread where nobody can tell what must happen next. The taxonomy below is the whole protocol. It is short on purpose, because a scale that needs a legend will not be used.
| Level | Meaning | Effect on the merge |
|---|---|---|
| Blocker | Correctness, security, data loss, a broken contract, or a change that is the wrong size | Must be resolved before merge |
| Should fix | A maintainability defect this change introduces | Fix now, or link a follow-up issue in the thread |
| Question | The reviewer cannot establish intent or behaviour from what is present | The author answers; no code change is required |
| Nit | Style or preference that the formatter and linter do not enforce | Author’s choice, never blocks |
| Note | Context, praise, or a related observation | None |
Four rules keep the scale honest. Only a Blocker stops a merge. A Blocker states the consequence, never the preference, so “this drops the audit column and the retention job reads it” is a Blocker while “I would not structure it this way” is not. A Question is a legitimate outcome of a review, because Bacchelli and Bird found understanding to be the hard part, and a reviewer who cannot establish intent has found a real problem with the change. A Nit is raised once, and if the author declines it, it is not re-raised.
The review path
The diagram below is the path I ask teams to walk, and it is the same path whether the second reader is a person or a model. The important parts are the two early exits, because they are the ones that save the most time, and the single decision point at the end, which belongs to a human.
Part I, reviewing as a human
Read in four passes
One pass over a large diff produces a set of reactions, not a review. Separate the passes, because each one asks a different question and conflating them is how a style comment ends up blocking a correctness fix.
- Intent. Read the description and the linked ticket before the diff. Does the change do what it claims, and only that? A change that also refactors an unrelated module is two changes, and the second one has not been described.
- Tests. Read the tests before the implementation, because a test is a declaration of intended behaviour written in a smaller language than the implementation. If you cannot tell what the change is meant to do from its tests, that is the finding. Getting started with testing covers what a test should assert, and test doubles covers the case where the test asserts the mock instead of the behaviour.
- Contracts and failure paths. Look for the boundaries and the errors, which are the places review still finds functional defects. Empty and null inputs, off-by-one at the edges, a caught exception that is logged and swallowed, a resource that is not released on the error path, a migration with no reversal.
- Evolvability. This is the pass the evidence says pays best, and it is the one most often skipped for time. Names that describe what a thing is rather than what it was, duplication that the change introduced rather than moved, dead code and commented-out code, coupling that will make the next change harder, and whether a new failure path will be visible in logs or metrics.
If a pass exhausts your budget, stop and say which pass you did not complete. A partial review that names its own boundary is worth more than a complete-looking review that skimmed.
Write comments that survive the thread
A review comment has one job, which is to change the reader’s mind or their code. Bosu, Greiler and Bird showed that comment usefulness is measurable and that it decays with change size and varies with reviewer experience (Bosu et al., 2015), so the phrasing matters and it is a skill rather than a personality trait. Three habits carry most of the value.
Be specific and citable. Point at the line, state the consequence, and say what you would accept instead. A comment that cannot be acted on without a follow-up conversation costs two rounds instead of one.
Ask rather than assume. “What happens when the list is empty here?” invites an answer and occasionally reveals that the reviewer missed a guard clause three lines above. “This will fail on an empty list” invites a defence and sometimes reveals that it cannot.
Separate the level from the tone. The taxonomy carries the weight, so the prose can stay plain. Escalating the language to escalate the level is how a Nit gets read as an attack, which is the point at which the author stops reading and starts replying.
A worked example, the same finding written badly and well
The change adds a retry around a payment call.
The weak version, Blocking by tone and unclear by content:
This is wrong. We can't just retry a payment, this will double charge customers.
Have you not considered idempotency? Please fix.
The version that can be acted on, Blocker by level and specific by content:
Blocker, payments/charge.php line 84.
The retry re-sends the same request without an idempotency key, so a timeout
followed by a retry is charged twice. Our provider deduplicates on the
Idempotency-Key header, so the smallest fix is to generate the key once per
charge attempt, outside the retry loop, and send it on every attempt.
If the provider does not support it for this endpoint, say so and I will
raise the level change as a product decision instead.
The second version names the consequence, the mechanism, the smallest fix and the condition under which the finding would change. The author can resolve it without a conversation, and a reader can verify the claim against the provider documentation.
The author’s side of the bargain
Understanding is the bottleneck, so the author owns most of the cost of the review before it starts. A description that carries four things makes the difference between a review that reads for fifteen minutes and one that reads for an hour.
- What the change does and why, in two sentences, including the ticket or the decision it implements. If the area has an architecture decision record, link it.
- The scope, explicitly. “This touches the invoice total only, the tax calculation is untouched” saves the reviewer from auditing code the change did not go near.
- How to verify it. The command, the page, the seed data. A reviewer who can run the behaviour gives a better review than one who can only read it.
- What the author is unsure about. A description that names its own weak point directs attention to the highest risk area, which is exactly where the reviewer should spend the budget.
What approving means
Approval is not a statement that the change is good. It is a statement that you can defend it. Concretely, you can explain what it does, why the trade-offs were accepted, and what would tell you it has gone wrong. If you cannot, the honest action is a Question, not a thumbs up. The Google study found that reviewers value speed highly (Sadowski et al., 2018), and speed is compatible with this standard, because the answer is to send a change back for a split rather than to approve a change you cannot account for. That is the same instinct postmortems rely on, where the question is not who wrote it but what the system allowed.
Part II, directing a reviewing agent
What the evidence supports, and what it does not
The most directly relevant study here is a two-phase empirical study at WirelessCar, a field study of current practices followed by a field experiment with two prototypes, one producing an upfront AI review and one answering questions on demand. Both assembled context through retrieval over diffs, source files and requirement tickets. The field study found the real pain points were frequent context switching, insufficient contextual information and a concern about false positives. In the experiment developers generally preferred the AI-led mode, and the preference was strongest for large or unfamiliar pull requests, while it varied with how well the reviewer knew the code base and how risky the change was. Their recommendation is a hybrid of the proactive and on-demand modes, because the combination is what balances efficiency, trust and reviewer control (A\dhalsteinsson et al., 2025).
Read that against the findings in the first half of this guide and the division of labour becomes obvious. Agents are strongest exactly where humans are weakest, which is large or unfamiliar changes, and the failure mode that erodes their usefulness is a high rate of findings that do not survive scrutiny. So the job is not to make the agent thorough. It is to make it precise and to cap it.
The second relevant result is about the code the agent is reviewing rather than the agent doing the reviewing. A study of bugs in LLM-generated code built a taxonomy of the defects those models actually produce and validated it with over fifty practitioners (Tambon et al., 2025). Machine-authored changes have their own characteristic failure patterns, which means a reviewer who treats AI-written code as ordinary code will miss the patterns that are specific to it. Ask the author to say which parts were generated, not to stigmatise it but to aim the review.
| Task | Delegate to an agent | Requires a human |
|---|---|---|
| Summarising a large or unfamiliar diff | Yes | No |
| Running the checklist against a diff that fits in context | Yes | No |
| Flagging missing tests for new branches, unhandled errors, resource leaks | Yes | No |
| Reproducing a failure from a report | Yes | No |
| Deciding whether the change should exist at all | No | Yes |
| Judging whether an accepted trade-off is acceptable for this product | No | Yes |
| Approving the merge | No | Yes |
| Declaring a failing check unrelated and silencing it | No | Yes |
Give the agent what it cannot infer
The WirelessCar result is that missing context, not missing capability, is what makes an AI review unreliable, and their solution was retrieval over the artefacts. Do the same deliberately rather than hoping the diff is self-describing.
- The diff alone is not the change. Supply the ticket and its acceptance criteria, and the source of the files the diff calls into.
- Conventions are invisible unless they are written. An unstated convention is a convention the agent will invent, and it will invent one, confidently. Point it at the conventions file, the linter configuration and the ADR for the area, or tell it explicitly that style is out of scope because a formatter owns it.
- State the scope. “Review the diff and its direct callers, ignore the rest of the module” produces fewer findings you have to dismiss.
- Say what a good finding looks like in your code base. One example of a finding you kept is worth more than a page of instructions about tone.
A review prompt with the rules inside
This is the prompt shape I use, and it is short because the rules do the work. Replace the bracketed parts with real paths and identifiers.
Role: you review a diff. You report findings. You never approve a merge.
Inputs
- The diff, plus the surrounding context of every changed function.
- [path to conventions], [path to the ADR for this area].
- [ticket id] and its acceptance criteria, copied in full.
Rules
1. Every finding cites file and line, in the form path/to/file.ext:LNN, and states
the consequence in one sentence.
2. Use only these levels. Blocking, Should fix, Question, Nit.
3. Report at most 10 findings. If a finding does not change behaviour,
maintainability or the test suite, drop it before reporting.
4. For each finding, give the smallest change that resolves it.
5. Do not report anything the formatter or linter enforces. If you cannot see the
linter configuration, say so rather than guessing.
6. If the diff does not contain enough context to decide, ask one question and do
not speculate in place of an answer.
7. Finish with what you checked and found clean, as a short list.
Output
A table with the columns level, location, finding, consequence, smallest fix.
No summary paragraph, no restatement of the diff.
Rule 3 and rule 7 are the two that change behaviour most. The cap forces the agent to rank, and the clean list tells you what was checked, which is the part a human review never records and the part that makes the next review cheaper.
Guardrails that keep the agent useful
- Cap the output. An uncapped list buries the two findings that decide the merge, and a reviewer who learns to skim an agent’s output has lost the agent.
- Require the location for every finding, so the human can verify without re-reading the diff. A finding without a line is an opinion.
- Never let the agent resolve a thread it opened. Resolution is the author’s act and the decision is the human’s, and an agent that closes its own thread has removed the human from the loop while appearing to keep them in it.
- Never let the agent approve, merge, dismiss a check, or label a failure as unrelated. Those are the four actions that transfer risk to a party that cannot carry it.
- Measure the false positive rate per level and remove the levels that mostly produce noise. If the Question level is never answered, delete the Question level from the agent’s allowed set.
- Accept an empty result. If the prompt implies that a report must always contain findings, an agent that finds nothing will invent something, and that is the fastest way to lose the reviewer’s trust.
The cap, the location requirement and the false positive measure are the same three ideas that make a human reviewer worth reading, which is why the agent is best understood as a junior reviewer with a checklist rather than a new kind of process.
A checklist both reviewers can use
One list, used by both readers, with a default level per item. The level is the default, not the verdict, and either reviewer may raise it with a reason.
| Area | Check | Default level |
|---|---|---|
| Intent | The change does what the linked ticket says, and nothing else | Blocker |
| Scope | The change is within the review budget, or it is mechanical | Blocker |
| Tests | Every new branch has a test that fails without the change | Blocker |
| Tests | The tests assert behaviour, not the implementation detail | Should fix |
| Correctness | Empty, null and boundary inputs are handled | Blocker |
| Correctness | Failure paths are explicit, with no silent catch | Blocker |
| Data | Migrations are reversible, or the irreversibility is stated and accepted | Blocker |
| Safety | Input is validated at the boundary and secrets are not logged | Blocker |
| Evolvability | Names describe what the thing is | Should fix |
| Evolvability | Duplication is introduced, not merely moved | Should fix |
| Evolvability | Dead code and commented-out code are removed | Should fix |
| Observability | A new failure path is visible in logs or metrics | Should fix |
| Documentation | An ADR or README is updated when a decision changed | Should fix |
| Dependencies | A new dependency is justified in the description | Question |
| Style | Anything the formatter or linter already enforces | Not reported |
The evolvability rows are the ones the research says pay, and they are the ones a checklist written around bug hunting leaves out. The two rows that carry most of the weight in practice are the size row and the missing test row, because both are cheap to check and both prevent the expensive version of the review.
Check what you have learned
The quiz below covers the parts of the taxonomy that most often get argued about in a real thread. Each answer explains why, including the ones you get wrong.
Code Review Quiz
Measuring whether the process works
If you cannot tell whether review is working, you will optimise the wrong thing, and the wrong thing is comment count. Metrics for software teams covers the general approach. For review specifically, these are the signals I track, and the trap in each.
| Signal | What healthy looks like | The trap |
|---|---|---|
| Review coverage | Every change to the trunk is reviewed | Coverage without participation still correlates with post-release defects (McIntosh et al., 2014) |
| Time to first review | Hours, not days | Speed measured alone rewards approving a change nobody understood |
| Change size | Most changes under the budget, with a long tail of mechanical ones | Splitting into meaningless commits to hit a number |
| Comments per change | Flat or falling | A rising count usually means nit inflation, and it is easy to mistake for rigour |
| Agent false positive rate | Falling as the prompt is tightened | Raising the cap to “cover everything” buries the finding that matters |
| Defects that escaped | Falling over quarters | Only meaningful if the postmortem loop is honest, and postmortems without review changes are theatre |
The fourth row is the one I would defend hardest. A team whose comment count is falling while its escaped defect count is also falling has understood the taxonomy. A team whose comment count is rising has usually taught its reviewers that volume is the contribution, which is the opposite of the finding that useful comments fall as a change grows.
Where this fits with the rest of the practice
Code review does not stand alone, and most review problems are symptoms of a decision made elsewhere.
- Small changes come from trunk-based development, and a change that cannot be made small is usually a design problem rather than a review problem.
- The tests the reviewer reads first are the same tests that let the pipeline gate the merge, which is the subject of continuous integration and delivery. A change behind a feature toggle can reach the trunk without users ever seeing it, which changes what the reviewer has to guarantee.
- The evolvability checks overlap with software design and refactoring, and the boundary between them is the same boundary between a verdict and a suggestion.
- Pair programming is the synchronous version of the same conversation. It shortens the review round trip rather than replacing the review.
- A defect that review missed is an input to the postmortem process, not evidence that review is pointless.
Conclusion
Two claims are worth carrying away. The first is that code review mostly finds evolvability defects rather than functional ones, which means the checklist a team uses should be aimed at naming, duplication, coupling and error handling rather than at bug hunting, and that a reviewer’s time is best spent understanding the change, because understanding is the documented bottleneck.
The second is that the size of the change decides the quality of the review more than the diligence of the reviewer does. A reviewer who cannot send a change back for a split has been given the responsibility of review without the authority to make it work.
On the agent side, the evidence points at a narrow, well-fed, tightly capped assistant that summarises context and runs a checklist, used hardest on the large and unfamiliar changes where humans do worse. It is a reviewer that should never approve, never resolve its own threads and never silence a failing check, because those are the actions where an error becomes somebody else’s consequence. Give it the same taxonomy as the humans, cap it, measure its false positives, and it earns its place in the thread.
References
- Fagan, M. E. (1976). Design and Code Inspections to Reduce Errors in Program Development. IBM Systems Journal, 15(3), 182–211. https://doi.org/10.1147/sj.153.0182
- Mäntylä, M. V., & Lassenius, C. (2009). What Types of Defects Are Really Discovered in Code Reviews? IEEE Transactions on Software Engineering, 35(3), 430–448. https://doi.org/10.1109/TSE.2008.71
- Bacchelli, A., & Bird, C. (2013). Expectations, Outcomes, and Challenges of Modern Code Review. Proceedings of the 2013 International Conference on Software Engineering, 712–721. https://doi.org/10.1109/ICSE.2013.6606617
- Rigby, P. C., & Bird, C. (2013). Convergent Contemporary Software Peer Review Practices. Proceedings of the 2013 9th Joint Meeting on Foundations of Software Engineering, 202–212. https://doi.org/10.1145/2491411.2491444
- Bosu, A., Greiler, M., & Bird, C. (2015). Characteristics of Useful Code Reviews: An Empirical Study at Microsoft. Proceedings of the 12th Working Conference on Mining Software Repositories, 146–156. https://doi.org/10.1109/MSR.2015.21
- Sadowski, C., Söderberg, E., Church, L., Sipko, M., & Bacchelli, A. (2018). Modern Code Review: A Case Study at Google. Proceedings of the 40th International Conference on Software Engineering: Software Engineering in Practice, 181–190. https://doi.org/10.1145/3183519.3183525
- A\dhalsteinsson, F. S., Magnússon, B. B., Milicevic, M., Davidsson, A. N., & Cheng, C.-H. (2025). Rethinking Code Review Workflows with LLM Assistance: An Empirical Study. 2025 ACM/IEEE International Symposium on Empirical Software Engineering and Measurement (ESEM), 488–497. https://doi.org/10.1109/ESEM64174.2025.00013
- Tambon, F., Moradi-Dakhel, A., Nikanjam, A., Khomh, F., Desmarais, M. C., & Antoniol, G. (2025). Bugs in Large Language Models Generated Code: An Empirical Study. Empirical Software Engineering, 30(3), 65. https://doi.org/10.1007/s10664-025-10614-4
- McIntosh, S., Kamei, Y., Adams, B., & Hassan, A. E. (2014). The Impact of Code Review Coverage and Code Review Participation on Software Quality. Proceedings of the 11th Working Conference on Mining Software Repositories, 192–201. https://doi.org/10.1145/2597073.2597076
About this post
This post was written by a human with AI assistance, which was used for research, outlining, editing and code suggestions. Claims, code samples and references were reviewed before publication, and the sources are cited so you can judge them yourself.