Reviewing a change an agent made

A change arrives with a description of what it does, written as confidently as the code. It might come from an agent in your editor, from a delegated agent working on a branch, or from a colleague who used one. AI code review, refactoring and maintenance covers what an automated first pass costs. This guide is the order a person works in when the judgement is theirs.

The order matters. The early checks are cheap, and they decide whether the expensive one, reading every line, is worth doing yet. Each pass below can end the review with the change sent back.

The example is the refund flow's provider call, built from the brief in Writing a brief for an unattended agent: send a requested refund to the provider, record the answer, and reuse the stored idempotency key on any retry.

Before you open the diff

• Open the brief, the requirements it names and the acceptance tests. Without them you can judge whether the change reads well, and not whether it is the right change. A change can pass every check and still be wrong, which is the argument of Code review is the new bottleneck.
• Leave the description for last. It is the author's claim about the change, not evidence of it. Read the diff first, then compare the two. Where they disagree, the diff is what ships.
• Decide whether it can be reviewed in one sitting. If it cannot, send it back to be split. A diff too large to hold in your head gets approved on trust. Sizing tasks an AI coding agent can finish is about sizing the work so it arrives reviewable in the first place.

Pass 1: what it touched

Before reading any code, look at the shape of the change.

# Every file changed on the task branch, with line counts
git diff --stat main...task/03-provider-call
 
# Anything under a path the brief said to leave alone
git diff --name-only main...task/03-provider-call -- src/payments/ migrations/ specs/
 
# Test files, which pass 2 reads first
git diff --name-only main...task/03-provider-call -- tests/

Then ask four questions of the list.

1. Does every file belong to the behaviour in the brief? A file that does not is either the brief's gap or the agent reaching past it, and either way it needs a reason.
2. Did anything change under a path the brief fenced off? That stops the review until you know why.
3. Did the dependency manifest change? Every package it added is one you now maintain, so ask whether the problem needed it.
4. Did configuration, the pipeline or a test fixture change? Those change how everything else is judged, so they are read as carefully as the code.

Pass 2: the tests, before the code

Read the changes to test files first. An edited assertion in an existing test is an edited requirement, and it needs a reason you accept, or it is reverted. A test file the brief said not to edit, edited, ends the review.

Then map each acceptance criterion to the test that checks it, in a short note kept with the review:

| Criterion                                | Test                      | Fails if broken? |
| ---------------------------------------- | ------------------------- | ---------------- |
| R5: confirmed answer sets `succeeded`    | provider-call "confirmed" | yes              |
| R5: declined sets `failed`, keeps reason | provider-call "declined"  | yes              |
| R6: a retry sends the row's stored key   | provider-call "retry"     | no, see below    |
 
Not in the brief, found in the diff:
- a 30 second timeout on the provider call (task 4 owns timeouts)

A criterion with no test is unverified, whatever the description says. A test that maps to no criterion is scope nobody asked for, or a requirement nobody wrote down, and both are worth knowing about.

The last column is the important one. A test that passes is only evidence if it would fail without the code it covers, and the quickest way to find out is to break that code on purpose. For each criterion, change the one line it depends on, run its tests, and expect red:

-  const key = refund.idempotencyKey;
+  const key = newIdempotencyKey(); // reviewer's break: a fresh key on every call

If the suite stays green, nothing checks R6, and the "retry" test is checking something else. Restore the line and send the gap back: a test that fails when the key changes between two calls for the same refund. This is the check AI testing and debugging recommends, done on the two or three lines that carry a criterion rather than across the codebase.

Pass 3: the code

Now read the code, against the design the brief pointed to. Some things deserve a deliberate look, because they are what a plausible change hides.

• Decisions nobody made. Compare the diff with the hand-back list of decisions the agent says it made. Anything visible in the diff and missing from the list is a decision it did not mention. Look hardest at every caught error, every default value and every new state, because that is where a failure gets quietly logged and swallowed.
• Stand-ins. A hardcoded response, a fake service, a placeholder table or a comment promising to finish later, in code that is not a test. The search after this list finds the obvious ones.
• The symptom fixed instead of the cause. A new null check, a wider catch, a retry or a loosened type, in a change meant to fix behaviour. Ask for the cause in one sentence before accepting the fix.
• A second way of doing something. A new helper beside an existing one, a new error style, a lookalike component. If the codebase already has a way, the change should use it, and if the rules file did not say so, that is a line to add to it.
• Money, identity and migrations, line by line. Elsewhere, reading for shape and intent is enough. On these paths, read every line and be able to defend each one, because the mistakes there are the ones that cannot be undone by the next change.

git diff main...task/03-provider-call -- src/ | grep -nE '^\+.*(TODO|FIXME|placeholder|stub|fake|hardcoded)'

A hit is not proof of a stand-in, and a clean result is not proof there is none. It is a fast way to find the ones that announce themselves.

Pass 4: run it

Let the checks that need no judgement run without you, on every change: tests, lint and type check. A green result reported in the description is a claim like the rest of it, so the result that counts is the pipeline's, on a clean checkout.

Then try the behaviour once yourself, the way the requirement describes it. For the provider call, that is a refund sent to the stub that confirms, one sent to the stub that declines, and one sent twice.

Merge, fix, or rewrite the brief

Three outcomes, and the choice between the last two is the one people get wrong.

• Merge when every criterion has a test that fails without the change, nothing crossed a boundary, and you could explain every decision in the diff to whoever is on call. "The agent wrote it" is not an explanation, the point Vibe coding builds fast, but can you explain it later? makes about the incident nobody can debug.
• Fix it in place when the gap is local and the brief was right: a missing test, a misleading name, a line in the wrong module.
• Reject and rewrite the brief when the change is wrong because the brief allowed it: an invented decision you disagree with, a stand-in, a data shape nobody chose. Patching the diff fixes this change and leaves the brief to produce the same mistake on the next run. Fix the brief, discard the branch and run the task again. A branch per task is what makes discarding one cheap.

Whichever you choose, finish the review note: the criteria, the tests, which checks you broke on purpose, and the decision. When this change breaks something weeks later, that note, not the conversation that produced the code, is what explains it, the record Delegated execution says you will want.

Make the next review cheaper

The same finding twice is a sign the fix belongs upstream. A convention the agent keeps missing becomes a line in the rules file. A kind of gap it keeps leaving becomes a line in every brief. A mistake a machine can catch becomes a check in the pipeline, so nobody has to look for it again. Review that keeps finding the same thing is review standing in for something cheaper, and review time is the scarcest thing in the pipeline.