feat: feed prior bot comments to model for conceptual dedup

AGENTS.md

@@ -25,7 +25,7 @@ src/
   openrouter/              # OpenRouter I/O: @openrouter/sdk wrapper, structured-output retry ladder, cost summary
   diff/                    # pure transforms over parse-diff output
   context/                 # workspace I/O: conventions file, changed files, import-trace scan, doc-mention scan, priority docs
-  review/                  # pure review logic: finding schema, phases, prompt, selection, comment mapping
+  review/                  # pure review logic: finding schema, phases, prompt, non-finding filter, selection, comment mapping, summary
   orchestrate.ts           # pipeline + createPromptedGenerateFindings — fully testable with stub clients
 ```
 

README.md

@@ -93,19 +93,19 @@ The `@umm review` comment trigger lets you re-request a review on any PR by comm
 
 ## Outputs
 
-| Output           | Description                                                               |
-| ---------------- | ------------------------------------------------------------------------- |
-| `findings_count` | Number of new findings posted (after threshold, cap, and cross-run dedup) |
-| `review_url`     | URL of the submitted review; empty when no review was posted              |
-| `model_used`     | Model that produced the accepted response                                 |
-| `skipped_reason` | Non-empty when the review was skipped (e.g. diff too large)               |
+| Output           | Description                                                                                   |
+| ---------------- | --------------------------------------------------------------------------------------------- |
+| `findings_count` | Number of new findings posted (after non-finding filter, threshold, cap, and cross-run dedup) |
+| `review_url`     | URL of the submitted review; empty when no review was posted                                  |
+| `model_used`     | Model that produced the accepted response                                                     |
+| `skipped_reason` | Non-empty when the review was skipped (e.g. diff too large)                                   |
 
 ## How it works
 
 1. Resolves the PR from the triggering event (supports `pull_request`, `pull_request_target`, and `issue_comment` events)
 2. Fetches the unified diff via the GitHub API — PRs that exceed the API's diff size limit are skipped
 3. Reads the conventions file and changed source files (token-budgeted), traces imports to find related code files, and scans doc files (`.md`, `.json`) for mentions of changed paths
-4. Builds a structured prompt with randomized delimiter nonces (prompt injection defense) and sends it to OpenRouter
+4. Builds a structured prompt with randomized delimiter nonces (prompt injection defense); on re-runs, prior bot comment bodies are included so the model can self-suppress conceptual duplicates. Sends it to OpenRouter
 5. Validates the response against a strict Zod schema, retrying with a fallback model if the primary fails
 6. Drops non-findings (see [Non-finding filter](#non-finding-filter)), then on re-runs deduplicates against previously posted inline comments (by hidden HTML anchor)
 7. Filters remaining findings by severity threshold, deduplicates overlapping findings within the run, and caps if configured
@@ -137,7 +137,7 @@ umm-actually is in early development — the core review pipeline works but ther
 - Skip-path handling with posted reasons (oversized diff, empty diff, API limits)
 - Cost transparency (per-run model/token/USD report in workflow summary)
 - `@umm review` comment trigger for on-demand re-reviews
-- Cross-run finding dedup — re-runs detect previously posted inline findings via hidden HTML anchors and post only new ones, with an updatable summary comment tracking totals
+- Cross-run finding dedup — re-runs detect previously posted inline findings via hidden HTML anchors and post only new ones; prior bot comment bodies also feed into the prompt for conceptual dedup (the model self-suppresses even when positional anchors differ). An updatable summary comment tracks totals
 - Non-finding filter — deterministic drop of findings that amount to "no bug here" (`N/A` prefixes, "no action needed" suggestions, "…is correct" titles) before threshold and cap
 
 **In progress**

src/__tests__/orchestrate.test.ts

@@ -1648,6 +1648,81 @@ describe("orchestrate", () => {
         data: { error: "[Error]: API rate limit" },
       })
     })
+
+    const priorBotCommentsFrom = (stubs: RecordingStubs): string[] => {
+      const context = stubs.generateFindingsCalls[0]
+      if (!context) {
+        throw new Error("expected at least one generateFindings call")
+      }
+      return context.priorBotComments
+    }
+
+    it("passes prior bot comment bodies (anchor-stripped) to generateFindings", async () => {
+      const commentBody =
+        "**[high/correctness]** Fix null check\n\nDescription.\n\n<!-- umm-actually:src/greeter.ts:correctness:99 -->"
+      const stubs = makeOrchestrateDeps({
+        githubClient: {
+          fetchBotReviewComments: async () => [
+            existingComment(commentBody, { line: 99 }),
+          ],
+          fetchBotIssueComments: async () => [
+            statusComment,
+            issueFinding("src/other.ts:security:50"),
+          ],
+        },
+      })
+      const logger = createTestLogger()
+
+      await orchestrate(stubs.deps, logger)
+
+      expect(priorBotCommentsFrom(stubs)).toEqual([
+        "**[high/correctness]** Fix null check\n\nDescription.",
+        "finding text",
+      ])
+    })
+
+    it("excludes the status comment from prior bot comments", async () => {
+      const stubs = makeOrchestrateDeps({
+        githubClient: {
+          fetchBotIssueComments: async () => [statusComment],
+        },
+      })
+      const logger = createTestLogger()
+
+      await orchestrate(stubs.deps, logger)
+
+      expect(priorBotCommentsFrom(stubs)).toEqual([])
+    })
+
+    it("caps prior bot comments at 30", async () => {
+      const comments = Array.from({ length: 40 }, (_, index) =>
+        existingComment(
+          `finding ${index}\n\n<!-- umm-actually:src/a.ts:correctness:${index + 1} -->`,
+          { line: index + 1 },
+        ),
+      )
+      const stubs = makeOrchestrateDeps({
+        githubClient: {
+          fetchBotReviewComments: async () => comments,
+        },
+      })
+      const logger = createTestLogger()
+
+      await orchestrate(stubs.deps, logger)
+
+      expect(priorBotCommentsFrom(stubs)).toEqual(
+        Array.from({ length: 30 }, (_, index) => `finding ${index + 10}`),
+      )
+    })
+
+    it("passes empty prior bot comments on a first run", async () => {
+      const stubs = makeOrchestrateDeps()
+      const logger = createTestLogger()
+
+      await orchestrate(stubs.deps, logger)
+
+      expect(priorBotCommentsFrom(stubs)).toEqual([])
+    })
   })
 })
 
@@ -1690,6 +1765,7 @@ describe("createPromptedGenerateFindings", () => {
       relatedDocs: [],
       annotatedDiff: annotateDiff(files),
       priorFindings: [],
+      priorBotComments: [],
     })
 
     expect(requestReviewCalls).toHaveLength(1)
@@ -1736,6 +1812,7 @@ describe("createPromptedGenerateFindings", () => {
       relatedDocs: [],
       annotatedDiff: annotated,
       priorFindings: [],
+      priorBotComments: [],
     })
 
     const call = first(requestReviewCalls)
@@ -1777,6 +1854,7 @@ describe("createPromptedGenerateFindings", () => {
       relatedDocs: [],
       annotatedDiff: annotated,
       priorFindings: [],
+      priorBotComments: [],
     }
 
     await generate(context)

src/orchestrate.ts

@@ -48,6 +48,7 @@ export type ReviewContext = {
   relatedDocs: PromptFile[]
   annotatedDiff: string
   priorFindings: Finding[]
+  priorBotComments: string[]
 }
 
 export type GenerateFindings = (
@@ -72,6 +73,14 @@ export type OrchestrateDeps = {
   generateFindings: GenerateFindings
 }
 
+/** Bounds token cost of prior bot comments in the prompt (~200 tokens each). */
+const PRIOR_COMMENT_CAP = 30
+
+/** Strips the trailing dedup anchor from a comment body so the model
+ *  doesn't see the dedup infrastructure in the prior-comments section. */
+const stripAnchorComment = (body: string): string =>
+  body.replace(/\n*<!-- umm-actually:.+? -->\s*$/, "")
+
 const buildSkipBody = (reason: string): string =>
   `**umm-actually** — review skipped\n\n${reason}\n\n---\n*umm-actually*`
 
@@ -81,29 +90,38 @@ const describeError = (error: unknown): string => {
     : String(error)
 }
 
-/** Anchors from the bot's inline review comments (live positions). Empty on
+type InlineCommentState = {
+  anchors: AnchorEntry[]
+  commentBodies: string[]
+}
+
+/** Anchors and raw bodies from the bot's inline review comments. Empty on
  *  fetch failure — findings then post as new; duplicates beat losing them. */
-const fetchInlineAnchors = async (
+const fetchInlineCommentState = async (
   { githubClient, prNumber }: { githubClient: GithubClient; prNumber: number },
   logger: Logger,
-): Promise<AnchorEntry[]> => {
+): Promise<InlineCommentState> => {
   try {
     const existingComments = await githubClient.fetchBotReviewComments({
       prNumber,
     })
-    return extractAnchors(existingComments)
+    return {
+      anchors: extractAnchors(existingComments),
+      commentBodies: existingComments.map((comment) => comment.body),
+    }
   } catch (fetchError) {
     logger.warn(
       "failed to fetch inline comments — treating their findings as new",
       { error: describeError(fetchError) },
     )
-    return []
+    return { anchors: [], commentBodies: [] }
   }
 }
 
 type IssueCommentState = {
   statusCommentExists: boolean
   anchors: AnchorEntry[]
+  findingBodies: string[]
 }
 
 /** The bot's issue comments carry the rest of the cross-run state: the
@@ -116,25 +134,27 @@ const fetchIssueCommentState = async (
 ): Promise<IssueCommentState> => {
   try {
     const comments = await githubClient.fetchBotIssueComments({ prNumber })
+    const findingComments = comments.filter(
+      (comment) => !comment.body.startsWith(STATUS_ANCHOR),
+    )
     return {
       // startsWith, not includes: a finding comment's model-generated text
       // could quote the marker mid-body and misclassify the run as a re-run.
-      statusCommentExists: comments.some((comment) =>
-        comment.body.startsWith(STATUS_ANCHOR),
-      ),
+      statusCommentExists: comments.length > findingComments.length,
       anchors: extractAnchors(
-        comments.map((comment) => ({
+        findingComments.map((comment) => ({
           body: comment.body,
           line: null,
           originalLine: null,
         })),
       ),
+      findingBodies: findingComments.map((comment) => comment.body),
     }
   } catch (fetchError) {
     logger.warn("failed to fetch issue comments — treating as a first run", {
       error: describeError(fetchError),
     })
-    return { statusCommentExists: false, anchors: [] }
+    return { statusCommentExists: false, anchors: [], findingBodies: [] }
   }
 }
 
@@ -396,6 +416,24 @@ export const orchestrate = async (
     )
   }
 
+  // Step 9.5: fetch prior bot comments — needed both for the prompt (the
+  // model sees what's already posted and self-suppresses conceptual dupes)
+  // and for positional dedup after generation.
+  const inlineState = await fetchInlineCommentState(
+    { githubClient, prNumber: prContext.prNumber },
+    logger,
+  )
+  const issueState = await fetchIssueCommentState(
+    { githubClient, prNumber: prContext.prNumber },
+    logger,
+  )
+  const priorBotComments = [
+    ...inlineState.commentBodies,
+    ...issueState.findingBodies,
+  ]
+    .map(stripAnchorComment)
+    .slice(-PRIOR_COMMENT_CAP)
+
   // Step 10–11: generate findings (V1: single combined phase)
   const phase = phases[0]
   if (!phase) {
@@ -410,6 +448,7 @@ export const orchestrate = async (
     relatedDocs,
     annotatedDiff,
     priorFindings: [],
+    priorBotComments,
   })
 
   const { modelUsed, attempts } = structuredResult
@@ -428,22 +467,15 @@ export const orchestrate = async (
   // is just the case where no bot comments exist yet. Sources: the bot's
   // inline comments (live positions) and its beyond-diff issue comments
   // (anchor lines). Runs before the cap so duplicates don't consume slots.
-  const inlineAnchors = await fetchInlineAnchors(
-    { githubClient, prNumber: prContext.prNumber },
-    logger,
-  )
-  const issueState = await fetchIssueCommentState(
-    { githubClient, prNumber: prContext.prNumber },
-    logger,
-  )
-  const existingAnchors = [...inlineAnchors, ...issueState.anchors]
+  const existingAnchors = [...inlineState.anchors, ...issueState.anchors]
   const newFindings = realFindings.filter(
     (finding) => !isDuplicateFinding(finding, existingAnchors),
   )
 
   logger.info("cross-run dedup against prior bot comments", {
     statusCommentFound: issueState.statusCommentExists,
     existingAnchorCount: existingAnchors.length,
+    priorBotCommentCount: priorBotComments.length,
     findingsAfterFilter: realFindings.length,
     findingsSurvivedDedup: newFindings.length,
   })

src/review/__tests__/prompt.test.ts

@@ -45,6 +45,7 @@ const makeUserPromptParts = () => ({
   annotatedDiff:
     "=== src/greeter.ts ===\n@@ -1,1 +1,1 @@\n     1 + export const greet",
   priorFindings: [],
+  priorBotComments: [],
   delimiterNonce: "abc123def456",
 })
 
@@ -288,6 +289,53 @@ describe("buildUserPrompt", () => {
     expect(userPrompt).toContain(priorFinding.title)
   })
 
+  it("omits the prior bot comments section when empty", () => {
+    const userPrompt = buildUserPrompt(makeUserPromptParts())
+
+    expect(userPrompt).not.toContain("prior_bot_comments")
+  })
+
+  it("includes prior bot comments with a do-not-re-report instruction when present", () => {
+    const userPrompt = buildUserPrompt({
+      ...makeUserPromptParts(),
+      priorBotComments: [
+        "**[high/correctness]** Fix null check\n\nDescription here.",
+        "**[medium/security]** Sanitize input\n\nAnother finding.",
+      ],
+    })
+
+    expect(userPrompt).toContain(
+      [
+        '<prior_bot_comments-abc123def456 note="findings already posted on this PR — do not re-report the same issues, even at different locations">',
+        "**[high/correctness]** Fix null check\n\nDescription here.",
+        "",
+        "---",
+        "",
+        "**[medium/security]** Sanitize input\n\nAnother finding.",
+        "</prior_bot_comments-abc123def456>",
+      ].join("\n"),
+    )
+  })
+
+  it("places prior bot comments after the diff and before prior findings", () => {
+    const priorFinding = makeFinding()
+
+    const userPrompt = buildUserPrompt({
+      ...makeUserPromptParts(),
+      priorFindings: [priorFinding],
+      priorBotComments: ["**[high/correctness]** Some prior finding"],
+    })
+
+    const diffIndex = userPrompt.indexOf("diff-abc123def456")
+    const botCommentsIndex = userPrompt.indexOf(
+      "prior_bot_comments-abc123def456",
+    )
+    const priorFindingsIndex = userPrompt.indexOf("prior_findings-abc123def456")
+
+    expect(diffIndex).toBeLessThan(botCommentsIndex)
+    expect(botCommentsIndex).toBeLessThan(priorFindingsIndex)
+  })
+
   it("labels related files with their inclusion reason", () => {
     const userPrompt = buildUserPrompt(makeUserPromptParts())
 

src/review/__tests__/review-summary.test.ts

@@ -174,4 +174,14 @@ describe("renderReviewSummary", () => {
     expect(summary).toContain("`abc123d`")
     expect(summary).not.toContain(baseStats.prContext.headSha)
   })
+
+  it("escapes pipe characters in paths so they cannot break the markdown table", () => {
+    const summary = renderReviewSummary({
+      ...baseStats,
+      changedFilePaths: ["src/a|b.ts"],
+    })
+
+    expect(summary).toContain("| Changed files | 1 | src/a\\|b.ts |")
+    expect(summary).not.toContain("| src/a|b.ts |")
+  })
 })