diff --git a/src/review/review-grounding.ts b/src/review/review-grounding.ts index 378efaafec..a9c3922af7 100644 --- a/src/review/review-grounding.ts +++ b/src/review/review-grounding.ts @@ -135,8 +135,11 @@ export async function fetchFullFileContents( ): Promise { if (!flags.fullFileContext || !ref) return undefined; // Source-first ordering (the diff's own priority) so the most-relevant files are inlined before the budget runs out. + // A newly ADDED file is excluded here (#3897): every line of it is already a `+` line in the diff itself + // (sent in the same prompt), so fetching + inlining its full content again is a byte-for-byte duplicate -- + // wasted GitHub API round-trip and wasted prompt budget that a genuinely modified file needs more. const candidates = files - .filter((file) => file.status !== "removed" && !SKIP_EXT.test(file.filename)) + .filter((file) => file.status !== "removed" && file.status !== "added" && !SKIP_EXT.test(file.filename)) .sort((a, b) => diffFilePriority(a.filename) - diffFilePriority(b.filename)); const out: ChangedFileContent[] = []; let used = 0; diff --git a/test/unit/review-grounding.test.ts b/test/unit/review-grounding.test.ts index 14b7ec8d8d..f4159ee337 100644 --- a/test/unit/review-grounding.test.ts +++ b/test/unit/review-grounding.test.ts @@ -174,13 +174,17 @@ describe("review-grounding: fetchFullFileContents (injected FileFetcher, fail-sa ["assets/icon.heic"], ["dist/pkg.tgz"], ["old.ts", "removed"], + ["src/new.ts", "added"], ), fetcher, ); expect(out).toBeDefined(); - // source (priority 0) before docs (priority 2); binary + removed excluded before fetch + // source (priority 0) before docs (priority 2); binary + removed + added excluded before fetch expect(out?.map((f) => f.path)).toEqual(["src/a.ts", "README.md"]); expect(reads).toEqual(["src/a.ts", "README.md"]); + // #3897: an ADDED file's entire body is already every `+` line of the diff -- fetching it again + // would duplicate that content in the prompt, so it's excluded the same way "removed" is. + expect(reads).not.toContain("src/new.ts"); for (const path of binary) expect(reads).not.toContain(path); });