The AI Code Review Cheat Sheet

Forty files changed. Two thousand lines. The agent says it's done.

I'll go through all of it. About a hundred of those lines get a proper read, and I trust that review more than one where every line got the same attention.

Attention is a budget. Spread it evenly over two thousand lines of plausible code and it fails: everything looks fine, focus runs out around file twelve, and the real problem sits in file thirty-one. The type checker, the linter, the scanners and the test suite have already checked every character without getting tired. That's their job.

So the real question is where my attention changes the outcome. What follows is the routine I've settled on after a few years of this, and after a couple of outages that showed me where the expensive mistakes live.

Most of the diff gets a glance. A small part gets my full attention, slowly, twice. The skill is knowing which part.

This is assisted work, not vibe coding. I own the design, the data model, and the decision about what we leave out. The AI writes much of the code and I steer. I know what's in that diff, because I decided what would be in it. I don't need to watch it being typed.

Disclaimer. This only works because the repo is already in decent shape: typed, linters that fail the build, real tests, clear layers, boring conventions. The method is comparison, and a fresh repo gives you nothing to compare against. AI is an amplifier: a good codebase gets better, a bad one gets worse. I described the machine that does the line by line reading in Programming by Coercion. This post is the other half: once that machine exists, what's left for me?

What AI actually gets wrong

Here's the part nobody says out loud: the boring stuff comes back correct.

Add a field and carry it down to the database. Write the tenth endpoint that looks like the other nine. Parse a format from a sample. Rename something across forty files. The agent gets all of that right, faster than me, and reading it teaches me nothing. That's most of the two thousand lines.

Correctness gets settled before the code exists. I want what a unique constraint gives you: a world where the duplicate can't exist. Types that make the bad state impossible to write. Constraints in the database, where nobody can forget to call them. Tests written first, so the only acceptable result is one that passes them.

With a clear description and tests up front, it rarely fails. What's left is what no test can express:

• Intent. It answers the question it understood. If the description was loose, you get something confident, coherent, and slightly off target.
• Architecture. It invents structure instead of using mine. It writes a new helper instead of finding the existing one, because writing is easier than looking.
• Scale. A cathedral when you asked for a garden shed.
• Cost over time. It optimises for what works today. It doesn't ask whether this shape can absorb the next three changes, or whether it forces a refactor with a heavy migration attached.

Everything below falls into one of those four.

0. The decisions, before the code

Zero indexed, because it happens before a line exists. It's also the change that helped me most.

Every prompt I write ends with some version of this:

Plan this before writing any code. Ask me the questions that would make the spec precise, and challenge anything that looks wrong or missing.

Without it, the agent takes a vague request and starts typing. With it, I get ten questions back, and two or three cover things I hadn't thought about. What about the rows that already exist? Per user or per organisation? You said recent: this week, or this session?

Those questions are the review. They happen while changing my mind still costs nothing. Half of what I used to catch in diffs now never gets written.

The conversation also produces the reasons. Why one column and not a second table. Why this is a shed. What we're deliberately not building. So I have the agent write them down as ADRs: one short file per decision, in the repo, with the choice and the options that lost.

I read those ADRs first when the diff lands, before any code. If one describes something I never agreed to, the diff is already wrong, and I've saved twenty minutes.

It compounds. Next month, in another session with another agent, nobody who missed the conversation quietly reverses the decision.

1. The model and the migration

Most of my attention goes here, because mistakes don't cost the same everywhere.

A frontend bug is usually reversible. Wrong label, broken layout: revert or hotfix, and twenty minutes later nobody remembers.

Data has memory. Once a migration runs on production, the mistake lives in the data. A dropped column is gone, and getting it back costs a restore, a maintenance window and a day of your life. I've paid that bill once, which is why this section comes first. A wrong type means a full table rewrite. A missing unique constraint means three weeks of quiet duplicates, and then you decide, row by row, which one is real.

So I ask two questions, every time.

Can this be undone? Every migration has a downgrade function. The real question is whether undoing it destroys something I can't recompute.

Does this take the table offline, and for how long? A nullable column with no default is instant at any row count. The same column with a computed value rewrites the whole table under an exclusive lock. One word apart in the diff. A millisecond versus a maintenance window in production. Same family: an index built without CONCURRENTLY blocks writes for the whole build, and a new foreign key or NOT NULL scans the table under lock unless you add it NOT VALID and validate it in a second step.

Then the short version of everything else. Is this the shape of the data, or the shape that made the code easy to write? What happens on the second insert of the same thing? Does NULL mean unknown, not applicable, or not computed yet? And how big is that table, really? I know the order of magnitude of mine. The agent doesn't.

A bad model is worse than a bad migration, because it trips neither question. It never fails, so no incident. It never locks anything, so no window. It just makes everything slightly harder forever, and the fix on day 400 is a migration over a hundred million rows plus every line of code that ever touched it.

Get the model right and a bad implementation is a refactor. Get it wrong and everything above it is built on sand.

2. How big should this have been?

I guess the size before I open anything. Two lines, two hundred, two thousand? Then I look.

Two lines expected and four hundred delivered means it reinvented something we already have. Two thousand expected and thirty delivered means it did the happy path and left the rest as an interface with a hopeful name.

The gap is the signal, both ways. The guess has to come first, or I'll happily justify whatever number shows up. I'm very good at that.

Same question one level up: cathedral or garden shed? Sometimes it really is a cathedral. Usually it's the wooden cabin you build for your kids at the bottom of the garden. One thing, built to spec, unlikely to change. Cheap, honest, solid enough not to fall on anyone.

AI defaults to cathedral. Ask for a CSV export and you get a strategy pattern, a format registry and a config object with eleven optional fields, so adding XML later will be easy. XML is not coming. I built that cathedral myself, twice, long before AI, which is probably why I spot it so fast.

So I ask out loud: is there less? I can follow the code if I concentrate. What I want to know is what could come out.

Clever is the failure mode, and it now has a running cost. Clever code needs a stronger AI to review it, on every future change. A small, cheap, fast model can handle boring code, so the loop stays quick and gives the same answer twice. for ... else. Two decorators whose order matters, with nothing saying so. A type computed from three other types, so the error lands four files from the mistake. None of that is wrong. All of it costs attention on every pass, mine and every AI's after me. (I wrote a whole post about this: Boring Is a Feature.)

One more question at the same level: is this actually our problem? Business logic is ours, so we write it. Parsing a date, retrying a request with backoff, validating an email, sanitising a filename: thousands of people solved those before us. A library already covers edge cases we haven't thought of, and our version is the one that breaks on a timezone. Agents write that code happily, because writing is easy and knowing a library exists is hard.

3. What things are called

Names are the cheapest thing to check in a diff and the most expensive to get wrong, so I read all of them. do_stuff, rename_file and sanitize_file_name_ascii can sit on the same twenty lines and tell three very different stories about what the code is for.

Mostly I check that we speak the same language. The business says client, so the code says client. An agent that switched to customer halfway through has added a second vocabulary everyone must translate from now on. Same for archive and delete, provider and vendor, or whatever word your domain settled on years ago.

Then vagueness. process_data, handle, manager, utils. A name that could describe anything usually means the function does more than one thing, and the name shows it first.

A precise name is a promise. sanitize_file_name_ascii tells me someone chose the encoding on purpose, and that I'll find that choice in exactly one place the day it turns out wrong.

Then every method, name against body. Does the name say everything the method does, and nothing more? A save_user that also sends a welcome email is two methods sharing one name. Then the signature: which arguments are positional, which are keyword-only, and whether each one is typed. Seven parameters, or a boolean flag that switches between two behaviours, means two jobs in one function. When every name tells the truth and every signature is tight, separation of concerns mostly takes care of itself. The type checker keeps it that way, because it checks every caller against that signature on every change.

4. Where the blast radius is

Two things earn slow reading.

What everything calls. The engine, the computation, the path every request takes. An error there multiplies across every caller and surfaces six weeks later as "the numbers look weird", the worst bug report there is. It's a few hundred lines out of two thousand, and the one place my reading beats the pipeline. The optional export and the admin screen three people open per week get a lighter read, on purpose: a bug there is visible, contained, and fixed the same day. Accepting a cheap miss there is what lets me go slow where a miss is expensive.

Lines in files that already existed. New files are low risk: nothing depended on them yesterday. Regressions live in modified files.

git diff --stat master... -- $(git diff --name-only --diff-filter=M master...)

Deleted lines get the most attention. Added code is a proposal. Deleted code is a decision, and nobody ever wrote down why the line was there.

5. Anything that touches security

In a properly layered app, security lives in four places: the auth dependency on the route, the permission check in the service, the row filter in the database, the guard on the frontend route. A new endpoint inherits all of it without knowing. Same trick as a database constraint: most features can't open a hole, because they never touch what closes it.

So it's a yes or no question. Does this diff touch the security layer? Almost always no, and I move on, grateful to whoever set up the layers.

When it's yes, I read all of it, whatever the size, and ignore every other rule in this post. This is where the pipeline is weakest. A missing auth dependency on a route is two characters. The code is correct, the linter is happy, and the tests pass because they call it as a logged-in user. The endpoint is wide open.

The other thing I check by hand: right verb, wrong subject. Can this user read this row, or any row? The agent checks "is the caller logged in" and forgets "is this theirs", because no happy-path test uses somebody else's id.

Secrets, injection and vulnerable dependencies stay in CI where they belong (CI-Embedded Security). Scanners find known-bad patterns. They don't find a correct-looking check on the wrong thing.

6. The test names

I read all of the tests, but the names get most of my attention.

The tests came out of the design, before the implementation (Backend Tests). They're the spec. So I'm reading a map, and twenty seconds of scrolling shows me everything we thought could go wrong.

I'm hunting for absence. Empty. Duplicate. Too large. Wrong order. Concurrent. Not allowed. If I expected an edge case and no name mentions it, it isn't covered, and I found that out without opening a file. test_service_works is a shrug with an underscore in it.

One grep, though: did any existing test change? A weak new test is a gap. An old test relaxed so the new code could pass is a different problem. Full disclosure: I've never caught one. It takes two seconds to check.

7. The dependencies

I read every line of the lockfile diff. Shortest file in the changeset, longest consequences. Adding a library takes one second and lasts five years: a licence, a maintainer you've never met, a transitive tree, a CVE feed.

Is it necessary? Is it maintained? Is it already in the project under another name? Could it be thirty lines we own? Bumps are fine. Additions need an argument.

8. A second AI reads it

Then I give the diff to a different AI, in a fresh session, and ask what's wrong with it.

Yes, I know. I don't fully trust AI, so my answer is more AI. But we already apply this rule to people and nobody finds it strange: you don't ask the developer who just wrote the code to review it five minutes later. Same assumptions, same blind spots, still a bit in love with the solution. A fresh session is that developer a week later. A different AI is a different developer.

Narrow questions only, because "review this" gets you twelve remarks about naming. What breaks in production? Can this migration be reverted, and does it lock the table? Which of these tests would still pass if the feature were deleted?

Then I sort the answers. That part can't be automated. Three buckets.

True, and it changes nothing. A naming preference, a micro-optimisation nobody will ever measure, "consider extracting this helper" about six lines. Agents produce these by the kilo, because they're trained to be useful and "looks fine to me" doesn't feel useful. I drop them without guilt.

The code is right, but the reviewer couldn't tell. A gift in disguise. If an AI with the whole file in front of it can't work out why the code does that, the human arriving in March has no chance. I write the explanation down, leave the code as it is, and the finding has paid for itself.

The finding is real. It understood the code and its purpose, and found something genuinely wrong: a case we never handle, a query that dies at production volume, logic in the wrong layer, a function whose name promises something it doesn't do. This is where code actually changes, and it's the smallest bucket.

Telling the first bucket from the third takes judgement. It needs someone who knows the system, the roadmap, and what we already decided not to build. I can't delegate that, and it's the most interesting twenty minutes of the whole review.

9. The thing itself

Then I use it. Click the button. Call the endpoint. Run it with real, ugly input, because the tidy example from the docstring already works. Then check what actually landed in the database.

Half of what an agent gets wrong is a misunderstanding of what I asked for. The code flawlessly implements the wrong idea, so the diff reads clean all the way down. The screen catches it in thirty seconds.

When I find something

It goes back through the loop. Patching by hand is tempting and quick, and it's exactly how the same bug comes back next month in a different hat.

So I always argue about it with the AI first. It holds more context than I do: the documentation, the log, the decision records from section 0, the reason we chose this three months ago. I have a vague feeling something's off. Half the time the answer is "that was decided in the ADR about X", and the thing I was about to fix wasn't broken. The other half, it agrees on the spot, and now we both know why.

Either way, the reasoning gets written down, and that's the point. A silent fix takes two minutes and teaches nobody anything, me included.

Then I fix the input. Almost every real finding traces back to a vague description. So I fix the description, add the test that should have existed, and run it again. That removes the whole class of mistake, not just the one instance.

Even typos go through the loop, and that took me a while to accept. Fixing a typo by hand takes four seconds. But in a sixty file diff, how do I know the same mistake isn't in the other fifty-nine? I don't, and reading won't tell me. The agent will, in one pass.

The cheat sheet

1. Decisions first. Read the ADRs before the code. Anything I didn't agree to?
2. Model and migration. Can it be undone without losing data? Does it lock a big table?
3. Size. Did I guess the size before looking? Is there less? Did a library already solve this?
4. Names and signatures. Same words as the business? Does each method's name say everything it does? Is the signature tight and typed?
5. Blast radius. What does everything call? Which existing lines changed or disappeared?
6. Security. Does the diff touch the security layer? If yes, read every line. Does it check "logged in" or "is this theirs"?
7. Tests. Which edge case has no test name? Did an old test get relaxed?
8. Dependencies. Any new package? What's the argument for it?
9. Second AI. Narrow questions, fresh session. Sort the answers: noise, unclear, real.
10. Use it. Real input, then check the database.

Anything I find goes back through the loop: fix the description, add the missing test, regenerate.

The honest part

No trick here. I do read the diff: about 5% of it at walking pace, and the other 95% at a glance. The type checker, the linter, the scanners, the tests and a second AI have all been over it far more carefully than I would.

For years I gave every line the same attention, and by file twelve I was skimming anyway, like everyone does. That pass took an hour and caught less than this one, because now my attention lands where the risk is, not wherever it happened to run out.

This assumes I know my own system. Which tables are big, which code is load-bearing, what we already decided not to build. That took years. It doesn't transfer to a new codebase, and that knowledge is the asset now. It used to be typing speed, then knowing the framework by heart. Today the machine writes the code, and the one thing it can't do is know what your system is for.

None of it is free. Something will get through. That's why monitoring, backups and a rehearsed rollback belong to the same system, not an afterthought. The bet is that the costs are lopsided: a bad screen is a revert, a bad schema is a year, an open endpoint is a phone call on a Sunday.

And yes, a post preaching simplicity ships with a ten item checklist. In my defence, most items are a no in about ten seconds.

Nobody reads as fast as this stuff gets written, and that speed is here to stay. What's left to decide is where you slow down, and how much of everything else you design so it can't be wrong.