AI Coding PR Review with Acceptance Criteria

How to review an AI-generated pull request against acceptance criteria: what to check by eye, what to run, and the failure modes that LLM-authored code slips past a quick skim.

In practice: mutate one line before approving

When an AI-generated test does not earn trust on its own, break the implementation on purpose. If the test still passes, the PR is not ready. That tiny ritual catches more fake confidence than a long review comment thread.

Mutation check:
1. Comment out the retry branch.
2. Run the retry test.
3. If it still passes, reject the PR.
4. Ask for a test that fails when the required behavior is removed.

The Mental Shift That Changes Everything

A pull request an engineer wrote by hand gets read for style, idiom, and judgment. One an LLM wrote should be read for one thing only: did it satisfy the acceptance criteria in the spec, without doing anything the spec did not authorize? That is the whole job. The spec is the rubric. If the spec does not cover a change, there is no meaningful way to review it, so send the author back to write a better spec instead of approving something nobody has a way to judge.

Stop asking "does this code look reasonable?" and start asking "which acceptance criterion does this line exist to satisfy?" Any line that cannot answer is suspect.

Run the Tests, Do Not Trust the Checkmark

CI passes. The green tick is there. That says almost nothing. Pull the branch and run the tests locally, or at minimum open the CI log and read what actually executed. A familiar pattern in AI-generated PRs is a clean green check because the test file imported the module, constructed the class, and asserted almost nothing — or mocked the one function that would have caught the bug. Watch also for tests that pass while the production path they claim to cover sits behind a feature flag the test never enables. Green. Useless.

Map Every Criterion to a Test

Now open the spec. For each acceptance criterion, search the diff for the test that demonstrates it. If you cannot point at a specific test and say "this covers criterion three," criterion three is not covered, regardless of how thorough the PR looks. Expect more rejections at this step than any other. The fix is not for the reviewer to add the tests; it is to return the PR with the missing criteria listed and let the author try again. This is also the place to insist that PR descriptions list the criteria closed. If the author cannot write "closes AC-1, AC-2, AC-4" at the top of the PR, they do not know what they built. Neither does the reviewer.

Smuggled Scope and Missing Error Paths

LLMs love to be helpful. A spec that asked for a date-parsing function comes back as a date-parsing function plus a new utility file, plus a dependency on a date library the project did not use, plus a refactor of an unrelated helper because the model decided it "could be cleaner." None of that was authorized. Walk the diff for any file, import, or abstraction the spec did not call for, and cut it or reject the PR. Unauthorized scope is how codebases rot one helpful suggestion at a time.

Then the other direction. LLMs are happy-path machines. Look specifically for: input null, network fails, file missing, user unauthorized, queue full. If the spec lists those cases, expect tests. A common find is a try/except that swallows everything, logs nothing, and returns a default. That is worse than a crash; it is a crash with the evidence deleted.

Hunt Hallucinations, Then Hand-Mutate

The model confidently calls a method that does not exist, imports a function the module never exported, passes three arguments to something that takes two. Type checkers catch most. Some slip through on code paths the tests never trigger. Grep every imported symbol and method call against the real library, by hand. Ten minutes of suspicion beats a 2am page.

Then the step most reviewers skip and the one most worth trusting. Pick one line in the new code and change it to something wrong. Flip a boolean. Return null. Comment out the line. Run the tests. If nothing fails, the tests are decorative. They exercised the code; they did not test it. Two minutes, and it catches a failure AI-written tests are especially prone to: tests that assert what the code does rather than what the spec requires.

Acceptance Criteria Belong in Given/When/Then

The review is only as good as the criteria. When a spec uses Given/When/Then, review becomes mechanical:

Given a logged-in user with an expired session token
When they submit the checkout form
Then the request is rejected with a 401
And the client shows the re-authentication modal
And no partial order is written to the database

Four assertions, four tests. If the PR has three, it is not done. Had the criterion been "handle expired sessions gracefully," the PR could be anything and the reviewer would have no grounds to push back. Vague criteria produce vague reviews produce shipped bugs.

A PR That Looked Fine

Here is the kind of PR the mutation check exists for. Consider a change adding retry logic to a payment webhook handler. Clean diff. Green CI. Two new tests, both passing. The agent even wrote a tidy PR description. Run the mutation check and comment out the retry loop entirely, and both tests still pass. The tests assert on the shape of the request object, not on whether retries occurred — the happy-path test succeeds on the first attempt, and the failure-path test mocks the HTTP client in a way that silently short-circuits the retry. In production the webhook would fail once and give up forever. The right response is to send it back with the retry criterion quoted and ask for a test that fails when retries are removed. A second attempt costs the agent minutes; the missed bug would cost an incident.

Send It Back, Do Not Fix It Yourself

The tempting move is to patch the small stuff and merge. Every time a reviewer does that, the team learns that reviewers clean up after the model, and the next PR arrives with more holes. A better default: list the failures against the spec, close the PR, ask for another attempt with a tightened spec if the original was ambiguous. The agent can try again in seconds. It is also worth grepping for the recurring offenders on every AI PR — TODOs left in, silent fallbacks to defaults when a config key is missing, bare except blocks that log nothing, hardcoded constants where the spec named an env var. None are style issues. All violate the spec.

Rubber-Stamping Is the Real Risk

Teams drift. The first AI PR gets a careful review. The tenth gets a skim. By the fiftieth, someone is approving on the green check. That is how LLM-authored code quietly takes over a codebase. The discipline is boring: pull the branch, run the tests, check the criteria, mutate a line. Every time. When the spec is sharp, review is quick and every approval is defensible. When it is vague, decline to review until it is fixed. Acceptance criteria are not a formality — they are what lets a reviewer tell signal from confident-sounding noise.

AI Review Packet to Copy

Use this when an AI-assisted PR looks tidy but the reviewer still needs to compare it against written acceptance criteria. The packet keeps the review anchored to behavior, not code style.

AI coding review packet: AI Coding PR Review with Acceptance Criteria

Decision to make:
- How to review an AI-generated pull request against acceptance criteria: what to check by eye, what to run, and the failure modes that LLM-authored code slips past a quick skim.

Owner check:
- Product owner:
- Engineering owner:
- QA or operations reviewer:

Scope boundary:
- In scope:
- Out of scope:
- Assumption that still needs approval:

Acceptance evidence:
- Test or fixture:
- Log, metric, or screenshot:
- Manual review step:

AI boundary: generated changes must stay inside the written scope and attach evidence for each acceptance criterion.

Reviewer prompt:
- What would still be ambiguous to someone who missed the planning meeting?
- What evidence would make this safe enough to ship?