Code Review When Half the PRs Are AI-Generated

Sometime around the middle of last year, I stopped being able to tell.
 

Not in a dramatic way. Nobody announced anything. But the PRs coming into review started looking different. Cleaner, mostly. Better named variables. Docstrings on functions that never had docstrings. JSDoc comments on a codebase where we'd argued for two years about whether JSDoc was worth the maintenance cost. And bigger. Much bigger. A ticket that used to arrive as a 90-line diff was now showing up as 400 lines, half of it a defensive wrapper nobody had asked for.

About the "half" in the title: I don't actually know the number. Nobody on my team is logging it and I'm not going to ask them to. Half is my honest guess from reading diffs most days. The fact that it's a guess is the interesting part, though. I can't tell by looking anymore, and not being able to tell is the thing this post is about.

Here's the shape of it. For most of my career, writing code and understanding code were the same activity. You wrote the implementation, and in the process of writing it you found the edge cases, felt where the API was awkward, and bumped into the constraint nobody had documented. Understanding wasn't a separate step. It was a side effect of the work.

That's come apart. Code production got dramatically faster. Code understanding didn't get faster at all, because it can't — it's still a human reading a system and building a model of it. Everything in this post follows from that gap.

The first place a lead notices it is in review. Reading a diff used to give me a decent read on the person who wrote it. You could see where someone was confident and where they were guessing. Guessing looks a certain way in code: awkward names, a comment apologising for something, a slightly wrong abstraction they clearly fought with. Most of that signal is gone now. Diffs arrive with an even confidence that's unrelated to how well anyone understands them.

AI didn't make review harder because the code is worse. It made review harder because the code stopped telling me how sure the author was.

The failure mode changed

Human-written bugs cluster at the edges. Off-by-one. Null on the empty case. The timezone thing. Someone forgot the third branch of a conditional because they were tired at 6pm on a Thursday. You learn to look there, and after enough years you look there without quite deciding to.

Generated code tends to fail differently. It handles the empty case. It handles null. It'll often handle three edge cases you hadn't thought about, which is useful and also a little disarming, because it builds trust right before the part where you need to be sceptical.

It fails in the middle instead. Locally correct, globally wrong. Every individual line does what it looks like it does. The function is coherent. It just doesn't fit the system it lives in, because the model had the file and maybe a few neighbouring ones, and it didn't have the fifteen unwritten things your team knows about how data moves through this particular application.

That's the review problem in one sentence: the code is fine, the context is missing, and the missing context is invisible in the diff.

The one that got through

Here's the one I bring up when people ask whether I'm being dramatic about this.

We had a multi-tenant billing service. Invoice numbers were per-organisation sequential, so every org had an invoice 1001, an invoice 1002, and so on. A design decision I'd probably have argued against if I'd been there when it was made, but it was five years old and it was fine.

Someone picked up a ticket about a slow invoice summary endpoint. Reasonable fix, cache it. The PR came in looking like this:
 

const SUMMARY_TTL = 300;

export async function getInvoiceSummary(orgId: string, invoiceNumber: number) {
  const cacheKey = `invoice:summary:${invoiceNumber}`;

  const cached = await redis.get(cacheKey);
  if (cached) {
    return JSON.parse(cached);
  }

  const summary = await invoiceRepo.buildSummary(orgId, invoiceNumber);
  await redis.set(cacheKey, JSON.stringify(summary), 'EX', SUMMARY_TTL);

  return summary;
}

Look at it for a second before you read on.

It's clean. The TTL is a named constant instead of a magic number. The repo call is correctly scoped, orgId is right there being passed through. There's a cache hit path and a cache miss path and both do the obvious thing. It came with tests. The tests passed. It took about 400ms off p95 on that endpoint in staging, which is what the ticket asked for.

The cache key doesn't include orgId.

So org A requests invoice 1042 and it gets cached. Org B requests its invoice 1042 within the next five minutes, gets a cache hit, and receives another company's billing summary. Line items, amounts, customer name. Everything.

The bug isn't clever. Any of us would spot it if someone said "find the bug in this function." What's worth talking about is everything around it:
 

• orgId was right there in the function. A parameter, used correctly two lines below. Your eye reads scoping is handled and moves on. If the parameter had been missing entirely I'd have caught it in four seconds.
• The tests passed, and they were decent tests. They were just written against fixtures with one org, because our fixtures had one org, because that's how they were seeded in 2021.
• Staging had one real tenant. The environment where we measured the improvement was structurally incapable of reproducing the bug.
• The author was good. An engineer I'd have put on an on-call rotation without thinking about it. They didn't skim it. They read it. It read as correct to them, and honestly, at PR number seven on a Thursday afternoon, it would probably have read as correct to me too.
   

None of our process caught it. What caught it was a contractor who'd joined three weeks earlier, didn't know the codebase, and left a comment on the PR asking whether the key needed the org in it. Not a review technique. Not a check we'd designed. Someone who hadn't yet absorbed the assumption that tenant scoping was handled, asking the question the rest of us had already answered without looking.

We shipped nothing, so this isn't a disaster story. But I've thought about it more than most of the incidents that did ship. The bug wasn't really in the code. The model didn't know invoice numbers weren't globally unique, it had no particular way to find that out, and nothing in the diff flagged the gap.

AI didn't create that weakness. It exposed it.

This is the part I'd have missed a year ago.

Our fixtures had been single-tenant since 2021. Our staging environment had one customer in it. That means a cross-tenant cache bug was always shippable in that codebase. Any of us could have written that line by hand on a bad afternoon and nothing in our pipeline would have stopped it. The hole was five years old.

What changed is throughput. When you're producing 90-line diffs by hand, you roll those dice a few times a week. When you're producing 400-line diffs, you roll them a lot more, and you roll them in code you didn't sit with long enough to feel uneasy about.

So when a generated change causes an incident, the useful question usually isn't why did the model do that. It's why couldn't our system catch it. Those are different questions with different fixes, and only one of them is actually in your control.

I'd rather be honest about where that leaves the checklist below. It's a compensating control. It exists because our fixtures are wrong and fixing fixtures is unglamorous work nobody wants to fund. A multi-tenant seed script would have caught this bug automatically, permanently, without depending on whether I was sharp that afternoon. Human vigilance is the weakest kind of safeguard there is. Sometimes it's the one you've got.

What I actually check now

The checklist came out of that PR. It's not "review more carefully" — that isn't a process, it's a promise to feel guiltier next time. It's a different set of things to look at, and I do them in this order on purpose. Takes me eight to ten minutes on a normal PR.
 

1. Read the PR description for evidence the author understood the problem. Not to work out whether a model wrote it. That's unanswerable and it's the wrong question. But there's a real difference between a description that restates the diff — adds a caching layer to the invoice summary endpoint using Redis with a 5 minute TTL — and one that says caching this because the summary does four joins and the export job hits it in a loop; five minutes of staleness is fine here because finance reconciles daily. The first summarises the output. The second shows someone who thought about the problem. The second gets a faster review.

2. Read the tests before the code. Then ask whether these tests would fail if the implementation were subtly wrong. Generated tests are good at asserting that a function does what the function does. They're weaker at encoding what the function is for. If I can mentally break the core logic and every test still passes, that's not a test suite, it's a description with a green checkmark on it. In the cache PR the tests asserted that a cache hit returned the cached value. Which it did. Beautifully.

3. Trace every scoping identifier through the whole diff. Tenant ID, org ID, user ID, environment, region — whatever keeps one customer's data away from another's. I search the diff for each one and check every place it appears and every place it should appear and doesn't. Cache keys, log lines, queue message payloads, file paths, metric labels, background job arguments, anything memoised in process. Anywhere a value gets flattened into a string or crosses a boundary, scope is easy to drop. The dangerous version isn't a missing where clause, which is loud and obvious. It's scope being present everywhere you'd expect it and absent in the one place that matters.

4. Find the loop, find the network call, check they're not in the same block. await inside for is the obvious one. The subtler one is Promise.all over an array whose length comes from user data, which looks like the sophisticated fix and is the same problem in a nicer shirt. We had one of these ship: a Promise.all over a customer's contact list, fine for months, until an enterprise customer imported 40,000 contacts and it took out the connection pool for everyone at about 2am. The code was valid. The system wasn't.

5. Ask what happens on the second call. Users double-click. Load balancers retry. Webhooks arrive twice. Queues redeliver. Generated code is usually written as though the happy path happens once, because that's how the ticket was phrased. So: can this run twice with the same input, and if it can't, what stops it? A unique constraint, an idempotency key, a status check inside a transaction. If the answer is "it won't happen twice," that's not an answer, that's a Thursday waiting to happen.

6. Read config and dependency changes as a separate review. A bumped minor version, a line in a .env, a new package pulled in to do something the standard library already does. These get waved through because they're two lines, and their blast radius has nothing to do with their line count. Adding a dependency is close to free now, which is exactly why it deserves the question it used to get: do we need this, and what are we signing up to maintain?

7. Ask one question the author has to reason about. Not a gotcha. What happens when an org has 50,000 invoices? Why five minutes and not sixty? What breaks if this cache is stale for an hour? What happens if the external call succeeds and the DB write fails? I'm not checking whether they memorised the diff. I'm checking whether they've built a model of it. "I hadn't thought about that" is a perfectly good answer and it's what review is for. Twenty minutes of silence followed by a paragraph in a suspiciously familiar register is a different answer, and that conversation isn't about the code.

That last one is closest to the point. I've mostly stopped opening a review by asking whether the code is correct. I start with whether the author understands what it'll do in our system. In my experience correctness tends to follow from that, and it's rarer for it to work the other way round.

What I tell my team

Four norms. None of them exciting, all of them load-bearing.

You are the author. I don't care what typed it. Your name is on the commit, you're the one getting paged, and "the model wrote that part" isn't a sentence that appears in our postmortems. That's not a position on AI. It's just how accountability has to work if it's going to work at all.

Tell me where you're unsure. One line in the PR description: not confident about the transaction boundary here. That's the signal we lost, and it's recoverable for free, as long as nobody has ever been made to feel stupid for offering it. That part's on the lead.

Keep PRs small. Generation made 400 lines nearly free to produce and did nothing to make them cheaper to review. That cost didn't vanish, it moved onto the reviewer, and reviewers absorb it quietly until they start skimming. Pushing back on PR size is the highest-leverage habit I know of, and it was already true before any of this.

Don't interrogate people about tooling. "Did AI write this?" is unanswerable, faintly insulting either way, and it turns review into a conversation about tools instead of about the system. Ask about the code. If someone doesn't understand their own change, that surfaces on its own in about ninety seconds.

The part I'm still working out

I don't have this solved. Reviews take me longer than they did two years ago, and I haven't found a version of this that scales cleanly past a team of twelve — the checklist depends on me having enough context to know what should be in that cache key, and that doesn't distribute easily.

Some days I think the honest answer is that most of this belongs in infrastructure rather than in review. Better fixtures, multi-tenant seed data, a staging environment that resembles production. Boring, expensive, permanent. Some days I think the reviewer's job just got harder and we should say so out loud instead of pretending the tooling made everything faster across the board.

What I'm fairly confident of is the thing underneath all of it. Code production and code understanding used to move at the same speed, because they were the same activity. They've decoupled, and everything in this post — the missing confidence signal, the bug that read as correct, the 400-line diffs, the checklist — is downstream of that one change.

Which is why "is this code correct" has become the less useful question, and "does this author understand what this code will do in our system" has become the more useful one. They used to be nearly the same question. A fair amount of the review advice going around still reads as though they are.

If you're leading a team through this, I'd like to hear what's working for you. I don't think anyone has it figured out yet.