Lesson 0005 · Core Designer · Module 1
Structural Issues in AI-Generated Code
AI output is legacy code from an author who won't answer questions. This lesson turns your structural eye on the five defects AI code most reliably carries.
Mission tie-in: you'll generate more code with AI, not less. The skill that keeps you in control is the ability to review structure the way you'd review a stranger's pull request — this lesson is that review checklist.
Knowledge: the five recurring defects
AI produces fluent, plausible code — and fluently repeats the same structural defaults:
- God functions and helper sprawl. One function that does everything, or the opposite: twenty micro-helpers with no shape, built to keep each output chunk short.
- Hidden global state. Module-level mutable containers, caches, and singletons that make behavior depend on call order.
- No invariants. Data accepted and mutated freely; invalid states possible by construction.
- Bare exception handling.
except Exceptionwithpass, or errors converted toNone— failure becomes invisible and travels. - Missing seams. Hardcoded I/O, sleep, and environment access inside logic that should be pure and testable.
Treat AI output as legacy code from an unknown author. The discipline that applies is the one Michael Feathers wrote for legacy code: find the seam, lock the behavior with a characterization test, then restructure with confidence.
def process_all(users: list[dict]) -> None:
processed = []
cache = {} # rebuilt every call: dedup never spans batches
for u in users:
if u["email"] in cache:
continue
try:
resp = requests.post("https://api.example.com/notify", json=u)
if resp.status_code != 200:
return # silently stops mid-batch
cache[u["email"]] = True
processed.append(u)
except Exception:
pass # swallows everything
return processed # annotated -> None, yet returns a list on the happy path
Name what's wrong — and this snippet carries nearly every defect on the list: network I/O welded to logic (no seam, so no test runs without a network), a bare except Exception that swallows a typo as readily as a timeout, a mid-loop return that abandons the remaining users and hands back None, a return type that contradicts the annotation, and a cache rebuilt on every call — so the de-duplication it appears to buy never survives a second batch.
The five defects are not random. They follow from what a language model is doing: producing the most plausible continuation given your prompt. The corpus that defines "plausible" is dominated by short, self-contained, illustrative code — tutorials, documentation snippets, question-and-answer posts. That code has properties its authors were right to choose and you are wrong to inherit: no dependency injection, because a snippet has nobody to inject; a defensive try/except, because an example that raises looks broken; module-level state, because a snippet has no lifecycle to hang it on; and every step inline, because a reader wants one screen. Ask for a function and you get an excellent snippet. A snippet in a system is a defect.
Three consequences follow, and each changes how you review.
The defects are correlated, so a checklist works. With human code you need judgment, because the mistakes are idiosyncratic. With generated code the same five shapes recur, which means a fixed list catches most of it — a rare case where a checklist genuinely outperforms attention.
The code is locally correct and globally wrong. Every line survives scrutiny; the problem is the shape, and shape is invisible one line at a time. This is precisely the failure mode line-by-line review is worst at, which is why reviewers who are good at spotting off-by-one errors miss AI structure entirely.
Fluency inverts the trust signal. A junior's messy code triggers care; polished code with docstrings and type hints triggers a skim. The generator produces the surface features that humans use as a proxy for diligence, and it produces them regardless of whether the diligence happened. Your calibration was trained on a world where those two things travelled together, and it no longer holds.
There is also a throughput mechanism worth naming. Generation has become nearly free; review has not. On a team of five that merges thirty per cent more code per week with unchanged review capacity, the binding constraint quietly moves from writing to reviewing, and any process that assumed "code produced ≈ code read carefully" starts failing without announcing it. The response is not to review harder; it is to make structure checkable by a machine (lesson 0010) so human attention is spent on the things a machine cannot judge.
Where it stops applying. This checklist is about structure, not correctness — it will never tell you the discount rule is wrong; only an example-based test written from the business rule can. It is also waste on genuinely disposable code: a one-off analysis script that runs once and is deleted has no future readers to protect. And when the model is given a strong frame, the defect profile changes — a well-specified generation fails in different places, so the list should be revisited rather than recited.
How a senior designer reviews that snippet
They read the signature before the body. def process_all(users: list[dict]) -> None — and the function returns a list. That contradiction takes four seconds to find and tells you the annotation was decoration, not a claim anyone checked. Contracts are where lies are cheapest to detect, which is why they are read first.
Then one question about testability: what would I have to stand up to run this? A network. That single answer settles the seams question without reading the loop: any test of this function is an integration test, so in practice there will be no test.
Then they hunt for silent exits — every path on which the function ends without saying anything. There are two: the return inside the loop and the except Exception: pass. The question that turns those into a business statement is what does the caller see when thirty of five hundred users fail? The answer is None, which is indistinguishable from the result of an empty input. A partial failure has been laundered into a success.
The rejected fix. The reflex is to wrap the call site:
try:
process_all(users)
except Exception:
logger.exception("notification batch failed")
This is treating the symptom. The function still cannot be tested without a network, still abandons the batch mid-loop, and still returns nothing the caller can act on — and now there is a log line that fires only in the cases the function did not swallow, which is the opposite of the ones you need to hear about. A reviewer's tell: if the fix is entirely outside the function, you have decided not to fix it.
The consequence over time. Five hundred users, item twelve raises, four hundred and eighty-eight notifications are never sent, and the batch reports success. Nobody learns anything until a customer asks why they were not told about something three weeks ago — and by then there is no log line, no failure record, and no way to reconstruct which users were skipped, because the only artefact that ever knew was the traceback that pass deleted. The cost of the defect is not the outage; it is that the system destroyed the evidence needed to understand it.
| You see this | What it costs | The move |
|---|---|---|
except Exception: pass | The failure is erased at the moment it was most informative. | Catch the specific error; record it or re-raise. |
| _cache = {} at module scope | Behaviour now depends on import order and call order. | Pass the cache in, or scope it to one object. |
| requests.post inside a rule | No seam: the test needs a network to run at all. | Inject a port; the adapter owns requests. |
| return inside a batch loop | One bad item silently abandons every item after it. | Collect a result per item; report the whole batch. |
| six one-line helpers, each used once | Helper sprawl: shape without structure, nothing named. | Inline them; keep only boundaries that mean something. |
When this is the wrong move
Skip the checklist when the code is genuinely disposable — a scratch analysis, a one-shot migration, a spike you will read once and delete — because structure is an investment in future change and there will be none. Skip it when the generated unit is a leaf: a pure function with no I/O, fully pinned by example tests. There, read the examples against the business rule instead; structure is not the risk. And be honest about the arithmetic on small pieces: if applying the five points costs more than writing the twenty lines yourself, write them yourself. The checklist earns its place on code that will be changed by someone else, later, under time pressure.
- AI writes bad code.
- It writes average code, which is a harder problem. Bad code announces itself and gets reviewed carefully. Average, fluent, well-formatted code with type hints defeats the instinct that something needs a second look — the defect and the camouflage arrive together.
- Better prompting removes the need to review.
- Framing shifts the distribution of defects; it does not truncate the tail, and the tail is where the expensive ones live. What good framing actually buys you is that the remaining defects are in the filling rather than the frame, so a review can be shallow in the right places.
- Tests generated alongside the code prove it works.
- Tests written from the same context tend to assert what the code does, including its bugs — they are characterization tests in a unit test's costume. That makes them genuinely useful as a refactoring safety net and near-worthless as evidence of correctness. Correctness evidence comes from examples derived from the requirement, not from the implementation.
- Careful line-by-line review catches it.
- Line-by-line review is the mode AI output passes most easily, because every individual line is defensible. The defects live in what is absent: the seam that was never created, the invariant nobody stated, the failure path that returns nothing. Review the shape and the contract, then spot-check the lines.
Skill: run the AI review checklist
In the snippet, the mid-loop return is dangerous because:
The worst thing about except Exception: pass in that loop is:
Best first step when AI code "works" but scares you:
Practice on your own code
Take a function AI generated for you recently. Run the five-point checklist (god function, hidden global state, missing invariants, bare exceptions, missing seams). Before fixing anything, answer the two senior questions: what would I have to stand up to run this, and what does the caller see when part of the work fails? Then fix exactly one defect, with a test that would have failed before.
In the lab: parts of the returns-resolution service were generated rather than designed, and carry these defects verbatim. The phase-1 checkpoint asks you to run the five-point review over one generated module, write a characterization test that pins its current behaviour — including the behaviour you disagree with — and only then change the structure.
Reveal: the snippet's refactor direction
Split into two seams: a pure de-duplication decision the caller owns, and a Notifier port the HTTP call hides behind. Every item gets a recorded outcome, so a partial failure is a fact the caller can act on rather than a silence.
class Notifier(Protocol):
def notify(self, user: dict) -> None: ...
def process_all(users: list[dict], notifier: Notifier) -> BatchResult:
result = BatchResult()
seen: set[str] = set()
for user in users:
if user["email"] in seen:
result.skipped(user)
continue
seen.add(user["email"])
try:
notifier.notify(user)
except NotifyFailed as exc:
result.failed(user, exc)
else:
result.done(user)
return result
Read what changed structurally, not cosmetically. The network moved behind a port, so the test runs offline with a list-backed fake. The except names one error type, so a typo still crashes the way a typo should. No path leaves the loop early, so five hundred users produce five hundred outcomes. And the return type is now a thing the caller can branch on, which is what makes "twelve failed" a fact rather than an absence. Failure semantics get their own treatment in lesson 0014.
- Explain to a junior, in two sentences, why fluent, well-formatted generated code deserves more structural suspicion than messy human code, not less.
- Find one generated function in your own repository this week and name its cost concretely: what a test of it currently requires, and what a partial failure currently tells the caller.
- Predict what breaks in a year if your team keeps merging at the current rate without a machine-checkable structural rule — name the first architectural property that will quietly stop being true.
Your win
You can now review AI output with a five-point checklist instead of vibes — and you've learned the deeper move: treat it as legacy code, lock it with tests, then restructure.
Read and watch deeper
- Working Effectively with Legacy Code, Feathers — ch. 4 "The Seam Model" (where a seam is defined as a place you can change behaviour without editing in that place) and ch. 13, "I Need to Make a Change but I Don't Know What Tests to Write", which is where characterization tests are introduced.
- Google's code review guide — what to look for: a primary source on reviewing design before lines, written for exactly the case where you did not write the code.
- A Philosophy of Software Design, Ousterhout — ch. 16 "Modifying Existing Code": how to improve unfamiliar code without breaking it.
- Clean Code, Martin — ch. 7 "Error Handling": why bare excepts and silent returns are structural defects.
- Watch: ArjanCodes YouTube — search "AI code review" or "AI coding best practices".
- Next: lesson 0010 covers steering AI toward better structure from the start.
- Reference: Patterns — seam / characterization test; glossary — seam, characterization test.
Paste an AI-generated function to your agent-teacher and ask for the five-point review — then argue with its findings.