I pre-registered a small study, pointed a 12-reviewer pipeline at three merged Copilot PRs in major .NET organisations, and the failure mode I found was not the one everyone talks about.
Disclosure first: I maintain review-pro, the open-source review system used as the instrument here. This article exists because I wanted to know whether its premise survives contact with real data. Part of it didn't. That's in here too, along with the two findings I got wrong myself and the tool caught.
The setup
You have an agent write a feature. You ask an agent to review it. What comes back is unhelpful in one of two directions: forty nitpicks, or a cheerful approval.
The usual explanation for why AI-written code needs its own review discipline is hallucination — invented APIs, imports that don't exist, config keys nobody defined. My tool ships a reviewer dedicated to exactly that, so I had every incentive to find it.
I went looking for it in the most honest place I could think of: agent-authored pull requests that real maintainers already merged into serious codebases.
The method, in brief
Before running anything, I wrote down what would count — a pre-registration with the corpus criteria, the classification buckets, and anti-gaming rules (first run counts, no diff dropped after seeing its findings, false-positive rate published whatever it is, negative results published). The full document and per-case records are public; nothing below was decided after seeing results, and the two things that were are marked as dated amendments in it.
The instrument: review-pro doesn't ask one LLM to review the diff. A triage pass reads the change and dispatches only the relevant specialist reviewers — each with a single mandate and its own scoped context — then a synthesis pass dedups overlapping findings, resolves conflicts by domain ownership, and emits one verdict. The rule that matters most for this study: reviewers are required to locate evidence in the repository before making a claim; an unverified "this looks wrong" is forbidden by their rubric.
diff
└── triage — which reviewers does this change actually need?
├── correctness ┐
├── api-contract │ only the relevant specialists,
├── tests │ in parallel — each must locate
├── craft │ repo evidence for every claim
└── ai-antipatterns ┘ (12 exist; the chore diff got 4)
└── synthesis — dedup, resolve conflicts, one verdict
That's the shape under test, not just the tool: whether specialised, evidence-required review finds what a single diff-reading pass cannot.
The corpus: merged PRs authored by GitHub's Copilot coding agent (author:app/copilot-swe-agent — GitHub itself attributes them, no vibes-based "this looks AI-written"), in established organisations, 30–400 changed lines, taken by recency rather than by browsing for juicy diffs. Three cases:
-
A .NET versioning library — 71 lines adding
HEAD~2-style ancestor parsing to the core lookup path. Merged with zero review comments and an empty approval, 12/12 CI green. - A widely-used AI extensions library — 125 lines deleting a raw-JSON workaround for an upstream SDK bug, plus a dependency bump. Merged with two human approvals and inline discussion.
- E2E test infrastructure in a large app framework — 48 lines of timeout hardening. A chore, included deliberately as a noise-floor test.
I anonymise the repos in this article because the argument is about a pattern, not about naming maintainers who merged something. The full records — PR links included — are published with the pre-registration, since "verified by hand" is only worth something if you can check it; the article body keeps them unnamed so the piece stays about the pattern, not the people.
One observation before any review ran, recorded in the pre-registration because it cuts against my own thesis: merged agent PRs in serious orgs skew heavily mechanical. Version bumps, lint fixes, dependency updates, a disabled flaky test. If most agent code that actually ships is mechanical, the surface for "confidently invented an API" is narrower than the discourse assumes.
Result 1: in this corpus, the falsifiable categories fired once — never for hallucination itself
My tool's AI-antipatterns reviewer has three falsifiable categories — claims that are objectively true or false, where I could verify every finding by hand:
| Category | Case 1 | Case 2 | Case 3 |
|---|---|---|---|
| Hallucinated API / symbol / import | 0 | 0 | 0 |
| Invented config / env key | 0 | 0 | 0 |
| Needless dependency | 0 | 1 | 0 |
Every symbol, import, and config key these agents wrote existed. I checked by hand; the reviewer checked independently; we agreed. The one hit was a dependency bump whose stated rationale didn't hold up — more on that below, because it's the most interesting defect of the lot.
If you came here for "LLMs make up functions," this is a negative result, and I'm publishing it as one. In this corpus, merged agent code simply doesn't look like that. Tool access — agents grepping before they import — may well be part of why, but that's a hypothesis about mechanism, and this study didn't test it.
Result 2: what fired instead
The category that fired repeatedly — and produced every finding that would have changed a merge decision — was ignored convention. Not inventing things the codebase doesn't have. Failing to notice things it does.
Case 1. The new ancestor-walking code calls a throwing API inside a method whose documented contract is "return null when not found." That alone is an ordinary bug. What makes it interesting: the repository already knows about this exact failure mode. There is a canonical guard elsewhere, with a comment that says, verbatim, "Our managed git implementation throws this on shallow clones." The agent's code sits outside that guard, so get-version HEAD~1 in a shallow CI clone regresses from a clean "bad ref" exit code to a raw internal error. Same story twice over: the repo knows how to peel annotated tags — there's a helper that does it — and the new code doesn't, so <release-tag>~1 cannot resolve at all. In a tag-driven versioning tool, where the ancestor of a release tag is arguably the headline use case.
Case 2. The PR deletes ~97 lines of defensive raw-JSON parsing, citing an upstream SDK bug as fixed.
What the PR assumed — the SDK bug is fixed; the workaround can go.
What the upstream source shows — the fix covers exactly one of the two fields the workaround defended.
What that means on the wire —"created_at": nullstill throws the exact exception class the PR claims is gone.
The review consequence — adopting the typed API was fine; retiring the whole guard on the strength of a one-field fix was not.
I verified the middle two lines from the SDK's generated deserializer at both version tags:
// "bytes" — null guard present in BOTH the old and new SDK version
if (prop.NameEquals("bytes"u8))
{
if (prop.Value.ValueKind == JsonValueKind.Null) { sizeInBytes = null; continue; }
...
}
// "created_at" — no guard in either version, non-nullable target
if (prop.NameEquals("created_at"u8))
{
createdAt = DateTimeOffset.FromUnixTimeSeconds(prop.Value.GetInt64());
continue;
}
And the falsifiable-category hit: the version bump justifying all this was unnecessary for its stated purpose. The fix was already present in the version the repo was on. The bump is only needed because the agent rewrote one call to the newer version's API shape — a self-inflicted dependency change on a shipped package, framed as a bug-fix requirement. Two human reviewers approved it.
Case 3. Even the chore had a version of this. The new timeout constant's doc comment says it "mirrors the explicit budget an existing helper already sets." It inverts it — the existing helper derives the budget from the wait; the new code derives the wait from the budget. And the constant got applied to two call sites that launch via dotnet run, where the environment variable it's calibrated against is never set, turning a documented invariant into dead prose and a 2-minute failure into a 4-minute one.
Here's the pattern in one sentence: the defects were not in what the agent wrote, but in what the agent didn't know the codebase already knew. The evidence for every one of these lives in files the diff never touched — a guard in another module, an upstream deserializer, a sibling helper's coupling direction. Which is precisely why a human skimming the diff, or an agent reviewing only the diff, won't catch them.
Result 3: green CI certified none of the things that mattered
Case 2's test suite is good — 900+ lines driving the real SDK through a fake HTTP handler. It still could not test the PR's central premise:
- Not one fixture in the project sends an explicit
"bytes": null. They all omit the field — a different deserializer path. The suite passes identically whether the upstream bug is fixed or not. - The only
has_morevalue in the project isfalse. The pagination rewrite — replacing careful manual paging with SDK auto-pagination — has zero multi-page coverage. The upstream bug was reported as crashing during auto-pagination.
Twelve green checks on case 1. Green CI everywhere. In no case did CI exercise the actual risk the change carried. "Tests pass" and "someone traced the shallow-clone path" are different claims, and only one of them is cheap.
Result 4: the tool corrected me. Twice.
I wrote my own ground-truth notes before each dispatch, so I could score the tool against an independent read. Embarrassingly, the scoring went both ways:
- I flagged
HEAD~2000000000as an unbounded loop. Two reviewers independently traced that the loop exits at the root commit — bounded by history depth, not by user input — and both cited the PR's ownHEAD~999test as proof. I was wrong. - I flagged a hard-coded env-var name where the repo "has a canonical constant." Three reviewers established the constant is
internal, in an assembly the test project doesn't reference, withInternalsVisibleTogranted to two other projects only — and that the literal is the file's pre-existing idiom. Wrong again.
Both of my false positives were exactly the genre of confident-but-unverified claim this whole article is about. The discipline that caught them wasn't mine; it was a rule in the reviewers' rubric: no finding without located, verified evidence. If you take one implementation idea from this piece, take that one.
The traffic also flowed the other way: a reviewer read the PR's commit history (I had only read the diff) and found that the PR's own final commit was a build-break fix necessitated by its hand-rolled index arithmetic — evidence for a simpler implementation, sitting in the PR itself.
What didn't work
Honesty section. Three things, all going in my issue tracker:
- One finding was right for a wrong reason. A reviewer correctly flagged missing multi-page test coverage, but claimed the test harness needed rework to support it. False — the harness already takes a request-predicate function, and other tests use it. A reader acting on that remedy does an unnecessary refactor. Core claim true, supporting claim wrong; I'm counting these separately from clean true positives.
- Leaving the repo is not automatic. Two reviewers correctly flagged the case-2 premise as "unverified by the repo" — but didn't go check the upstream SDK themselves. The two that did (and nailed it) included one I had explicitly told to. Repo-grounded verification is reliable; external fact-checking currently depends on the mandate you give the reviewer.
- n = 3. This is a pattern I observed under pre-registered rules, not a proven population claim. I'd genuinely like to see someone run the same protocol on Rust or TypeScript corpora and publish disagreement.
Why twelve reviewers and not one
The shape was sketched in the method section; here's why the data justifies it:
- The strongest case-1 finding was independently produced by four reviewers from four different angles (contract, error path, maintainability, convention) and collapsed into one item by a synthesis pass. That convergence is signal you can't get from one context.
- Reviewers disagreed — one flagged a partial rollout, another scoped the same observation out as pre-existing. Both defensible; synthesis resolves it by domain ownership. A single agent would never even surface the tension.
- Silence stayed cheap. On the chore diff, triage dispatched 4 reviewers instead of 7, one returned zero findings, and another explicitly declined five would-be findings with reasons. Noise is the failure mode everyone predicts for fan-out review; the noise floor is what I most wanted to measure, and it held.
Takeaway
The distribution of agent-code failure has shifted somewhere the discourse hasn't followed. In this corpus, at least, it wasn't invention. It was locally plausible code written with no memory of the codebase's accumulated knowledge: the guard someone added after a production incident, the peel helper, the coupling direction a sibling function established. Every one of those is invisible in the diff and discoverable only by tracing what the diff touches.
That's a reviewable property. But it means review has to leave the diff — search the repo, read the callers, check the upstream source — and it means "reviewed" has to mean more than an approval on green CI. The diff is where the change is. The repository is where the evidence is.
If you want to poke at this, here's what running it looks like — install once, then invoke the skill on a branch inside your agent tool:
/plugin marketplace add tufantunc/review-pro # Claude Code
/plugin install review-pro@review-pro
# then, in a session on the branch under review:
"review this branch with review-pro"
And here's what a finding looks like. This one is real — case 1 of the pilot, condensed from the full record:
- severity: High
category: api-contract.breaking
file: src/.../ManagedGit/GitRepository.cs
line: 379
title: new `~` path makes public Lookup throw where its
documented contract is "otherwise null"
evidence: GetCommit (line 329) throws GitException on a missing
object; every pre-existing path in Lookup returns null
impact: get-version HEAD~1 in a shallow CI clone regresses from
a clean BadGitRef exit to a raw InternalError
remedy: resolve the parent via the non-throwing TryGetObjectBySha
before walking, keeping the documented null contract
confidence: high
The tool is MIT, plain markdown, runs in Claude Code / opencode / Cursor / Codex (repo). The most useful thing you can send me is not a star — it's a false positive or a missed finding, with the code. There's an issue template for exactly that; the rubrics get calibrated from real examples, and this article is what that calibration process looks like when you point it at the tool itself.
Pre-registration, amendments, and full per-case findings: studies/2026-08-copilot-pr-pilot.
Top comments (3)
The “ignored repo memory” framing matches the failure I keep running into with coding agents. The cheap guard I like is forcing the reviewer to name the older file or helper that should have changed its answer. If it can’t point to one, it’s probably just grading the diff in isolation.
That guard matches the pilot data almost exactly. In case 1, every finding that would have changed the merge decision cited an unchanged file. The sharpest one pointed at a guard in another module whose comment named the exact failure mode the new code reintroduced. The findings that only referenced the diff were the ignorable ones.
review-pro encodes the per-finding version of your rule: a finding without located evidence is forbidden by rubric, and "located" usually means outside the diff. What it doesn't have yet is your aggregate version — if an entire review of a substantive change contains zero references to unchanged files, the review itself is suspect. That's cheap to check at the synthesis stage, and I'm adding it: github.com/tufantunc/review-pro/is...
One boundary I'd put on it: it can't be a hard per-finding gate, since some real defects live entirely inside new code (the off-by-one variety). As a review-level tripwire, though, I think you're right and it costs nothing. 🙌
This matches the review failure I keep seeing. The diff gives the reviewer a neat boundary, but the bug usually lives in the caller graph or an old guardrail outside that boundary. Green CI only proves the sampled path survived.