fix(review): stop stubbing files whose worktree content the size probe cannot find

[backnotprop]

TLDR

In v0.26.0 and v0.26.1, code review silently drops file content from diffs. A file that was renamed and then edited, and any file in a partial clone whose blob is not local, renders as an empty card: no hunks, no counts, no explanation. /api/file-content then refuses it as binary, so there is no way to recover the content in the UI. A reviewer can approve a change without ever seeing it.

On the harness fixture the renamed-and-edited Panel.tsx (a real +8/-2) shows as h0 +0/-0 on both released versions. Nothing in the product says anything is missing.

Mechanism

buildBoundedTrackedDiff (packages/shared/review-core.ts) runs an oversized preflight: git diff --no-textconv --raw -z --no-abbrev <base>, then batch-probes every object id with cat-file --batch-check.

For tree-vs-worktree diffs, any path pulled into rename/copy detection has its working-tree content hashed by git, and that computed hash is printed in --raw output. The blob is never written to the object database. getGitObjectSizes skipped missing answers, and the backfill loop mapped every id it had no size for to Number.POSITIVE_INFINITY.

The consumer only opened the filesystem-stat door for all-zero object ids (isNullObjectId), so those entries were read as oversized, excluded with a :(top,exclude,literal) pathspec, and replaced by buildOversizedTrackedStub: zero hunks plus Binary files ... differ. The client parsed that to 0/0 and rendered a header with an empty body, and the isBinaryPatchFile gate in /api/file-content (both the Bun server and the Pi mirror) then returned nulls.

Verified affected shapes:

• committed or staged rename plus an unstaged edit (R stub)
• tracked add plus an unstaged edit while rename detection is active (A stub)
• copies
• index blobs missing from the object database, for example a partial clone with an unreachable promisor (M stub)
• old-side blobs not yet fetched in a partial clone

Affected views: since-base, uncommitted, unstaged, and the GitButler and jj callers of getWorkingTreeDiffFromBase. Affected versions: v0.26.0, v0.26.1, and dev.

The fix

1. The stat door now covers unreadable new sides, not just unhashed ones. workingTreeInfo is resolved whenever the new side has no readable object behind it: an all-zero id, or an id the probe that ran could not find. The working-tree file is exactly what git formats for those entries, so its stat size is the authoritative bound. This is the invariant already documented above the loop: core.bigFileThreshold does not bound working-tree sides, so their sizes have to come from stat.

2. Missing means unknown-but-bounded, never infinite. getGitObjectSizes now returns Map<string, number | null>, where null is "the probe ran and could not size this object" (missing, omitted, or an unparseable size). An unknown blob size reads as not-oversized, which is safe because every rendered diff already carries BOUNDED_DIFF_GIT_CONFIG (core.bigFileThreshold, from #1205), so git itself stubs oversized object-database content. A missing blob must never stub a file git can plainly diff. If git genuinely cannot produce the content (a dead promisor), the diff now fails loudly through assertGitSuccess rather than silently blanking a file. Genuinely oversized files still stub, via real probe sizes and the stat door.

The door being closed is the per-object missing door only. #1205's invariants are preserved exactly:

• the batch-failure degraded path is untouched: a probe that fails outright still says nothing about any single object, so those ids keep relying on core.bigFileThreshold rather than on a working-tree file that may not even be the diff's new side (a staged or commit-to-commit diff's new side is not the worktree).
• the fingerprint null-means-fresh contract is untouched.
• the memory bound itself is untouched.
• oversized stubs keep their content-sensitive id: a synthesized worktree hash is now used only when git printed no id at all. When git printed a real id it had already hashed that exact worktree content, so the stub keeps it instead of re-hashing a multi-megabyte file.

3. An empty card now says why it is empty. New shared predicate isContentlessBinaryPatch in packages/shared/diff-paths.ts (binary marker, no hunks) plus a small BinaryFileNotice component, mounted in both the all-files view (AllFilesCodeView) and the single-file viewer (DiffViewer). Copy: "Binary or oversized file, content not shown." This is defense in depth: whatever the cause, a card with no body now states that content was not shown instead of reading as "no changes here".

Failing-then-passing proof

New tests drive the real runGitDiff / getWorkingTreeDiffFromBase against scratch git repos built to the reproduction shapes.

test	unfixed	fixed
renders a renamed file whose edited worktree blob the probe cannot find	FAIL: got similarity index 94% / rename ... / Binary files a/src/Card.tsx and b/src/Panel.tsx differ	PASS
renders a tracked add whose worktree blob the probe cannot find	FAIL: got new file mode 100644 / Binary files /dev/null and b/replacement.txt differ	PASS
renders a file whose index blob is missing from the object database	FAIL: got new file mode 100644 / Binary files /dev/null and b/added.txt differ	PASS
still stubs an oversized worktree file whose blob the probe cannot find (5 MB+, rename-detected, id missing)	PASS (guard)	PASS
DiffViewer DOM: a hunkless stub explains its empty body / a genuine binary explains its empty body	FAIL (no notice in the DOM)	PASS

The #1205 test a single object the probe reports missing still excludes only that path asserted the exact door being closed, so it is rewritten rather than deleted, and it now covers strictly more: a probed-oversized object still excludes only its own path (asserted on the emitted pathspec list), while a missing one excludes nothing and keeps rendering. Plus six unit tests for isContentlessBinaryPatch, including a text patch whose content quotes the binary marker.

Full runs: bun test 2936 pass / 0 fail; bun test apps/pi-extension 177 pass / 0 fail (Pi vendors review-core.ts and diff-paths.ts verbatim through vendor.sh, so parity is automatic); bun run typecheck clean; bun run --cwd apps/review build then bun run build:hook both clean.

Version-matrix harness (dev, same fixture, since-base)

metric	v0.24.2	v0.25.1	v0.26.0	v0.26.1	dev before	dev after	git truth
total +	35	35	26	26	26	34	35 (tracked +25, plus untracked)
total -	22	22	20	20	20	22	22
Panel.tsx (renamed + edited)	R h2 +8/-2	R h2 +8/-2	R(bin) h0 +0/-0	R(bin) h0 +0/-0	R(bin) h0 +0/-0	R h2 +8/-2	R050
binary-stub files	none	none	6MB file; Panel.tsx	6MB file; Panel.tsx	6MB file; Panel.tsx	6MB file only	n/a
/api/file-content on Panel.tsx	old=279ch new=413ch	old=279ch new=413ch	null, null	null, null	null, null	old=279ch new=413ch	n/a
big-file representation	M h1 +1/-0	M h1 +1/-0	binary stub	binary stub	binary stub	binary stub	M +1/-0

dev now matches git ground truth on every file, with one deliberate difference: the 6 MB file stays a binary stub, which is #1205's memory bound doing its job. That single stubbed line is the whole +34 vs +35 gap. The renamed file is back to real content and its file-content probe is non-null again.

Lineage

#1167 is the size-cap work this preflight came from; #1205 made the memory bound git-native (core.bigFileThreshold) so a failed probe stops blanking the whole review. This change closes the remaining per-object door, the missing one, and keeps the memory bound exactly where #1205 put it.

Merged with #1219

#1219 landed first, so main is merged in here and the predicted overlap is resolved in this branch. Rule applied: specific beats general, and the two never stack.

• packages/shared/diff-paths.ts keeps both detectors, ordered specific then general. isOversizedReviewStubPatch (marker-based, from fix(review): mint content-derived diff cache keys so single-file tabs render fully #1219) stays first; isContentlessBinaryPatch follows as the documented fallback, with a note that callers ask the marker first. No duplicate helpers.
• Both mount points (AllFilesCodeView and DiffViewer) render exactly one notice: a marker-carrying stub gets fix(review): mint content-derived diff cache keys so single-file tabs render fully #1219's OversizedFileNotice, which knows the file is over the size cap; anything else hunkless and binary (a genuine binary, or a stub shape the marker does not cover) gets BinaryFileNotice. The fallback is explicitly gated on the marker so the two can never both appear on one card.
• review-core.ts merged without conflict: fix(review): mint content-derived diff cache keys so single-file tabs render fully #1219's marker injection is in buildOversizedTrackedStub, this change is in getGitObjectSizes and the preflight loop.
• The new DOM test moved into fix(review): mint content-derived diff cache keys so single-file tabs render fully #1219's isolated real-renderer CI step rather than the shared list, for the reason that step exists. fix(review): mint content-derived diff cache keys so single-file tabs render fully #1219's afterAll restore of the leaked mock.module('@pierre/diffs') in AllFilesCodeView.lifecycle.test.tsx is untouched.

A new DOM case pins the harmonization directly: a marker-carrying stub renders exactly one oversized notice and zero fallback notices.

Post-merge verification: bun test 2938 pass / 0 fail; all three DOM CI steps green, with the shared list and the isolated diff-renderer step each run forward and reversed (226 pass, 8 pass, 6 pass); typecheck clean; review and hook builds clean. The harness re-run on dev still reports +34/-22, Panel.tsx back to R h2 +8/-2 with real content, bigassets/metrics-6mb.txt the only remaining stub, and its served patch carries #1219's marker, so it renders the specific size-cap notice rather than the fallback.

AI-assisted.