Previous: CLAUDE.md & Session Notes

Reviewing AI Code

An agent can generate a week of code in an afternoon. Whether that is a superpower or a liability is decided entirely by what happens next: the review. This module is about the discipline that keeps the throughput without inheriting the risk — how to read agent code, what to hunt for, when to go deep, and how to make every finding permanent.

Review Is the Bottleneck Now

For most of software history, the scarce resource was writing code. Typing was slow, thinking was slow, and review was a tax you paid on top of the real work. Agents invert that completely. Generation is now effectively free: you can have three hundred lines of plausible, compiling, test-passing code before your coffee cools. The scarce resource — the thing your throughput actually depends on — is your ability to read that code, understand it, and decide whether it is true.

This is not a small reframe. It means review skill is engineering productivity now. The engineer who can review a diff quickly and accurately ships more agent-generated work per day than the engineer who generates twice as much and reviews half as well — and the second engineer's extra output is where incidents come from. If you think of agentic work as the Rotating Door — delegate a chunk, take a lap, come back, review, land, delegate the next chunk — then your review cadence is the rotation speed. The agent is never the slow part of that loop. You are. Getting faster and sharper at review is how the whole door spins faster.

From that inversion follows the prime directive of this module, and it does not bend for deadlines, demos, or diffs that look fine:

House Standard

Never merge code you have not read and understood, and review the plan before you review the diff. You own every line you merge — its bugs, its security holes, its maintenance burden. "The agent wrote it" is not a defense in the postmortem, any more than "I copied it from a forum post" ever was. The merge button is where accountability transfers to you, whole and undivided.

Take the ownership point literally. Six months from now, someone will git-blame a line in production, and it will have your name on it. The on-call engineer debugging it at 2 a.m. will not care who typed it. The security team tracing an injection path will not accept "that was Claude" as a root cause. Every process in your organization — blame, incident review, code archaeology, promotion packets — treats the merger as the author. Agentic coding does not change that; it just means the gap between what you merged and what you understood can grow much larger, much faster, if you let it.

The rest of this module is the practical answer to an obvious objection: "if I have to deeply read everything, have I really gained anything?" Yes — because review effort is not uniform. It front-loads into the plan, concentrates on a specific set of AI failure modes, scales with consequence, and compounds through a feedback loop. Read on.

The Cheapest Review Happens Before Generation

The most expensive review mistake is doing all of your reviewing at the end. A wrong plan caught before generation costs you one message: "no, don't add a caching layer, just fix the N+1 query." The same wrong plan caught in a 40-file diff costs you an afternoon — first to understand what the agent built, then to understand why it is wrong, then to either untangle it or throw it away and re-prompt. Same defect, two orders of magnitude difference in cost, purely based on when you caught it.

So for any nontrivial task, the sequence is: ask for the approach first, review that, approve it, and only then let the agent build. This mirrors the buy-in step of the house planning method — you would not let a new teammate disappear for two days on a feature without agreeing on the approach, and an agent is a teammate who works at 100x speed, which makes the pre-agreement more important, not less. Concretely:

  • Ask for the plan explicitly. "Before writing code, give me your approach: what files you'll touch, what the data flow is, what you'll test." Most agents have a planning mode; use it for anything beyond a trivial fix.
  • Review the plan like a design doc. Is the approach the one you would take? Does it reuse what exists? Does the file list match the blast radius you expect? A plan that touches auth middleware for a copy change is a red flag you can catch in ten seconds.
  • Edit the plan, not just the code. Corrections at the plan stage are one-line messages. Say what to change and what to keep, then approve.
  • Or invert it: write the spec yourself. For work where you already know the shape, a tight spec — files to touch, contract to satisfy, tests that must pass — is a plan review you did in advance. The agent executes; the diff review becomes a conformance check against your own spec, which is dramatically faster than reverse engineering intent from code.

There is a second benefit that has nothing to do with the agent: articulating or reviewing the plan forces you to understand the task before the code exists. Reviewers who skip this step end up learning the problem domain from the agent's diff — which means the diff's framing becomes their framing, and they lose the independent viewpoint that review depends on. You cannot notice that the agent solved the wrong problem if your entire understanding of the problem came from the agent's solution.

Pro Tip

A fast plan-review heuristic: read the agent's proposed file list before anything else. Files you expected but don't see mean the plan is incomplete; files you didn't expect mean either the agent knows something you don't (worth asking about) or scope is creeping (worth stopping). Thirty seconds on the file list routinely saves the afternoon.

What to Hunt For: AI-Specific Smells

Reviewing agent code is not the same skill as reviewing human code, because agents fail differently than humans do. A human teammate's bugs cluster where the code is genuinely hard — concurrency, off-by-ones, gnarly edge cases — and their confusion shows: hesitant naming, TODO comments, a question in the PR description. Agent code shows none of those signals. It is uniformly fluent, uniformly confident, and its defects hide in specific, recurring patterns. Learn the patterns and your review speed doubles, because you know where to point your attention.

SmellWhat it looks likeWhat to do
Plausible-but-wrongCode that reads beautifully — clean naming, idiomatic structure, sensible comments — and is subtly incorrect: an inverted condition, a timezone assumption, a pagination boundary off by one, the wrong field with the right-sounding nameTreat fluency as zero evidence of correctness. Slow down precisely where the code reads smoothest, trace the logic against the requirement line by line, and execute the changed path yourself
Invented APIsMethods, config options, or parameters that do not exist — retryWithBackoff on a client that has no such method, a config flag from a different library, an option from a version you don't run. Most common on less-popular libraries and recent versions, where training data is thinVerify any API you don't personally recognize against the actual installed version's docs or source. Types and compilers catch many of these; in dynamic languages and config files (YAML, JSON), nothing catches them until runtime — check by hand
Quiet reimplementationThe agent rebuilt a helper that already exists in your repo — a second date formatter, a third retry wrapper, a bespoke validation function next to your shared one — because it didn't find or wasn't told about the originalFor any new utility-shaped function in a diff, search the repo for an existing equivalent before accepting it. This is the DRY module's duplication smell with a new cause; the fix is the same (use the existing helper) plus a feedback step (tell the agent where shared helpers live — see the feedback loop below)
Over-engineeringAbstractions, options, and error hierarchies nobody asked for: a strategy pattern around one strategy, a config object with six unused fields, a custom exception tree for a function with one failure mode, interfaces with a single implementationAsk of every abstraction in the diff: what second use case justifies this? If the answer is hypothetical, request the simple version. Every speculative layer is code you will review, maintain, and eventually delete
Test theaterTests that assert the mock returned what the mock was told to return; tautological asserts (expect(x).toBe(x) in disguise); assertions on incidental structure instead of behavior; or — the worst variant — tests rewritten to pass instead of code fixed to workFor each test, ask: what real bug would make this fail? If you can't name one, the test is theater. And any diff where a previously-passing test was modified deserves your full attention: was the test wrong, or was it inconvenient?
Silent scope creepThe diff touches files the task did not require: a drive-by refactor of a neighboring module, reformatting that buries the real change, a "while I was here" rename across twelve files, config tweaks nobody discussedDiff the file list against the approved plan. Out-of-scope changes get split out or reverted — not because they're necessarily bad, but because they arrived unreviewed inside a diff about something else, which is exactly where regressions hide
Deleted or weakened checksError handling removed, validation relaxed, types loosened (unknown to any, a non-null assertion, a broadened catch block, a lint rule disabled inline) — usually to make something compile or a test passRead every red line in the diff, not just the green ones. Deletions and loosenings are the highest-risk lines in any agent diff, because they are how an agent "fixes" an obstacle it doesn't understand. Each one needs an explicit justification or a revert

Warning

The fluency trap: confident prose is not correct code. Your brain uses reading ease as a proxy for quality — smooth, idiomatic code feels reviewed as you scroll it. Agent code weaponizes that instinct, because it is uniformly smooth, including the parts that are wrong. The old heuristic "messy code hides bugs, clean code is probably fine" actively misleads you here. Recalibrate: with agent diffs, polish tells you nothing, and your suspicion has to come from tracing logic and executing code, not from how the diff reads.

Verify, Don't Trust: Claims Require Execution

An agent's summary of its own work is a claim, not evidence. "All tests pass" sometimes means all tests pass; sometimes it means the tests that pass, pass; occasionally it means the failing test was edited until it passed. The agent is not lying — it is summarizing optimistically, and its summary is generated by the same process that generated the code. You would not accept "it works, trust me" from a human PR with no CI run attached. Apply the same standard here, with a concrete verification sequence:

  • Run the tests yourself. Full suite, your machine or CI, not the agent's transcript of a test run. This takes one command and converts the biggest claim in the summary into a fact.
  • Run the app and exercise the changed path.Tests verify what someone thought to test. Actually clicking through the changed flow — or curling the changed endpoint — catches the category of bug where the code and tests agree with each other and both diverge from what you asked for.
  • Read the test diff first. This is the highest leverage habit in this module. Tests encode what the agent thinks it built — expected inputs, outputs, and edge cases in a few readable lines. Mismatches between the ask and the build show up fastest there: if you asked for case-insensitive matching and no test exercises mixed case, you've found the gap before reading a line of implementation. The test diff is the agent's interpretation of the spec, and checking an interpretation is faster than checking an implementation.
  • Check the dependency diff. New entries in package.json, requirements.txt, or go.mod need the same scrutiny as any supply-chain addition, because that is what they are. Is the package real, maintained, and the one you'd have chosen — or a plausible-sounding name the agent reached for? Agents have recommended packages that are abandoned, obscure, or nonexistent — and attackers now register commonly hallucinated package names precisely because agents recommend them. An unrecognized dependency is a stop-and-check, every time.
  • Escalate for security- and money-adjacent code.Anything touching auth, permissions, payment flows, PII handling, crypto, or input validation gets line-by-line review with the same rigor you'd give a junior engineer's first payment-path PR — because that is the trust level the author has earned, regardless of how senior the code sounds.

Note

The unifying principle: every claim in an agent's summary is either verified by execution or it is unverified. There is no third category where a claim becomes true by being stated confidently in a bulleted list. The good news is that verification is cheap — running tests and clicking through a flow is minutes — and it composes: a repo with fast tests and one-command local runs makes verify-don't- trust nearly free, which is itself an argument for investing in that infrastructure.

Right-Sizing Review: Depth Scales with Consequence

"Review everything deeply" is not a policy, it is a bottleneck with a virtue costume on. The sustainable policy is the chunking policy's mirror image: just as you size the chunks you delegate by how much you can confidently review, you size the review by the consequence of the chunk being wrong. Three tiers cover almost everything:

TierWhat qualifiesReview depth
ThrowawayPOC one-shots, spike branches, demo scaffolding, internal scripts — code whose failure costs you a shrug and a re-prompt, and which will not be merged to a shared branchSkim the structure, smoke-test that it does the thing. Deep review here is wasted attention — the code's job is to answer a question, not to survive. The one discipline that matters: throwaway code must actually be thrown away, because "the POC that quietly became production" is how tier-one code ends up carrying tier-three consequences
ProductionVertical slices headed for main: features, fixes, refactors — the default tier for real workFull review: plan conformance, the complete smell hunt from this module, tests run, changed path exercised, every red line read. This is the tier the prime directive was written for
CriticalAuth, payments, permissions, data migrations, PII flows, anything irreversible or externally visible at scaleEverything in the production tier, plus a second human reviewer. Agent-generated or not, one-way-door code gets two pairs of eyes — the agent's involvement raises the bar here, never lowers it

Within whatever tier you are in, watch yourself for the rubber-stamp signals — the same ones from the parallel- agents module, because running multiple agents multiplies review volume and rubber-stamping is how reviewers drown quietly:

  • You approved a diff faster than you could have read it.
  • You scrolled to the end without forming a single question or objection. Real review of nontrivial code almost always surfaces at least one "why this way?"
  • You cannot summarize the diff's approach in two sentences without looking back at it. This is the acid test: if you cannot state what the change does and how, you have not reviewed it — you have scrolled it.
  • Your comments are all about formatting and naming. Surface- level comments on a substantial diff usually mean the substance went unexamined.

When you catch these signals, the fix is not "try harder" — it is structural. Shrink the chunks you delegate so each diff fits in your genuine attention span, or slow the rotation. A smaller diff genuinely reviewed beats a larger one approved on vibes, every single time it matters.

Don't Do This

Don't let the tier be decided by the diff's appearance or your calendar. "It's a small change" describes the diff, not the consequence — a one-line change to a permissions check is critical-tier. And "I'm in a hurry" is precisely the condition under which rubber stamps happen. Tier by blast radius, decided before you open the diff, and hold the line when you're busiest — that is when it protects you most.

Closing the Loop: Findings Become Rules

A review finding you fix once is a correction. A review finding you fix twice is missing context — that is the context module's doctrine, and review is where it pays out. The engineers who get compounding returns from agents are not the ones who review hardest; they are the ones who convert what review teaches them into standing infrastructure, so each class of finding happens approximately once. Three practices:

Recurring findings become CLAUDE.md rules

Every time your review catches the same smell twice, write the rule down where the agent will see it next session. Caught a second reimplemented date helper? Add "shared utilities live in lib/utils — search there before writing new helpers." Caught weakened types again? "Never widen a type or add a non-null assertion to fix a compile error; surface the error instead." Caught test edits masquerading as fixes? "Never modify an existing test to make it pass without explicit approval." Each rule is one line, takes thirty seconds, and permanently deletes a category from your future review load. Your review checklist and your CLAUDE.md should converge over time — everything you reliably catch is something the agent could have been told.

Prompt a self-review before you look

Before you spend your attention, spend the agent's: "List what could be wrong with this diff. What edge cases did you not handle? What assumptions did you make?" This catches a surprising amount — asked to critique rather than defend, agents readily surface unhandled cases, shaky assumptions, and shortcuts they took, because critique is a different generation task than justification. It does not replace your review; the agent shares blind spots with itself, and a self-review will not catch the wrong-problem-solved failure. But it is a nearly free first-pass filter, and its output tells you where to aim: everything on the agent's own worry list is a place you should look first.

Track your own review misses

When a bug escapes to production through a diff you approved, run the personal postmortem: which smell category was it, and why did your review not catch it? Skipped the red lines? Trusted a test summary you didn't run? Rubber-stamped tier-two code because it arrived at 5 p.m.? Your misses are the most precise data you will ever get about where your review attention goes and where it doesn't — far better than any generic checklist, because they are fitted to your actual blind spots. Each miss updates two artifacts: your personal review habits, and (per the rule above) the CLAUDE.md line that makes the agent less likely to produce that failure again. Review discipline that learns is the only kind that keeps up with generation that scales.

The Discipline in One Paragraph

Generation is cheap; review is the bottleneck; therefore review skill is the productivity skill, and the prime directive is absolute: never merge what you have not read and understood, because you own every merged line. Spend review effort where it is cheapest — on the plan, before generation — and hunt where agent defects actually live: plausible-but-wrong logic, invented APIs, quiet reimplementation, over-engineering, test theater, scope creep, and weakened checks, with special suspicion for the smoothest-reading code. Claims require execution: run the tests, exercise the path, read the test diff first, audit new dependencies, and go line-by-line on anything security- or money-adjacent. Scale depth by consequence — skim the throwaway, fully review the production slice, add a second human on the critical path — and watch for the rubber-stamp signals that mean you are scrolling, not reviewing. Then close the loop: recurring findings become CLAUDE.md rules, agents self-review before you spend attention, and your misses teach you where to look next. That loop is what lets review keep pace with generation — which is the whole game.

Knowledge Check

Five review scenarios. For each, pick the call a review-disciplined engineer would make.

Loading quiz...