How to Review a 1,000+ File PR Without Losing Your Mind

Let me present you the PR in question: ticket DAED-190, “Switch gw edit to new UI stack”.

In one PR, a colleague replaced the legacy JSP gateway-edit page with a React single-page app. Because that meant changing the UI stack, the backend had to move with it: new REST controllers, a new application service, new tests, a Maven frontend build, and an early commit that deleted everything nobody referenced anymore.

The diff, against master:

1186 files changed, 19809 insertions(+), 473971 deletions(-)

Bitbucket helpfully informed me that only the first 1000 files are shown. That number was the whole reason this turned into a project of its own.

So I sat down and asked myself: how do I actually do this well?

Review fails through lack of coverage, not lack of skill

Doing it manually was not realistic. I’d read for days, hold a shrinking amount of the change in my head, get tired, and still miss things, and the things I’d miss would be the boring, important ones (a field that quietly lost its validation or an action that no longer exists).

The failure mode of manual review at this size is not “I review it wrong”, it’s “I review 15% of it and quietly convince myself that was enough”.

The obvious alternative, throwing the diff at an LLM and asking “review this”, is worse. The diff alone doesn’t fit in the context window. And even if it did, the agent has no baseline: “is this line correct?” is a much weaker question than “does this behave like the thing it replaces?” When you’re replacing a UI, the interesting question is never does the new code look right, it’s does the new code still do what the old code did. You can’t answer that from the diff.

I needed an approach that kept the context small enough for the agent to be reliable, while still covering the whole change.

What I had going for me (and what I didn’t)

Before choosing an approach, it’s worth being honest about the shape of the thing you’re reviewing.

In my favor: this is a relatively small CRUD service with only a handful of flows. A list page, a copy flow, an edit form, not a sprawling distributed system with a dozen integration points. That matters, because it means the surface a rewrite can break is finite and enumerable. Users had already been testing the new UI on IO (Integrating Environment), so the obvious, everyday path (open a gateway, change a field, save) had human eyes on it before the PR ever reached me.

Which is exactly why I didn’t trust it. People test the happy path. They click through the flow that works, confirm the screen looks right, and move on. Nobody spends their afternoon feeding a 4000-character string into a 100-column field, or pulling the session out from under a save.

So I deliberately weighted the review toward the unhappy paths: missing validations, error handling, what happens when a write fails halfway through. A green light from testers on IO told me the good path worked but told me almost nothing about the bad ones.

And one thing working against me: I already knew, from weeks of troubleshooting this service, that it had holes: plenty of its fields were not properly validated or handled to begin with. So “the new code matches the old code” was never going to be enough on its own. If the baseline is leaky, faithfully reproducing the baseline reproduces the leaks.

That’s part of why the review ended up covering DB constraints and failure modes rather than stopping at parity, and a big reason so many of the findings turned out to be inherited rather than new.

Divide and conquer

Two things made the difference.

First, clone both sides side by side. I checked out master and the PR branch into two separate directories:

./master/infobip-gateway-edit      ← the old JSP page, the baseline
./pr_branch/infobip-gateway-edit   ← the React/Bepo SPA

That sounds trivial but it changes the whole exercise. Now “what did master do here?” is a file read, not a guess. Every claim the review makes can be traced back to a line in the old code. The agent is comparing two concrete states, and not reasoning a diff.

(I also cloned the two SQL repositories that hold the table definitions, because part of the review ended up being “does the new code respect the actual database constraints?”. More on that later.)

Second, break the review into small steps and do one per session. This is the part that made the agent reliable. A single “review this PR” prompt would have produced a confident, shallow wall of text. Sixteen focused prompts produced something I could actually trust, because each one had a narrow scope, a written-down baseline, and a bounded amount of context to hold.

The prompt I gave the agent did not say “review the changes”. It asked a narrower, more provable question, and it did so in very concrete terms.

The instruction that started it

Here is the first prompt in full: it names what we’re checking, the order, the output files, and how we’ll work together.

Two things are doing most of the work here. The prompt leads with a negative instruction (don’t review the diff), which steers the agent away from the obvious-but-wrong approach. And it hands over a concrete first output (three named files), a stopping rule, and a coordination protocol (stop, update the plan, report back). None of that is clever; it’s just specific. That specificity is what kept sixteen sessions coherent.

Why the plan lives in a file

The move that mattered most wasn’t clever. It was one of the earliest practices in agentic coding: a plan file (REVIEW_PLAN.md) with a status table at the top:

#	Step	Output file	Status
0	Create review plan	REVIEW_PLAN.md	DONE
1	Document all master fields	MASTER_FIELDS_DOCS.md	DONE
2	Document all master actions	MASTER_ACTIONS_DOCS.md	DONE
3	Document all master validations	MASTER_VALIDATIONS_DOCS.md	DONE

The workflow was:

1. Agent does one step.
2. Agent stops, updates the status table, and reports back.
3. I read the result and decide if it’s good enough.
4. I compact the session (context reset), give the green light, and the agent starts the next step.

Two things come out of this. The agent never has to hold more than one step in mind, which keeps the context (and therefore the hallucination risk) small. And I never have to hold the whole PR in mind either, because the plan file does that for me. If I come back a week later, REVIEW_PLAN.md tells me exactly where we are. (This turned out to matter: I later re-checked comments after the developer pushed new commits, and the plan file meant I didn’t have to reconstruct anything.)

The other side of the workflow: the intermediate documents are baselines. MASTER_FIELDS_DOCS.md lists every field on the old page with its type, options, and visibility rules. MASTER_ACTIONS_DOCS.md lists 17 groups of user actions.

MASTER_VALIDATIONS_DOCS.md lists 47 numbered validations (V1–V47). Each entry names its source file and line, so the parity check in a later step is mechanical rather than interpretive: you can diff a document against the new code, which is much more reliable than asking an agent to “look for missing validations”.

Building on the baseline (phase 2)

Once the three parity axes were done (fields, actions, validations), I had a solid, evidence-backed answer to “did the rewrite lose anything?” That’s the question a UI rewrite can actually get wrong in a way users notice.

But parity isn’t correctness. A rewrite can keep every field and every rule and still be broken in ways the old page wasn’t. So I extended the plan with a second phase, still small steps, still one per session, this time walking the application like an engineer building it from scratch and asking which parts have to work for this to be safe to ship?

I kicked off that phase with a second prompt, the same shape as the first: a concrete goal, the scope I was deliberately excluding, and an ordered list of steps.

#
7	Validations against DB constraints (length, nullability, numeric width, uniqueness, CHECK)	REVIEW_VALIDATIONS_AGAINST_DB.md
8	Data integrity, transactions & concurrency	REVIEW_FINDINGS_DATA_INTEGRITY.md
9	Error handling & failure modes	REVIEW_FINDINGS_ERROR_HANDLING.md
10	Security: authorization transfer only	REVIEW_FINDINGS_AUTHORIZATION.md
11	API contract & integration	REVIEW_FINDINGS_API_CONTRACT.md
12	Frontend runtime correctness	REVIEW_FINDINGS_FRONTEND_RUNTIME.md
13	Caching (HTTP + Spring + frontend)	REVIEW_FINDINGS_CACHING.md
14	File upload (keystore)	REVIEW_FINDINGS_KEYSTORE_UPLOAD.md
15	Observability & operability	REVIEW_FINDINGS_OBSERVABILITY.md
16	Build, packaging & assets	REVIEW_FINDINGS_BUILD_ASSETS.md

I wrote these steps the way I’d audit a service: transactions, then failure paths, then who’s allowed in, then the contract, then the runtime, then the edges. Each one is a standalone unit of “does this part work?”, so the agent could check it in isolation without needing the other fifteen steps in context.

Two of the steps earned their place in ways I didn’t predict:

• DB constraints (step 7) turned into the single most valuable step. Comparing the form metadata against the actual DDL found that 43 text columns had no length enforcement anywhere, client or server, so an over-long value would travel all the way to SQL Server and come back as “String or binary data would be truncated”, with the save handler flattening every database error into one generic message. There’s no way to find that by reading a diff; you find it by resolving each form field to its column and checking the constraint is enforced somewhere.
• Caching (step 13) found that the new SPA loads its bundles from fixed, unversioned URLs, so a deploy could leave browsers running last week’s JavaScript against this week’s backend. That one was real, and it got fixed. (Which is its own small lesson: a review that produces at least one concrete, fixed bug has paid for itself.)

What I didn’t review on purpose

I ignored code quality and organization. Not because it doesn’t matter, but because I had to choose, and at 1186 files I couldn’t do everything (see “the line” below). What I prioritised was app correctness and the things a user would notice. These include a missing field, a missing validation, a save that half-commits, an error page instead of an error message or a bundle that won’t refresh.

I’m confident there is dead code left in that PR. There is certainly some code that could be written more clearly, or factored differently. I flagged the small amount of dead code I tripped over while looking for correctness issues (unused imports, unreferenced configuration properties), but I didn’t go looking for it.

That was a conscious trade, not an oversight.

Tools

I used Pi with an AI model served from our own infrastructure.

I keep Pi deliberately bare (no MCPs and no skills) because I also use it with models running locally on my Mac, where RAM limits how much context I can hold, so I keep the harness lean to leave as much of the window as possible for the actual work. This review ran against the company-hosted model, so that constraint didn’t really bite here, but the bare setup is what I’m used to.

When a task genuinely needs tooling, I reach for other agents (Codex, Claude Code) that have it wired up. For this review what I needed wasn’t tools; it was a patient reader with one small, well-defined job per session, and that’s what the step-by-step structure provided.

What the review produced

All told, the process produced 16 review steps, 13 findings documents, and a set of PR-ready comments. Not every step produced a comment: several produced an explicit “no regression, no comment needed” verdict, which is just as valuable, because it’s a documented answer instead of a silent omission. A few of the things it turned up:

• A real caching bug in the new SPA bundle URLs (found, commented, fixed by the developer).
• A dropped keystore on copy-without-reupload (the copy path read the source gateway and then discarded it, so the row ended up with the old filename and password but a NULL keystore blob).
• A blank page when the SPA is opened at a URL that isn’t a copy URL: the URL parser throws before the promise .catch that renders the “cannot open the gateway” message, and there’s no error boundary, so the operator gets nothing.
• The dead action envelope field, and assorted smaller items.
• And on the other side, a genuine improvement worth calling out: the branch is stricter than master on server-side bounds: 18 @Range checks and the GatewaySpeed @DecimalMin/Max rules now run on save where master had them dormant. I asked the developer to confirm the stricter rejections were intentional, but it’s a good change.

A handful of the findings resolved in the developer’s favour, too: two “missing” grids turned out to be non-functional on master in the first place (their buttons were disabled and the permission helper always returned false), and one authorization concern from the plan turned out not to be a real gate at all.

Part of doing this well is being willing to be wrong in writing, and the baseline documents made that checkable rather than a matter of opinion.

The line you have to draw

Somewhere in a PR this size you have to decide you’re done. You will not read everything. You will miss nuances. There is dead code you didn’t flag and duplication you didn’t mention.

That’s not failure. It’s the job. The alternative is holding a 1186-file change to an impossible standard and never shipping the review. So I drew the line at app correctness, accepted that code-quality nits would survive, and moved on, because other things were waiting for my attention.

If you take one thing from this: scope the review to a small number of axis you can actually prove, write the baseline down, and let the structure (not willpower) keep you honest.

Credit where it’s due

A list of findings can give the wrong impression, so I want to be explicit about something: most of what this review flagged was not introduced by this PR. It was copied, faithfully, from the old master implementation.

The non-atomic copy (step 8), the generic “database error” that names no field (step 7), the audit that is best-effort and swallows its own failures (step 15), the permission helper that always returned false, the grids whose buttons did nothing (steps 5/10), the lengthless text columns: the new code reproduces the old behaviour, warts and all.

That is exactly what a rewrite is supposed to do, and finding inherited behaviour is not the same as finding a defect. I said as much in the write-ups: several findings are marked “parity” or “no regression” rather than “bug” for precisely that reason.

Which reframes the numbers. In a PR this size (a full UI stack replacement, plus the backend changes to support it), the count of genuinely new defects was small. Most of the review’s output is a map of what the old system already did, not a list of things the rewrite broke.

Large part of that is the developer’s work. He kept parity deliberately, wrote tests that pin behaviour, and fixed the real issues (the caching bug, the keystore drop, the case-sensitivity bug in the new field check) as they came up. But I also think it’s a genuine sign of how far LLM-assisted development has come.

Producing a large, coherent, mostly-correct rewrite, where the majority of what a careful reviewer finds is the old code’s pre-existing behaviour rather than new mistakes, is not something I would have expected a couple of years ago. The human still matters: someone had to guide the change, land it, and decide what was worth reviewing.

But the baseline quality of the large machine-written change was high, and the remaining edge cases were the exceptions, not the rule.

Caveats

This is one developer’s approach to one specific PR. It is not a general methodology and it isn’t a claim that everyone should review big PRs this way.

It worked well here for a particular reason: this PR was a rewrite of an existing behaviour, which gives you a natural baseline (the old code) and a natural set of questions (does the new code still do all of it?). A 1000-file PR that adds a new feature has no such baseline, and the same divide-and-conquer axes wouldn’t transfer directly.

A PR that’s mostly dependency bumps or generated code would want something else again.

Treat it as a worked example you can borrow from, not a rulebook. The transferable part isn’t the sixteen steps. It’s the habits: clone both sides, pick a small number of provable axes, write the plan down, keep each session narrow, and stop when you’ve covered what you decided mattered.