diff --git a/src/review/inline-comments.ts b/src/review/inline-comments.ts index cda032cb5d..3521917238 100644 --- a/src/review/inline-comments.ts +++ b/src/review/inline-comments.ts @@ -65,6 +65,27 @@ export type ReviewInlineComment = { path: string; line: number; side: "RIGHT"; b * wall (the model is also asked to be selective, and composeInlineFindings already caps at 10). */ const MAX_INLINE_COMMENTS = 10; +/** PURE (#2140): the subset of {@link rightSideLinesFromPatch} that are genuinely ADDED ("+") lines — GitHub + * suggested-change blocks 422 unless the anchor is an added line; context (" ") lines may still take a plain + * inline comment. */ +export function addedLinesFromPatch(patch: string): Set { + const lines = new Set(); + let right = 0; + for (const raw of patch.split("\n")) { + 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 === "+") lines.add(right); + right += 1; + } + return lines; +} + /** PURE: the set of NEW-file (RIGHT-side) line numbers a unified-diff patch makes commentable — every added * ("+") and context (" ") line inside a hunk. GitHub 422s an inline comment whose line is NOT one of these, so * {@link selectInlineComments} validates each finding against this set. Deleted ("-") lines are LEFT-side only @@ -130,9 +151,13 @@ export function selectInlineComments( minFindingSeverity: ReviewFindingSeverity | null | undefined = null, ): ReviewInlineComment[] { const rightLinesByPath = new Map>(); + const addedLinesByPath = new Map>(); for (const file of files) { const patch = typeof file.payload?.patch === "string" ? file.payload.patch : ""; - if (patch) rightLinesByPath.set(file.path, rightSideLinesFromPatch(patch)); + if (patch) { + rightLinesByPath.set(file.path, rightSideLinesFromPatch(patch)); + addedLinesByPath.set(file.path, addedLinesFromPatch(patch)); + } } const out: ReviewInlineComment[] = []; const seen = new Set(); @@ -144,7 +169,15 @@ export function selectInlineComments( const key = `${finding.path}:${finding.line}`; if (seen.has(key)) continue; seen.add(key); - out.push({ path: finding.path, line: finding.line, side: "RIGHT", body: formatInlineBody(finding, suggestionsEnabled, categoriesEnabled) }); + const suggestionAnchorable = Boolean( + suggestionsEnabled && finding.suggestion && addedLinesByPath.get(finding.path)?.has(finding.line), + ); + out.push({ + path: finding.path, + line: finding.line, + side: "RIGHT", + body: formatInlineBody(finding, suggestionAnchorable, categoriesEnabled), + }); } return out; } diff --git a/test/unit/inline-comments.test.ts b/test/unit/inline-comments.test.ts index f3451880a1..e6286ddfc6 100644 --- a/test/unit/inline-comments.test.ts +++ b/test/unit/inline-comments.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { generateKeyPairSync } from "node:crypto"; import type { InlineFinding } from "../../src/services/ai-review"; -import { isInlineCommentsEnabled, maybePostInlineComments, postInlineReviewComments, rightSideLinesFromPatch, selectInlineComments, shouldRenderFindingCategories, shouldRenderSuggestions, shouldRequestInlineFindings } from "../../src/review/inline-comments"; +import { isInlineCommentsEnabled, maybePostInlineComments, postInlineReviewComments, addedLinesFromPatch, rightSideLinesFromPatch, selectInlineComments, shouldRenderFindingCategories, shouldRenderSuggestions, shouldRequestInlineFindings } from "../../src/review/inline-comments"; import { createTestEnv } from "../helpers/d1"; function envWithKey() { @@ -73,6 +73,13 @@ describe("rightSideLinesFromPatch (#inline-comments)", () => { }); }); +describe("addedLinesFromPatch (#2140)", () => { + it("returns only ADDED (+) line numbers, excluding context lines", () => { + const patch = "@@ -1,3 +1,4 @@\n ctx1\n-removed\n+added2\n+added3\n ctx4\n\\ No newline at end of file"; + expect([...addedLinesFromPatch(patch)].sort((a, b) => a - b)).toEqual([2, 3]); + }); +}); + describe("selectInlineComments (#inline-comments)", () => { const files = [fileWith("src/a.ts", "@@ -1,1 +1,2 @@\n ctx\n+added2"), { path: "src/no-patch.ts", payload: {} }]; @@ -151,6 +158,25 @@ describe("selectInlineComments (#inline-comments)", () => { expect(out[0]?.body).toBe("**Blocker:** Fix this."); expect(out[0]?.body).not.toContain("escape attempt"); }); + + it("keeps a plain inline comment but strips an un-anchorable suggestion on a context line (#2140)", () => { + const contextPatch = "@@ -1,1 +1,2 @@\n ctx\n+added2"; + const contextFiles = [fileWith("src/a.ts", contextPatch)]; + const onContext: InlineFinding = { path: "src/a.ts", line: 1, severity: "nit", body: "Context note.", suggestion: "const x = 1;" }; + const out = selectInlineComments([onContext], contextFiles, true); + expect(out).toEqual([{ path: "src/a.ts", line: 1, side: "RIGHT", body: "**Nit:** Context note." }]); + expect(out[0]?.body).not.toContain("```suggestion"); + }); + + it("keeps both comment and suggestion when the anchor is an added line (#2140)", () => { + const out = selectInlineComments([withSuggestion], files, true); + expect(out[0]?.body).toContain("```suggestion"); + }); + + it("never emits a suggestion on a file with no usable patch (#2140)", () => { + const out = selectInlineComments([withSuggestion], [{ path: "src/a.ts", payload: {} }], true); + expect(out).toEqual([]); + }); }); describe("category tags (#1958)", () => {