feat(review): multi-line suggestion blocks with range validation (#2141)

src/github/pr-actions.ts

@@ -68,7 +68,14 @@ export async function createPullRequestReviewComments(
   repoFullName: string,
   pullNumber: number,
   commitId: string,
-  comments: Array<{ path: string; line: number; side: "RIGHT" | "LEFT"; body: string }>,
+  comments: Array<{
+    path: string;
+    line: number;
+    side: "RIGHT" | "LEFT";
+    body: string;
+    start_line?: number;
+    start_side?: "RIGHT" | "LEFT";
+  }>,
   mode: AgentActionMode,
 ): Promise<{ id: number }> {
   const { owner, repo } = splitRepo(repoFullName);

src/review/inline-comment-range.ts

@@ -0,0 +1,48 @@
+/** Multi-line inline comment range validation (#2141). */
+
+import { rightSideLinesFromPatch } from "./inline-comments-select";
+import type { InlineFinding } from "../services/ai-review";
+import type { PullRequestFileRecord } from "../types";
+
+/** Normalized [start, end] for an inline finding — `line` is always the start; invalid/inverted/absent `endLine`
+ *  collapses to a single-line anchor. */
+export function parseInlineLineRange(finding: Pick<InlineFinding, "line" | "endLine">): { start: number; end: number } {
+  const start = finding.line;
+  const end = finding.endLine != null && finding.endLine > start ? finding.endLine : start;
+  return { start, end };
+}
+
+/** True when every line in the inclusive [start, end] range is present in `lines`. */
+export function everyLineInSet(start: number, end: number, lines: Set<number>): boolean {
+  for (let line = start; line <= end; line += 1) {
+    if (!lines.has(line)) return false;
+  }
+  return true;
+}
+
+/** Build per-file RIGHT-side commentable line sets from PR file records. */
+export function rightLinesByPath(
+  files: Pick<PullRequestFileRecord, "path" | "payload">[],
+): Map<string, Set<number>> {
+  const out = new Map<string, Set<number>>();
+  for (const file of files) {
+    const patch = typeof file.payload?.patch === "string" ? file.payload.patch : "";
+    if (patch) out.set(file.path, rightSideLinesFromPatch(patch));
+  }
+  return out;
+}
+
+/** Resolve the GitHub inline-comment anchor for a finding. Multi-line ONLY when every line in [start,end] is
+ *  commentable on the RIGHT side; otherwise downgrade to the single start line (fail-safe, no 422). */
+export function resolveInlineCommentAnchor(
+  finding: Pick<InlineFinding, "path" | "line" | "endLine">,
+  rightLines: Map<string, Set<number>>,
+): { start: number; end: number; multiLine: boolean } {
+  const { start, end } = parseInlineLineRange(finding);
+  const validLines = rightLines.get(finding.path);
+  if (!validLines || !everyLineInSet(start, end, validLines)) {
+    return { start, end: start, multiLine: false };
+  }
+  if (end > start) return { start, end, multiLine: true };
+  return { start, end: start, multiLine: false };
+}

src/review/inline-comments.ts

@@ -11,6 +11,7 @@
 import { createPullRequestReviewComments } from "../github/pr-actions";
 import { isConvergenceRepoAllowed } from "./cutover-gate";
 import { formatInlineCommentSeverityLabel } from "./inline-comment-label";
+import { resolveInlineCommentAnchor, rightLinesByPath } from "./inline-comment-range";
 import { addedLinesByPath, anchoredSuggestionBlock } from "./inline-suggestion-anchor";
 import { selectAnchoredInlineFindings } from "./inline-comments-select";
 export { rightSideLinesFromPatch } from "./inline-comments-select";
@@ -60,8 +61,16 @@ export function shouldRenderFindingCategories(
   return inlineCommentsEnabled && manifestToggle === true;
 }
 
-/** A GitHub inline review comment anchored to a line on the RIGHT (added/context) side of the PR diff. */
-export type ReviewInlineComment = { path: string; line: number; side: "RIGHT"; body: string };
+/** A GitHub inline review comment anchored to a line on the RIGHT (added/context) side of the PR diff. Multi-line
+ *  comments set `start_line`/`start_side` with `line` as the inclusive end (#2141). */
+export type ReviewInlineComment = {
+  path: string;
+  line: number;
+  side: "RIGHT";
+  body: string;
+  start_line?: number;
+  start_side?: "RIGHT";
+};
 
 /** Hard cap on inline comments posted per PR review — a focused review leaves a handful of precise notes, not a
  *  wall (the model is also asked to be selective, and composeInlineFindings already caps at 10). */
@@ -107,12 +116,23 @@ export function selectInlineComments(
     perCategoryCap,
   });
   const addedLines = addedLinesByPath(files);
-  return selected.map((finding) => ({
-    path: finding.path,
-    line: finding.line,
-    side: "RIGHT" as const,
-    body: formatInlineBody(finding, suggestionsEnabled, categoriesEnabled, addedLines),
-  }));
+  const rightLines = rightLinesByPath(files);
+  return selected.map((finding) => {
+    const anchor = resolveInlineCommentAnchor(finding, rightLines);
+    const anchoredFinding: InlineFinding =
+      anchor.multiLine ? finding : { ...finding, endLine: undefined };
+    const comment: ReviewInlineComment = {
+      path: finding.path,
+      line: anchor.end,
+      side: "RIGHT" as const,
+      body: formatInlineBody(anchoredFinding, suggestionsEnabled, categoriesEnabled, addedLines),
+    };
+    if (anchor.multiLine) {
+      comment.start_line = anchor.start;
+      comment.start_side = "RIGHT";
+    }
+    return comment;
+  });
 }
 
 /** Post the model's inline findings as ONE quiet, non-blocking review (`event: COMMENT`) on the PR. Fully

src/review/inline-suggestion-anchor.ts

@@ -1,5 +1,6 @@
 /** Suggestion anchor-safety for inline PR review comments (#2140). */
 
+import { parseInlineLineRange } from "./inline-comment-range";
 import type { InlineFinding } from "../services/ai-review";
 import type { PullRequestFileRecord } from "../types";
 
@@ -38,11 +39,16 @@ export function addedLinesByPath(
 
 /** True when a finding's line is an ADDED RIGHT-side line that can carry a ```suggestion block. */
 export function isSuggestionAnchorable(
-  finding: Pick<InlineFinding, "path" | "line">,
+  finding: Pick<InlineFinding, "path" | "line" | "endLine">,
   addedLines: Map<string, Set<number>>,
 ): boolean {
   const validLines = addedLines.get(finding.path);
-  return validLines != null && validLines.has(finding.line);
+  if (validLines == null) return false;
+  const { start, end } = parseInlineLineRange(finding);
+  for (let line = start; line <= end; line += 1) {
+    if (!validLines.has(line)) return false;
+  }
+  return true;
 }
 
 /** GitHub suggestion fence — dropped when blank or when the text would break the fence (#1956). */

src/services/ai-review.ts

@@ -374,6 +374,9 @@ export type InlineFinding = {
   severity: "blocker" | "nit";
   body: string;
   suggestion?: string | undefined;
+  /** Optional end line (inclusive) for a multi-line inline comment / ```suggestion block (#2141). When absent or
+   *  invalid (`endLine` ≤ `line`), the finding is treated as single-line. */
+  endLine?: number | undefined;
   /** `.gittensory.yml` `review.finding_categories` (#1958): the kind of issue (security/correctness/performance/
    *  maintainability/tests/style), when the model was asked to self-categorize and emitted a value in the fixed
    *  enum. Absent when the feature is off (the model was never asked) OR the model's value didn't parse — callers
@@ -638,6 +641,8 @@ export function parseModelReview(text: string): ModelReview | null {
               // JSON numbers are always finite (NaN/Infinity can't appear), so a numeric `line` is real; trunc a
               // float, and the `line > 0` guard below drops 0/negative anchors.
               const line = typeof o.line === "number" ? Math.trunc(o.line) : 0;
+              const endLineRaw = typeof o.endLine === "number" ? Math.trunc(o.endLine) : undefined;
+              const endLine = endLineRaw != null && endLineRaw > line ? endLineRaw : undefined;
               const body = typeof o.body === "string" ? o.body.trim() : "";
               const suggestion =
                 typeof o.suggestion === "string" ? o.suggestion.trim() : "";
@@ -653,6 +658,7 @@ export function parseModelReview(text: string): ModelReview | null {
                       body,
                       ...(suggestion ? { suggestion } : {}),
                       ...(category ? { category } : {}),
+                      ...(endLine != null ? { endLine } : {}),
                     },
                   ]
                 : [];
@@ -1325,13 +1331,15 @@ function mergeSameLineFindings(first: InlineFinding, next: InlineFinding): Inlin
   const weak = nextStronger ? first : next;
   const suggestion = strong.suggestion ?? weak.suggestion;
   const category = strong.category ?? weak.category;
+  const endLine = strong.endLine ?? weak.endLine;
   return {
     path: first.path,
     line: first.line,
     severity: strong.severity,
     body: strong.body,
     ...(suggestion ? { suggestion } : {}),
     ...(category ? { category } : {}),
+    ...(endLine != null ? { endLine } : {}),
   };
 }
 
@@ -1353,6 +1361,7 @@ export function composeInlineFindings(reviews: ModelReview[]): InlineFinding[] {
       // `category` is a fixed enum literal (never free text), so it carries through as-is — no public-safe
       // scrubbing needed, unlike body/suggestion.
       ...(finding.category ? { category: finding.category } : {}),
+      ...(finding.endLine != null ? { endLine: finding.endLine } : {}),
     };
     const key = `${finding.path}:${finding.line}`;
     const existing = byLine.get(key);

test/unit/ai-review.test.ts

@@ -43,6 +43,7 @@ type InlineFinding = {
   severity: "blocker" | "nit";
   body: string;
   suggestion?: string;
+  endLine?: number;
   category?: "security" | "correctness" | "performance" | "maintainability" | "tests" | "style";
 };
 type ModelReviewShape = {
@@ -3034,6 +3035,25 @@ describe("pure helpers", () => {
     ]);
   });
 
+  it("parseModelReview parses endLine for multi-line inline findings and drops inverted ranges (#2141)", () => {
+    const json = JSON.stringify({
+      assessment: "ok",
+      blockers: [],
+      nits: [],
+      suggestions: [],
+      inlineFindings: [
+        { path: "src/a.ts", line: 1, endLine: 3, severity: "nit", body: "Multi." },
+        { path: "src/b.ts", line: 5, endLine: 3, severity: "nit", body: "Inverted." },
+        { path: "src/c.ts", line: 2, endLine: 2, severity: "nit", body: "Equal." },
+      ],
+    });
+    expect(parseModelReview(json)?.inlineFindings).toEqual([
+      { path: "src/a.ts", line: 1, endLine: 3, severity: "nit", body: "Multi." },
+      { path: "src/b.ts", line: 5, severity: "nit", body: "Inverted." },
+      { path: "src/c.ts", line: 2, severity: "nit", body: "Equal." },
+    ]);
+  });
+
   it("parseModelReview drops malformed inline findings (non-object / missing path|line|body / non-positive line), never partial", () => {
     const json = JSON.stringify({
       assessment: "ok",
@@ -3084,6 +3104,21 @@ describe("pure helpers", () => {
     ).toEqual([]);
   });
 
+  it("composeInlineFindings carries endLine through compose and merge (#2141)", () => {
+    const out = composeInlineFindings([
+      reviewWithFindings([
+        { path: "src/a.ts", line: 1, endLine: 3, severity: "nit", body: "Multi-line note." },
+      ]),
+      reviewWithFindings([
+        { path: "src/a.ts", line: 1, severity: "blocker", body: "Stronger body." },
+        { path: "src/a.ts", line: 1, endLine: 4, severity: "nit", body: "Weaker with wider range." },
+      ]),
+    ]);
+    expect(out).toEqual([
+      { path: "src/a.ts", line: 1, endLine: 3, severity: "blocker", body: "Stronger body." },
+    ]);
+  });
+
   it("composeInlineFindings MERGES same-(path,line) findings across reviewers: max severity, suggestion carried from whichever had it; distinct lines untouched (#2158)", () => {
     const out = composeInlineFindings([
       reviewWithFindings([

test/unit/inline-comment-range.test.ts

@@ -0,0 +1,81 @@
+import { describe, expect, it } from "vitest";
+import {
+  everyLineInSet,
+  parseInlineLineRange,
+  resolveInlineCommentAnchor,
+  rightLinesByPath,
+} from "../../src/review/inline-comment-range";
+
+const multiPatch = "@@ -1,0 +1,3 @@\n+one\n+two\n+three";
+const mixedPatch = "@@ -1,2 +1,4 @@\n ctx\n+add2\n ctx4\n+add4";
+
+describe("parseInlineLineRange (#2141)", () => {
+  it("collapses absent, equal, or inverted endLine to a single-line range", () => {
+    expect(parseInlineLineRange({ line: 2 })).toEqual({ start: 2, end: 2 });
+    expect(parseInlineLineRange({ line: 2, endLine: 2 })).toEqual({ start: 2, end: 2 });
+    expect(parseInlineLineRange({ line: 5, endLine: 3 })).toEqual({ start: 5, end: 5 });
+  });
+
+  it("keeps a valid forward range", () => {
+    expect(parseInlineLineRange({ line: 1, endLine: 3 })).toEqual({ start: 1, end: 3 });
+  });
+});
+
+describe("everyLineInSet (#2141)", () => {
+  it("requires every line in the inclusive range", () => {
+    const lines = new Set([1, 2, 3]);
+    expect(everyLineInSet(1, 3, lines)).toBe(true);
+    expect(everyLineInSet(1, 4, lines)).toBe(false);
+  });
+});
+
+describe("rightLinesByPath (#2141)", () => {
+  it("omits files with empty or non-string patches", () => {
+    const map = rightLinesByPath([
+      { path: "src/empty.ts", payload: { patch: "" } },
+      { path: "src/bad.ts", payload: { patch: 42 as unknown as string } },
+      { path: "src/a.ts", payload: { patch: multiPatch } },
+    ]);
+    expect(map.size).toBe(1);
+    expect(map.has("src/a.ts")).toBe(true);
+  });
+});
+
+describe("resolveInlineCommentAnchor (#2141)", () => {
+  const files = [{ path: "src/a.ts", payload: { patch: multiPatch } }];
+
+  it("emits a multi-line anchor when every line in the range is commentable", () => {
+    const rightLines = rightLinesByPath(files);
+    expect(resolveInlineCommentAnchor({ path: "src/a.ts", line: 1, endLine: 3 }, rightLines)).toEqual({
+      start: 1,
+      end: 3,
+      multiLine: true,
+    });
+  });
+
+  it("downgrades to the start line when any line in the range is not commentable", () => {
+    const rightLines = rightLinesByPath([{ path: "src/a.ts", payload: { patch: mixedPatch } }]);
+    expect(resolveInlineCommentAnchor({ path: "src/a.ts", line: 2, endLine: 99 }, rightLines)).toEqual({
+      start: 2,
+      end: 2,
+      multiLine: false,
+    });
+  });
+
+  it("downgrades when the file path is missing from the RIGHT-side line map", () => {
+    expect(resolveInlineCommentAnchor({ path: "src/missing.ts", line: 1, endLine: 3 }, new Map())).toEqual({
+      start: 1,
+      end: 1,
+      multiLine: false,
+    });
+  });
+
+  it("keeps a single-line anchor when the range collapses to one commentable line", () => {
+    const rightLines = rightLinesByPath(files);
+    expect(resolveInlineCommentAnchor({ path: "src/a.ts", line: 2 }, rightLines)).toEqual({
+      start: 2,
+      end: 2,
+      multiLine: false,
+    });
+  });
+});