Skip to content

Commit 0e1ea9e

Browse files
committed
fix(review): use merge-base SHA for PR file contents
gh pr diff computes diffs against the merge-base (common ancestor), but file contents were fetched at baseSha (base branch tip). When the base branch has moved since the branch point, line counts don't match the diff hunks, causing iterateOverDiff trailing context mismatch crashes. Fetch the merge-base SHA via GitHub's compare API and use it for old file contents. Added try/catch around processFile as a safety net. Fixed in both Bun and Pi servers. For provenance purposes, this commit was AI assisted.
1 parent 566df72 commit 0e1ea9e

5 files changed

Lines changed: 35 additions & 8 deletions

File tree

apps/pi-extension/server/serverReview.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,8 +531,9 @@ export async function startReviewServer(options: {
531531

532532
if (isPRMode && prRef && prMeta) {
533533
try {
534+
const oldSha = prMeta.mergeBaseSha ?? prMeta.baseSha;
534535
const [oldContent, newContent] = await Promise.all([
535-
fetchPRFileContent(prRef, prMeta.baseSha, oldPath || filePath),
536+
fetchPRFileContent(prRef, oldSha, oldPath || filePath),
536537
fetchPRFileContent(prRef, prMeta.headSha, filePath),
537538
]);
538539
json(res, { oldContent, newContent });

packages/review-editor/components/DiffViewer.tsx

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -279,11 +279,16 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
279279
// against the complete file (isPartial: false), enabling expansion.
280280
const augmentedDiff = useMemo(() => {
281281
if (!fileContents || fileContents.forPath !== filePath || (fileContents.old == null && fileContents.new == null)) return fileDiff;
282-
const result = processFile(patch, {
283-
oldFile: fileContents.old != null ? { name: oldPath || filePath, contents: fileContents.old } : undefined,
284-
newFile: fileContents.new != null ? { name: filePath, contents: fileContents.new } : undefined,
285-
});
286-
return result || fileDiff;
282+
try {
283+
const result = processFile(patch, {
284+
oldFile: fileContents.old != null ? { name: oldPath || filePath, contents: fileContents.old } : undefined,
285+
newFile: fileContents.new != null ? { name: filePath, contents: fileContents.new } : undefined,
286+
});
287+
return result || fileDiff;
288+
} catch {
289+
// Fall back to partial diff if file contents don't match hunks
290+
return fileDiff;
291+
}
287292
}, [patch, filePath, oldPath, fileContents, fileDiff]);
288293

289294
const previousScrollFilePathRef = useRef(filePath);

packages/server/review.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -462,10 +462,13 @@ export async function startReviewServer(
462462
return Response.json(result);
463463
}
464464

465-
// PR mode: fetch from platform API using base/head SHAs
465+
// PR mode: fetch from platform API using merge-base/head SHAs.
466+
// The diff is computed against the merge-base (common ancestor), not the
467+
// base branch tip. File contents must match the diff for hunk expansion.
466468
if (isPRMode) {
469+
const oldSha = prMetadata.mergeBaseSha ?? prMetadata.baseSha;
467470
const [oldContent, newContent] = await Promise.all([
468-
fetchPRFileContent(prRef!, prMetadata.baseSha, oldPath || filePath),
471+
fetchPRFileContent(prRef!, oldSha, oldPath || filePath),
469472
fetchPRFileContent(prRef!, prMetadata.headSha, filePath),
470473
]);
471474
return Response.json({ oldContent, newContent });

packages/shared/pr-github.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,21 @@ export async function fetchGhPR(
101101
url: string;
102102
};
103103

104+
// Fetch the merge-base SHA — the common ancestor commit GitHub uses to compute the PR diff.
105+
// baseSha (baseRefOid) is the tip of the base branch, which may have moved since the branch point.
106+
// File contents must be fetched at the merge-base to match the diff hunks.
107+
let mergeBaseSha: string | undefined;
108+
try {
109+
const compareResult = await runtime.runCommand("gh", hostnameArgs(ref.host, [
110+
"api",
111+
`repos/${ref.owner}/${ref.repo}/compare/${raw.baseRefOid}...${raw.headRefOid}`,
112+
"--jq", ".merge_base_commit.sha",
113+
]));
114+
if (compareResult.exitCode === 0 && compareResult.stdout.trim()) {
115+
mergeBaseSha = compareResult.stdout.trim();
116+
}
117+
} catch { /* fallback to baseSha if compare API fails */ }
118+
104119
const metadata: PRMetadata = {
105120
platform: "github",
106121
host: ref.host,
@@ -114,6 +129,7 @@ export async function fetchGhPR(
114129
headBranch: raw.headRefName,
115130
baseSha: raw.baseRefOid,
116131
headSha: raw.headRefOid,
132+
mergeBaseSha,
117133
url: raw.url,
118134
};
119135

packages/shared/pr-provider.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ export interface GithubPRMetadata {
7070
headBranch: string;
7171
baseSha: string;
7272
headSha: string;
73+
/** Merge-base SHA — the common ancestor commit used to compute the PR diff. Differs from baseSha when the base branch has moved. */
74+
mergeBaseSha?: string;
7375
url: string;
7476
}
7577

0 commit comments

Comments
 (0)