Chapter 39 - Review Patterns For AI Code

Part: VIII - The Field Manual

Thesis

AI-generated code fails at plausibility boundaries, not complexity boundaries. Review patterns for AI code target those boundaries — not the surface coherence the model optimizes for.

Key Line

Review the system, not the prose.

Why AI Code Needs Different Review

Human code fails where humans get things wrong: complex algorithms, edge cases they did not think of, interfaces they misunderstood.

AI code fails differently. The implementation is often fluent and well-structured. The function names are good. The comments are clear. The failure is in a place the model did not know was wrong: a deprecated API, a version assumption, a state that is almost always true but sometimes is not. The failure is masked by the confidence of the surrounding code.

A reviewer applying human-code review instincts to AI code will miss these failures. The review asks “is this clear and correct-looking?” The plausibility trap (Ch 31) answers yes, and the bug reaches production.

AI-code review asks: “what would make this wrong while still looking right?” That question requires a different set of lenses.

Read The Code Before The Explanation

The model often explains what it generated before or alongside the code itself.

The explanation is correct. The explanation describes what the code does. The code has a bug.

The explanation creates a mental model in the reader. The mental model filters the code reading. The bug is in a line where the explanation said the code would be right, so the reviewer’s attention skips over it.

Read the code before reading the explanation. Form an independent understanding of what the code does. Then read the explanation. If the explanation matches your reading, the review is coherent. If they diverge, the divergence is where the problem lives.

The Two-Pass Review

Pass one — behavior: What does this code do? Not what does the model say it does. What does the code actually do when run? Trace the execution path for the main case, then for the empty input, then for the error case.

If you cannot trace the execution for the error case, that is a finding. The error handling is not reviewable.

Pass two — failure modes: For each section of the diff, ask: what would make this wrong while still looking correct? Then name the test that would catch each failure mode. If no test exists for a named failure mode, either add the test or accept the risk and document it.

Observable Markers Review

When the session used the Observable Markers pattern (Ch 38), the review is guided by the markers themselves.

Every VCB-MARKER comment names what was changed and what to test. Every colored UI border marks a new element. The review lane for markers:

Find every marker in the diff.
For each marker: verify the described change is present and correct.
Run or watch the demo with the markers visible. The reviewer sees exactly what the session touched.
When satisfied, ask the agent to sweep all markers. Verify the sweep is complete.
The clean version is what goes to review.

Worked example. A demo recording shows a login flow with a yellow-bordered form component and the console output [VCB-MARKER] New login form rendered. The reviewer knows: this form is the change. Everything else in the flow was already passing. The review focuses exclusively on the new form. Adjacent flows do not need re-review. When the form passes, the border is removed and the console log is deleted in a single targeted sweep.

This pattern transforms “review the entire PR” into “review the marked changes.” The diff may touch five files. The markers show that only two sections represent new behavior. The other three are plumbing changes that the existing tests cover.

The Lane Structure

Different types of changes carry different risk profiles. Review each lane separately.

Lane	The question	What AI gets wrong here
Diff scope	Does every changed file belong to the stated task?	AI touches adjacent code, adds unrequested refactors
Test quality	Would each test fail before the fix and pass after?	AI generates tests that mirror the implementation, not the requirement
Security	Did any capability expand, even slightly?	AI adds permissions or opens surfaces it was not asked to touch
Version assumptions	Does this code work with the installed library version?	AI describes APIs from its training cutoff, not the installed version
State assumptions	Does this code assume state that is not always true?	AI assumes authentication, open connections, or cached values
Error paths	Are the error handlers reachable and correct?	AI adds error handling that looks complete but handles wrong conditions
Dependencies	Was any new package added and is it justified?	AI adds convenience libraries without weighing the dependency cost
Migration	If this touches data, is the migration reversible?	AI generates one-way migrations without rollback paths

Do not run all lanes on every PR. Match lanes to risk. A CSS change needs Diff scope and maybe UX. A database migration needs all of them.

The Behavioral Equivalence Test for Refactors

A refactor must preserve observable behavior. That is its definition.

When reviewing a refactor diff, the question is not “is this cleaner?” The question is “does this do the same thing?”

Specific things to check:

Renamed variables: does the new name have the same type and semantics as the old?
Changed default values: does the caller chain expect the old default?
Modified guard clauses: does the new guard reject the same inputs as the old?
Altered error handling: does the new handler propagate the same error types to callers?
Restructured control flow: does every path through the new code exist in the old code?

If any of these is unclear from reading the diff, run the old and new versions against the same test suite and compare. If the test suites are not sufficient to surface the difference, the refactor is not yet reviewable.

Reviewing AI-Native Architecture Changes

Everything above reviews a diff. An AI-native system needs a second kind of review, aimed at a different question: not “is this code correct,” but “what new thing can the model’s output now become real.”

A change to a system prompt, a tool’s schema, a validator’s rule, which model handles a request, or what gets retrieved into context is an architecture change even when it touches no application logic at all. Reviewing it as if it were a copy edit misses what actually changed: the model’s proposal surface, or the checks standing between a proposal and a commit.

The reviewer’s question for this class of change is not “does this code do what it says.” It is “what new output can now become real that could not before, and what checks it.” A widened tool schema, a loosened validator, a new model with different failure characteristics routed into an existing pipeline - each of these can leave the surrounding code completely unchanged while quietly expanding what the system will accept.

Model role. Did this change what the model is being asked to propose, interpret, or decide? A model that was narrating game state and is now also proposing the outcome has crossed a line worth naming explicitly.

Tool capability. Did any tool’s exposed capability widen? A tool that gained a new optional field, a wider numeric range, or a new action type has a larger proposal surface than it had yesterday.

Permission scope. Does this change touch who or what may invoke a capability, and was that scope narrowed or widened?

Schema changes. A schema loosened to accept more shapes accepts more invalid shapes along with the valid ones it was loosened for. Chapter 16’s unknown-field and versioning discipline applies at review time, not just at design time.

Validator order. If a pipeline runs structural, authorization, and domain checks in sequence, did this change reorder them, skip one, or make one conditional in a way that could let a proposal through a gate it used to have to clear?

State ownership. Did this change move a decision that used to belong to a deterministic component - a rules engine, a solver, a permission table - toward the model, even implicitly?

Prompts and retrieval. Did the specification the model receives change - a different retrieval source, a shorter prompt, a removed example - in a way that could weaken what the model is being told before it generates?

Model routing. Did this change which model handles which requests? A cheaper or faster model routed into a path that used to use a stronger one may need specification that was previously optional, per Chapter 7’s discussion of how stronger models hide weak specifications.

Trace behavior. Does this change affect what gets logged, and would a rejection or an escape from this path still be visible after the change?

The Sink-First Review Pattern

A useful discipline for AI-native review: instead of starting from the diff and asking what changed, start from each consequential sink the system has - a database write, an executed tool call, a public publish call, a user-visible render - and trace backward to the gates that currently guard it.

For each sink: what can reach it? What checks run before it’s reached? Did this diff touch any of those checks, directly or by changing something upstream that feeds them? A diff that never mentions the sink by name can still be the diff that weakened its gate - a loosened schema three components upstream, a retrieval source that now includes less-trusted content, a routing change that sends more traffic through a path with a known validation gap.

Sink-first review catches what diff-first review misses: changes whose consequence is somewhere else entirely from where the lines actually moved.

Reviewing The Proposal vs. Reviewing The Control System

These are two different reviews, and a change can pass one while failing the other.

Reviewing the proposal asks: is this specific patch, this specific tool call, this specific claim correct? That is the two-pass review earlier in this chapter, applied to one instance of output.

Reviewing the control system asks: given everything this system can now produce, is there still a check standing between every consequential proposal and its commit boundary? That is architecture review, and it does not require any specific proposal to have gone wrong - it asks whether the system would catch one if it did.

A PR that adds a well-tested, well-reviewed new tool can still weaken the system if nobody reviewed whether that tool’s capability was properly gated. The proposal review passed. The control-system review was never run.

What CI Cannot Catch

CI runs the test suite. The test suite catches what it covers.

CI cannot catch:

Failure modes that no test exercises
State assumptions that are almost always true in the test environment
Version drift between what the model generated and what the installed library actually does
Behavioral changes that happen to produce the same output for all tested inputs
Observable markers that were supposed to be removed before merge

Human review covers the gaps CI leaves open. The review lanes above are organized around these gaps.

The Sign-Off Question

Before approving any AI-generated change, answer this question aloud: “If this change causes a production incident, can I explain what happened and why the change was approved?”

If the answer is no, the review is not complete. Something in the change is not understood.

This is not a gatekeeping exercise. It is the ownership test from Ch 30. The reviewer who cannot explain a change cannot own the decision to ship it. The change may be correct — but correctness that is not understood is not owned.

Practical Artifact — Review Lane Checklist

Lane	Required for	Question
Diff scope	All changes	Does every changed file belong to this task?
Test quality	All changes	Would each test fail before this change and pass after?
Observable markers	Sessions using markers	Were all VCB-MARKER annotations removed before review?
Security	Any auth, data, or capability change	Did capabilities expand? Did any boundary open?
Version assumptions	Any library usage	Does the generated code match the installed library version?
State assumptions	Any function with external dependencies	What state does this assume? Is that state always true?
Error paths	Any error handling	Are error handlers reachable and do they handle the right conditions?
Dependencies	Any new package	Is this package necessary? What is its maintenance status?
Migration	Any schema or data change	Is this reversible? Has it been tested on production-sized data?
Architecture (AI-native)	Any change to prompts, schemas, tools, validators, model routing, or retrieval	What new output can now become real, and what checks it?
Sink-first	Any change touching a consequential sink, directly or upstream	For each sink this system has: what reaches it, and did this diff touch any gate in front of it?
Sign-off	All changes	Can you explain what happened and why you approved it if it fails?

Export

Copy this block into your CLAUDE.md, agent instructions, or project checklist.

Review the system, not the prose.

vcb_chapter: 39
title: "Review Patterns For AI Code"
key_line: "Review the system, not the prose."
thesis: "AI-generated code fails at plausibility boundaries. Review must target those boundaries, not the surface coherence the model optimizes for."
checklist:
  - item: "Read the code before reading the model's explanation"
    protects: "against explanation-guided review that misses bugs in coherent code"
  - item: "Run the two-pass review: behavior first, then failure modes"
    protects: "against reviews that assess clarity without testing correctness"
  - item: "Verify observable markers were swept before merge"
    protects: "against prototype markers reaching production code"
  - item: "Check version assumptions against installed library versions"
    protects: "against deprecated API usage that passes review but fails at runtime"
  - item: "For refactors: verify behavioral equivalence, not just syntactic change"
    protects: "against refactors that introduce behavioral bugs under clean-looking diffs"
  - item: "Answer the sign-off question before approving"
    protects: "against approving changes that are not understood and therefore not owned"

Code read before explanation? — protects against explanation-filtered review
Behavior traced for main case, empty input, and error case? — protects against untested paths
Observable markers swept before this review? — protects against prototype artifacts in production
Version assumptions checked against installed library? — protects against training-cutoff drift
State assumptions named and verified? — protects against almost-always-true failures
For refactors: does every changed guard, default, and error handler match the original? — protects against invisible behavioral change
Can you explain this change if it causes an incident? — protects against unowned approvals