Code Review That Actually Catches Things 
 

Almost every engineering team does code review, and most of them get very little from it. The comments cluster around naming, formatting, and the reviewer's personal preference about early returns, while the change that silently removes a tenant check on a query gets approved in ninety seconds with a thumbs up. The ritual is universal; the value is wildly uneven. And the gap is not about reviewer skill — it is about the fact that almost nobody has ever written down what their review process is supposed to catch, which means every reviewer invents their own scope on the spot and the important categories fall between them.

What review is actually for

It is worth separating the four things review is doing, because they have different urgency and different substitutes. First, catching defects a reviewer can see and the author cannot — the second pair of eyes on logic, edge cases, and blast radius. Second, catching the things tests structurally cannot: missing authorization, a data-exposure path, an irreversible migration, a change that is correct but will behave badly at your data volume. Third, knowledge distribution, so that the person who wrote the payments module is not the only person who understands it, which is the benefit that pays off in eighteen months when they leave. Fourth, a shared sense of what the codebase should look like. Only the second of those is genuinely irreplaceable, and it is the one most review processes spend the least attention on. The first is partly covered by tests and types. The fourth is almost entirely automatable, and every minute spent on it manually is a minute not spent on the second. That reordering — automate style, spend human attention on correctness and security — is the single largest improvement available to most teams, and it costs a formatter and a linter.

The things reviewers should be looking for

• Authorization on every new data path. Not 'is there a check' but 'is the check scoped to the right tenant and the right user'. In a multi-tenant system this is the highest-severity class of bug that exists and it is invisible to a test suite that only runs as an admin.
• Data exposure: new fields on an API response, new logs containing personal data, error messages that leak internal state, a debug endpoint that survived the branch.
• Irreversibility. A migration that drops a column, a backfill with no dry run, a destructive operation with no confirmation. These deserve disproportionate scrutiny because the cost of being wrong is not symmetric.
• Behaviour at scale: a query with no index behind it, an N+1 introduced by a convenient ORM relation, an unbounded fetch that is fine with fifty rows and fatal with fifty thousand.
• Failure handling — what happens when the third-party call times out, the job is retried, the response is partial. The happy path is usually correct; the interesting bugs live in the other branches.
• Whether the change matches the intent. Reviewers rarely ask whether the code solves the right problem, and it is one of the few things review can catch that nothing else will.

Size is the variable that determines everything

The most reliable predictor of whether a review finds anything is the size of the change, and the relationship is not gentle. A change of under two hundred lines gets read; a change of a thousand lines gets skimmed and approved, because genuine review of it would take an hour of concentrated attention that the reviewer does not have in the middle of their own work. Everyone knows this and almost every team tolerates large pull requests anyway, usually because the work was not decomposed before it started. The practical fix is upstream of review: split the work at the point of planning rather than at the point of submission, separate mechanical changes from behavioural ones so that a rename of four hundred call sites does not arrive in the same diff as the logic change that matters, and land refactors before or after the feature rather than inside it. Where a large change is genuinely unavoidable — a framework upgrade, a generated-code update — say so explicitly, describe what the reviewer should actually check, and do not pretend it received line-by-line attention. The second variable is latency. A pull request that waits nine hours for review costs far more than the review itself, because the author context-switches, starts something else, and returns to a stale branch. Most teams would get more from a norm of reviewing within two hours than from any change to how they review.

What to automate so humans can stop looking at it

Everything a machine can decide should be decided by a machine before a human opens the diff. Formatting belongs to a formatter with no configuration debate. Lint rules belong in CI as failures rather than as review comments. Type checking should be strict enough that a reviewer never has to reason about whether a value can be null. Static analysis and security scanning — dependency vulnerabilities, secrets committed by accident, obvious injection patterns — should run automatically on every branch, as part of your supply chain controls. Test coverage should be visible but not enforced as a threshold, because a mandated percentage produces tests written to touch lines rather than to assert behaviour. Migration safety checks are worth automating specifically, because the failure mode is severe and the patterns are detectable: flag any migration that drops a column, adds a non-nullable column without a default, or takes a lock on a large table. What is left after all of that automation is a small, high-value set of questions that genuinely require judgement, and a reviewer who is only asked those questions gives much better answers than one wading through a diff full of noise.

The social half of the problem

Review is the most emotionally loaded process in engineering, and teams that ignore that get worse technical outcomes, not just unhappier engineers. A few conventions do most of the work. Distinguish blocking objections from suggestions explicitly, because 'consider extracting this' and 'this will drop rows' read identically in a comment thread and authors cannot always tell which is which. Comment on the code rather than the person, which sounds like a platitude until you notice how often reviews say 'you forgot' where 'this misses' would be equally clear and carry none of the charge. Ask questions when you are uncertain rather than asserting — the reviewer who says 'why is this safe when two requests arrive together?' finds more bugs than the one who says 'this is a race condition' and is wrong a third of the time. Approve with minor comments rather than blocking on preferences, since a round trip for a variable name costs a day of latency for nothing. And be alert to the seniority dynamic in both directions: junior reviewers rarely challenge senior authors, which is where several of the most expensive bugs get through, and senior reviewers can turn a review into an unstructured rewrite request that demoralises the author without improving the code. Naming what review is for, out loud, fixes more of this than any tooling.

Where review is the wrong instrument

Some problems keep appearing in review comments, and their recurrence is the signal that review is not the right place to solve them. If reviewers keep catching the same missing authorization check, the fix is a framework-level default that makes the unsafe version hard to write, not a more vigilant reviewer. If they keep catching N+1 queries, add a test-time detector. If every review contains a debate about structure, the team has an unresolved architecture disagreement that a pull request thread is a terrible venue for. Review is a filter, and any defect class you can eliminate at the source is one fewer thing the filter has to catch. It is also worth being honest that review does not scale to everything: for genuinely high-risk changes — a payment path, a permissions model, an irreversible migration — a fifteen-minute conversation before the code is written is worth more than any amount of scrutiny afterwards, because by review time the design decision is expensive to reverse and the reviewer is under implicit pressure to approve work that already exists.

How Infiniti Tech Partners approaches this

We join client codebases regularly, which means we experience a lot of review cultures from the inside within the first fortnight, and the pattern is consistent: the teams getting value from review have written down what it is for, and the teams that have not are relying on each reviewer's instincts. When we work alongside a team, we tend to bring three things into the process — a checklist scoped to the risks in your system rather than a generic one, automation for everything on it that a machine can decide, and the habit of reviewing within a couple of hours so the loop stays tight. On our own engagements every change is reviewed by a second senior engineer on our side before it reaches your team, because a review queue full of work that is not ready is a tax on your engineers rather than a service to them. We keep a small number of concurrent engagements deliberately, which is part of how that stays true. If your review process has become a formality that everyone performs and nobody trusts, it is usually a week of work to make it useful again — mostly deletion and automation, not new ceremony.