In January about a fifth of the pull requests on our main service were opened by a coding agent, usually with a person driving it. By May it was over half. The agent was better at the work than we had expected and the volume went up because the cost of a change went down. That part was fine.

What was not fine was review. Our process was three years old and it was built on an assumption we had never written down: the person who opened the PR understood every line in it, and review was a conversation between two people who both knew what the change was for. Neither half held any more. The author had often not read every line. The reviewer was now the first human to look at some of the code.

We got a production incident out of it in April, a schema change that the agent made correctly and that the author approved without noticing that it dropped a default. After that we changed the process. This is what changed and why.

The failure modes were specific#

Reading more carefully was the first suggestion and it was wrong. Reviewers were already spending longer on agent PRs than on human ones, and the incident PR had two approvals. The problem was not attention. It was that the things reviewers were looking at were the wrong things, because the failure modes of agent written code are different from the failure modes of human written code.

Human PRs tend to have a small number of deliberate changes and the bugs are in the logic of those changes. Agent PRs tend to have the requested change plus a halo of small adjacent changes that the agent made because they seemed reasonable: a renamed variable, a reformatted block, a removed default that looked unused, an added null check that changes behaviour, an updated test that now asserts the new behaviour instead of the old one. Each is plausible. Each is invisible in a 400 line diff. The one that dropped our default was three lines in a 380 line PR that was mostly a correct feature.

The second failure mode was in tests. The agent writes tests, and the tests pass, and reviewers took passing tests as evidence. But the agent wrote the tests and the code from the same understanding, so a misunderstanding in the code is mirrored in the test, and the test passes because it checks that the code does what the code does. A green suite from the same author as the code is weaker evidence than a green suite from a different author, and with agents the author is always the same.

The third was scope. A human asked to fix a bug fixes the bug. An agent asked to fix a bug sometimes fixes the bug and also refactors the module it lives in, because the refactor made the fix cleaner. The refactor might be good. It is also a second change riding inside the first one's review.

What a PR has to contain now#

The fix for all three was to change what a pull request has to have before a reviewer is assigned. Not guidelines. Checks, in CI, that fail.

The first check is a diff budget by intent. The PR description has to declare the intended scope, as a list of files or directories, and the diff has to stay inside it. Anything outside the declared scope fails the check with the list of files that strayed. The author can widen the scope, and that is a visible act in the description that the reviewer sees, or they can tell the agent to drop the extra changes. In practice it is the second one nine times out of ten, and the halo of adjacent changes mostly stopped appearing, because the agents we use read the check's failure output and learn what the scope means for that repository.

# .github/workflows/scope.yml
- name: Enforce declared scope
  run: |
    scope=$(gh pr view "$PR" --json body -q .body \
      | sed -n '/^## Scope/,/^## /p' | grep '^- ' | sed 's/^- //')
    changed=$(git diff --name-only origin/main...HEAD)
    out=$(echo "$changed" | grep -v -F -f <(echo "$scope") || true)
    if [ -n "$out" ]; then
      echo "Files outside declared scope:"; echo "$out"; exit 1
    fi

The second check is that behavioural changes have to be listed. The description has a section for every observable change in behaviour: a default changed, an error now thrown, a response field added, a migration. It is generated by the agent as part of opening the PR and the reviewer's first job is to check the list against the diff. Our incident PR would have had "removes the default on orders.currency" in that list, or the reviewer would have asked why it did not, because the schema file was in the diff.

The check for this one is weaker, it only verifies that the section exists and is not empty when certain paths change. The value is in making the list a required artefact. Agents are good at producing it. Reviewers are good at spotting a diff that contradicts it.

The third is that test changes are reviewed separately from code changes. The diff view in our review tool now opens with tests collapsed and a banner: tests changed, 4 files. The reviewer reads the code first, decides what they think it does, and then opens the tests and checks whether the tests agree with them, not with the code. That reordering is the single most useful thing we did. Reading the test first primes you to accept the code. Reading the code first and then the test turns the test into a claim to be checked.

Where a test was modified rather than added, the check requires a one line justification in the description for each one. "Updated assertion to match new behaviour" is a red flag sentence and reviewers treat it as one.

What a reviewer does now#

With those artefacts in place the review itself changed shape.

Read the declared scope and the behaviour list before the diff. Ask whether the scope is right for the task. A bug fix that declares a scope of eleven files is a question before it is a review.

Read the code, form a view of what it does, then read the tests as claims and check them. For any test that was modified, decide whether the modification is a correction or a capitulation.

Look for the halo. Even with the scope check, changes inside the scope can be adjacent. A reformatted block is fine. A removed line is never fine without a reason in the description.

Run it. This one was controversial and it is the change with the most disagreement on the team. For any PR that touches persistence, a queue, or an external call, the reviewer pulls the branch and exercises the change by hand, or through the agent with a specific instruction to demonstrate the behaviour in the list. It costs ten to twenty minutes. The argument for it is that a passing test suite from the same author as the code is not evidence, and a person watching the thing happen is.

Do not review the parts you cannot review. A 1,200 line PR does not get a review. It gets a comment asking for it to be split, and the agent splits it, and that takes five minutes rather than the two hours it took a human to split a PR in 2023. The upper limit we settled on is 400 changed lines excluding generated files and lockfiles, and a check enforces it.

What we stopped doing#

We stopped trusting green. A passing suite is the floor, and a PR where the agent also ran the tests it wrote and reports them passing is the same floor described twice.

We stopped treating agent PRs as needing more reviewers. The incident PR had two approvals. The second approver assumed the first had read the schema change. Two half reviews are one review with a diffused sense of responsibility. One reviewer, named, who owns the approval.

We stopped reviewing style. Biome does that and the agent follows Biome. Every comment about naming or formatting is a comment not spent on the behaviour list.

Where the author fits#

The person who drove the agent is still the author, and the thing we ask of them changed the most. Before, their job was to write the code. Now their job is to have read the code, to have written or checked the scope and the behaviour list, and to be able to answer questions about any line in the diff. If the answer to a review question is "I'll ask the agent", the PR was opened too early.

We put that in the template: by opening this PR you confirm you have read every changed line and can explain it. It sounds heavy. It is the exact bar we always had for human written code, and the only reason it needs writing down now is that the tool made it possible to skip.

What the agent sees#

One more change that cost nothing. The checks above produce failure output, and the agents we use read failure output and adjust. So the messages were written for the agent as much as for the human. "Files outside declared scope" lists the files and says: either add them to the Scope section with a reason, or revert the changes to them. "Behaviour changes section is empty but the diff touches a migration" says which migration and asks for one line per observable change.

After a month of that, most PRs arrive with the scope declared and the behaviour list filled in before a human has seen them, because the agent learned what the repository asks for from the checks that told it. The process taught the tool, and the humans got to spend their attention on the part of the review that needs a person.

Numbers, for what they are worth#

From May to August, with the checks in place: median time to first review went down, from 5 hours to about 2, mostly because the scope and behaviour sections make a PR easier to start on. Median PR size went down from 310 lines to 160. Reverts in the fortnight after merge went from 6 in the quarter before the change to 1 in the quarter after. The one was a human PR.

None of the checks are clever. They exist because the assumption our old process rested on, that the author understood their diff, is no longer something you can take for granted, so it has to be made into something you can verify.