Where the Review Point Moved

Test Names First

A scenario index is what a well-named test suite is. Every test name is a claim the team is making about what the system does under a specific set of conditions. The name Loyalty_members_get_a_ten_percent_discount_on_orders_over_fifty_dollars is not decoration. It is a specification.

The absence of a test named Loyalty_members_with_expired_tier_status_do_not_get_the_discount is a gap in the specification, and that gap will not appear anywhere in the diff. The diff will contain the code that handles expired tiers or it won't. The diff will not contain the claim that the team thought about expired tiers and made a deliberate choice.

The Builder API

The fluent builders your team maintains - aLoyaltyMember(), aCartReadyForCheckout(), anOrder() - are a vocabulary index for the domain. When a test reads:

var order = aCartReadyForCheckout()
    .forCustomer(aLoyaltyMember().withExpiredTier())
    .containing(aProduct().costing(60.dollars()))
    .build();

that line is doing vocabulary work. The builder API names the domain concepts your team has decided are real. When a new test introduces aLoyaltyMember().withDiscountOverride(0.15m) without a corresponding builder method, using a raw decimal instead, that's a vocabulary drift: a concept that exists in the implementation but has no named, composed home in the test language. It will be invisible in the diff, which will show a test that passes.

Contract and Interaction Expectations

When a collaboration test specifies that the checkout service calls the discount calculator with a particular shape of request, that expectation is a claim about the system's boundary. It defines what crossing the boundary means. A reviewer who reads that expectation can tell whether the agent understood the collaboration correctly: whether the request carries the right data, whether the response is consumed through the right abstraction, whether the mock was set up in a way that actually constrains the implementation or just greenlights any call.

Three surfaces. None of them live in the diff. All of them carry more intent than the diff does.

What a Review Looks Like Now

The sequence matters. Most teams do it backwards.

Start with test names. Skim the full list of new and modified scenario names. You are looking for gaps, not for correctness. What scenarios are present? What scenarios should be present but aren't? Does the naming stay inside the team's vocabulary, or has a synonym crept in that will start a split in how the team talks about the domain? A five-minute pass over test names tells you more about whether the task was specified correctly than a forty-five-minute line-by-line diff read.

A useful discipline here: before looking at any test body, write down the scenarios you would expect to see, given the task description. Then compare that list to the scenario names in the PR. The gaps are your first review comment. They are more valuable than any line-level observation, because they identify what the agent was not asked to verify, which is where the real risk lives.

Move to the builder API. Look at how existing builders were used and whether any new test setup was written by hand instead of through the builder layer. Raw object construction in test setup is almost always a signal: either the builder is missing a needed method, or the test is exploring a scenario the builder vocabulary hasn't named yet. Both are design conversations, not implementation conversations. A raw new Customer { TierStatus = "expired" } in a test is a reviewer flag: the team hasn't decided what to call this concept, and the agent picked the path of least resistance.

Compare that flag to what the test would look like if the vocabulary were right:

[Fact]
public void Expired_tier_members_do_not_receive_the_loyalty_discount()
{
    var order = aCartReadyForCheckout()
        .forCustomer(aLoyaltyMember().withExpiredTier())
        .containing(aProduct().costing(60.dollars()))
        .build();

    var receipt = checkout.Process(order);

    receipt.Discount.Should().Be(Money.Zero);
}

The builder call .withExpiredTier() is not implementation detail. It is a named claim that the team recognizes expired tier status as a first-class domain concept. The raw alternative, TierStatus = "expired" as a string literal, is the absence of that claim. The reviewer's job is to notice the difference and ask which one the team actually wants.

Look at the contract and interaction expectations next. For each mock expectation or contract assertion introduced in the new tests, ask whether the boundary being described matches the system's existing collaboration graph. Does the expectation reveal the right collaborator being called at the right time with the right shape? A misspecified mock expectation is one of the most expensive bugs a review can miss: it will pass CI, it will read as reasonable code, and it will leave the system's actual boundary behavior untested.

Then, and only then, look at the diff. At this point the diff is a confirmation, not an investigation. You are checking that the implementation plausibly corresponds to the contracts you already reviewed. You are not trying to reconstruct intent from implementation, because you already found the intent in the three surfaces that came before. The diff review is minutes, not hours.

The checklist, in order: test names first, builder API second, contract expectations third, diff last. Everything else is archaeology.

Diff-First Tooling Is the Wrong Granularity

The pull request as a review artifact was designed for a specific workflow: a human writes code, a human reviews it. The granularity - line-by-line diff with inline comments - was calibrated for that pair. You can comment on line 47 because line 47 was where the human made a decision that can be questioned.

When an agent generates the diff, line 47 is not where the decision was made. Line 47 is where the decision was rendered. The decision was made in the test name, the builder call, the contract expectation. Those artifacts don't have a canonical location in a diff-first review tool. They're buried in the same flat list of changed lines as the implementation. A reviewer who finds a problematic test name has to comment on the line where the test name appears, in a file that also contains the assertions and the setup, in a review thread that will be displayed in the same UI as a comment about a typo in a docstring. The tooling cannot distinguish between intent-bearing artifacts and implementation artifacts, because it wasn't built to. It sees lines.

Test-suite-aware review tooling is the next frontier, and the shape of it is visible even if the implementations aren't mature yet. The right granularity is not the line; it's the scenario. A review surface that groups changed tests by coverage area, surfaces missing scenarios relative to the builder API's vocabulary, and flags builder API calls that don't use the team's established composition patterns would give reviewers something the diff never could: a structured view of what the agent decided to specify and what it left unspecified. The diff view would still exist. It would just be the last tab you open, not the first.

Teams that wait for the tooling to catch up will spend the intervening period doing the right review by discipline, not by structure. That's fine. The discipline is what the tooling will eventually automate. Understanding the right review sequence now is the prerequisite for adopting the tooling when it arrives.

The Habit Lag Is the Real Bottleneck

Here is the real situation in most teams: the developers who read this post will agree. They will nod through the argument. They will leave the post with genuine conviction that test-name-first, builder-second, contract-third, diff-last is the right review sequence.

Then they will open the next PR and start at the diff.

Habits are faster than convictions. The PR opens, the diff tab is there, the eyes go to the lines. The pattern was laid in over years of reviewing human-written code where the diff was, in fact, the right place to look. The hands don't wait for the mind to remember that the situation has changed. The stickiness of the habit is not a character flaw. It is a calibration problem: the tooling, the muscle memory, the team norms, all of them are still tuned for a world where the diff was the right unit of analysis. That world is gone. The calibration hasn't caught up.