fix(web): keep review comments inside the pull request's hunks

apps/web/src/reviewCommentContext.test.ts

@@ -1,3 +1,4 @@
+import { hydratePartialDiff } from "@pierre/diffs";
 import { parsePatchFiles } from "@pierre/diffs/utils/parsePatchFiles";
 import { describe, expect, it } from "vite-plus/test";
 
@@ -6,6 +7,7 @@ import {
   buildFileReviewComment,
   formatReviewCommentFence,
   inferReviewCommentFenceLanguage,
+  resolveDiffReviewPosition,
   restoreDiffReviewCommentRange,
 } from "./reviewCommentContext";
 
@@ -112,4 +114,78 @@ describe("review comment context parsing", () => {
       endSide: "additions",
     });
   });
+
+  it("anchors review comments only on lines inside the host's hunks", () => {
+    const oldLines = Array.from({ length: 30 }, (_, index) => `line ${index + 1}\n`);
+    const newLines = [
+      ...oldLines.slice(0, 6),
+      ...Array.from({ length: 10 }, (_, index) => `new ${index + 1}\n`),
+      ...oldLines.slice(6),
+    ].map((line) => (line === "line 25\n" ? "line twenty-five\n" : line));
+    // What the host returns: new lines 4 to 19 and 32 to 38. The insertion shifts the new side,
+    // so new line 25 (old line 15) sits between the hunks while old line 25 is inside one.
+    const fileDiff = parsePatchFiles(
+      [
+        "diff --git a/src/app.ts b/src/app.ts",
+        "--- a/src/app.ts",
+        "+++ b/src/app.ts",
+        "@@ -4,6 +4,16 @@",
+        " line 4",
+        " line 5",
+        " line 6",
+        ...Array.from({ length: 10 }, (_, index) => `+new ${index + 1}`),
+        " line 7",
+        " line 8",
+        " line 9",
+        "@@ -22,7 +32,7 @@",
+        " line 22",
+        " line 23",
+        " line 24",
+        "-line 25",
+        "+line twenty-five",
+        " line 26",
+        " line 27",
+        " line 28",
+      ].join("\n"),
+      "review-position-hunks",
+    )[0]!.files[0]!;
+    // The Code tab tries the original partial patch before the hydrated viewer file.
+    expect(resolveDiffReviewPosition(fileDiff, 25, "additions")).toBeNull();
+    expect(resolveDiffReviewPosition(fileDiff, 23, "additions")).toBeNull();
+    expect(resolveDiffReviewPosition(fileDiff, 15, "deletions")).toBeNull();
+    // Expanding the file loads both sides whole, so the gaps around the hunks become lines
+    // the viewer can select but the host will not anchor a comment to.
+    hydratePartialDiff("merge", fileDiff, {
+      oldFile: { name: "src/app.ts", contents: oldLines.join("") },
+      newFile: { name: "src/app.ts", contents: newLines.join("") },
+    });
+    expect(fileDiff.isPartial).toBe(false);
+
+    expect(resolveDiffReviewPosition(fileDiff, 1, "additions")).toBeNull();
+    expect(resolveDiffReviewPosition(fileDiff, 25, "additions")).toBeNull();
+    expect(resolveDiffReviewPosition(fileDiff, 39, "additions")).toBeNull();
+    expect(resolveDiffReviewPosition(fileDiff, 1, "deletions")).toBeNull();
+    expect(resolveDiffReviewPosition(fileDiff, 15, "deletions")).toBeNull();
+    expect(resolveDiffReviewPosition(fileDiff, 29, "deletions")).toBeNull();
+    expect(resolveDiffReviewPosition(fileDiff, 33, "additions")).toEqual({
+      kind: "context",
+      oldLine: 23,
+      newLine: 33,
+      side: "right",
+    });
+    expect(resolveDiffReviewPosition(fileDiff, 23, "deletions")).toEqual({
+      kind: "context",
+      oldLine: 23,
+      newLine: 33,
+      side: "left",
+    });
+    expect(resolveDiffReviewPosition(fileDiff, 7, "additions")).toEqual({
+      kind: "added",
+      newLine: 7,
+    });
+    expect(resolveDiffReviewPosition(fileDiff, 25, "deletions")).toEqual({
+      kind: "deleted",
+      oldLine: 25,
+    });
+  });
 });

apps/web/src/reviewCommentContext.ts

@@ -2,6 +2,8 @@ import type { FileDiffMetadata, SelectedLineRange, SelectionSide } from "@pierre
 import { PullRequestContextMetadata, type PullRequestReviewPosition } from "@t3tools/contracts";
 import * as Schema from "effect/Schema";
 
+import { isLineInFileDiff } from "./components/pullRequest/pullRequestDiff.logic";
+
 const ReviewCommentSelectionSchema = Schema.Struct({
   start: Schema.Number,
   side: Schema.Literals(["additions", "deletions"]),
@@ -344,13 +346,25 @@ export function resolveDiffReviewPosition(
   })[0];
   if (line === undefined) return null;
 
+  // An explicit selection must not resolve to a same-numbered line on the other side.
+  if (
+    (side === "additions" && line.newLineNumber !== lineNumber) ||
+    (side === "deletions" && line.oldLineNumber !== lineNumber)
+  ) {
+    return null;
+  }
+
   switch (line.change) {
     case "add":
       return line.newLineNumber === null ? null : { kind: "added", newLine: line.newLineNumber };
     case "delete":
       return line.oldLineNumber === null ? null : { kind: "deleted", oldLine: line.oldLineNumber };
     case "context":
-      return line.oldLineNumber === null || line.newLineNumber === null
+      // An expanded file also shows the gaps between hunks, and GitHub rejects a whole review
+      // over one comment anchored there.
+      return line.oldLineNumber === null ||
+        line.newLineNumber === null ||
+        !isLineInFileDiff(fileDiff, "right", line.newLineNumber)
         ? null
         : {
             kind: "context",