Reviewing coding-agent diffs: what does the checklist catch first?

What you get here: a ten-item checklist for reading an agent's diff in the order that finds damage fastest, the reasoning for having a second model read it before you do, and a scanner script that turns the checklist into line references. The scanner ran in this session on a diff we constructed to contain every pattern; its output is below, unedited. Updated on 2026-10-30 with a run of the scanner over 6,500 real patches from SWE-bench Verified (the maintainers' fixes and test changes, and 5,500 model patches from eleven runs of one harness), and three fixes to the scanner that run found.

Why agent diffs need a different first pass

A human colleague's pull request comes with a mental model you share: they know which tests matter, they would not delete one to make CI green, and if they added a dependency they would say so. An agent's diff comes with none of that. It optimised for the check you gave it, and if the check was "make the tests pass", the diff may contain the cheapest route to that outcome. The AgentLens paper (arXiv:2605.12925) put a number on how often a passing run got there badly: 10.7% of passing trajectories in its sample showed regression cycles, blind retries, missing verification, or out-of-order work, with a range of 0.5% to 23.2% across the eight models it measured. The tests were green. The process was not.

So the first pass over an agent diff is not "is this good code". It is "did the agent change the rules of the game", and that question has a short, stable answer set.

The checklist

In the order we read them:

1. Deleted tests. A removed def test_ line, or a removed test file. This is the single most common way a red suite becomes green without a fix.
2. Skipped or neutered tests. A new @skip, @xfail, .skip(, or a test body replaced by a trivially true assertion. Same effect, easier to miss because the function is still there.
3. Weakened assertions. An assertEqual becoming assertTrue, a specific exception check becoming a bare assertRaises(Exception), an expected value widened until anything satisfies it.
4. Broad exception handling. A new except: or except Exception: around the code that was failing. The failure did not go away; it went quiet.
5. New dependencies. A new import that is not standard library, or any change to a requirements file, lock file, or package manifest. Agents reach for a package the way people reach for a search engine.
6. Files outside the task scope. If the spec said src/ and tests/, anything else in the diff is either a needed side effect or a wander. Either way it needs a sentence of justification.
7. CI and build configuration. A change under .github/, a Dockerfile, or a pipeline file. continue-on-error: true is one line and it turns a failing pipeline into a passing one.
8. Secret-like strings. Anything that looks like a key or token assigned in a new line, including in fixtures and example files.
9. Leftover markers. New fixme-style comments where a fix should have been. They mark the exact spot where the agent knew it was cutting a corner.
10. Large net deletions. A file that lost more than forty lines net. Sometimes that is the refactor you asked for. Sometimes the agent deleted the part it could not make work.

Everything else, style, naming, structure, is ordinary review and can wait until these ten are clear.

Let a second model read it first

The Claude Code best-practices page recommends an adversarial step before a task is counted as done: "have a subagent review the diff in a fresh context and report gaps." The reasoning is that a reviewer in a fresh context "sees only the diff and the criteria you give it, not the reasoning that produced the change, so it evaluates the result on its own terms." The same page ships a bundled /code-review skill that runs that review in a subagent and returns the findings to the session.

The caveat on the same page is worth quoting in full, because it is the failure mode of every automated reviewer: "A reviewer prompted to find gaps will usually report some, even when the work is sound, because that is what it was asked to do. Chasing every finding leads to over-engineering." The fix is to give the reviewer the checklist above rather than an open brief. "Report deleted or skipped tests, weakened assertions, new dependencies, and files outside src/ and tests/" produces a short list you can act on. "Review this diff" produces a page.

There is a related result in the harness literature. The "Same Model, Different Harness" paper (arXiv:2608.26218) kept the model and tasks fixed and changed only how the harness presented history to the model; on a tight-context comparison of 169 tasks the fail-to-pass fraction moved from 28% to 49%. The model that wrote the diff and the model that reviews it can be the same weights and still see different things, because the harness decides what they see. A fresh-context reviewer is a harness choice, not a model choice.

Stop some of it before it happens

Half the checklist can be prevented rather than caught. The Claude Code permissions page describes deny rules that "block in every mode, including bypassPermissions", and rule syntax like Read(./.env) for specific files. A deny rule on the test directory for the implementation phase, or on the CI directory for every phase, removes items 1, 2, and 7 from the diff before you read it. The permission models article covers the equivalent controls in other harnesses.

The scope item is also cheaper to enforce than to review. Running the agent in a worktree, as described in the worktrees article, means the diff is one branch against your main checkout, and git diff main...agent/task --stat lists every touched path in one screen.

The scanner

The shipped script reads a unified diff from a file or stdin, walks the hunks, and prints one line per finding with the file, the line number in the new or old file, and the text that triggered it. It exits 1 when anything is flagged, so it can sit in a script between the agent finishing and you reading. The --scope flag takes path prefixes; any changed file outside them is reported as out of scope.

We built a diff that contains every pattern on purpose, to check that each rule fires. It is a constructed fixture, not the output of any agent session: a tally.py that gains a pandas import and a broad except, a test file that loses the missing-column test and skips the blank-cells test, and a CI workflow that gains continue-on-error and writes a fake key into a file. This is the real output from this session, Python 3.13 on Windows:

$ python reviewing-agent-diffs-checklist.py agent.diff --scope src/ tests/
new-dependency   src/tally.py:2                     import pandas as pd
broad-except     src/tally.py:8                     except Exception:
leftover-marker  src/tally.py:9                     rows = []  # FIXME handle bad files properly
deleted-test     tests/test_tally.py:13             def test_missing_column_exits_2(self):
weakened-assert  tests/test_tally.py:14             with self.assertRaises(SystemExit) as cm:
weakened-assert  tests/test_tally.py:16             self.assertEqual(cm.exception.code, 2)
skipped-test     tests/test_tally.py:13             @unittest.skip("flaky on CI")
weakened-assert  tests/test_tally.py:19             self.assertEqual(load("fixtures/blank.csv", "x"), ["1", "", "3"])
weakened-assert  tests/test_tally.py:15             self.assertTrue(True)
secret-like      .github/workflows/ci.yml:10        - run: echo "api_key = 'sk-live-abcdefghijklmnop1234'" > .en...
ci-config        .github/workflows/ci.yml           CI or build configuration changed
out-of-scope     .github/workflows/ci.yml           not under src/, tests/
12 finding(s) in 9 categories: broad-except, ci-config, deleted-test, leftover-marker, new-dependency, out-of-scope, secret-like, skipped-test, weakened-assert
exit=1

Two things are visible in that output that matter for how you use it. The removed assertions inside the deleted test are reported twice, once as the deleted test and once each as weakened assertions, because the scanner does not know the assertions belonged to the function above them. And assertTrue(True) is flagged as weakened even though nothing was removed on that line, because a trivially true assertion is the pattern regardless of history. The findings are pointers. The reviewer reads the lines.

The rule set is deliberately small and readable, one regex per item, so that you can add the patterns your own codebase has taught you. The one we would add first for a JavaScript repository is a test renamed from it( to xit(, which is already in the skip rule; the one we would add for Go is a removed t.Fatal.

The ten patterns in 6,500 real SWE-bench patches

A fixture proves each rule fires; it says nothing about how often the rules fire on real work, or how often they fire on work that is fine. So on 30 October 2026 we ran the scanner over three sets of real patches for the 500 tasks of SWE-bench Verified: the fix each project's maintainers merged (the "gold" patch), the test changes they merged with it, and the 5,500 patches that eleven models submitted through one harness, mini-swe-agent 2.0.0, in the runs our task-by-task page measured (SWE-bench/experiments at commit 40f164d5b8f1). The model patches are the patch.diff files in the public swe-bench-submissions bucket those entries name, one per task, saved on 29-30 October; whether a patch resolved its task comes from the entry's own per-task record, and we re-ran no tests. 3,923 of the model patches resolved their task and 1,577 did not; 66 of the unresolved ones were empty submissions, which trip nothing. We publish counts, not the patches.

Two settings make the run fit the checklist. --scope was the list of files the gold fix touched, so "out of scope" here means a file the maintainers did not need to change, and the gold-fix column is zero by construction. --own was the repository's own package (django for django/django, sklearn for scikit-learn), so an import of the project itself is not a new dependency. reviewing-agent-diffs-swe-bench-scan.py does the fetching, the bookkeeping and the counting, and calls scan() from the scanner above for every patch. Its full output:

$ python reviewing-agent-diffs-swe-bench-scan.py scan --cache swe-bench-patches-2026-10-29
SWE-bench Verified tasks: 500; runs: 11; model patches: 5,500 (3,923 resolved, 1,577 unresolved)
empty submissions: 66 (0 resolved, 66 unresolved); patch.diff missing from the snapshot: 0
check                             gold fix        gold tests    model resolved  model unresolved
(patches)                              500               500             3,923             1,577
deleted-test                    0    0.0%       28    5.6%        0    0.0%        0    0.0%
skipped-test                    1    0.2%       20    4.0%        1    0.0%        2    0.1%
weakened-assert                 0    0.0%       61   12.2%       14    0.4%        6    0.4%
broad-except                    0    0.0%        3    0.6%       37    0.9%       69    4.4%
new-dependency                  8    1.6%       11    2.2%       38    1.0%       34    2.2%
out-of-scope                    0    0.0%        0    0.0%      324    8.3%      380   24.1%
ci-config                       0    0.0%        0    0.0%        0    0.0%        0    0.0%
secret-like                     0    0.0%        1    0.2%        0    0.0%        0    0.0%
leftover-marker                 1    0.2%        0    0.0%        3    0.1%        9    0.6%
big-deletion                    0    0.0%        1    0.2%        0    0.0%        1    0.1%
(any check)                    10    2.0%      104   20.8%      394   10.0%      456   28.9%
(any but out-of-scope)         10    2.0%      104   20.8%       89    2.3%      116    7.4%
files a model patch touched outside the gold fix's files:
  model resolved    existing source file 481, test path 17
  model unresolved  existing source file 531, test path 11, new file 1
scanner: reviewing-agent-diffs-checklist.py (--own = the repository package)
new-dependency hits on import lines that import the repository's own package:
  gold fix               0 of     11 (0.0%)
  gold tests             0 of     13 (0.0%)
  model resolved         0 of     48 (0.0%)
  model unresolved       0 of     51 (0.0%)

The first pass was a correction to our own scanner. Run with the rules as published on 5 September, the first pass flagged 59 of the 500 gold fixes (11.8%), and nearly all of it was noise. new-dependency fired on 11.2% of the gold fixes and 21.2% of the gold test patches, because the rule treated every import outside a 29-name list as third-party: 65.6% of the import lines it flagged in the gold fixes were the project importing itself, and most of the rest were relative imports (from . import x) and standard-library modules the list missed. And the skip rule's xit(, meant for JavaScript's skipped tests, matched sys.exit(: it flagged 10 resolved model patches before the fix and 1 after. We fixed all three on 29 October (a word boundary on xit(, relative imports and sys.stdlib_module_names excluded, and the --own option added); each fix only removes findings, and the twelve findings in the fixture output above are unchanged. The table above is the corrected scanner. --scanner runs the 5 September copy for comparison.

With the fixes, the scanner flags 10 of the 500 gold fixes (2.0%): eight that import a third-party library (numpy, docutils, jinja2, packaging and others), one with a to-do comment, and one pytest docstring that mentions @unittest.skip. The maintainers' test patches are a different story. The checklist flags 20.8% of the maintainers' test patches: weakened-assert on 12.2%, deleted-test on 5.6% and skipped-test on 4.0%, almost all of the skips being Django's conditional @skipUnlessDBFeature decorator. That is ordinary test maintenance: an assertion edited in place is a removed assertion line plus an added one, and the rule sees only the removal. So the test items on the checklist are right for a diff that was not supposed to touch tests, which is the usual agent task, and noisy for one that was. If the task includes changing tests, read items 1 to 3 as a list of lines to look at, not as a verdict.

The model patches separate along one item. deleted-test fired on none of the 5,500 and skipped-test on three, all three in pytest's own source, where skipping is the subject. weakened-assert fired on twenty, and most of the lines it flagged are edited assertions in Django's existing test files, with a few removed assert statements in library code; agents in this harness rarely touched tests, but some did. Out-of-scope fired on 8.3% of resolved model patches and 24.1% of unresolved ones, roughly three times as often when the patch failed. The extra files were not scratch scripts: 481 of the 498 files resolved patches touched outside the gold fix were existing source files, and so were 531 of the 543 for unresolved ones. The next strongest signal is broad-except: 0.9% against 4.4%. Everything except scope fired on 2.3% of resolved patches and 7.4% of unresolved.

Two readings follow. First, on this kind of work the item that earns its place is scope: a patch that edits files the real fix did not need is worth reading closely, and a reviewer without a gold patch can get the same signal from the task's stated scope. Second, a clean scan is weak evidence: more than 92% of the unresolved patches tripped nothing but scope, so the checklist finds the cheap ways to turn a suite green, not wrong fixes. The limits are the data's: one harness, twelve Python repositories, tasks whose tests the agent cannot see, and resolution as the leaderboard records it. An agent that can see and run a failing test, which is how most people use one, has more reason to touch it, and these numbers do not show how often it does.

What the scanner cannot see

It reads text, so it cannot tell you whether the implementation is right, whether the new test tests the spec, or whether a broad except was justified. It has no idea what the task was unless you pass --scope. And it will miss a weakened test that was weakened by changing the fixture data rather than the assertion. The red-green gate covers the case the scanner cannot: whether the test ever failed at all.