ai-workflow-course/modules/10-reviewing-code-you-didnt-write

Module 10 — Reviewing Code You Didn't Write

The AI wrote a diff that reads beautifully and is wrong in one line you'll skim right past. Reviewing for plausibility traps — not just bugs — is the highest-leverage, least-taught skill in this whole space. This module gives you a gate to run it at and a checklist to run.

---

Prerequisites

• Module 2 — Version Control as a Safety Net. You read changes with git diff. This module turns that one-off habit into a disciplined review pass over a whole change.
• Module 8 — Remotes and Hosting. Your repo lives on a host now, and a change arrives as a pull request (GitHub/Gitea/Forgejo) or merge request (GitLab) — same thing, different name. We'll write "PR" throughout; it's the unit of review.
• Module 9 — Issues and the Task Layer (helpful, not required). A PR usually answers an issue; the issue is the "what I asked for" you review the diff against.

If you only have Modules 1–2, you can still do the core skill of this module locally — reviewing a diff between two branches with git diff — and skip the part where you open it as a PR on a host.

---

Learning objectives

By the end of this module you can:

1. Use a pull request as a review gate: nothing reaches the main branch without passing through a diff someone (or something) signed off on — even on a solo repo.
2. Read an AI-generated diff the right way: against the request, deletions first, the diff over the AI's own description of it.
3. Name and spot the four plausibility traps — invented APIs, silent scope creep, deleted edge-case handling, and convincing-but-wrong logic — that pass a human skim and a quick run.
4. Run a repeatable AI-diff review checklist and end every review with an explicit approve / request changes decision you can defend.

---

Key concepts

The gate, not the formality

A pull request proposes merging a branch into another (usually main) and pauses there so the change can be looked at before it lands. On a team that pause is where review happens. The trap is treating it as a rubber stamp — "looks good, merge" — which is exactly how bad changes get the institutional blessing of "it was reviewed."

Reframe it the way you already think about change control: a PR is a change gate, and merge is a one-way door. Once it's on main, it's in everyone's next clone, in CI, on its way to a deploy. The cheapest place to catch a problem is in the diff, before the door closes. You can recover after (that's Module 12), but recovery is always more expensive than the review you skipped.

This holds even when you're the only human on the repo. That's not bureaucracy for its own sake — the syllabus's own course repo opens a PR for every module for exactly two reasons that apply to you solo:

• Traceability. The PR is a durable record of what changed and why, linked to the issue it answers. git log tells you the change happened; the PR tells you the reasoning, the discussion, and what was rejected.
• A forced read. Opening the PR makes you look at the whole change as one diff, away from the editor you wrote it in. That context switch is where you catch the thing you were too close to see while generating it.

When the author is an AI, both reasons get sharper. The AI produced the change with total confidence and no memory of why; the PR is where a human supplies the judgment and the record the AI can't.

Why this is a genuinely new skill

You already know how to review human code. Reviewing AI code is not the same activity, and assuming it is gets people burned.

When a human writes a function, the bugs cluster where the human was uncertain — the gnarly edge, the bit they rushed, the TODO they meant to come back to. You can often feel the soft spots, and the code's roughness is a signal: confusing code is suspicious code.

AI output inverts that signal. It is uniformly fluent. The variable names are good, the structure is clean, the comment above the broken line confidently states the correct intention, and the one wrong line looks exactly as polished as the forty right ones. The fluency is constant; the correctness is not — and your eye has spent a career using fluency as a proxy for correctness. That proxy is now actively misleading.

So the question shifts. With human code you mostly ask "is this good code?" With AI code you have to ask "is this code true?" — does it do what it claims, against the request I actually made, using things that actually exist. That's reviewing for plausibility traps: code engineered (by a process optimizing for plausible-looking output) to pass exactly the skim you're tempted to give it.

The four plausibility traps

These are the failure modes to hunt for specifically. They're not random bugs; they're the characteristic ways fluent-but-untrue code goes wrong.

1. Invented APIs. The model reaches for a function, method, keyword argument, flag, config key, or endpoint that should exist by analogy — and doesn't, or exists with a different signature. It's the same generative move behind hallucinated package names (the supply-chain version of this gets its own treatment in Module 15). The tell is that it reads more natural than the real API, because it was generated to be plausible rather than recalled from docs. Classic shape: assuming list.pop(i, default) works because dict.pop(k, default) does. Verify every unfamiliar symbol against real docs or source — confidence in the surrounding prose is not evidence.

2. Silent scope creep. You asked for one thing; the diff does that thing and quietly "improves" three others it was never asked to touch — reformatting a file, reshuffling imports, renaming a variable across the module, "simplifying" an unrelated function. Each extra edit is an unrequested change you now have to review with no stated intent behind it, and it's where regressions hide. The discipline: every hunk must trace back to the request. Anything that doesn't is guilty until proven innocent, and the right move is often "take it out and do it in its own PR."

3. Deleted edge-case handling. The most dangerous trap, because it lives in the - lines you skim. While implementing the feature, the model drops a bounds check, removes a None guard, collapses a try/except into the happy path, or — worst — replaces a real error with a silent swallow (except: pass) under the banner of "making it robust." The code now looks cleaner and passes every test you'd casually run, because you'd test the path that works. The bad input that the deleted guard existed to catch now fails silently. Read every deletion. Deletions are where behavior disappears.

4. Convincing-but-wrong logic. An inverted condition (if not x where it meant if x), an off-by-one, < where it meant <=, and where it meant or, a filter quietly dropped from a comprehension. On the happy path it often produces a believable-enough result, and the comment above it cheerfully describes the correct behavior — so the comment actively vouches for the bug. The defense is to trace one real call through the changed code yourself instead of trusting the narration.

A real AI diff usually has most lines correct and one trap buried in legitimate work — which is what makes it dangerous. The feature genuinely works when you try it; the trap is somewhere you didn't look.

How to actually read the diff

Mechanics first. You want the change as one reviewable unit, separate from the code you wrote it in:

git fetch                       # get the branch the PR is built from
git diff main..feature-branch   # the whole change, as one diff

On your host's PR page you get the same diff with line comments, file-by-file navigation, and the CI results attached — use it. But the content of the review is the same whether you read it in the browser or the terminal.

Then run the pass in this order (the full version is in lab/ai-diff-review-checklist.md — keep it open while you work):

1. State the request in one sentence. This is your scope yardstick. If it answers an issue (Module 9), that's your sentence.
2. Read the diff, not the AI's summary. The summary tells you what it intended; the diff is what it did. Only the diff is real.
3. Scope check. Every hunk maps to the request. Flag everything that doesn't.
4. Deletions first. Read every - line and ask what behavior just left the codebase.
5. Verify the unfamiliar. Every API, flag, and key you don't personally know exists — check it.
6. Trace one real call, including a failure case. Not the happy path — the bad input.
7. Decide. Approve only if you can explain every hunk. Otherwise request changes. The burden of proof is on the diff, not on you.

That last point is the whole posture: a diff is guilty until proven correct. "It runs" is the weakest evidence there is — the traps above are designed to run.

---

The AI angle

Every other module here makes a tool more valuable because of AI. This module is the one where the human stays in the loop on purpose, and it's worth being precise about why.

The thing AI is best at — producing fluent, confident, well-structured output — is precisely the thing that defeats the review reflex you built reviewing humans. You learned to trust clean code and distrust messy code; AI produces uniformly clean code regardless of whether it's correct, so that heuristic now points the wrong way. Reviewing AI diffs means consciously overriding an instinct that served you well for years.

And the volume cuts against you. AI makes generating a 300-line PR almost free, which quietly shifts the bottleneck from writing to reviewing — and tempts everyone to review at the speed they generate. The economics of the team now hinge on review being the gate that writing no longer is. The fluent-but-wrong line costs nothing to produce and everything to miss.

This is the human half of a loop you'll keep building. Module 11 wires this review gate into the full issue → branch → PR → review → merge motion with humans and agents as contributors. Much later, Module 24 looks at AI reviewers that comment on PRs automatically — but an automated reviewer is an assistant to this skill, not a replacement for it. You can't supervise a review bot you couldn't do yourself.

---

Hands-on lab

Lab language: shell + the Python tasks-app. You won't write Python; you'll open a PR for a real change, then review a diff the "AI" produced and catch the trap planted in it.

You'll need:

• Git, Python 3.10+, and your AI assistant.
• The starter base app in lab/tasks-app/ (tasks.py, cli.py). It's the Module 1/2 app with one addition: complete() validates the index and done turns a bad index into a clean error. Note that behavior — the trap will mess with it.
• The planted AI change in lab/ai-change.patch.
• The review checklist in lab/ai-diff-review-checklist.md.
• Optional (Part A as a real PR): the repo you pushed to a host in Module 8. If you don't have one, do Part A locally as a branch — the review skill in Parts B–C is identical either way.

Part A — Open a PR as a gate

1. Set up the base app as a repo and confirm its baseline behavior:

   mkdir -p ~/workflow-course/review-lab && cd ~/workflow-course/review-lab
   cp /path/to/modules/10-reviewing-code-you-didnt-write/lab/tasks-app/*.py .
   git init -q && git add . && git commit -qm "base: tasks-app"
   
   python cli.py add "write the review module"
   python cli.py done 99        # baseline: prints "error: no task at index 99", exits non-zero
   echo "exit code: $?"
   

   Remember that last result. A bad index is a clean, loud error today.

2. Make a small honest change of your own on a branch — ask your AI for a one-line tweak, e.g. "make the empty-list message say '(nothing to do)' instead of '(no tasks yet)'" — apply it, commit it, and open it as a PR:

   git switch -c tweak-empty-message
   # apply the AI's one-line change to tasks.py, then:
   git add . && git commit -m "Friendlier empty-list message"
   

   If you have a Module 8 remote: git push -u origin tweak-empty-message, then open the PR on your host and read your own diff in the PR view. If you're local-only: git diff main..tweak-empty-message. Either way, review your own one-line change as a diff before merging it. Get used to the gate on a trivial change so it's a reflex on a dangerous one. Merge it when you're satisfied (git switch main && git merge tweak-empty-message).

Part B — Review the AI's diff (the real exercise)

3. Now a teammate-who-is-an-AI has opened a PR. The prompt it was given was exactly: "Add a delete <index> command to the tasks app." Bring its change in on its own branch:

   git switch main
   git switch -c ai-delete-command
   git apply /path/to/modules/10-reviewing-code-you-didnt-write/lab/ai-change.patch
   git add . && git commit -m "Add delete command"
   

4. Review it before you run it. Open the checklist and read the diff as one unit:

   git diff main..ai-delete-command
   

   Work the checklist. The request was one sentence: add a delete command. Hold every hunk up to it. Read the - lines. Find the line that does something the request never asked for and that changes behavior you tested in Part A. Write down what you think the trap is before step 5.

Part C — Confirm the trap by running the failure case

5. Now verify your read by running the failure path, not the happy one:

   python cli.py add "a real task"
   python cli.py delete 0        # the requested feature: works fine on the happy path
   python cli.py add "another"
   python cli.py done 99         # the trap: compare this to your Part A baseline
   echo "exit code: $?"
   python cli.py list            # did task 99 (which doesn't exist) get marked done? did anything?
   

   In the base app, done 99 was a clean error with a non-zero exit. After this "add a delete command" change, it prints updated and exits 0 — silently claiming success while marking nothing. The diff only said it was adding delete. While in the file it also rewrote complete() to swallow the IndexError "for robustness," deleting the edge-case handling and turning a loud failure into a silent lie. That's three traps in one small hunk: scope creep (it touched complete, which the request never mentioned), deleted edge-case handling, and convincing-but-wrong logic wearing a reassuring comment.

6. Play it out. On your host's PR you'd leave a line comment on the complete() hunk — "out of scope, and this swallows the error done relied on; please drop it" — and request changes rather than approve. The feature you were asked for was fine; the PR still doesn't merge. That's the gate doing its job.

---

Where it breaks

• A checklist is a floor, not a ceiling. It catches the characteristic traps reliably; it will not catch a deep logic error that requires understanding the whole system. For changes in code you don't know, reviewing the diff in isolation isn't enough — that harder case (pointing AI at an unfamiliar codebase, and reviewing safely there) is Module 23.
• Tests catch what review misses, and vice versa. This module is human review; it pairs with automated testing and CI (Modules 13–14), which catch the regressions a tired reviewer skims past. Neither replaces the other — the trap in this lab passes a casual run and would pass a test suite that only tests the happy path. Review is what notices the test you should have.
• Review fatigue is real and AI makes it worse. Twenty fluent PRs in a day will wear down the exact attention this skill needs, and a rubber-stamped review is worse than none because it launders the change as "reviewed." Smaller PRs are the mitigation: insist the AI's changes stay small and single-purpose so each one is reviewable in full. A PR too big to review honestly should be sent back to be split, not skimmed.
• You can't review what you don't understand. If a diff uses an API or a corner of the language you don't know, "looks fine" is not a review — that's the moment to verify it exists and does what it claims, or to pull in someone who knows. The honest output of a review is sometimes "I'm not qualified to approve this," and that's a valid result.

---

Check for understanding

You're done when:

• You've opened (or branched) a change and reviewed it as a diff before merging — the gate is a reflex, even on a one-liner.
• You found the planted trap in ai-change.patch by reading the diff against the one-sentence request, and named why it's a trap (it changed complete(), which the request never mentioned, and swallowed the error done depended on).
• You confirmed it by running the failure case (done 99) and seeing the silent updated + exit 0, instead of trusting the happy path (delete 0) that worked fine.
• You can name the four plausibility traps from memory — invented APIs, silent scope creep, deleted edge-case handling, convincing-but-wrong logic — and you treat a diff as guilty until proven correct.

When "it runs" stops feeling like sufficient evidence and "I read every - line" starts feeling mandatory, you've got the skill. Module 11 takes this gate and wires it into the full collaboration loop — issues, branches, PRs, and merges — with both humans and agents as contributors.