How I review AI generated code as a CTO: the failure modes unique to it, why a clean analyzer proves nothing, adversarial verification, and who owns the bug.
A pull request lands. It's 340 lines across nine files, it compiles, flutter analyze comes back clean, the tests are green, and the description is a tidy three-bullet summary of exactly what was asked for. A couple of years ago that meant a competent engineer had done competent work. Today it means almost nothing, because a model can produce all four of those signals in ninety seconds without ever having been correct about the problem.
I'm not going to relitigate whether to use AI to write code. We do, heavily — at Shpper, across my Flutter tools family, and on my own site, where 98 browser tools and 32 games are now far more generated than hand-typed. That argument is over. The question that matters to me as a CTO is narrower: what does code review have to become when the author of the diff is fast, fluent, tireless, and confidently wrong in ways a human author almost never is?
A junior who doesn't understand a problem writes code that looks like it. A model that doesn't understand the problem writes code that looks like a senior engineer wrote it: same conventions, same layout, same doc comments, same defensive null checks in plausible places. The signals we've spent careers using as cheap proxies for care have been decoupled from care. This post is the discipline I use instead — the failure modes specific to generated code, why a clean analyzer is the weakest possible evidence, reading a diff for what's missing, demanding a failure scenario instead of an explanation, adversarial verification, and the two team questions nobody wants to answer.
You review differently once you know what you're hunting. These five have actually bitten us.
The happy path is almost always right — that's what most published code is, so that's what the model learned. The edges are where it invents, and the tell is a boundary handled with suspicious smoothness: an empty list returning a sensible-looking default, with no comment explaining why that default is correct.
In the QA sweep I ran across the games last month I found twelve real correctness bugs — three high, nine medium — and nearly all lived at edges. My favourite: a keno payout table off by one row at the top end, so the single rarest and most valuable outcome paid the second-best amount. It read as entirely reasonable in review.
First rule: spend review attention inversely to how confident the code looks. The blocks that read as obviously fine are the blocks nobody checked.
This is the one that frightens me most, because it survives every automated gate we have. Twenty lines of correct control flow with one comparison backwards: isBefore where it needed isAfter, a guard clause returning early on the case it was meant to handle. Everything around it is right, which is what makes it invisible — your eye checks whether the shape of the logic is sensible, and the shape is sensible.
Our Solitaire implementation shipped with a canStack check that returned true when card colours matched instead of alternated. Correct rank comparison, correct empty-column handling, correct king rule, one inverted colour test. The type system has nothing to say about it, and neither does the analyzer; a reviewer reading for structure sees a well-formed function. You catch it by playing one hand, or by having written the rule down before you read the code. The defence generalises: stop reading conditionals and start evaluating them — one input that should pass, one that should fail, traced by hand, on the branches where being wrong is expensive.
Hallucinated methods are the least dangerous failure in this post — the compiler catches them for free. The dangerous versions compile: where where the intent was removeWhere, putIfAbsent when the code needed an unconditional overwrite. And the truly ugly case, configuration, which has no compiler at all.
I've had a generated change add a lint rule to analysis_options.yaml that had been renamed three releases earlier. The analyzer shrugged, the build was green, and a check I believed I'd enabled did nothing for weeks. An invented key in YAML or JSON is silently ignored, and silence is indistinguishable from success. Any generated diff touching config gets verified by observing the behaviour change, never by reading the file.
The most insidious of the five, and a direct consequence of how we prompt. Ask for an implementation and its tests in the same pass and the tests get written against the implementation, not the requirement. The suite becomes circular: it proves the code does what the code does. Our Solitaire bug had a test, and the test asserted that a red seven could be stacked on a red eight. Green tick, wrong game.
The related smells I treat as blockers: assertions on isNotNull instead of a value, no negative cases, and golden tests generated from current output.
The fix is ordering. Review the test expectations against the requirement before the implementation is visible. I read the test file first, on its own, and ask whether those assertions are the ones the ticket implies. Two extra minutes, and it's the difference between a suite that verifies and a suite that ratifies.
The diff touches files nobody asked about: a formatting pass on a file that happened to be open, a variable renamed across a module, a dependency bumped, a default changed. Individually defensible; collectively, an unreviewed change surface bolted onto a reviewed one. It matters because review quality falls off a cliff with diff size — a 340-line diff gets skimmed, a 90-line diff gets read. So a change touching files outside its stated scope gets sent back rather than reviewed: a policy, not a mood, and cheap to enforce because regenerating a narrower diff costs nothing.
It's also how safety rails quietly come off. An analysis_options.yaml with one rule newly commented out is the highest-signal line in any generated diff, because it usually means the code couldn't satisfy the rule and deleting the rule was the shortest path. Same for a fresh // ignore: or a skipped test — that isn't cleanup, it's the model routing around an obstacle that existed for a reason.
Here's the one that changed how I run reviews, because both gates I trusted let it through. My portfolio is a Flutter web app: about 130 routes across the tools and games, all client-side, hosted static. Last quarter I had a refactor done to hoist shared page chrome — header, breadcrumb, footer — out of every page and into one shell widget. Mechanical, high-repetition: exactly what generated code should excel at. Around 400 lines, mostly deletions. flutter analyze: clean. flutter build web --release: succeeded. And I reviewed it properly — every hunk, the shell's constructor, three sample pages.
The shell replaced a LayoutBuilder-driven ConstrainedBox with a Column whose routed child was wrapped in Expanded. In a Scaffold body that's correct, idiomatic, and what you'd write yourself. But the shell was mounted inside a SingleChildScrollView, because that's how the footer sits correctly on short pages, and a scroll view hands its child unbounded height. In debug that combination throws a loud framework error:
RenderFlex children have non-zero flex but incoming height constraints are unbounded.
In a release build the assertion behind that error doesn't exist, so nothing throws — the framework computes with infinity and paints nothing. Every tool page rendered a correct header, a correct footer, and an empty rectangle where the tool should have been. In production, and not for the first time.
Why each gate failed, because the pattern generalises:
Expanded in a Column is type-correct and idiomatic. Flutter constraint errors are runtime by design; no static check catches this without knowing the widget's eventual parent.It was caught forty minutes after deploy, not by an alert but by a screenshot — a habit borrowed from the iOS side of the tools family, where I build, launch on a simulator and screenshot the artifact before believing it works. That's now a gate: a headless pass loads every route in the release build and screenshots it, and any route whose content region falls under a threshold of non-background pixels fails the build. Crude, and it has caught two more blanks since.
The lesson survives outside Flutter: static analysis proves your code is well-formed, not that it does anything. The only evidence a UI works is a picture of it working, and this is precisely the class of bug generated code produces most — a model optimises the file in front of it against a contract expressed somewhere it never looked. Most teams stop their evidence ladder at "a human read the diff." For a diff a human didn't author, "someone ran it and looked at the output" is the entry fee.
Review is pattern-matching on what's in front of you. Generated code is superb at that and weak at what isn't there, because absence has no token to generate. The highest-yield question isn't "is this right?" — it's "is this all?" The absences I find most often:
else. For each meaningful if: what happens when it's false? Alarmingly often the honest answer is "nothing, silently."setState after an await with no mounted check. Generated Flutter widgets forget dispose more than anything else.The technique that raised my hit rate most is embarrassingly simple: before opening the diff, write three bullets describing what a correct change would have to touch. Then compare your list to theirs. It flips review from "does this look right," which generated code always wins, to "is this complete," which it often loses.
When I question a generated change, the default reply is an explanation. Explanations are cheap, fluent and nearly always convincing — producing convincing explanations is what these models are best at, which makes it the least informative response you can ask for. So I stopped asking "why is this correct?" and started asking "under what conditions is this wrong?"
"Why is this correct" invites a rationalisation of the code as written, and succeeds whether or not the code is correct. "When does this break" requires modelling the input space, and the answers are checkable — either the scenario exists or it doesn't, and I can go run it. Three prompts I use nearly verbatim, on models and engineers alike.
That last one is my favourite: if nobody can describe the user-visible symptom of a failure, nobody has thought about the failure.
The same rule applies to fixes. "Fixed" is not a claim I accept. The claim I accept is: here is the test that failed before this change and passes after it. Without it you have a change correlated with a symptom disappearing, which is how the same bug returns in three weeks wearing a different hat.
Nearly everyone runs a second AI pass over the diff, and nearly everyone does it in the way that extracts the least value: they ask it to review. Ask a model to review code and you get a review — balanced, professional, a few style notes, "overall this looks good." It's agreeable by construction: tell it the code is fine and it agrees, tell it the code is suspect and it agrees with that instead. That's not something you can review your way out of; it's what you get for asking a confirming question. So structure the second pass adversarially:
That's exactly how the games sweep worked. Instead of "are these 32 games correct?", I pasted each game's published rules alongside the implementation and asked where the two diverge. Framed that way it produced twelve bugs; framed as a review, the same model told me the games looked solid.
This is the part I think about most and am least confident I've solved. The risk isn't juniors using AI. It's a junior whose entire loop is: read ticket, prompt, paste, CI green, open PR, forward my review comments back into the prompt, repeat. That person is a router. They ship, sometimes quite fast, and they are not becoming an engineer, because judgment is built by the friction of not knowing and the loop removes precisely that friction.
The signal is worth checking deliberately: ask an engineer "what did you try before this?" and "what would break if we did the opposite?" about their own PR. If the answers stay generic, that's a process gap, and it's mine to fix. What works for us:
Short answer: the human who merged it. Not the person who prompted, not the model, not "the AI." The merger. Say it early, because otherwise the ambiguity gets discovered during an incident. The first time "the AI wrote that" appears in a postmortem timeline and nobody pushes back, quality is finished — you've minted an unfalsifiable excuse available for every line in the repository. What that costs in practice:
git blame has to return someone who can be asked a question.Ownership is what keeps the rest honest: every discipline here costs time, and the only durable reason anyone pays it is that the bug will carry their name.
None of this is about distrusting the tool — most of the code I ship starts as a generated diff. It's about noticing that the signals we used to read as evidence of care are now free, and free signals carry no information. What still costs something is running the thing, looking at the output, naming the conditions under which it breaks, and putting your name on the merge. Review the behaviour, not the prose.