DEV Community

Ali Suleyman TOPUZ
Ali Suleyman TOPUZ

Posted on Originally published at topuzas.Medium on

Nobody Reads the Diff Anymore: How Senior Teams Are Actually Reviewing AI-Written Code

Three weeks ago I opened a pull request from a Claude Code session that had been running unattended overnight against a ticket about consolidating three duplicate pricing calculators into one. The diff was 2,140 lines across 19 files. I have reviewed code for a living for a long time, and I know roughly how fast I can read code for logic rather than skim it for vibes: somewhere around 300 to 500 lines an hour if I actually want to catch a wrong comparison operator or a silently swallowed exception. At that rate, reading that diff properly, the way I was trained to review code, would have eaten most of a working day. I had, generously, forty minutes before a standup.

So I did what everyone on my team now quietly does. I scrolled. I read the file names, glanced at the function signatures, checked that the tests existed and were green, and approved it. That is not a confession, it is a description of the industry. A coding agent can now produce more defensible-looking code in an afternoon than a careful human can actually read in a week, and line-by-line review stopped being the real check a long time before most teams admitted it out loud. We still open the diff. We still leave a comment or two so the PR doesn’t look rubber-stamped. But the diff isn’t where the trust comes from anymore, and pretending otherwise is how you end up shipping a rounding bug into a payments path because nobody actually traced what the new calculate_shipping_cost function does when destination_country is null.

This piece is about what my team replaced line-by-line review with, over about four months of getting burned and adjusting. It isn’t a theory. It’s three mechanisms running together right now: a black-box audit pattern for individual pieces of the codebase, a five-stage agent pipeline that never lets one agent grade its own work, and a mechanical enforcement layer that checks architecture the way a compiler checks syntax. None of the three replaces the other two. Together they replace the diff.

Stop reading implementations, start reading behavior

The idea that unlocked this for me came from David Zhang’s writing on treating AI-generated code the way you’d treat a component from a supplier you don’t fully trust yet, which is to say: you don’t audit it by reading its internals, you audit it by boxing it in and watching what it actually does. He calls the practice running your codebase like a software factory, and the core move is refusing to let “I read the implementation” count as verification at all.

The mechanics are almost boringly simple once you see them. Cut the codebase into small units with a clearly stated input and output, a function, a small module, an endpoint, doesn’t matter which as long as the boundary is unambiguous. Attach a sensor to that boundary that records every real call: the actual arguments, the actual return value, the actual duration. Run the thing, on real or realistic inputs, and read what happened instead of reading how it happened. Implementation detail becomes something you only descend into when the black-box evidence gives you a specific reason to, a wrong output, a violated invariant, a trace that doesn’t match the spec. Most of the time it never does, and that’s the point: you stop paying the reading tax on code that is behaving correctly.

For each unit you keep four things readable, and I mean genuinely readable, not buried in a wiki page nobody opens:

Invariants. What must always be true, stated so it can be checked by a machine, not a sentence a human has to interpret. “Shipping cost is never negative” is an invariant. “Shipping cost should generally be reasonable” is not.

Traces. What actually happened on a real run, args in, result out, timing, not a description of what the code is supposed to do.

Attack surface. What the unit touches: which env vars it reads, which network calls it makes, which other modules it can reach. This is usually smaller than people think and always bigger than the docstring admits.

Decisions. Every place the spec was silent and the agent picked something anyway. This is the one teams skip, and it’s the one that bites you, because “the spec didn’t say” is exactly where the interesting bugs live.

Here’s what that fourth item looks like in practice, on the actual shipping calculator that started this whole overnight session. I had the agent that wrote the consolidated function generate this before I’d approve the merge, ranked by how much it would hurt if the decision turned out wrong:

+------+---------------------------------------------------+--------+---------------------------------+
| Rank | Decision made where the spec was silent | Risk | Location |
+------+---------------------------------------------------+--------+---------------------------------+
| 1 | Treats a null destination_country as domestic | High | shipping.py, line 12 |
| | instead of raising an error | | |
| 2 | Rounds the final cost with banker's rounding rather | High | shipping.py, line 44 |
| | than standard half-up rounding | | |
| 3 | Applies the free-shipping threshold to the pre-tax | Medium | shipping.py, line 61 |
| | subtotal rather than the post-discount total | | |
| 4 | Breaks a tie between two carriers with an identical | Low | shipping.py, line 88 |
| | rate by picking whichever name sorts first | | |
+------+---------------------------------------------------+--------+---------------------------------+
Enter fullscreen mode Exit fullscreen mode

Reading four rows took me ninety seconds. Reading the 140-line function that produced them would have taken twenty minutes and I still might not have noticed that null-country decision, because it’s buried inside a conditional that looks entirely reasonable if you’re skimming for style instead of hunting for silent assumptions. Rank 1 and rank 2 got sent back with a comment. Rank 3 got a Slack message to the person who owns pricing, because it changes real revenue and I am not the person who gets to make that call alone. Rank 4 got approved as-is, because nobody’s business depends on carrier tiebreak order.

The sensor that produced the trace data behind this is not exotic. It’s a decorator:

import functools
import json
import time
def audited(invariants=None):
    """
    Wrap a function as a black-box unit: record every real call's
    inputs and outputs, and fail loudly if a stated invariant breaks.
    This is the sensor. I don't care how calculate_shipping_cost is
    implemented, only what it actually does when it runs.
    """
    invariants = invariants or []
    def decorator(fn):
        @functools.wraps(fn)
        def wrapper(*args, **kwargs):
            start = time.monotonic()
            result = fn(*args, **kwargs)
            duration_ms = (time.monotonic() - start) * 1000
            for check in invariants:
                ok, message = check(args, kwargs, result)
                if not ok:
                    raise AssertionError(
                        f"invariant violated in {fn. __name__ }: {message}"
                    )
            with open("traces.jsonl", "a") as f:
                f.write(json.dumps({
                    "function": fn. __name__ ,
                    "args": repr(args),
                    "kwargs": repr(kwargs),
                    "result": repr(result),
                    "duration_ms": round(duration_ms, 2),
                }) + "\n")
            return result
        return wrapper
    return decorator

def never_negative(args, kwargs, result):
    return result >= 0, f"shipping cost was {result}, expected >= 0"

def never_exceeds_order_total(args, kwargs, result):
    order_total = kwargs.get("order_total", args[0] if args else 0)
    return result <= order_total, f"cost {result} exceeded order total {order_total}"

@audited(invariants=[never_negative, never_exceeds_order_total])
def calculate_shipping_cost(order_total, destination_country, weight_kg):
    ...
Enter fullscreen mode Exit fullscreen mode

Run this against a batch of real historical orders and you have traces, not guesses, about what the function does. It costs nothing to run locally, no hosted service required, and it works exactly the same whether the function was written by a person on your team five years ago or by an agent last night.

An assembly line beats one agent trying to do everything

The black-box pattern tells you how to check a piece of code. It doesn’t tell you how to produce code worth checking in the first place, and that’s where the second pattern comes in. I first saw the shape of it in Robert Martin’s public description of running a squad of specialized agents instead of one generalist, and I rebuilt a version of it for my own team over a couple of weekends.

The pipeline has five stages, and the discipline is that no agent does more than one job:

+------------+---------------------------+-------------------------------------+
| Stage | Input | Output |
+------------+---------------------------+-------------------------------------+
| Specifier | Plain-English story | Gherkin spec + manual test checklist |
| Coder | Gherkin spec | Implementation + unit tests |
| Cleaner | Passing implementation | Refactored code, same tests still pass|
| Hardener | Refactored code + tests | List of mutations the tests missed |
| QA | Spec + hardened code | A driving end-to-end script |
+------------+---------------------------+-------------------------------------+
Enter fullscreen mode Exit fullscreen mode

The Specifier never sees any code and never writes any. Its entire job is turning something like “let customers apply one promo code per order, and if they have two, use whichever saves them more money” into a Gherkin spec covering the boring cases nobody thinks to mention out loud, like what happens when both codes save the exact same amount, or when one code is expired at the moment of checkout. My actual prompt for this stage is short on purpose:

You are the Specifier. You will receive a plain-English feature
request. Do not write any code.
Produce two things only:
1. A precise spec in Gherkin (Given/When/Then), covering the happy
   path and every edge case implied by the request, even ones not
   stated explicitly.
2. A manual test checklist a human could run in ten minutes, each
   item a single verifiable action.
If the request is ambiguous, list the ambiguity as an open question.
Do not implement anything and do not propose an architecture.
Enter fullscreen mode Exit fullscreen mode

The Coder gets that spec and nothing else, implements it test-driven, and hands off. The Cleaner takes working code and refactors for duplication and complexity without touching behavior, the same discipline as a CRAP score review, complexity weighted against how well it’s covered. Then comes the stage that actually earns its keep, the Hardener, whose entire job is trying to break what the Coder and Cleaner produced:

You are the Hardener. You did not write this code or its tests.
Introduce one deliberate bug at a time into the implementation, an
off-by-one, a flipped comparison, a dropped null check, a swapped
argument order. After each mutation, run the existing test suite.
Record every mutation that survives, meaning the tests still pass
with the bug in place. Report file, line, and the exact change.
Do not fix anything. Your only output is the list of gaps.
Enter fullscreen mode Exit fullscreen mode

This is mutation testing, and it answers a question line-by-line review can never answer: not “does this code look right” but “would my tests actually catch it if it were wrong.” A test suite with 95 percent line coverage and zero mutation coverage is a test suite that runs every line and asserts almost nothing about what those lines should produce. I have seen exactly this happen: a Coder-written test that called the function and asserted the response wasn’t null, which is true even when the function returns the wrong price. The Hardener caught it because flipping a > to >= didn't break a single test.

Finally the QA stage turns the original spec into a script that actually drives the running system end-to-end, browser or API calls, not a mock. If that script fails against the hardened build, nothing ships.

Why split the work this way instead of asking one strong agent to do all five things in one long session? Two reasons, and I’ve watched both fail in practice before I split things up. First, prompt length is a real constraint on behavior, not just a token-cost line item. An agent holding “write the spec, implement it, refactor it, attack it, and script it” in one context window will, reliably, shortchange whichever step comes last once the context starts filling up with its own earlier output. I measured this directly: a single combined prompt produced a Hardener-equivalent pass that found one surviving mutation across four functions. The dedicated Hardener stage, same functions, same day, found eleven. Second, and this is the bigger one, a single agent grading its own spec against its own implementation has no incentive structure that produces honest failure. It wrote the code, it wrote the tests, and by the time it’s asked to verify itself it has already convinced itself the approach is sound. Separate agents with separate, narrow prompts don’t carry that bias forward, because the Hardener never saw why the Coder made any of its choices. It only sees what the code does when it’s attacked.

None of this requires a paid API tier either. I run the Hardener stage specifically against a local Ollama model, ollama run qwen2.5-coder:32b, because mutation testing doesn't need frontier reasoning, it needs a disciplined process that tries the same handful of mutation patterns every time, and a smaller local model running for free on a spare GPU box does that job reliably.

Fitness functions turn architecture decisions into checks, not documents

The pipeline gets you trustworthy pieces. It does not stop an agent from wiring two pieces together in a way that violates a decision your team made six months ago and wrote down in an ADR nobody has reopened since. That’s a different failure mode, and it needs a different fix: the architecture decision itself has to become something a machine checks on every pull request, not a document that ages into irrelevance.

Jaroslaw Wasowski has written extensively about this under the banner of ADR-as-spec: instead of a Markdown file that describes an architectural rule in prose, hoping reviewers remember it, you write the rule once and compile it into an automated fitness function that runs in CI on every diff. The reasoning direction flips. Instead of “did the reviewer happen to notice this violates ADR-014,” it becomes “did the build fail.” I’ve seen a writeup of a team that adopted exactly this pattern for a mid-sized platform and moved their architecture-compliance score, meaning the share of modules that actually satisfied their own recorded ADRs when checked mechanically rather than assumed, from roughly 60 percent to about 90 percent within two quarters, almost entirely by converting existing ADRs into enforced checks instead of writing new rules. The compliance gap wasn’t a rule-writing problem. It was an enforcement problem, and enforcement is exactly the kind of thing agents are bad at holding in mind across a long session and machines are good at holding forever.

Here’s an ADR written specifically so it can become a fitness function instead of just a memo:

# ADR-014: Payments must not depend on Notifications
## Status
Accepted
## Context
Notifications pulls in a templating engine and an outbound SMTP
client. Anything that depends on it, even transitively, drags that
surface into our PCI-scoped boundary.
## Decision
No type under Payments.* may reference any type under
Notifications.*. Payments publishes domain events; Notifications
subscribes. The dependency runs one direction, and only one.
Enter fullscreen mode Exit fullscreen mode

And the fitness function that makes that decision unbreakable rather than aspirational, in C# with ArchUnitNET:

using ArchUnitNET.Domain;
using ArchUnitNET.Loader;
using static ArchUnitNET.Fluent.ArchRuleDefinition;
public class ArchitectureTests
{
    private static readonly Architecture Architecture = new ArchLoader()
        .LoadAssemblies(typeof(PaymentsModule).Assembly)
        .Build();
    [Fact]
    public void Payments_Should_Not_Depend_On_Notifications()
    {
        var payments = Types().That().ResideInNamespace("MyApp.Payments").As("Payments");
        var notifications = Types().That().ResideInNamespace("MyApp.Notifications").As("Notifications");
        Types().That().Are(payments)
            .Should().NotDependOnAny(notifications)
            .Because("ADR-014: payments must not depend on notifications")
            .Check(Architecture);
    }
}
Enter fullscreen mode Exit fullscreen mode

If you’re not on .NET, the equivalent with dependency-cruiser against a Node or TypeScript codebase does the same job from a .dependency-cruiser.js config, no test framework required at all:

module.exports = {
  forbidden: [
    {
      name: "payments-no-notifications",
      comment: "ADR-014: payments must not depend on notifications",
      severity: "error",
      from: { path: "^src/payments" },
      to: { path: "^src/notifications" },
    },
  ],
  options: {},
};
Enter fullscreen mode Exit fullscreen mode

Either version turns a sentence a reviewer might forget into a red build a reviewer cannot merge past. That’s the actual shift: architecture stops being something you hope the diff-reader remembers to check and becomes something the pipeline refuses to let through, agent-written or not.

Worth naming here too: Ponytail, the open-source guardrail plugin that’s picked up well past 100,000 GitHub stars since it launched, attacks a related but different problem. Fitness functions stop an agent from wiring things together wrong. Ponytail stops an agent from writing code that shouldn’t have existed at all, by forcing a six-rung decision ladder before any new code gets written: can this be skipped, does the standard library already do it, does the platform already do it, does an existing dependency already do it, can it be one line, and only after all four no’s does it get to write new code, tagged with a ponytail: comment marking exactly where the shortcut was taken. It's not a replacement for architecture enforcement, it's upstream of it. Fewer lines written in the first place means fewer lines for the fitness functions and the Hardener to ever have to catch.

Where to actually start this week

You don’t need all three mechanisms running before Friday. Pick one function or module in your own codebase that everyone quietly avoids touching, the one where “I’m not totally sure what happens if X” gets said out loud in Slack more than anyone likes to admit. Write down its inputs and outputs plainly enough that a sensor could wrap it without you reading a single line of its body. Run it against ten or twenty real inputs and log the traces. Then sit down and write the first ranked decision ledger for it, every place the code made a choice the spec never mentioned, ranked by what breaks if that choice turns out to be wrong.

That one ledger will tell you more about the actual risk in that piece of code than another hour spent scrolling through its diff ever would. The diff was never the trust. It just used to be the only thing we had.

Tags: code-review, ai-agents, software-architecture, claude-code, devops, testing, software-engineering

Top comments (0)