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.
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.
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:
HEAD~2
-style ancestor parsing to the core lookup path. Merged with 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.
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.
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": null
still throws the exact exception class the PR claims is gone.
The review consequence— adopting the typed API was fine; retiring thewholeguard 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.
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:
"bytes": null
. They all has_more
value in the project is false
. The pagination rewrite — replacing careful manual paging with SDK auto-pagination — has zero multi-page coverage. The upstream bug was reported as crashing 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.
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:
HEAD~2000000000
as 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 own HEAD~999
test as proof. I was wrong.internal
, in an assembly the test project doesn't reference, with InternalsVisibleTo
granted 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.
Honesty section. Three things, all going in my issue tracker:
The shape was sketched in the method section; here's why the data justifies it:
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
"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.