From 0347dab39c064da9533b4f61ac2b6855e4ff97e7 Mon Sep 17 00:00:00 2001
From: shin-core <153108882+shin-core@users.noreply.github.com>
Date: Sun, 26 Jul 2026 09:14:37 +0900
Subject: [PATCH] fix(review): keep a fetched file distinguishable from an
 omitted one under thin budget

review-grounding.ts guarantees a file whose content is successfully fetched is
never rendered as the empty '(no content available)' placeholder a never-fetched
file uses. fetchFullFileContents broke that: when the remaining share fell below
MIN_SAMPLE_CHARS, sampleHeadAndTail returned '' and the fetched file was pushed as
{ text: '', truncated: true } -- identical to an unreadable file, so the AI
reviewer couldn't tell 'fetched but budget exhausted' from 'never fetched'. Sample
at the MIN_SAMPLE_CHARS floor for a genuinely-fetched file so it always carries a
minimal, distinguishing head+tail (a small bounded overrun only on the last thin
file). A never-fetched file (text == null) still skips/omits, unchanged. Updates
the existing 1MB-fixture test to assert the corrected, distinguishable output.
---
 src/review/review-grounding.ts     | 13 ++++++-------
 test/unit/review-grounding.test.ts | 17 ++++++++++++-----
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/review/review-grounding.ts b/src/review/review-grounding.ts
index 576f76d847..84b5e11b0f 100644
--- a/src/review/review-grounding.ts
+++ b/src/review/review-grounding.ts
@@ -307,13 +307,12 @@ export async function fetchFullFileContents(
       used += text.length;
       continue;
     }
-    const sampled = sampleHeadAndTail(text, share);
-    if (!sampled) {
-      // The remaining share was too thin for even a head+tail sample to carry signal — same as unreadable.
-      out.push({ path: file.filename, text: "", truncated: true });
-      used = FILE_CONTENT_BUDGET;
-      continue;
-    }
+    // This file WAS genuinely fetched, so it must never render as the empty "(no content available)"
+    // placeholder a never-fetched file uses -- that breaks the module's own "never omitted again"
+    // guarantee (#8646). When the remaining share is thinner than MIN_SAMPLE_CHARS, sample at that floor so
+    // a fetched file always yields at least a minimal, distinguishing head+tail (a small, bounded overrun of
+    // the overall budget on the very last thin file, not a per-file unbounded cost).
+    const sampled = sampleHeadAndTail(text, Math.max(share, MIN_SAMPLE_CHARS));
     out.push({ path: file.filename, text: sampled, truncated: true });
     used += sampled.length;
   }
diff --git a/test/unit/review-grounding.test.ts b/test/unit/review-grounding.test.ts
index 4b47a6e35f..83f545547b 100644
--- a/test/unit/review-grounding.test.ts
+++ b/test/unit/review-grounding.test.ts
@@ -545,10 +545,12 @@ describe("review-grounding: fetchFullFileContents (injected FileFetcher, fail-sa
     expect(after).toEqual({ path: "src/after.ts", text: "ok" });
   });
 
-  it("falls all the way back to full omission when the remaining share is too thin for even a sample", async () => {
-    // Two fillers each just under MAX_SINGLE_FILE leave only 200 chars of the 96k budget for the third file
-    // -- below MIN_SAMPLE_CHARS, so sampleHeadAndTail itself declines rather than rendering a garbled sliver,
-    // and fetchFullFileContents degrades that to the same full-omission shape as an unreadable file.
+  it("still yields a minimal distinguishing sample for a genuinely-fetched file under extreme budget pressure (#8646)", async () => {
+    // Two fillers each just under MAX_SINGLE_FILE leave only ~200 chars of the 96k budget for the third file --
+    // below MIN_SAMPLE_CHARS. Previously fetchFullFileContents degraded that fetched file to the same empty
+    // { text: "", truncated: true } shape a NEVER-fetched file uses, breaking the module's "never rendered as
+    // omitted again" guarantee. It must now sample at the MIN_SAMPLE_CHARS floor so a fetched file always
+    // carries at least some distinguishing real content.
     const filler = "f".repeat(MAX_SINGLE_FILE - 100);
     const map: Record<string, string> = { "src/a.ts": filler, "src/b.ts": filler, "src/huge.ts": "z".repeat(1_000_000) };
     const fetcher: FileFetcher = { getFileContent: async (path) => map[path] ?? null };
@@ -559,7 +561,12 @@ describe("review-grounding: fetchFullFileContents (injected FileFetcher, fail-sa
       fetcher,
     );
     const huge = out?.find((f) => f.path === "src/huge.ts");
-    expect(huge).toEqual({ path: "src/huge.ts", text: "", truncated: true });
+    // The fetched file is truncated but NOT empty -- it carries real sampled bytes + the omission marker, so
+    // its rendered output is distinguishable from a never-fetched file's empty placeholder.
+    expect(huge?.truncated).toBe(true);
+    expect(huge?.text.length).toBeGreaterThan(0);
+    expect(huge?.text).toContain("omitted from the middle of this file");
+    expect(huge?.text).toContain("z"); // genuine content from the fetched file, not just the marker
   });
 
   it("returns undefined when nothing readable was inlined", async () => {