Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/review/review-grounding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 46 additions & 0 deletions test/unit/review-grounding.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down