Chapter 11 — Code Review

“Beloved, do not believe every spirit, but test the spirits to see whether they are from God.” — 1 John 4:1

“The eye of the master fattens the calf.” — old farming proverb, recorded in every culture that ever raised livestock

Why This Matters

For two weeks you’ve been writing prompts and reading what comes back. You’ve discovered something every working engineer eventually does: most AI bugs are not loud. The compiler doesn’t complain. The one example works. The output looks plausible. The code ships. And then, two weeks later, you find a bug that has been lying in wait the whole time because nobody read carefully enough to see it.

That gap — between code that compiles and code that is correct — is the gap code review exists to close. This chapter is a sustained tour of how AI fails in ways that compile, run, and look fine, and how a senior engineer reads to catch them.

Here is the rule the chapter will defend: code review is a discipline, not a feeling. It has a method. It has categories. It is something you do the same way every time, the way a doctor takes vital signs the same way every time — not because every patient is identical, but because the process is what makes sure nothing gets missed.

The discipline pairs with Chapter 4’s testing discipline. Tests catch the bugs your eyes missed. Review catches the bugs your tests didn’t think to write. Both are needed. Neither replaces the other. A great Phase 2 engineer is the one who reads the AI’s code as if it might be subtly wrong (because it might) and as if writing more tests is part of reading (because it is).

Coach’s Note — The students who skipped review last week are reading this chapter feeling caught. Good. The discipline this chapter teaches is the one that separates real software engineers from people who paste AI output into files. The rest of your career rewards review. Spend this week getting it into your hands.

11.1 — Code Review as a Discipline

When a doctor takes vital signs, they don’t decide on the spot what to measure. They measure pulse, blood pressure, respiration, temperature, oxygen — the same five things, in roughly the same order, every time. Because the discipline is the point. The discipline is what catches the thing the doctor wasn’t looking for.

Code review works the same way. You go through a checklist, every time, in roughly the same order. The checklist is not exotic. It is short. But you follow it, even when you “feel like the code is fine.”

Here is the senior’s checklist for reviewing a piece of AI-generated Java code:

1. Does every method called actually exist? (Hallucinated API check.)
2. Does every type and import resolve? (Compiler check, before reading.)
3. Are nulls and empties handled correctly? (Defensive review.)
4. Are boundaries (off-by-one, edge values) correct? (Boundary review.)
5. Does the code do what the spec says — not what the example shows? (Spec-vs-example review.)
6. Is the performance reasonable for the expected input size? (Big-O sniff test.)
7. Does the code follow the conventions of the surrounding codebase? (Consistency review.)
8. Are the names good? (Taste review.)
9. Is there code that shouldn’t be there at all? (Cruft review.)
10. Would I sign my name to this? (Accountability check.)

Ten questions. You ask all ten, every time. Some are answered fast (the compiler check is the compiler running). Some take real attention (the spec-vs-example check). The total time for a careful review of one AI-produced class is roughly 5–15 minutes, depending on size. That is cheap relative to the cost of shipping a bug.

The rest of the chapter walks each category. Then we hand you Project 11, in which you find planted bugs in real AI-shaped code.

11.2 — Category 1: Hallucinated APIs

Already introduced in Chapter 9. We say more here because this is the most common AI failure and the easiest to catch.

A hallucinated API is a method, class, library, or function the AI confidently uses that does not exist. Java 17’s standard library is closed — every class and every method is documented at docs.oracle.com/en/java/javase/17/docs/api/. If a method is not in the docs, it does not exist.

Common hallucination patterns you will see:

• A String.splitCsv() or String.toCamelCase() that sounds right but isn’t there.
• A Files.readJson() or Files.writeJson() — JSON is not in java.nio.file.
• A Math.median() (only Math.min, Math.max, and the usual arithmetic).
• A List.findFirst(Predicate) — Stream has findFirst(), List does not.
• A Map.getOrThrow(K, String) — that’s a Kotlin idiom; Java’s Map.get(K) returns null and you check.
• A Optional.getOrElse(T) — the Java name is Optional.orElse(T).
• Use of var in a context where it isn’t allowed (e.g., as a field type — var is local-variable-only in Java 17).

The diagnostic move:

1. Compile first, read second. If the compiler rejects the code, the compiler is telling you the truth. Pay attention.
2. For any method name you don’t recognize, look it up. Two-second check. Java 17 API docs.
3. For any import you don’t recognize, look it up. Especially imports from packages that don’t usually appear (com.something.someother).

Bad — AI output you should be suspicious of:

// BAD AI OUTPUT — looks plausible, contains hallucinations
import java.util.List;
import java.util.stream.Stream;

public class TextUtil {
    public static List<String> readLines(String path) {
        return Stream.fromFile(path)         // <-- no such method
            .map(String::trimEnd)            // <-- no String.trimEnd in Java 17
            .filter(String::nonEmpty)        // <-- no String.nonEmpty
            .toList();
    }
}

Three hallucinations in five lines. None of those methods exist in Java 17.

Good — the same intent, written with real APIs:

// GOOD — uses only real Java 17 APIs
import java.nio.file.Files;
import java.nio.file.Path;
import java.io.IOException;
import java.util.List;

public class TextUtil {
    public static List<String> readLines(Path path) throws IOException {
        return Files.readAllLines(path).stream()
            .map(String::stripTrailing)      // real — added in Java 11
            .filter(s -> !s.isEmpty())       // real — String.isEmpty exists
            .toList();
    }
}

Coach’s Note — When you see a clean fluent pipeline in AI output, your first instinct should be verify each method exists. Fluent pipelines are where hallucinations love to hide because they read smoothly. A method that sounds idiomatic is exactly the kind the AI is most likely to invent.

11.3 — Category 2: Null and Empty Handling

The second most common AI failure: code that works on a normal input and breaks on null, the empty string, the empty list, or the missing key.

The AI’s default is usually one of:

• Permissive: silently returns a “reasonable” default ("", 0, an empty list) on bad input.
• Crashy: NullPointerException at the first method call on a null.
• Inconsistent: sometimes throws, sometimes returns, depending on which code path the input takes.

None of these are necessarily wrong — but none of them are necessarily right either. The right behavior is the one your spec specifies. If the spec says “throw on null,” the code should throw on null. If the spec says “treat null as empty,” it should do that. The bug is when the code does neither because the AI guessed.

Bad — AI output that handles null inconsistently:

// BAD AI OUTPUT — inconsistent null handling
public class Roster {
    private final List<String> names = new ArrayList<>();

    public void add(String name) {
        names.add(name);              // happily adds null
    }

    public boolean contains(String name) {
        return names.contains(name);  // returns true if names contains null
    }

    public String greet(String name) {
        return "Hello, " + name.toUpperCase();  // NPE if name is null
    }
}

Three methods, three different policies on null. add allows it. contains happily checks for it. greet crashes on it. The class has no consistent contract. This is not a bug in any one method; it’s a design failure across the class.

Good — same class, with a stated and enforced null policy:

// GOOD — consistent: no null allowed anywhere in this class's API
public class Roster {
    private final List<String> names = new ArrayList<>();

    public void add(String name) {
        if (name == null) throw new NullPointerException("name");
        names.add(name);
    }

    public boolean contains(String name) {
        if (name == null) throw new NullPointerException("name");
        return names.contains(name);
    }

    public String greet(String name) {
        if (name == null) throw new NullPointerException("name");
        return "Hello, " + name.toUpperCase();
    }
}

java.util.Objects.requireNonNull(name, "name") is the idiomatic one-liner for that pattern; the AI sometimes uses it and sometimes doesn’t. Either is acceptable; consistency is the point.

Empty handling has the same shape. An empty string is not a null. An empty list is not a null. A Map without a key is not a null. Each is its own case. The AI will sometimes conflate them. Your review catches the conflations.

The questions to ask, every time:

• What does this method do on a null argument? Is that what the spec says?
• What does this method do on an empty input? Is that what the spec says?
• What does this method return if its result would be empty? Empty collection? Null? An Optional.empty()?
• Are these answers consistent with the rest of the class?

11.4 — Category 3: Off-by-One and Boundary Errors

Off-by-one bugs are old. They predate AI by half a century. AI inherits them because the training data is full of them.

A real-shaped example. AI is asked: “Write a Java method that returns the last n elements of a list.”

// BAD AI OUTPUT — off-by-one at the boundary
public static <T> List<T> lastN(List<T> list, int n) {
    if (n >= list.size()) return list;
    return list.subList(list.size() - n - 1, list.size());  // off by one
}

subList(list.size() - n - 1, list.size()) returns n + 1 elements, not n. The -1 is the bug.

A subtler one:

// BAD AI OUTPUT — boundary error on n == 0
public static <T> List<T> lastN(List<T> list, int n) {
    if (n > list.size()) return list;       // <-- should be >=
    return list.subList(list.size() - n, list.size());
}

This one is correct on most inputs. But if n == list.size(), the first branch (n > list.size()) is false, and we fall through to subList(0, list.size()) — which returns the full list, which is what we want, but via a subList view of the original. If the caller mutates the returned list, they mutate the original. Subtle.

What catches both of these: a test that exercises the boundary. The boundaries in code are always 0, 1, n - 1, n, n + 1, Integer.MAX_VALUE, and negative. Your test suite includes at least one of each, every time the spec admits one.

The review questions:

• For every numeric range (for loops, slice indices, array lengths), what are the off-by-one risks?
• Is the loop condition < or <=? Which is correct?
• Is the start index 0 or 1? Inclusive or exclusive?
• What happens when n == 0? When n == list.size()? When n is negative?
• Are these boundaries tested?

Phase 1’s testing discipline (Chapter 4) is exactly the muscle that catches these. Now you use it on AI output instead of your own.

11.5 — Category 4: Plausible But Wrong

The most insidious category — and the one the rest of the chapter circles back to.

Plausible-but-wrong code:

• Compiles.
• Runs on the obvious example.
• Reads like it’s doing the right thing.
• Is doing the wrong thing on inputs the AI didn’t think about.

Classic example: a method to find the median of an array.

// BAD AI OUTPUT — plausible, wrong on even-length arrays
public static double median(int[] nums) {
    Arrays.sort(nums);
    return nums[nums.length / 2];
}

Works on {1, 2, 3} → returns 2. Correct.

Works on {1, 5} → returns 5. Wrong. The median of two numbers is their average — (1 + 5) / 2 = 3.0. The AI returned the larger of the two.

The bug is silent. The compiler is fine with it. The example the AI mentally tested (“a 5-element array”) works. The case the AI didn’t test (even-length arrays) is wrong.

Good — same problem, both cases handled:

// GOOD — handles both odd and even lengths
public static double median(int[] nums) {
    if (nums == null || nums.length == 0) {
        throw new IllegalArgumentException("median requires a non-empty array");
    }
    int[] sorted = nums.clone();
    Arrays.sort(sorted);
    int mid = sorted.length / 2;
    if (sorted.length % 2 == 1) {
        return sorted[mid];
    } else {
        return (sorted[mid - 1] + sorted[mid]) / 2.0;
    }
}

(Bonus: the original mutates the caller’s array by sorting in place. The fix clones first. Another silent bug the AI introduced.)

The review questions for plausible-but-wrong:

• What inputs would most likely break this code? Did the AI handle them?
• Does the code distinguish cases the problem actually has (odd/even, positive/negative, present/absent)?
• Are there hidden side effects (mutated arguments, modified shared state)?
• If I had to write a test designed specifically to break this code, what would it look like? Run that test.

That last question is the discipline. A senior reviewer asks “what would break this?” before “does this work?” The answers reveal the bugs.

11.6 — Category 5: Performance Traps

AI sometimes produces code that works correctly but slowly. The most common pattern: an O(n²) algorithm where O(n) would do.

Real-shaped example. AI is asked: “Given a list of integers, return the list of integers that appear more than once.”

// BAD AI OUTPUT — O(n²), correct but slow
public static List<Integer> duplicates(List<Integer> nums) {
    List<Integer> result = new ArrayList<>();
    for (int i = 0; i < nums.size(); i++) {
        for (int j = i + 1; j < nums.size(); j++) {
            if (nums.get(i).equals(nums.get(j)) && !result.contains(nums.get(i))) {
                result.add(nums.get(i));
            }
        }
    }
    return result;
}

The inner loop is O(n). The outer loop is O(n). The result.contains inside is O(n). Total: roughly O(n³) in the worst case. On a 10,000-element list with many duplicates, this code takes seconds where the alternative takes milliseconds.

Good — O(n) version using a Set:

// GOOD — O(n), uses a Set to track seen values
public static List<Integer> duplicates(List<Integer> nums) {
    Set<Integer> seen = new HashSet<>();
    Set<Integer> dups = new LinkedHashSet<>();   // preserves order
    for (int n : nums) {
        if (!seen.add(n)) dups.add(n);
    }
    return new ArrayList<>(dups);
}

HashSet.add returns false if the element was already present. So if seen.add(n) is false, we’ve seen n before — add it to dups. One pass. O(n).

The review questions for performance:

• What’s the loop’s complexity in terms of input size?
• Are there nested loops over the same collection? Why?
• Are there List.contains calls inside loops? Could a Set replace the List?
• For the expected input size, is this fast enough? (Hint: if input is “small,” it doesn’t matter. If input is “any user-supplied collection,” it matters.)

You are not optimizing prematurely. You are noticing when O(n²) is clearly worse than O(n) on a problem that doesn’t need O(n²). The taste develops with reps. Project 11 will give you several.

11.7 — Category 6: Codebase Convention Drift

The AI does not know your codebase. The AI was trained on the global corpus of Java. When the AI writes a method for your file, it writes the method as a generic Java author would, not as your file would. Sometimes that’s fine. Sometimes it produces code that is inconsistent with everything around it.

Examples of convention drift:

• Your project uses 4-space indentation; the AI gives you 2.
• Your project uses LOGGER.info(...); the AI uses System.out.println(...).
• Your project’s package is com.litman.coding2.catalog; the AI assumes the default package.
• Your project never throws checked exceptions from public APIs; the AI throws IOException.
• Your project names its tests MethodNameTest_describesScenario; the AI names them testMethodName.
• Your existing code returns Optional<T>; the AI returns T or null.

None of these are bugs in isolation. All of them are bugs in the context of your codebase, because a future reader will trip over the inconsistency.

The fix is twofold:

1. In the prompt, tell the AI what the conventions are. Paste an existing class as a style reference.
2. In the review, check that the AI’s code matches the surrounding code’s conventions. If it doesn’t, fix it before merging.

This category is where Phase 1’s reading discipline (Chapter 1 — Reading Code Like Scripture) pays off. You can only enforce conventions you can recognize. You spent Phase 1 building the recognition. Use it now.

11.8 — Reading Speed vs. Reading Depth

A common new-engineer mistake: trying to speed-read AI output. Skim, see “looks right,” save, move on. This produces exactly the failure mode this chapter exists to prevent.

There is a place for fast reading and a place for slow reading. Learn which is which.

The fast read happens first. If the fast read fails (“no, this isn’t the kind of answer I wanted”), re-prompt without spending the time to slow-read. If the fast read passes, immediately switch to slow read. Most students stop after the fast read. The bugs live in the slow read.

A useful exercise: time yourself. Take an AI output of about 30 lines. Try to slow-read it in under 5 minutes. Then re-read it slowly with no time pressure. Note what the time-pressured read missed.

Coach’s Note — Slow reading is not “reading carefully” in a vague sense. It is reading with the ten-question checklist actively in your mind. The checklist forces you to slow down at each question. Without the checklist, slow reading collapses into rereading the same paragraph three times without learning anything new.

11.9 — Tests Must Catch What Review Misses

This is the Chapter-4 principle, restated for Phase 2.

Code review is necessary. Tests are also necessary. Neither replaces the other.

Review catches:

• Obvious wrongness (“this method doesn’t exist”).
• Style and convention issues.
• Design concerns (“this class shouldn’t have this method”).
• Categories of bugs the reader thinks about while reading.

Tests catch:

• Specific input-output failures.
• Boundary conditions the reader didn’t think to check by hand.
• Regressions when code changes.
• Bugs that involve interactions between methods.

A bug that survives review is a bug the test suite must catch. Always. Every time you find a bug in AI code that your review missed, write a test for it — both to verify the fix and to prevent the bug from coming back. That test becomes a permanent guard.

The pattern, in practice:

1. Spec the work (Chapter 2).
2. Prompt for the implementation (Chapter 10).
3. Review the AI’s output (this chapter).
4. Fix what review caught.
5. Write tests, including tests for cases review didn’t think about.
6. Run tests.
7. For each test failure, write a new test that would have caught the bug earlier, and fix.

That cycle is the working senior engineer’s day. Project 11 walks you through it on planted-buggy code so you can practice.

11.10 — Testing the Spirits

The chapter’s apologetic frame: how do we test the spirits? 1 John 4:1 is one verse but it earns a chapter. It is the canonical New Testament instruction on the discipline of discernment.

The verse continues: “because many false prophets have gone out into the world.” The early church faced a problem: people claiming spiritual authority who were not actually authoritative. Some sounded right. Some quoted Scripture. Some performed apparent signs. The instruction was not “trust your gut” and not “trust the credentials.” It was test. Try the spirit. Compare its teaching against what the apostles handed down. Compare its fruit against the fruit of the Spirit. Compare what it says about Christ against what the church confesses about Christ.

Discernment, in the church’s tradition, is not a feeling. It is a discipline. It has criteria. It is something you practice until you can do it under pressure.

The transfer to engineering work is direct. Code review is the engineering form of discernment. The AI is sometimes right and sometimes wrong, and it sounds the same in both cases. Your job is to test. You compare its output against the spec (the apostolic teaching). You compare its behavior against the tests (the fruit). You compare its API usage against the docs (the confession of the standard library). You do this every time, the same way, with the same discipline, until it becomes second nature — until your eyes catch the off-by-one before your conscious mind has named it.

There is also the deeper resonance. Discernment is something Christians practice because we are not the source of truth. We are not the standard against which other things are measured. The Word is. The confessions are. The Spirit testifies. We submit to a higher norm and test claims against it.

The engineer’s posture toward AI is something like that, in its own register. You are not the standard either — the spec is, the tests are, the language docs are. But you are the one who holds those standards up against the AI’s claim and checks. The AI is not “the authority”; it is a contributor whose work submits to review. You are the reviewer. That is the senior’s job in the room.

Beginners think senior engineers are smart. They are smart, yes. But they are also practiced at testing the work in front of them. That practice — that ten-question checklist run a thousand times until it is automatic — is the seniorness. It is not magic. It is discernment, in the old sense, applied to a new domain.

Coach’s Note — If you take one verse from this entire course, take 1 John 4:1. Test the spirits. Not because every spirit is malicious — many are well-intentioned. But because well-intentioned and right are not the same thing, and the discipline of testing is what closes the gap. Your AI assistant is not malicious; it is, on the whole, helpful. It is also wrong sometimes. Test.

11.11 — Common AI Pitfalls (Week 11 Edition)

Pitfall: You read AI output once at full speed and accept it. What’s happening: Fast-read only. The slow-read questions never got asked. Fix: After the first read, always do a slow read with the ten-question checklist.

Pitfall: You catch one bug in AI output and assume the rest must be fine. What’s happening: Bugs cluster. A method that’s wrong about null often also wrong about empty. A class that hallucinates one API often hallucinates more. Fix: When you find one bug, look harder for more — don’t relax.

Pitfall: Your tests pass on the AI’s code but the code is still wrong. What’s happening: Your tests cover only the cases the AI was thinking about (the obvious ones). The wrong cases were not in your test suite. Fix: For each bug class in §11.2–§11.7, write at least one test. Bugs you didn’t think of are bugs you can’t test for, so use the categories as a forcing function.

Pitfall: You see “the AI used Optional, that’s modern Java” and don’t think harder. What’s happening: Style impressions are easy to mistake for correctness. The AI might be using Optional while still mishandling nulls underneath. Fix: Don’t let style cues short-circuit the review. Run the checklist regardless of how clean the code looks.

Pitfall: You catch a bug, fix it without writing a test for it. What’s happening: Next time the AI produces similar code, you might not catch it. And anyone refactoring later might reintroduce the bug. Fix: Every fix gets a test. Every fix. No exceptions.

Pitfall: You can’t decide if a piece of AI code is right or wrong. What’s happening: The spec was ambiguous, so neither you nor the AI knows the intended behavior. Fix: Go back to the spec. Tighten it. Re-review. (This is also feedback that Chapter 2 is calling: tighten your specs.)

11.12 — Reps

Open the exercises. This week the reps are heavily review-centric — most of them give you AI-shaped code and ask you to find bugs.

Rep 1. Find three bugs in a given 20-line method (planted; we tell you they’re there).

Rep 5. Take AI output from Project 10 and run the ten-question checklist on it. Document what each question caught.

Rep 9. Write a test for a bug class you didn’t catch on your first read. Run it. Watch it fail. Now you know the bug.

Full set in the exercises.

11.13 — This Week’s Project: Find the Bugs in AI’s Code

You’re ready for Project 11: Find the Bugs in AI’s Code, in Project 11.

The setup: you receive 4 small AI-generated programs, each with at least 2 planted bugs from the categories in this chapter. For each: find every bug, classify it, fix it, and (for Medium) write a prompt template that would have prevented that bug class from being generated.

Three tiers:

• Normal — find and fix all bugs across 4 programs.
• Medium — for each bug class, write a prompt template that would have prevented it.
• Hard — generate your own AI-buggy code, build a test suite, and have a classmate try to write code that passes the tests but is still subtly wrong.

11.14 — Coach’s Final Word

Three weeks into Phase 2, you have learned to prompt and to review. Those are the senior’s two hands. With them you can direct an AI to produce code your team can trust. Without them you are a vibe coder with a faster keyboard.

Reading carefully is not glamorous. It does not feel like coding. It is, for most of your career, the majority of what an experienced engineer does — reading other people’s code (and now AI’s), catching what they missed, raising the quality of what ships. The students who internalize this in Phase 2 ship work the grader can trust. The students who don’t, don’t.

One chapter and one project left in the “AI mechanics” stretch of the book. Chapter 12 makes architecture the senior’s irreplaceable contribution — the thing AI fundamentally cannot do for you because it is a question of taste and judgment, not pattern-matching. After that, Chapter 13 puts iteration on top of the whole stack and Chapter 14 takes the honest measure of what you’ve built. We’re past halfway.

See you Monday. Bring your magnifying glass.

Up next: Read the exercises — heavy review work. Then open Project 11. After that, Chapter 12 — Architecture First.