Reviewing AI code: I check the intent, not the diff

The most common finding in generated code is not the bug. It is the effort. A task that needed one function and one field comes back as an interface plus an implementation plus a factory plus a configuration object. Everything works, everything is tested, none of it is wrong.

That is exactly what makes it awkward. A bug gets fixed. A superfluous abstraction gets merged, because it looks like the good craft you are shown in books.

Why the diff is the wrong place

In a diff the result looks defensible. Every single file is clean, named properly, covered by tests. The damage is not in any one line but in the sum: five files for something that belonged in one.

Findings like this have an unpleasant property: they are expensive to raise and cheap to overlook. Raising one in review effectively asks for working, tested code to be thrown away, and that has to be argued for. With your own code inertia is added; with generated code nobody is emotionally attached to it, and yet nobody deletes it either.

On top of that, automated checks contribute nothing here. Linters, type checking and tests measure correctness, not appropriateness. A factory with exactly one product is syntactically impeccable and stays green in every pipeline. Even a review pass against architectural guardrails only finds it if one of those guardrails describes this exact case, and "no more structure than necessary" is hard to phrase as a rule without becoming arbitrary.

The shift: checking before code exists

My conclusion was not to read diffs more carefully. It was to move the question earlier. Before a task goes to an agent there is a short specification and an implementation plan derived from it, and what gets reviewed is the plan, not the result.

That is the point at which superfluous structure is cheap. If the plan says an interface and a registry will be created, striking that costs one sentence. Once the same structure exists as code, with tests, it costs a discussion and a rewrite.

The second half of the same decision: cut tasks small. Small tasks are not solved better. They only keep down the amount of code I have to judge at the end. At two hundred lines I can see that one abstraction is too many. At two thousand I read structure and check correctness, and the question of appropriateness never gets asked, because I am busy following along.

What I look for in a plan

Three things, and all of them come down to the same question: can this structure be derived from a requirement that was actually stated?

An interface with exactly one implementation. Legitimate when a second one is coming, or when the seam is needed for testing. Not legitimate because dependency inversion sounds good in general. The question is: who is the second implementation, and when does it arrive?

Configurability without a requirement. Parameters, strategies, switches for cases nobody mentioned. Every option is a state that later has to be tested, documented and accounted for in every rewrite. Configurability is not a property, it is a debt you take on.

Distribution without a reason. One thing becomes four files in four folders, because that is what the architecture's layers are called. Layers exist to enforce dependency directions, not to spread every thought across four places.

An example, shortened

The task: a function that determines how far in advance a reminder goes out for an appointment. Two cases, switchable by configuration. What came back was, at its core, this:

interface ReminderStrategy {
    calculate(appointment: Appointment): Duration;
}

class DefaultReminderStrategy implements ReminderStrategy { /* ... */ }
class EarlyReminderStrategy implements ReminderStrategy { /* ... */ }

class ReminderStrategyFactory {
    constructor(private readonly registry: Map<string, ReminderStrategy>) {}
    create(kind: string): ReminderStrategy {
        const strategy = this.registry.get(kind);
        if (!strategy) throw new UnknownStrategyError(kind);
        return strategy;
    }
}

Four types, a registry, a dedicated error type and a string key that can fail at runtime. What the task called for was this:

function reminderLeadTime(appointment: Appointment): Duration {
    return appointment.isFirstVisit ? hours(24) : hours(2);
}

The difference is not the line count. It is that the second version has no runtime failure mode, needs no registration step at startup, and keeps its case distinction visible in one place. The first version is prepared for a third case nobody asked for, and pays for that preparation immediately.

If that third case does turn up later, the reversal is cheap: turning a two-branch function into a strategy is a contained change. The opposite direction, tearing out an abstraction that was guessed wrong, is not, because callers have grown used to it by then.

What the preparation costs

The preparation is the price. Writing a specification and a plan before every task takes noticeable time, and that time is spent before anything visible exists. On small changes the ratio is uncomfortable: the description is longer than the solution.

The second price: small tasks mean more tasks. More handovers, more merges, more administration. The effort does not disappear, it moves from reading to slicing.

And to be honest: I do not remove every superfluous abstraction that does make it into the code. If it works, is tested and sits somewhere rarely touched, it stays. That is not consistency, it is a calculation I make case by case.

Where more structure is right

When it is clear that the scaffolding for a larger undertaking is being laid, extensibility belongs in it from the start. Then the interface is not speculation but a requirement, and the second implementation has a name and a date.

The difference is not the amount of structure but where it comes from. Structure that follows from a known requirement is planning. The same structure without that requirement is a bet on how the product will develop, and that bet is usually lost.

Not the list, the timing

With generated code my checklist did not change. Its timing did. In a diff, superfluous structure is hard to argue against, because it works. In a plan, it is one sentence you delete.

What remains is a question no tool answers for me: which requirement demanded this abstraction. If I cannot find an answer, there isn't one.