diff --git a/src/review/review-grounding.ts b/src/review/review-grounding.ts index bb2a707ad6..613914bb89 100644 --- a/src/review/review-grounding.ts +++ b/src/review/review-grounding.ts @@ -172,7 +172,21 @@ export function diffFullyCoversFile(file: PullRequestFile): boolean { const newStart = Number.parseInt(match[3]!, 10); const newCount = match[4] !== undefined ? Number.parseInt(match[4], 10) : 1; if (oldStart > 1 || newStart > 1) return false; // leading unchanged lines exist before the hunk - return oldCount - file.deletions < DIFF_CONTEXT_LINES && newCount - file.additions < DIFF_CONTEXT_LINES; + + const observed = countObservedHunkChanges(file.patch); + if (observed.additions !== file.additions || observed.deletions !== file.deletions) return false; + + return oldCount - observed.deletions < DIFF_CONTEXT_LINES && newCount - observed.additions < DIFF_CONTEXT_LINES; +} + +function countObservedHunkChanges(patch: string): { additions: number; deletions: number } { + let additions = 0; + let deletions = 0; + for (const line of patch.split("\n").slice(1)) { + if (line.startsWith("+")) additions += 1; + else if (line.startsWith("-")) deletions += 1; + } + return { additions, deletions }; } /** Centrally fetch the FULL post-change content of changed files (the one grounding input no lane fetches diff --git a/test/unit/review-grounding.test.ts b/test/unit/review-grounding.test.ts index 9751f51cc9..4809baac3f 100644 --- a/test/unit/review-grounding.test.ts +++ b/test/unit/review-grounding.test.ts @@ -296,6 +296,52 @@ describe("review-grounding: fetchFullFileContents (injected FileFetcher, fail-sa ).toBe(true); }); + it("returns false when file-level totals include changes omitted from a truncated patch", () => { + expect( + diffFullyCoversFile({ + filename: "src/truncated.ts", + status: "modified", + patch: "@@ -1,5 +1,5 @@\n-old1\n-old2\n+new1\n+new2\n line3\n line4\n line5", + additions: 50, + deletions: 50, + }), + ).toBe(false); + }); + + it("fetches a modified file when the visible patch omits later changed hunks", async () => { + const reads: string[] = []; + const fetcher: FileFetcher = { + getFileContent: async (path) => { + reads.push(path); + return "export const hiddenTail = true;"; + }, + }; + const truncated: PullRequestFile = { + filename: "src/truncated.ts", + status: "modified", + patch: "@@ -1,5 +1,5 @@\n-old1\n-old2\n+new1\n+new2\n line3\n line4\n line5", + additions: 50, + deletions: 50, + }; + + const out = await fetchFullFileContents({ ciGrounding: false, fullFileContext: true }, "sha", [truncated], fetcher); + + expect(reads).toEqual(["src/truncated.ts"]); + expect(out).toEqual([{ path: "src/truncated.ts", text: "export const hiddenTail = true;" }]); + }); + + it("returns false when trailing context is ambiguous on the post-change side", () => { + expect( + diffFullyCoversFile({ + filename: "src/ambiguous-tail.ts", + status: "modified", + patch: "@@ -1,4 +1,5 @@\n-old\n+new1\n+new2\n line2\n line3\n line4", + additions: 2, + deletions: 1, + }), + ).toBe(false); + }); + it("returns false when the hunk does not start at line 1 on either side (leading unchanged lines exist)", () => { expect( diffFullyCoversFile({