fix(review): count blank context lines in addedLinesFromPatch so blocker anchors don't drift

src/review/inline-suggestion-anchor.ts

@@ -10,15 +10,25 @@ import type { PullRequestFileRecord } from "../types";
 export function addedLinesFromPatch(patch: string): Set<number> {
   const lines = new Set<number>();
   let right = 0;
-  for (const raw of patch.split("\n")) {
+  // Mirror rightSideLinesFromPatch (inline-comments-select.ts) line-for-line so both walkers agree on every
+  // RIGHT-side line number (#9663). A patch ending in "\n" splits to a trailing empty element that is a split
+  // artifact, not a diff line; drop it here so that a remaining empty element genuinely means "a context line
+  // whose leading space was stripped" and advances `right` like the context line it is.
+  const rawLines = patch.split("\n");
+  if (rawLines.length > 0 && rawLines[rawLines.length - 1] === "") rawLines.pop();
+  for (const raw of rawLines) {
     const header = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(raw);
     if (header?.[1]) {
       right = Number.parseInt(header[1], 10);
       continue;
     }
     if (right === 0) continue;
     const marker = raw[0];
-    if (marker === undefined || marker === "-" || marker === "\\") continue;
+    if (marker === "-" || marker === "\\") continue;
+    // A zero-length patch line (marker `undefined`) is a context line whose single space was stripped, not a
+    // line that does not exist (#9076/#9663). It is NOT added, but it must still advance `right` — skipping it
+    // desynced every subsequent added-line number, so a blocker anchored after a blank context line was dropped
+    // and one anchored on the blank context line itself was wrongly accepted.
     if (marker === "+") lines.add(right);
     right += 1;
   }

test/unit/inline-comments.test.ts

@@ -125,6 +125,22 @@ describe("selectInlineComments (#inline-comments)", () => {
     expect(out).toEqual([{ path: "src/a.ts", line: 2, side: "RIGHT", body: "**Blocker:** Must fix." }]);
   });
 
+  it("anchors a blocker after a blank context line correctly and drops one on the blank line itself (#9663)", () => {
+    // Patch with a blank context line (line 2) then an added line (line 3). Before #9663 addedLinesFromPatch
+    // returned {2} instead of {3}, so the blocker on the ADDED line 3 was dropped and a blocker on the blank
+    // CONTEXT line 2 was wrongly posted -- the exact "your bug is on a line you did not write" failure #9076
+    // exists to prevent, on the path (addedLines) that enforces it for blocker-severity findings.
+    const blankContextFiles = [fileWith("src/a.ts", "@@ -1,3 +1,3 @@\n one\n\n+three")];
+    const out = selectInlineComments(
+      [
+        { path: "src/a.ts", line: 3, severity: "blocker", body: "Real defect on the added line." },
+        { path: "src/a.ts", line: 2, severity: "blocker", body: "Anchored on a blank context line." },
+      ],
+      blankContextFiles,
+    );
+    expect(out).toEqual([{ path: "src/a.ts", line: 3, side: "RIGHT", body: "**Blocker:** Real defect on the added line." }]);
+  });
+
   it("caps the output at 10 comments", () => {
     const bigPatch = "@@ -1,0 +1,12 @@\n" + Array.from({ length: 12 }, (_, i) => `+line${i + 1}`).join("\n");
     const bigFiles = [{ path: "src/big.ts", payload: { patch: bigPatch } }];

test/unit/inline-suggestion-anchor.test.ts

@@ -20,6 +20,37 @@ describe("addedLinesFromPatch (#2140)", () => {
     expect(addedLinesFromPatch("").size).toBe(0);
     expect(addedLinesFromPatch("preamble only").size).toBe(0);
   });
+
+  describe("blank context lines and trailing-newline artifacts (#9663)", () => {
+    // A blank context line (its single leading space stripped by the split -> marker `undefined`) must still
+    // advance `right`, exactly as rightSideLinesFromPatch does. Without that, the added line after it desyncs.
+    const blankContextPatch = "@@ -1,3 +1,3 @@\n one\n\n+three";
+
+    it("advances right across a zero-length context line so the added line keeps its true number", () => {
+      // Before #9663 this returned Set{2}: the blank line was skipped without advancing right, so "+three"
+      // landed on 2 (a context line) instead of 3 (the line the contributor actually added).
+      expect([...addedLinesFromPatch(blankContextPatch)]).toEqual([3]);
+    });
+
+    it("produces the same set with or without a trailing newline", () => {
+      expect([...addedLinesFromPatch(blankContextPatch + "\n")].sort((a, b) => a - b)).toEqual(
+        [...addedLinesFromPatch(blankContextPatch)].sort((a, b) => a - b),
+      );
+      // And concretely: the trailing "\n" split artifact must not be counted as an extra line.
+      expect([...addedLinesFromPatch("@@ -1,0 +1,1 @@\n+only\n")]).toEqual([1]);
+    });
+
+    it("keeps '-' and '\\' markers skipped without advancing right", () => {
+      // "\ No newline at end of file" and removed lines never touch the RIGHT side.
+      expect([...addedLinesFromPatch("@@ -1,2 +1,2 @@\n-gone\n+new\n\\ No newline at end of file")]).toEqual([1]);
+    });
+
+    it("isSuggestionAnchorable follows the corrected numbering across a blank context line", () => {
+      const addedLines = addedLinesByPath([{ path: "src/a.ts", payload: { patch: blankContextPatch } }]);
+      expect(isSuggestionAnchorable({ path: "src/a.ts", line: 3 }, addedLines)).toBe(true);
+      expect(isSuggestionAnchorable({ path: "src/a.ts", line: 2 }, addedLines)).toBe(false);
+    });
+  });
 });
 
 describe("addedLinesByPath + isSuggestionAnchorable (#2140)", () => {