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

The oversized preflight in buildBoundedTrackedDiff mapped every object the cat-file batch could not size to infinity. `missing` is routine: for tree-vs-worktree diffs git hashes the WORKING-TREE content of any path pulled into rename/copy detection and prints that hash in --raw output without ever writing the blob, and partial clones report it for unfetched blobs. Those files were excluded by pathspec and replaced with a contents-free binary stub, so a renamed-and-edited file rendered as a silently empty card and /api/file-content refused it as binary. Missing now means unknown, not oversized, and an unreadable new side is bounded by the working-tree file's stat size instead. Every rendered diff stays bounded git-side by core.bigFileThreshold (#1205), genuinely oversized files still stub via real probe sizes plus the stat door, and a blob git truly cannot read now fails loudly through assertGitSuccess instead of blanking a file. Client side, a chunk with a binary marker and no hunks now renders an explicit placeholder in both the all-files view and the single-file viewer, so an empty card can never again pass for "no changes here". AI-assisted.
2026-09-14 14:17:26 +08:00 · 2026-08-06 00:47:19 -07:00
parent 7ba4e3b3e4
commit 75e8b78cc7
9 changed files with 482 additions and 39 deletions
@@ -55,6 +55,7 @@ jobs:
          DOM_TESTS=1 bun test
          packages/review-editor/components/DiffViewer.fullContentSwap.test.tsx
          packages/review-editor/components/DiffViewer.oversizedStub.test.tsx
+          packages/review-editor/components/DiffViewer.binaryNotice.test.tsx

      # Seam contracts + the remaining DOM-gated tests. Scoped to the DOM files
      # (not the whole ui suite) to keep this process light.
@@ -30,10 +30,11 @@ import { buildCodeNavRequest } from '../utils/buildCodeNavRequest';
 import { getDiffSelection, getLineNumberFromNode, getSideFromNode } from '../utils/diffSelection';
 import { isContentConsistentWithPatch } from '../utils/patchConsistency';
 import { hashString } from '../utils/hashString';
-import { isOversizedReviewStubPatch } from '@plannotator/shared/diff-paths';
+import { isContentlessBinaryPatch, isOversizedReviewStubPatch } from '@plannotator/shared/diff-paths';
 import { OversizedFileNotice } from './OversizedFileNotice';
 import { ToolbarHost, type ToolbarHostHandle } from './ToolbarHost';
 import { FileHeader } from './FileHeader';
+import { BinaryFileNotice } from './BinaryFileNotice';
 import { EditSessionHud } from './EditSessionHud';
 import { FileCommentBanner } from './FileCommentBanner';
 import { annotationMatchesPrScope, isFileScopedAnnotation, lineRangeForAnnotation } from '../utils/annotationScope';
@@ -2181,6 +2182,14 @@ export const AllFilesCodeView: React.FC<AllFilesCodeViewProps> = ({
        {!collapsed && isOversizedReviewStubPatch(file.patch) && (
          <OversizedFileNotice onHeightChange={() => refreshItem(item.id)} />
        )}
+        {/* The general fallback under that specific case: any OTHER hunkless
+            binary chunk draws nothing either. Gated on the marker so a
+            marker-carrying stub is explained exactly once, by the line above. */}
+        {!collapsed
+          && !isOversizedReviewStubPatch(file.patch)
+          && isContentlessBinaryPatch(file.patch) && (
+          <BinaryFileNotice onHeightChange={() => refreshItem(item.id)} />
+        )}
        {/* EXPERIMENTAL edit-session HUD: session controls + state in a slim
            strip below the header, above the file content. Appears/disappears
            with session start/end, which both go through a version-bumped
@@ -0,0 +1,31 @@
+import React, { useLayoutEffect } from 'react';
+
+/**
+ * Says why a file's card has no diff in it.
+ *
+ * A patch chunk with a binary marker and no hunks renders as an empty body:
+ * the card is a bare header with no counts and no reason, which reads as a
+ * broken diff. That shape covers genuine binary files and files the review
+ * core declined to read, so the copy commits to neither cause.
+ */
+export const BinaryFileNotice: React.FC<{
+  /** Re-measure hook for the virtualized all-files host, whose custom-header
+   *  slot heights are not auto-observed. */
+  onHeightChange?: () => void;
+}> = ({ onHeightChange }) => {
+  // Before paint, so the host re-measures without a one-frame overlap with the
+  // content below.
+  useLayoutEffect(() => {
+    onHeightChange?.();
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);
+
+  return (
+    <div
+      data-binary-file-notice=""
+      className="px-4 py-2 text-xs leading-relaxed text-muted-foreground border-b border-border bg-muted/30"
+    >
+      Binary or oversized file, content not shown.
+    </div>
+  );
+};
@@ -0,0 +1,161 @@
+/**
+ * A file whose card has no diff in it must SAY so.
+ *
+ * A patch chunk carrying a binary marker and no hunks renders as an empty
+ * body, so the card is a bare header with no counts and no reason. That is
+ * what a file dropped by the review size probe looked like (#1167): reviewers
+ * saw an empty card and could approve without ever seeing the content.
+ *
+ * DOM-gated (DOM_TESTS=1) and registered in .github/workflows/test.yml's
+ * "Run UI seam-contract + DOM tests" step.
+ */
+import { afterEach, describe, expect, mock, test } from 'bun:test';
+import React from 'react';
+import { act } from 'react';
+import { createRoot, type Root } from 'react-dom/client';
+import { OVERSIZED_REVIEW_STUB_MARKER } from '@plannotator/shared/diff-paths';
+
+mock.module('../workerPool', () => ({
+  useIsWorkerPoolReadyOrDisabled: () => true,
+  useWorkerPoolThemeSync: () => {},
+}));
+
+mock.module('../hooks/usePierreTheme', () => ({
+  usePierreTheme: () => ({ type: 'light', css: '' }),
+}));
+
+mock.module('./ToolbarHost', () => ({
+  ToolbarHost: React.forwardRef(function MockToolbarHost() {
+    return null;
+  }),
+}));
+
+const { DiffViewer } = await import('./DiffViewer');
+
+const hasDom = typeof document !== 'undefined';
+
+// The shape the review core emits for a file it declined to read: rename
+// metadata, no hunks. Before the fix a renamed-and-edited file could land here
+// purely because its size probe could not find the worktree blob.
+const STUB_PATCH = [
+  'diff --git a/src/Card.tsx b/src/Panel.tsx',
+  'similarity index 94%',
+  'rename from src/Card.tsx',
+  'rename to src/Panel.tsx',
+  'index bab081fdb737..99fffbd3cac3 100644',
+  'Binary files a/src/Card.tsx and b/src/Panel.tsx differ',
+  '',
+].join('\n');
+
+const REAL_BINARY = [
+  'diff --git a/assets/logo.png b/assets/logo.png',
+  'index 1111111111aa..2222222222bb 100644',
+  'Binary files a/assets/logo.png and b/assets/logo.png differ',
+  '',
+].join('\n');
+
+const TEXT_PATCH = [
+  'diff --git a/calc.ts b/calc.ts',
+  'index 0000000..1111111 100644',
+  '--- a/calc.ts',
+  '+++ b/calc.ts',
+  '@@ -1,3 +1,3 @@',
+  ' const a = 1;',
+  '-const b = 1;',
+  '+const b = 2;',
+  ' const c = 3;',
+  '',
+].join('\n');
+
+// The same shape PLUS the size-cap marker. The specific notice owns this one,
+// so exactly one explanation must appear on the card.
+const MARKED_OVERSIZED_STUB = [
+  'diff --git a/assets/blob.pack b/assets/blob.pack',
+  OVERSIZED_REVIEW_STUB_MARKER,
+  'index 1111111111aa..2222222222bb 100644',
+  'Binary files a/assets/blob.pack and b/assets/blob.pack differ',
+  '',
+].join('\n');
+
+const NOTICE_SELECTOR = '[data-binary-file-notice]';
+const OVERSIZED_NOTICE_SELECTOR = '[data-oversized-file-notice]';
+
+function view(patch: string, filePath: string) {
+  return (
+    <DiffViewer
+      patch={patch}
+      filePath={filePath}
+      diffStyle="unified"
+      annotations={[]}
+      selectedAnnotationId={null}
+      scrollTargetAnnotation={null}
+      pendingSelection={null}
+      onLineSelection={() => {}}
+      onAddAnnotation={() => {}}
+      onAddFileComment={() => {}}
+      onEditAnnotation={() => {}}
+      onSelectAnnotation={() => {}}
+      onDeleteAnnotation={() => {}}
+    />
+  );
+}
+
+describe.if(hasDom)('contentless binary card presentation (DOM)', () => {
+  let root: Root | null = null;
+  let host: HTMLDivElement | null = null;
+  const originalFetch = globalThis.fetch;
+
+  async function render(patch: string, filePath: string) {
+    // There is no expandable content for these shapes; keep the lookup inert.
+    globalThis.fetch = (async () =>
+      new Response(JSON.stringify({ oldContent: null, newContent: null }), {
+        headers: { 'content-type': 'application/json' },
+      })) as typeof fetch;
+
+    host = document.createElement('div');
+    document.body.appendChild(host);
+    root = createRoot(host);
+    await act(async () => {
+      root!.render(view(patch, filePath));
+      await new Promise((resolve) => setTimeout(resolve, 25));
+    });
+    return host;
+  }
+
+  afterEach(async () => {
+    if (root) {
+      await act(async () => root!.unmount());
+      root = null;
+    }
+    host?.remove();
+    host = null;
+    globalThis.fetch = originalFetch;
+  });
+
+  test('a hunkless stub explains its empty body', async () => {
+    const el = await render(STUB_PATCH, 'src/Panel.tsx');
+    const notice = el.querySelector(NOTICE_SELECTOR);
+    expect(notice).not.toBeNull();
+    expect(notice!.textContent).toContain('content not shown');
+  });
+
+  test('a genuine binary file explains its empty body too', async () => {
+    const el = await render(REAL_BINARY, 'assets/logo.png');
+    expect(el.querySelector(NOTICE_SELECTOR)).not.toBeNull();
+  });
+
+  test('an ordinary text diff is left alone', async () => {
+    const el = await render(TEXT_PATCH, 'calc.ts');
+    expect(el.querySelector(NOTICE_SELECTOR)).toBeNull();
+    expect(el.querySelector(OVERSIZED_NOTICE_SELECTOR)).toBeNull();
+  });
+
+  test('a marker-carrying stub is explained once, by the specific notice', async () => {
+    // Specific beats general: the size-cap notice knows WHY the body is empty,
+    // so the fallback must stand down rather than stack a second line on it.
+    const el = await render(MARKED_OVERSIZED_STUB, 'assets/blob.pack');
+    expect(el.querySelectorAll(OVERSIZED_NOTICE_SELECTOR).length).toBe(1);
+    expect(el.querySelectorAll(NOTICE_SELECTOR).length).toBe(0);
+    expect(el.textContent).not.toContain(OVERSIZED_REVIEW_STUB_MARKER);
+  });
+});
@@ -13,9 +13,10 @@ import { ToolbarHost, type ToolbarHostHandle } from './ToolbarHost';
 import { OverlayScrollArea } from '@plannotator/ui/components/OverlayScrollArea';
 import { useOverlayViewport } from '@plannotator/ui/hooks/useOverlayViewport';
 import { FileHeader } from './FileHeader';
+import { BinaryFileNotice } from './BinaryFileNotice';
 import { FileCommentBanner } from './FileCommentBanner';
 import { OversizedFileNotice } from './OversizedFileNotice';
-import { isOversizedReviewStubPatch } from '@plannotator/shared/diff-paths';
+import { isContentlessBinaryPatch, isOversizedReviewStubPatch } from '@plannotator/shared/diff-paths';
 import { isFileScopedAnnotation, lineRangeForAnnotation } from '../utils/annotationScope';
 import { lineAnnotationMetadata } from '../utils/annotationDisplay';
 import type { AnnotationScrollTarget } from '../types';
@@ -686,6 +687,15 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
  // renders as an empty body. Say so instead of showing a bare header.
  const isOversizedStub = useMemo(() => isOversizedReviewStubPatch(patch), [patch]);

+  // The general fallback under that specific case: any OTHER hunkless binary
+  // chunk (a genuine binary file, or a stub shape the marker does not cover)
+  // still renders an empty body and still has to say why. Gated on the marker
+  // so a marker-carrying stub is explained exactly once, by the message above.
+  const isContentlessBinary = useMemo(
+    () => !isOversizedStub && isContentlessBinaryPatch(patch),
+    [patch, isOversizedStub],
+  );
+
  // Replay a selected line/range comment's anchor as the controlled highlight so
  // clicking it (inline card or sidebar) lights up its lines. A live compose
  // selection (pendingSelection) wins while the toolbar is open; file-scoped
@@ -731,7 +741,10 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
        overflowX="scroll"
        onViewportReady={onViewportReady}
      >
+        {/* Specific first, general second, and never both: whichever applies,
+            a card with no hunks to draw says why instead of reading as empty. */}
        {isOversizedStub && <OversizedFileNotice />}
+        {isContentlessBinary && <BinaryFileNotice />}
        <FileCommentBanner
          comments={fileComments}
          selectedAnnotationId={selectedAnnotationId}
@@ -1,5 +1,80 @@
 import { describe, expect, test } from "bun:test";
-import { parseDiffFilePathLines, parsePatchPathToken, unquoteGitPath } from "./diff-paths";
+import {
+  isContentlessBinaryPatch,
+  parseDiffFilePathLines,
+  parsePatchPathToken,
+  unquoteGitPath,
+} from "./diff-paths";
+
+describe("isContentlessBinaryPatch", () => {
+  test("flags a git binary chunk with no hunks", () => {
+    expect(isContentlessBinaryPatch([
+      "diff --git a/logo.png b/logo.png",
+      "index 1111111111aa..2222222222bb 100644",
+      "Binary files a/logo.png and b/logo.png differ",
+      "",
+    ].join("\n"))).toBe(true);
+  });
+
+  test("flags a review stub for a file the server declined to read", () => {
+    expect(isContentlessBinaryPatch([
+      "diff --git a/src/Panel.tsx b/src/Panel.tsx",
+      "similarity index 94%",
+      "rename from src/Card.tsx",
+      "rename to src/Panel.tsx",
+      "index bab081fdb737..99fffbd3cac3 100644",
+      "Binary files a/src/Card.tsx and b/src/Panel.tsx differ",
+      "",
+    ].join("\n"))).toBe(true);
+  });
+
+  test("flags a literal GIT binary patch payload", () => {
+    expect(isContentlessBinaryPatch([
+      "diff --git a/logo.png b/logo.png",
+      "GIT binary patch",
+      "literal 12",
+      "",
+    ].join("\n"))).toBe(true);
+  });
+
+  test("does not flag a text patch", () => {
+    expect(isContentlessBinaryPatch([
+      "diff --git a/calc.ts b/calc.ts",
+      "--- a/calc.ts",
+      "+++ b/calc.ts",
+      "@@ -1,2 +1,2 @@",
+      "-const b = 1;",
+      "+const b = 2;",
+      "",
+    ].join("\n"))).toBe(false);
+  });
+
+  test("does not flag a text patch whose content mentions the binary marker", () => {
+    // Content lines always carry a +/-/space prefix, and the scan stops at the
+    // first hunk header, so quoted marker text cannot be mistaken for a header.
+    expect(isContentlessBinaryPatch([
+      "diff --git a/notes.md b/notes.md",
+      "--- a/notes.md",
+      "+++ b/notes.md",
+      "@@ -1,2 +1,2 @@",
+      "-old note",
+      "+Binary files a/x and b/x differ",
+      " GIT binary patch",
+      "",
+    ].join("\n"))).toBe(false);
+  });
+
+  test("does not flag a metadata-only chunk with no binary marker", () => {
+    // A pure mode change has no body either, but git says nothing about
+    // content there, so it keeps its existing rendering.
+    expect(isContentlessBinaryPatch([
+      "diff --git a/run.sh b/run.sh",
+      "old mode 100644",
+      "new mode 100755",
+      "",
+    ].join("\n"))).toBe(false);
+  });
+});

 describe("diff path parsing", () => {
  test("unquoteGitPath decodes octal (UTF-8 byte) escapes", () => {
@@ -210,6 +210,35 @@ export function isOversizedReviewStubPatch(patch: string): boolean {
  return patch.split("\n").some((line) => line === OVERSIZED_REVIEW_STUB_MARKER);
 }

+/**
+ * True when a single file's patch chunk carries a binary marker and no hunks,
+ * so a diff renderer has literally nothing to draw for it.
+ *
+ * The GENERAL case, of which `isOversizedReviewStubPatch` above is the one
+ * specific case we can name: git emits this shape for real binary files, and
+ * the review core emits it for files it declined to read. Either way the card
+ * renders as a bare header with no counts and no body, which reads as a broken
+ * or empty diff rather than as content that was deliberately not shown.
+ *
+ * Callers that can say something more specific should ask the marker predicate
+ * FIRST and fall back to this one, so a marker-carrying stub is explained once,
+ * by the message that knows why.
+ *
+ * Scanning stops at the first hunk header: content lines always carry a `+`,
+ * `-`, or space prefix, so a `Binary files ` line at column zero before any
+ * `@@ ` can only be the extended header git (or the stub builder) wrote.
+ */
+export function isContentlessBinaryPatch(patch: string): boolean {
+  let hasBinaryMarker = false;
+  for (const line of patch.split("\n")) {
+    if (line.startsWith("@@ ")) return false;
+    if (line.startsWith("Binary files ") || line === "GIT binary patch") {
+      hasBinaryMarker = true;
+    }
+  }
+  return hasBinaryMarker;
+}
+
 export function parseDiffMetadataPathLines(lines: string[]): DiffPathPair {
  let oldPath: string | undefined;
  let newPath: string | undefined;
@@ -845,11 +845,22 @@ describe("review-core", () => {
    }
  });

-  test("a single object the probe reports missing still excludes only that path", async () => {
+  test("a probed-oversized object excludes only its path, and a missing one none", async () => {
+    // Per-object evidence stays per-object: one oversized blob costs one path,
+    // never the whole review. An object the probe cannot SIZE is a different
+    // answer from one it sizes above the cap: `missing` is unknown, so that
+    // path keeps rendering under git's own core.bigFileThreshold bound.
    const smallOld = "1".repeat(40);
    const smallNew = "2".repeat(40);
-    const brokenOld = "3".repeat(40);
-    const brokenNew = "4".repeat(40);
+    const missingOld = "3".repeat(40);
+    const missingNew = "4".repeat(40);
+    const hugeOld = "5".repeat(40);
+    const hugeNew = "6".repeat(40);
+    const renderedPatch = [
+      "diff --git a/small.ts b/small.ts\n-old\n+new\n",
+      "diff --git a/unfetched.ts b/unfetched.ts\n-gone\n+restored\n",
+    ].join("");
+    const excludedPaths: string[] = [];
    const runtime: ReviewGitRuntime = {
      ...unavailableFileMethods,
      async runGit(args, options) {
@@ -857,7 +868,8 @@ describe("review-core", () => {
          return {
            stdout: [
              `:100644 100644 ${smallOld} ${smallNew} M\0small.ts\0`,
-              `:100644 100644 ${brokenOld} ${brokenNew} M\0broken.bin\0`,
+              `:100644 100644 ${missingOld} ${missingNew} M\0unfetched.ts\0`,
+              `:100644 100644 ${hugeOld} ${hugeNew} M\0huge.bin\0`,
            ].join(""),
            stderr: "",
            exitCode: 0,
@@ -866,16 +878,23 @@ describe("review-core", () => {
        if (args[0] === "cat-file" && args.some((arg) => arg.startsWith("--batch-check"))) {
          const input = (options as { stdin?: string } | undefined)?.stdin ?? "";
          return {
-            stdout: input.trim().split("\n").filter(Boolean).map((objectId) =>
-              objectId === brokenNew ? `${objectId} missing` : `${objectId} blob 10`,
-            ).join("\n"),
+            stdout: input.trim().split("\n").filter(Boolean).map((objectId) => {
+              if (objectId === missingNew) return `${objectId} missing`;
+              if (objectId === hugeNew) {
+                return `${objectId} blob ${MAX_REVIEW_FILE_CONTENT_BYTES + 1}`;
+              }
+              return `${objectId} blob 10`;
+            }).join("\n"),
            stderr: "",
            exitCode: 0,
          };
        }
        if (args[0] === "rev-parse") return { stdout: "/repo\n", stderr: "", exitCode: 0 };
        if (args[0] === "diff") {
-          return { stdout: "diff --git a/small.ts b/small.ts\n-old\n+new\n", stderr: "", exitCode: 0 };
+          excludedPaths.push(
+            ...args.filter((arg) => arg.startsWith(":(top,exclude,literal)")),
+          );
+          return { stdout: renderedPatch, stderr: "", exitCode: 0 };
        }
        throw new Error(`Unexpected git command: ${args.join(" ")}`);
      },
@@ -887,7 +906,12 @@ describe("review-core", () => {
    const result = await runGitDiff(runtime, "staged", "main", "/repo");

    expect(result.patch).toContain("+new");
-    expect(result.patch).toContain("Binary files a/broken.bin and b/broken.bin differ");
+    expect(result.patch).toContain("Binary files a/huge.bin and b/huge.bin differ");
+    expect(excludedPaths).toEqual([":(top,exclude,literal)huge.bin"]);
+    // The unfetchable object's path is not stubbed away: git renders it, and
+    // a git that truly cannot read it fails loudly instead of blanking it.
+    expect(result.patch).toContain("+restored");
+    expect(result.patch).not.toContain("Binary files a/unfetched.ts");
    expect(result.patch).not.toContain("Binary files a/small.ts");
  });

@@ -966,6 +990,90 @@ describe("review-core", () => {
    expect(second).not.toBe(first);
  }, 20_000);

+  test("renders a renamed file whose edited worktree blob the probe cannot find", async () => {
+    // Rename/copy detection makes git hash the WORKING-TREE content and print
+    // that hash in --raw output, but the blob is never written to the object
+    // database. The size probe answers `missing` for it. Treating that as
+    // "oversized" dropped the whole renamed file out of the review (#1167).
+    const repoDir = initRepo();
+    mkdirSync(join(repoDir, "src"));
+    const original = Array.from({ length: 40 }, (_, index) => `line ${index + 1}`).join("\n");
+    writeFileSync(join(repoDir, "src/Card.tsx"), `${original}\n`, "utf-8");
+    git(repoDir, ["add", "-A"]);
+    git(repoDir, ["commit", "-m", "add card"]);
+    const base = git(repoDir, ["rev-parse", "HEAD"]);
+    git(repoDir, ["mv", "src/Card.tsx", "src/Panel.tsx"]);
+    git(repoDir, ["commit", "-m", "rename card to panel"]);
+    writeFileSync(
+      join(repoDir, "src/Panel.tsx"),
+      `${original}\nline 41 unstaged\n`,
+      "utf-8",
+    );
+
+    const runtime = makeConfigForwardingRuntime(repoDir);
+    const patch = await getWorkingTreeDiffFromBase(runtime, base);
+
+    expect(patch).toContain("+line 41 unstaged");
+    expect(patch).not.toContain("Binary files");
+  }, 20_000);
+
+  test("renders a tracked add whose worktree blob the probe cannot find", async () => {
+    // A delete/add pair also feeds rename detection, so the added path carries
+    // a real-but-unwritten worktree hash in --raw output.
+    const repoDir = initRepo();
+    const base = git(repoDir, ["rev-parse", "HEAD"]);
+    writeFileSync(join(repoDir, "replacement.txt"), "fresh content\n", "utf-8");
+    git(repoDir, ["rm", "-q", "tracked.txt"]);
+    git(repoDir, ["add", "-A"]);
+    git(repoDir, ["commit", "-m", "replace tracked file"]);
+    writeFileSync(join(repoDir, "replacement.txt"), "fresh content\nedited later\n", "utf-8");
+
+    const runtime = makeConfigForwardingRuntime(repoDir);
+    const patch = await getWorkingTreeDiffFromBase(runtime, base);
+
+    expect(patch).toContain("+fresh content");
+    expect(patch).toContain("+edited later");
+    expect(patch).not.toContain("Binary files");
+  }, 20_000);
+
+  test("renders a file whose index blob is missing from the object database", async () => {
+    // Partial clones (and pruned object databases) can report `missing` for an
+    // index blob git can still diff perfectly well from the working tree.
+    const repoDir = initRepo();
+    writeFileSync(join(repoDir, "added.txt"), "content from the index\n", "utf-8");
+    git(repoDir, ["add", "added.txt"]);
+    const blobId = git(repoDir, ["rev-parse", ":added.txt"]);
+    rmSync(join(repoDir, ".git", "objects", blobId.slice(0, 2), blobId.slice(2)), { force: true });
+
+    const runtime = makeConfigForwardingRuntime(repoDir);
+    const result = await runGitDiff(runtime, "uncommitted", "main");
+
+    expect(result.patch).toContain("+content from the index");
+    expect(result.patch).not.toContain("Binary files");
+  }, 20_000);
+
+  test("still stubs an oversized worktree file whose blob the probe cannot find", async () => {
+    // The size probe cannot bound this file (its hash is missing) and
+    // core.bigFileThreshold does not bound working-tree sides, so the
+    // filesystem stat has to be the authoritative bound.
+    const repoDir = initRepo();
+    const largeSize = MAX_REVIEW_FILE_CONTENT_BYTES + 1;
+    writeFileSync(join(repoDir, "old-big.txt"), "a".repeat(largeSize), "utf-8");
+    git(repoDir, ["add", "-A"]);
+    git(repoDir, ["commit", "-m", "add big file"]);
+    const base = git(repoDir, ["rev-parse", "HEAD"]);
+    git(repoDir, ["mv", "old-big.txt", "new-big.txt"]);
+    git(repoDir, ["commit", "-m", "rename big file"]);
+    writeFileSync(join(repoDir, "new-big.txt"), "b".repeat(largeSize), "utf-8");
+
+    const runtime = makeConfigForwardingRuntime(repoDir);
+    const patch = await getWorkingTreeDiffFromBase(runtime, base);
+
+    expect(patch).toContain("Binary files");
+    expect(patch).not.toContain("bbbbbbbbbb");
+    expect(patch.length).toBeLessThan(4_000);
+  }, 40_000);
+
  test("synthesizes quoted rename and copy metadata from raw status details", async () => {
    const renamedFrom = 'old "rename" path';
    const renamedTo = "new \\ rename path";
@@ -832,19 +832,25 @@ function isGitlink(entry: RawDiffEntry): boolean {
 * oversized replaced the whole review with binary stubs on one failed probe.
 * Working-tree sizes come from filesystem stat and never from this probe.
 *
- * The per-object doors stay conservative on a probe that ran: an object the
- * batch reports as `missing`, omits from its output, or answers with an
- * unparseable/negative size maps to infinity. That is evidence about one
- * object only, and excluding just its path keeps the stub's rename/copy/mode
- * metadata rendering while the rest of the diff stays intact.
+ * A probe that ran answers per object with a size or with `null`, which means
+ * "unknown", never "oversized": an object the batch reports as `missing`,
+ * omits from its output, or answers with an unparseable/negative size has no
+ * usable size. `missing` is routine for tree-vs-worktree diffs — every path
+ * pulled into rename/copy detection gets its WORKING-TREE content hashed by
+ * git and that hash printed in `--raw` output without ever being written to
+ * the object database — and it also happens in partial clones. Mapping those
+ * to infinity excluded files git can plainly diff, so a renamed-and-edited
+ * file rendered as an empty binary stub (#1167). Callers bound an unknown new
+ * side by filesystem stat, and every ODB blob stays bounded by
+ * `core.bigFileThreshold` (`BOUNDED_DIFF_GIT_CONFIG`).
 */
 async function getGitObjectSizes(
  runtime: ReviewGitRuntime,
  objectIds: string[],
  cwd?: string,
-): Promise<Map<string, number> | null> {
+): Promise<Map<string, number | null> | null> {
  const uniqueObjectIds = [...new Set(objectIds.filter((objectId) => !isNullObjectId(objectId)))];
-  const sizes = new Map<string, number>();
+  const sizes = new Map<string, number | null>();
  if (uniqueObjectIds.length === 0) return sizes;

  const result = await runtime.runGit(
@@ -862,13 +868,10 @@ async function getGitObjectSizes(
    const [objectId, objectType, objectSize] = line.split(" ");
    if (!objectId || objectType === "missing") continue;
    const size = Number(objectSize);
-    sizes.set(
-      objectId,
-      Number.isFinite(size) && size >= 0 ? size : Number.POSITIVE_INFINITY,
-    );
+    sizes.set(objectId, Number.isFinite(size) && size >= 0 ? size : null);
  }
  for (const objectId of uniqueObjectIds) {
-    if (!sizes.has(objectId)) sizes.set(objectId, Number.POSITIVE_INFINITY);
+    if (!sizes.has(objectId)) sizes.set(objectId, null);
  }
  return sizes;
 }
@@ -1029,19 +1032,28 @@ async function buildBoundedTrackedDiff(
  //    the file for the index line and content-based binary detection wins),
  //    but their sizes come from filesystem stat, not the probe, so that
  //    exclusion door keeps working below regardless of the probe outcome.
+  //  - An id the probe answered for but could not size (`missing`, omitted,
+  //    unparseable) is unknown, not oversized. git prints such an id for every
+  //    worktree path that rename/copy detection hashed, and for blobs a
+  //    partial clone has not fetched; a file git can plainly diff must never
+  //    be replaced by a content-free binary stub (#1167).
+  const probedSize = (objectId: string): number | null =>
+    isNullObjectId(objectId) || objectSizes === null
+      ? null
+      : objectSizes.get(objectId) ?? null;
  for (const entry of entries) {
    if (isGitlink(entry)) continue;
-    const oldSize = isNullObjectId(entry.oldObjectId)
-      ? null
-      : objectSizes === null
-        ? null
-        : objectSizes.get(entry.oldObjectId) ?? Number.POSITIVE_INFINITY;
-    const newObjectSize = isNullObjectId(entry.newObjectId)
-      ? null
-      : objectSizes === null
-        ? null
-        : objectSizes.get(entry.newObjectId) ?? Number.POSITIVE_INFINITY;
-    const workingTreeInfo = isNullObjectId(entry.newObjectId)
+    const oldSize = probedSize(entry.oldObjectId);
+    const newObjectSize = probedSize(entry.newObjectId);
+    // The working-tree file is what git formats whenever the new side has no
+    // readable object behind it: an unhashed worktree side (all-zero id), or
+    // an id the probe that RAN could not find. Either way its stat size is the
+    // authoritative bound. A probe that FAILED outright says nothing about any
+    // single object, so those ids keep relying on core.bigFileThreshold rather
+    // than on a working-tree file that may not be the diff's new side at all.
+    const newSideUnreadable = isNullObjectId(entry.newObjectId)
+      || (objectSizes !== null && newObjectSize === null);
+    const workingTreeInfo = newSideUnreadable
      ? await getWorkingTreeFileInfo(runtime, root, entry.newPath)
      : null;
    const newSize = newObjectSize ?? workingTreeInfo?.size ?? null;
@@ -1059,14 +1071,18 @@ async function buildBoundedTrackedDiff(
        `large:${entry.newPath}:${workingTreeInfo.size}:${workingTreeInfo.mtimeMs}`,
      );
    }
-    const workingObjectId = !fingerprintMode && workingTreeInfo && entry.newPath
+    // Only an all-zero new side needs a synthesized content-sensitive id. When
+    // git printed a real id it already hashed this exact working-tree content,
+    // so the stub keeps it instead of re-hashing an oversized file.
+    const workingObjectId = !fingerprintMode
+      && isNullObjectId(entry.newObjectId)
+      && workingTreeInfo
+      && entry.newPath
      ? await hashOversizedWorkingTreeFile(runtime, entry.newPath, workingTreeInfo, cwd)
      : null;
    oversized.push({
      ...entry,
-      newObjectId: newObjectSize === null && workingObjectId
-        ? workingObjectId
-        : entry.newObjectId,
+      newObjectId: workingObjectId ?? entry.newObjectId,
    });
  }