Where the review context lost, on the 13 PRs that lost

The quality arm of the PR benchmark found 13 pull requests out of 60 where a model reviewing the naive context (diff + every file it touches) named the defect being fixed and the same model reviewing the trace-mcp context did not. This page is the diagnosis of those 13, run before any tuning.

The expected answer was a distribution: some unresolved import edges, some symbols dropped under the bundle’s 8,000-token budget, some file never pulled in. The actual answer has no distribution in it.

Finding: the context contained no source code at all — in all 13

Dumping both arms’ prompts (bench-pr-context.ts --only <prs> --dump-prompts) and reading them shows the same thing 13 times out of 13. The trace-mcp arm’s “context bundle” was a list of signatures. Not truncated bodies, not partial bodies — zero lines of source, in every section, on every PR. In full, this was the entire context axios#11073 was reviewed from, after the diff:

=== Primary Symbol ===
[namespace] __module__:server — sandbox/server.js
(module body) sandbox/server.js

[function] requestHandler — sandbox/server.js
function requestHandler(req, res)
## Impact — call sites this change can break

The PR changes const pathname to let pathname. Deciding whether that is a fix or a regression requires seeing the later reassignment inside the function body — which the naive arm had and this arm did not. The pattern repeats: every one of the 13 judge notes describes speculation (“focuses on hypothetical scenarios”, “treats the fix as a behavior change”, “falsely claims XSS”), which is what a reader does when it has names and no code.

Root cause: a bare require() under ESM, swallowed by a catch

FileReadCache.readSymbolSource in src/tools/navigation/context-bundle.ts read file bytes through require('node:fs'). The package is "type": "module", so under real ESM that is a ReferenceError — caught by the catch {} next to it, cached as “file unreadable”, and reported as a bundle that assembled fine. assembleContext then degraded every item to no_source, its documented behaviour when a body is unavailable.

It worked in the two places we look:

• the shipped build — tsup.config.ts injects a createRequire banner, so installed users get a working require and full bodies. No released version served signature-only bundles.
• the test suite — vitest defines require in every module it transforms, so all 10k+ unit tests, including this tool’s own behavioural suite, exercised the working path.

It failed in exactly one place: a consumer that imports src/ as real ESM. The published package ships dist only (bin → dist/cli.js), so no install path reaches that code; the two consumers that do are scripts/bench-pr-context.ts under tsx — which is how the benchmark measured a context with the source removed — and pnpm serve, the contributor’s run-from-source path.

This is the second instance of the class; TRA-542 was the same bare require in dropDecisionRows, also behind a non-fatal catch, also reporting success.

What it cost

Both halves of the benchmark measured the degraded arm, so both numbers were wrong, in opposite directions.

The token number was too good. Re-running the same 60 PRs, same corpus, same pinned SHAs, with bodies restored:

The baseline arm is untouched (13,595 median, identical), which is the control this correction rests on.

Per PR, on the 13:

The quality number was too bad. The re-run, same 60 PRs, same preregistered bars, same judge protocol. Each run is its own pair — the naive arm was re-measured alongside the trace arm, so the struck run’s numbers are only comparable to the struck run’s naive column, not to this one:

Read across the pairs: comprehension went from −15 pp to +1.7 pp, false positives from +0.55 to +0.22, and latency from 17% faster to level. The naive arm scored 65.0% in both runs — measured twice, independently, at the same value.

One row of the re-run needed a second attempt: honojs/hono#5283 failed with a transport error (claude exited 1) and was filled by re-invoking the same script, which re-runs only the directories with no cached judgement. Nothing inspects a row’s outcome before deciding to keep it — the retry closed a gap in the artifact, it did not re-roll a result.

Both bars are met (≤10 pp comprehension loss, ≤+0.5 false positives): the trace arm lands 1.7 points above the naive one, which the bar never asked for and which 60 pull requests cannot make significant — parity is the honest reading. The struck run’s latency advantage was the speed of a context with the code removed, and it is gone.

Why no test caught it, and what does now

Nothing asserted that a body ever reached the caller. The behavioural suite for this tool checked the result shape — { primary, dependencies, callers, totalTokens, truncated } — and the markdown branch was covered only by existence, not content. A signature-only bundle satisfies every one of those assertions.

Two gates were added:

• tests/tools/behavioural/get-context-bundle.behavioural.test.ts now asserts the markdown output contains the primary symbol’s body, not its signature. This is the assertion the coverage was missing; it does not catch the ESM mechanism, because vitest defines require.
• tests/ci/no-bare-require.test.ts scans src/ and fails on any bare require( outside a stated allowlist. This is the gate for the mechanism — static, because neither runtime we test in can reproduce it.

Two further latent instances of the same bug were fixed while the rule was being written: python-modules.ts (readdirSync for src-layout detection, which silently returned “not a src layout” under ESM) and install-app.ts.

The other lesson: changed_symbol_readable measured nothing

The token benchmark reported changed-symbol readability of 100% in both arms — including on all 13 of these PRs, while the trace arm contained no code. The metric recorded a span whenever the bundle listed a symbol, never checking that the bundle carried its body. It was the one indicator that should have caught this and it was structurally incapable of it.

Fixed, not renamed (TRA-1100). get_context_bundle now reports a detail field ('full' | 'no_source' | 'signature_only') per symbol — the same classification assembleContext already computed internally but never surfaced. changed_symbol_readable and dependent_readable in scripts/bench-pr-context.ts now count a span as readable only when detail === 'full'; dependent_pointed is unchanged, since “named with a location” was always the honest claim for that one. The same field backs a cheap CI gate (tests/ci/context-bundle-body-coverage.test.ts) that fails on partial body loss, not just total loss — see the preregistration’s release gate section.

And once it measured something, it failed. Under the corrected definition the trace arm reads 71% against the naive arm’s 100%: 98 of 338 changed symbols across the corpus arrive without their bodies. That misses a preregistered quality floor, which now publishes as MISSED.

The cause is not what this page first guessed. 93 of the 98 are the bundle budget truncating — raising it to 64,000 brings all but five bodies through, at 8,000: 240 bodies and 72.7% saved; 16,000: 263 at 32.3%; 32,000: 299 at −0.1%; 64,000: 333 at −0.3%. The saving crosses zero while 39 bodies are still missing, so there is no budget on this corpus that buys full coverage and a token win. The missed floor is therefore a packing problem — which symbols get the budget — not a budget-size problem.

Whole-file nodes — the suspect named here on 2026-09-07, meaning a __module__ / <module> node or a document node whose body is the file itself — are 65 of the bodyless, and they thin out with budget like everything else, so they were never a separate structural cause.

5 bodies arrive missing at any budget, down from 29 measured before the containment fix below: most of what the higher budgets were buying back was budget the bundle had been spending on duplicate bytes. Five is small enough to read one by one. Per-symbol counts: benchmarks/pr-context/symbol-detail.json; reproduce with --symbol-detail and --bundle-budget. —

Second pass, same 13 PRs: the context was paying for the same bytes twice

Added 2026-09-07 (TRA-1141), after the correction above.

With bodies restored, 13 of the 60 PRs cost more than reading the files outright. Breaking those 13 prompts down by section — token counts per section of the dumped prompts, gpt-tokenizer, same pinned SHAs — puts the excess in one place. In 10 of 13 the “Primary Symbol” section alone was larger than the naive arm’s entire file dump.

The reason is containment. __module__:foo spans its whole file and note:Readme spans its whole document, and a changed-symbol review bundle asks for both the container and the functions or headings inside it — so the members’ bytes shipped twice. axios#11118 sent InterceptorManager.js as a module body and then again as three functions, a class and two methods. Two related cases cost as much: an entire markdown document inlined because a wikilink mentions the symbol (32% of got#2379’s prompt), and an entire test file inlined as a “caller” (94% of axios#11039’s excess).

get_context_bundle now emits a contained symbol once, inside the container that already carries it, and keeps whole-file and prose symbols in the dependency and caller lists as pointers rather than bodies. Same 60 PRs, same SHAs, same corpus:

The loss count moved as well, 23 → 56, for a reason that has nothing to do with this table: the metric behind it changed in the same run. The two sections below are that story.

The 13 costliest PRs stay costlier, and that is structural rather than fixable: when the changed symbol is the module container, the bundle’s primary section is the file, so it can approach the cost of reading the file but never beat it, and the diff, callers and impact list sit on top. What changed is the size of the overrun — across those 13 prompts, 49,770 → 40,991 tokens.

What it cost on the quality side. Shaping a response without checking comprehension is the failure this whole page exists to prevent, so the two bundle versions were run head-to-head on the 13 PRs the change touched most: same judge protocol, same model, blind and order-randomised, arm A the old bundle and arm B the new one.

Comprehension is identical (9 of 13 each; 8 understood by both, one by each arm alone). The false-positive difference is two claims across 13 PRs — a number this sample cannot resolve, reported because it moved the wrong way, not because it means anything.

What the honest metric reads, now that it has been run

TRA-1100 landed detail and pointed both readability columns at it, in parallel with this work and without re-running the benchmark — so the published artifact still carried the figures the old metric produced. This is that re-run. Holding everything else constant, dependent_readable in the trace arm:

Thirty of those thirty-six points were the metric; six are real — bodies the new rules moved into the pointer list. dependent_pointed stays 100%: every dependent is still named with a location the agent can fetch. That trade is the change’s actual cost, and it belongs next to the 72.7%, not underneath it.

And the finding that costs the most: half the changed bodies never shipped

The same re-run moves the column that had read 100% since this benchmark was written. changed_symbol_readable counted a changed symbol as readable whenever the bundle listed it; scored against detail === 'full', on the same 60 pull requests:

Nothing about what the product serves changed between the first two columns. This is the same defect as the one at the top of this page, one level deeper: the budget was truncating changed-symbol bodies all along, and the metric recorded a span whenever the bundle listed the symbol. A third of the PRs never had the changed code in front of them.

The third column is this change, and it is the reason the median saving reads 72.7% rather than 75.2%. Deduplicating a member into its container is only free while the container’s body survives assembly; when the budget reduces the container to a signature, the member was the one thing that could still have fitted. The bundle now notices that and re-assembles with those members restored — which costs 2.4 points of saving and buys back four points of changed-symbol coverage against the old bundle, on top of the eleven the deduplication had cost.

The mechanism is visible in the extremes. axios#11119 edits a line of README.md; the changed symbol is the whole 24,000-token document, it cannot fit the primary category’s share of an 8,000-token budget, and what shipped was its first line. psf/requests#7371 fixes a typo in a comment inside a 30,000- token test module; what shipped was module tests/test_requests.py. Both are counted in the 72.7% median saving, and in both the saving is partly the cost of not sending the code — 349 tokens against 24,348, and 282 against 25,197.

This does not retract the token figure, which counts what the arms actually sent and is unchanged by the metric fix, and it does not contradict the comprehension parity measured in TRA-568 — that judgement was made on these same prompts. It does say the bundle needs a better answer than a signature when the changed symbol is larger than its budget: the sub-symbols the diff actually touched, or the hunks’ surroundings, rather than the container’s first line. That is TRA-1144, filed from this run, and it is now measurable because the metric finally moves when it happens.

Saying so when the budget refuses a body (TRA-1144, 2026-09-07)

Re-assembling with the restored members answers the case where there is something smaller to send. Some changed symbols have nothing smaller. axios#11119’s is a whole 24,000-token README.md; psf/requests#7371’s is a 30,000-token test module. No share of an 8,000-token budget delivers either, and sending them whole would buy the coverage column at a price that defeats the tool — so that ceiling stands.

What does not have to stand is how it was reported. The degraded entry shipped as (module body) tests/test_requests.py: one line, reading like the whole answer, indistinguishable from a symbol that genuinely has no body. It now carries the fact:

[namespace] __module__:test_requests — tests/test_requests.py
(module body) tests/test_requests.py
[body omitted — 29,847 tokens, over this section's budget]

Cost, same 60 PRs, same pinned SHAs, measured against master at the merge of the change above: median saving 72.8% → 72.7%, median trace tokens 3,286 → 3,291, +1,445 across the set — about 23 tokens on each of the 46 PRs that omit anything, nothing on the other 14. Changed-symbol coverage is unchanged at 71%, by design: the marker declares the loss, it does not repair it. The declaration sits inside the token accounting, so a longer degraded entry can consume budget a shorter one would not have; that is a property of the greedy assembler, not a defect, and it is what the +1,445 is.

The marker stays silent for the entries the bundle caps before the assembler runs — dependencies past MAX_FULL_SOURCE_DEPS, markdown files, whole-file containers. Those reach the same no_source tier with their source never read, so there is no omitted body to report, and a behavioural test pins the two routes apart.

This closes the third of the three answers TRA-1144 asked for. The first — send the sub-symbols the diff touched rather than their container — is the re-assembly rule above. The second, a line window around each hunk, was not built: get_context_bundle never sees the diff, and the sub-symbols the caller already asked for are the same information arriving through a channel that exists.

The comprehension result, which did not go the expected way

An implementation of the first answer, built in parallel with the one that shipped and discarded in its favour, was run head-to-head against the pre-fix bundle on the 13 PRs it moved most — the TRA-568 judge protocol, blind and order-randomised:

Not equal only on aggregate — identical row by row: all 13 landed on the same verdict in both arms (6 understood by both, 7 by neither, 0 by either alone). The check on the implementation that did ship reported 69.2% in both arms on its own 13 PRs. Different samples, same shape of answer: no movement.

So the coverage column’s climb from 50% to 71% has no comprehension result behind it yet. Both samples are underpowered and both were chosen for being hard, which does not settle whether the effect is absent or merely unmeasurable here — but until something does, the recovered bodies are worth reporting as delivery, not as quality.