Reviewing an AI-Assisted Pull Request

Kai opens his next pull request on a Thursday afternoon. When a place frees up on a full class, members now get the first offer, ahead of day-pass climbers who joined the waitlist earlier. The change is 1,150 lines across 19 files, written with the agent in one afternoon, and it comes with a confident three-paragraph description that the agent drafted from the session. Every test is green.

Ines reviews it the way Chapter 6 taught you to read your own diffs, with one difference: the author is a colleague, so some of the questions can now be asked out loud. The first thing she does is not read it.

Size Before Content

Ines sends the pull request back unread and asks for it in three pieces: the priority rule itself, the restructuring of waitlist that the agent did along the way, and the fixtures for the new tests. Her reason is arithmetic. Cragmoor splits any change over about 200 changed lines, because past that point nobody reads closely; they scroll. At 1,150 lines the review she would give is a skim with a green tick on the end, barely more reading than Kai's first waitlist got before it shipped its double offer.

Splitting used to be expensive, because the author had written every line by hand and untangling them took a day. With an agent it takes minutes: Kai asks for the restructuring alone on a fresh branch, then the rule on top of it, then the fixtures. The cost has moved. Generation is cheap; reading is the scarce step, and it belongs to the person with the least spare time on the team. A split pays the author a few minutes to save the reviewer a day.

What comes back is three pull requests of different kinds. The priority rule is 190 changed lines, small enough to read line by line. The restructuring is 360 lines that move code without changing it, which Ines reviews by checking that the existing tests pass untouched before and after. The fixtures are 600 lines of test data, which she reviews by sample. Each one can now be approved for what it is.

The Author Can Answer

Before she reads a line of the priority rule, Ines asks Kai one question the change raises. A day-pass climber holds an offer and is at minute 10 of the 15-minute hold. A member joins the waitlist. What happens to the offer?

There are three defensible answers. The offer stands and the member waits for the next place; the offer is withdrawn and goes to the member; or the offer stands but the member jumps ahead of everyone else still waiting. Each is a business decision, and each is a different program. If Kai can name which one his code does and why, the review can start. If his answer is "let me ask the agent", the pull request is not ready, whatever the tests say.

This is the professional contract of Chapter 2 applied between colleagues. The author is accountable for every line, and an author who cannot explain what the change does has not reviewed it. The agent can help Kai find the answer; it cannot be the answer. A reviewer who accepts "the agent says it handles that" is now reviewing a summary of a summary, and nobody on the team has read the code.

Tests First

Ines reads the tests before the code, the reading order from Chapter 6, and in the priority pull request she finds two problems. The first is in the new tests: their expected values were not worked out from the rule. Each test asserts exactly the order in which the code happened to offer places, copied from a run of the implementation, the pattern Chapter 8 warned about. A test written that way passes by construction and would have passed with the wrong rule too.

The second problem is in a test that already existed. The test for the 15-minute hold used to assert 15 minutes. In this pull request it reads the hold length from configuration instead, so the expected value and the code under test now come from the same place. The diff is small and looks like tidying.

diff --git a/tests/test_waitlist.py b/tests/test_waitlist.py
@@ -1,10 +1,11 @@
 from datetime import datetime, timedelta, timezone

+import settings
 from waitlist import make_offer

 NOON = datetime(2026, 10, 12, 12, 0, tzinfo=timezone.utc)


-def test_offer_is_held_for_15_minutes():
+def test_offer_is_held_for_the_hold():
     offer = make_offer("c-204", sent_at=NOON)
-    assert offer.expires_at == NOON + timedelta(minutes=15)
+    assert offer.expires_at == NOON + timedelta(minutes=settings.HOLD_MINUTES)

$ pytest -q tests/test_waitlist.py
.                                                       [100%]
1 passed in 0.00s

diff --git a/tests/waitlist.test.ts b/tests/waitlist.test.ts
@@ -1,9 +1,11 @@
 import { expect, test } from "vitest";
+import { HOLD_MINUTES } from "../src/settings.ts";
 import { makeOffer } from "../src/waitlist.ts";

 const NOON = new Date("2026-10-12T12:00:00Z");

-test("an offer is held for 15 minutes", () => {
+test("an offer is held for the hold", () => {
   const offer = makeOffer("c-204", NOON);
-  expect(offer.expiresAt).toEqual(new Date("2026-10-12T12:15:00Z"));
+  const expected = new Date(NOON.getTime() + HOLD_MINUTES * 60_000);
+  expect(offer.expiresAt).toEqual(expected);
 });

$ npx vitest run tests/waitlist.test.ts

 Test Files  1 passed (1)
      Tests  1 passed (1)

diff --git a/test/WaitlistTest.java b/test/WaitlistTest.java
@@ -1,5 +1,6 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;

+import java.time.Duration;
 import java.time.Instant;
 import org.junit.jupiter.api.Test;

@@ -7,8 +8,9 @@ class WaitlistTest {
     static final Instant NOON = Instant.parse("2026-10-12T12:00:00Z");

     @Test
-    void offerIsHeldFor15Minutes() {
+    void offerIsHeldForTheHold() {
         var offer = Waitlist.makeOffer("c-204", NOON);
-        assertEquals(Instant.parse("2026-10-12T12:15:00Z"), offer.expiresAt());
+        var hold = Duration.ofMinutes(Settings.HOLD_MINUTES);
+        assertEquals(NOON.plus(hold), offer.expiresAt());
     }
 }

$ java -jar junit.jar execute -cp out --scan-classpath
└─ WaitlistTest ✔
   └─ offerIsHeldForTheHold() ✔
[         1 tests successful      ]
[         0 tests failed          ]

The run under the diff is the whole problem in one line. Ines sets the hold to 20 minutes, which is wrong, and the rewritten test still passes. Before the change, the same mistake failed the test with a message naming both values. After it, the test checks that the code agrees with the configuration, which it always will, so it can no longer fail when the hold changes. The 15 minutes is a business rule, and a test of a business rule writes the number down.

The Description Is Not the Diff

The agent-written description says, in its second paragraph, "no change to offer expiry". The hold test in the same pull request says otherwise: the hold is now configurable, which is a change to offer expiry whether or not anyone means to use it. Nobody lied. The description was generated from the conversation, and in the conversation Kai never asked for a configurable hold; the agent made that change on the way past and summarised what it had been asked to do.

So Ines treats the description as a list of claims and checks each one against the diff. "Members are offered first": find the line. "Existing day-pass offers are not affected": find the test that proves it, or ask for one. "No change to offer expiry": false, see the test file. As Chapter 6 put it, an explanation helps you read and verifies nothing, because it is generated text too. A description is the same thing at pull-request scale.

What Disclosure Changes

Cragmoor's pull-request template has a line for what the agent did, and Kai filled it in: written with the agent, tests included. Knowing that does not raise the bar. The bar is the one it always was: every line understood by its author and by its reviewer. What it changes is where Ines looks first.

AI-assisted changes fail in characteristic places, and Chapter 7 named them: calls to functions that do not exist, tests bent until they pass, lines nobody asked to change, and abstractions built for a single caller. The configurable hold is two of those at once, an unrequested change carried by a weakened test. A reviewer who knows where problems cluster spends the first ten minutes there and finds them, instead of spending the same ten minutes on naming and indentation.

Comments That Land

Ines does not rewrite Kai's lines in her comments. The agent will regenerate those lines the moment Kai asks it to address the review, and a suggested rewrite of a line the agent is about to replace is wasted work. She comments on behaviour, and for each problem she asks for the test that would have caught it.

Her three comments on the priority pull request are short. Put 15 back in the hold test as a number, and move the configurable hold to its own pull request if anyone actually wants it. Work out the expected order in the new tests from the rule and say in each test which rule it checks. Add a test for a member who joins while a day-pass offer is held, with the answer Kai gave to her question about the held offer written down as the expected result. That is the advice of Chapter 6 for sending a diff back to the agent, with the one thing that was wrong, and it works between colleagues for the same reason: a narrow request gets a narrow, reviewable answer.