How to Review a 900 Line AI Pull Request Without Losing Your Mind

A 900 line pull request used to be a scheduling problem. Somebody spent three days on it, so you set aside an hour, and the size at least told you roughly how much thinking had gone in.

That correlation is gone. A 900 line diff can now be twenty minutes of prompting, and the size tells you nothing about how much judgement it contains.

Review practice has not caught up. Most teams are applying habits calibrated for human authored code to a volume and a failure distribution that are both different.

Why the usual approach fails

Research on code review has been consistent for years that effectiveness falls off sharply past a few hundred lines. Attention degrades, and past a point reviewers start pattern matching rather than reading.

Agent written code makes that worse in a specific way: it reads well. Consistent naming, even structure, no rough edges, correct formatting. Reviewers unconsciously allocate scrutiny to code that looks messy, so uniformly tidy code gets less of it.

I have started treating smoothness as a signal in itself. If a diff touching a genuinely gnarly part of the system comes back symmetrical and clean, the gnarl was probably ignored rather than resolved. Real code that has survived production usually has a weird branch with a comment explaining an incident.

Ask for the diff to be smaller

The first move is not a review technique. It is refusing the review.

A 900 line PR that does one thing is fine. A 900 line PR that adds a feature, refactors two modules, updates dependencies, and reformats a file is four PRs, and it is your job to say so. This has always been true and it matters more now, because the cost of producing the large version dropped to nothing while the cost of reviewing it did not.

The useful ask: separate mechanical changes from semantic ones. A rename touching 60 files is reviewable in two minutes if it is alone, and it hides a logic change beautifully if it is not.

Read it in a specific order

Not top to bottom. The file order in a diff is alphabetical, which is meaningless.

1. Read the requirement first. Open the ticket. Write down, in your own words, what this change should do. Do this before looking at the code, because otherwise the code will tell you what it does and you will evaluate it against itself. This single habit catches more real problems than any other.

2. Read the schema and type changes. Migrations, table definitions, interface changes, API contracts. These are the highest consequence and the hardest to reverse. A migration that drops a column is a different category of risk from a component that renders wrongly.

3. Read the tests, and read them adversarially. Covered below, because this is where the interesting failures are.

4. Read the error paths. Search the diff for catch, except, if err != nil, .catch(. Ask what the caller learns. Handlers that convert every failure into null are the most common defect in agent output and they are quick to find if you look for them directly.

5. Read anything touching auth, money, or tenancy. Line by line, slowly. These are the places where a subtle error is not a bug report but an incident.

6. Skim the rest. Genuinely skim. If steps one through five are clean and the mechanical checks pass, the remaining code is usually fine, and pretending you carefully read 600 lines of straightforward CRUD helps nobody.

Read the tests adversarially

This is where I find the most, and it inverts the usual instinct that tests are the reassuring part of a diff.

Does the test derive its expectation from the implementation?

def test_discount():
    assert apply_discount(100, 0.2) == 80.0

Ask where 80.0 came from. If the answer is "from running the function", the test asserts current behaviour rather than correct behaviour, and if the function is wrong the test now certifies the bug permanently. Ask instead what the spec says a 20 percent discount on 100 should be, and whether the second argument is a fraction or a percentage.

Is the thing under test mocked?

@patch("app.billing.charge")
def test_checkout(mock_charge):
    mock_charge.return_value = {"ok": True}
    assert checkout(cart).status == "success"

This asserts that checkout succeeds when charge succeeds. It contains no information. It will pass if the real function changes shape, starts throwing, or stops being called.

Are there only happy paths? Agents generate the success case reliably and the failure cases inconsistently. Check for: empty input, null, the dependency failing, the dependency timing out, duplicate submission, and a value at a boundary.

Would the test fail if you broke the code? The fastest check available. Pick one assertion, mentally invert the implementation, and ask whether the test notices. If not, it is decoration.

Make the machine do the boring parts

Reviewer attention is the scarce resource. Spend it on judgement and let tooling handle the rest.

Before a human looks at a PR, CI should already have run type checking in strict mode, linting including rules for empty catch blocks and broad exception handlers, the full test suite, a dependency diff, and ideally a query count assertion on any new endpoint.

Every one of those catches a category that reviewers are bad at catching by reading. Strict types catch invented APIs, lint rules catch swallowed errors, query count assertions catch N+1 patterns that are invisible in a small fixture.

If your review comments are frequently about things a linter could catch, add the linter rule and stop making that comment.

Questions worth asking out loud

A short list I keep returning to. Most of them are about context that is not in the diff.

How many rows will this loop over in production? The fixture has twelve. The table has forty million.

Is this handler concurrent? Two requests, same input, same millisecond. Does the read-then-write hold up?

What happens on the second run? Retries, duplicate webhooks, at-least-once queues. Is this idempotent?

Which of these defaults applies in production? os.getenv("DEBUG", "True") is a shipped incident.

What does this do when the dependency is down? Not slow. Down.

Is anything here reading input we do not control? User content, a webhook body, a scraped page, a model response.

Say what you actually think

One social note, because it changes review quality.

There is a temptation to be gentler about machine written code, since nobody's feelings are involved. In practice I have found the opposite is more useful: be blunter than you would with a colleague, because there is no relationship cost and the author can regenerate cheaply.

"This whole module is the wrong approach, here is why, please redo it" is a reasonable review comment when regenerating costs ten minutes. It would be a harsh one for three days of human work.

The corollary matters too. The human who submitted the PR is accountable for it. "The agent wrote it" is not a defence, and a team where that becomes an acceptable answer is a team that has stopped reviewing. Whoever opens the pull request owns every line in it, which means reading it properly before asking anybody else to.

The realistic summary

You cannot carefully review 900 lines. Nobody can, and pretending otherwise is how defects get through with a green checkmark on them.

What you can do is review the 90 lines that carry the risk, make machines check the categories machines are good at, and push back on diffs that bundle unrelated work. That is a smaller job than it sounds and it catches most of what matters.

More on the underlying failure modes in why AI generated code passes tests and still breaks production and writing an AGENTS.md that actually reduces bugs.