AI Coding

How to Review a Large Generated Pull Request

A generated pull request of four thousand lines has a smell to it. Not a bad smell exactly, more like the ozone before a storm. Something enormous has arrived, fully formed, and the diff view scrolls for what feels like minutes before reaching the bottom. The instinct is to close the tab and hope the CI catches whatever is wrong. That instinct is worth examining, because it is partly right.

Large machine-written PRs are now a normal part of code review, and the old habits do not transfer cleanly. A human contributor's big PR usually has a story: a feature that grew, a refactor that escaped its cage. A model's big PR has a shape that can be predicted before reading a single line, and that predictability is the reviewer's best tool.

The diff is a transcript, not a proposal

When a person opens a pull request, the diff is the residue of a decision they already made. When a model opens one, the diff is the decision-making itself, laid out in order. That distinction changes what a reviewer should look for first.

The top of a generated diff often contains the seed of every problem below it. A helper function introduced on line 40 gets called in nine places by line 900, and the ninth call passes arguments the helper was never designed for. A pattern established early, then bent, then abandoned, then quietly restored somewhere in the middle. Reading the first two hundred lines with full attention pays for itself many times over.

There is a specific failure mode worth naming. Models tend to write the happy path with great care and then bolt on error handling as an afterthought, often in a separate pass, which is why the exception branches in a large generated PR frequently contradict the assumptions of the code they wrap. A function that returns null on line 120 and throws on line 700 is not being inconsistent on purpose. Two different passes wrote those lines.

Read the tests before the implementation

Tests come first in a generated PR, for a practical reason. The implementation is long and the tests are short, and the tests reveal what the model believed it was building. If the tests mock a dependency that does not exist, or assert on a shape the real API never returns, the implementation below is built on that misunderstanding.

Watch for tests that pass for the wrong reason. A test asserting that a function returns a non-null value proves almost nothing. A test that constructs a realistic input, calls the function, and checks a specific field of the output is doing work. In a large generated PR, the ratio of the first kind to the second is a reliable signal of how much the author actually verified.

Count the assertions that would fail if the implementation were deleted. It is a crude measure and it takes about ninety seconds, and it has caught more than one PR where every test passed and nothing was tested.

The interfaces a machine gets wrong

Boundaries between systems are where generated code degrades. Inside a single file, a model can hold the whole context. At the edge of a database, an HTTP client, a message queue, or a third-party SDK, it is reconstructing behavior from training data, and training data contains every version of every API that ever existed.

So the review should spend disproportionate time on the seams. Does the pagination loop actually terminate when the API returns fewer items than the page size? Does the retry logic double the timeout on each attempt, and is there a cap? Does the client close its connections on the error path? These questions sound pedantic until a service starts leaking file descriptors at three in the morning.

A useful trick: list every external call the PR makes, then for each one, find the line where failure is handled. Any call without a corresponding failure line is a finding. It is mechanical work, which makes it well suited to a reviewer who is tired of reading logic.

Duplication hides in plain sight

Large generated PRs accumulate near-duplicate code the way a river accumulates silt. A validation function on line 300 and another on line 1400 that differ by one condition. Two utility modules that both format dates. Three helper functions that all normalize strings, each slightly differently.

The reviewer's eye slides over these because each individual block looks reasonable. The problem only appears later, when a bug fix lands in one copy and the other two keep the bug. Searching the diff for repeated function signatures or identical string literals catches a surprising amount of this in a few minutes.

Duplication is not automatically wrong. Two functions that look alike today may diverge tomorrow, and premature merging creates its own problems. The question worth asking is whether the duplication is intentional or accidental, and in a generated PR the default answer is accidental.

The parts that were never written at all

The most dangerous lines in a large generated PR are the ones that are not there. A model writing a new endpoint will produce the handler, the validation, the database call, and the response. It may not produce the authorization check, because authorization lives in a middleware file it never saw.

Absence is hard to review because there is nothing to click on. The practical approach is to compare the new code against a sibling that already works. Find the existing endpoint most similar to the new one, and diff the two by eye. What does the old one do that the new one does not? Logging, rate limiting, a feature flag, a transaction wrapper. The gap is the review.

This comparison also catches the model's tendency to invent a parallel structure rather than extend an existing one. A new file that mirrors an old file's purpose is a signal that the author did not know the old file existed.

When to stop reading line by line

At some point the reviewer has to admit that line-by-line reading of four thousand lines is not a review, it is a performance. Attention degrades. The twentieth file gets skimmed. The fortieth gets the benefit of the doubt.

Better to read the first two hundred lines closely, read the tests closely, audit every external call, search for duplication, and then run the code. Run it against a real database with real data, hit the new endpoint with a malformed payload, kill the connection mid-request. A generated PR that survives twenty minutes of adversarial use has earned a lighter read of the middle.

The middle of a large generated diff is often the least interesting part. It is where the boilerplate lives, the getters and the serializers, and the model is usually competent there. The risk concentrates at the edges: the first decisions, the external boundaries, the missing pieces, and whatever happens when the thing is actually turned on.

Asking for a smaller PR next time

The review is easier when the PR is smaller, and the reviewer usually has the standing to say so. A four thousand line generated PR can be regenerated in four hundred line increments, and the second attempt is often better because the prompt can absorb what the first attempt revealed.

There is a cost to this, of course. Regeneration takes time, and the model may produce a different set of problems. But the review of four smaller PRs, each with a clear boundary, is a fundamentally different task from the review of one monolith. The first is a series of decisions. The second is an act of faith.

The tell that a PR needs splitting is simple: if the reviewer cannot describe the change in one sentence that a colleague would recognize, the PR is doing too many things. A generated PR that adds an endpoint, refactors the auth layer, and upgrades a dependency is three PRs wearing a trench coat.

What the reviewer owes the next reader

Every large generated PR that gets merged becomes someone else's archaeology six months later. The reviewer is the last person who will ever read the whole thing at once, and the comments left on the diff are the only map that survives.

A comment that says "this looks fine" is worthless. A comment that says "this retry loop has no cap, so a persistent 503 will spin forever" is a gift to whoever debugs the incident. The reviewer's job is not to approve or reject. It is to leave behind enough understanding that the next person does not have to reconstruct it from scratch.

That is the real argument for reviewing generated code carefully rather than rubber-stamping it. The code will outlive the conversation that produced it. The diff will be closed and forgotten, and the repository will carry whatever was wrong with it for years.

Leave a Reply

Your email address will not be published. Required fields are marked *

Back to top button