The agent didn't hallucinate. It ignored what the repo already knew.
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 ~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 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_more value 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~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 ownHEAD~999 test 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, withInternalsVisibleTo granted to two other projects only - and that the literal is the f
Comments
No comments yet. Start the discussion.