From da76bf618eb8ad72f4ae2bdc4295ae9615948116 Mon Sep 17 00:00:00 2001 From: Tanisha Aberdeen <32620895+aliasunder@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:22:19 -0400 Subject: [PATCH 1/5] feat: extend non-finding filter to title + suggestion fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six confirmation findings escaped on PRs #10/#12 because the model routed the "no real finding" signal into whichever field the filter didn't inspect — the title ("N/A — …", "…is correct"), the suggestion ("No action needed — …"), or a rambling scenario's conclusion ("…No bug here."). The filter tested only failure_scenario prefixes. Filter (deterministic layer): - title, failure_scenario, and suggestion all tested against the existing prefix set plus a new separator-anchored confirmation set (none / no failure / no concrete failure scenario / no bug / no action needed / no change needed). The required separator ([—–:.-] or end-of-text) keeps real scenarios like "No failure occurs until…" and "None of the guards…" safe — and lets "none" return, resolving the PR #9 prompt/filter drift in the filtering direction. - Declarative confirmation titles ("…is correct" / "…is accurate") and leaked scenario conclusions ("…no bug here", "…analysis was wrong") drop via end-anchored suffixes. Prompt (advisory layer): OUTPUT_DISCIPLINE bans declarative confirmation titles + verification-task titles, and gains a suggestion-format line — a suggestion of "no bug"/"no action needed"/"N/A" means no finding. README: new Non-finding filter section documenting the drop contract and the anchoring rationale; What-it-does + How-it-works + Shipped surfaces updated in lockstep. Accepted tradeoffs (user-approved): genuine "Placeholder text …" titles and imperative "Ensure the timeout is correct" titles would drop — zero observed instances in ~70 bot findings vs 6 observed escapes. 17 new tests on the real escaped strings (398 total); 3 mutation checks verified (title-suffix rule, separator anchoring, title field removal each fail exactly their covering tests). --- README.md | 14 +- .../__tests__/filter-non-findings.test.ts | 186 ++++++++++++++++++ src/review/__tests__/prompt.test.ts | 9 + src/review/filter-non-findings.ts | 39 +++- src/review/prompt.ts | 7 +- 5 files changed, 250 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index c49bb1b..9194cd9 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,7 @@ LLM-powered pull request review as a GitHub Action. One consolidated review per - Reads your repo's conventions file (`AGENTS.md` by default) and reviews against it - Posts exactly **one** PR review with inline comments anchored to diff lines — no duplicate comments, no unrequested-reviewer badges - Structured output end to end: every finding carries a category, severity, confidence, and a concrete failure scenario +- **Drops non-findings before they post** — findings that conclude "no bug here" (an `N/A — …` title, a "no action needed" suggestion, a "…is correct" title) are filtered deterministically - Model-agnostic via OpenRouter — pick your model, see your per-call costs - Findings that can't be anchored to the diff (e.g. callers outside the changed files) are posted as standalone comments on the PR - PRs with oversized diffs are skipped gracefully with a body-only review stating the reason @@ -106,11 +107,21 @@ The `@umm review` comment trigger lets you re-request a review on any PR by comm 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 5. Validates the response against a strict Zod schema, retrying with a fallback model if the primary fails -6. Filters findings by severity threshold, deduplicates overlapping findings, and caps if configured +6. Drops non-findings (see [Non-finding filter](#non-finding-filter)), filters by severity threshold, deduplicates overlapping findings, and caps if configured 7. On re-runs, compares findings against previously posted inline comments (by hidden HTML anchor) and filters out duplicates 8. Maps findings to inline PR review comments anchored to diff lines, with a snap-to-nearest-hunk fallback 9. Posts one review with inline comments (invisible body); beyond-diff findings post as standalone PR comments; every run upserts a status comment with cross-run totals +## Non-finding filter + +Models sometimes report "findings" that conclude the code is fine — titled `N/A — …` or `…is correct`, with suggestions like "No action needed". The system prompt prohibits these, but models don't always comply, so every finding also passes a deterministic filter before severity threshold, cross-run dedup, and cap. A finding is dropped when: + +- its **title**, **failure_scenario**, or **suggestion** starts with a non-finding signal — `N/A`, `not applicable`, `placeholder`, or a separator-delimited confirmation phrase (`none — …`, `no failure — …`, `no concrete failure scenario — …`, `no bug — …`, `no action needed — …`, `no change needed — …`) +- its **title** ends with a declarative confirmation — `…is correct` or `…is accurate` +- its **failure_scenario** ends with a leaked conclusion — `…no bug here` or `…analysis was wrong` + +The patterns are deliberately anchored to the start or end of a field, so real findings survive: a scenario like "No failure occurs until the third retry…" or "None of the guards catch this input" never matches. Dropped counts are logged per run (`filtered non-findings`). + ## Status umm-actually is in early development — the core review pipeline works but there's more to build. Here's what's shipped and what's in progress: @@ -127,6 +138,7 @@ umm-actually is in early development — the core review pipeline works but ther - 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 +- 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** diff --git a/src/review/__tests__/filter-non-findings.test.ts b/src/review/__tests__/filter-non-findings.test.ts index 2581280..903118b 100644 --- a/src/review/__tests__/filter-non-findings.test.ts +++ b/src/review/__tests__/filter-non-findings.test.ts @@ -180,4 +180,190 @@ describe("filterNonFindings", () => { expect(result).toEqual({ findings: [], droppedAsNonFinding: 0 }) }) + + // --- Title signals (observed escapes on PRs #10/#12) --- + + it("drops a finding whose title starts with 'N/A' (observed on PR #12)", () => { + const finding = makeFinding({ + title: "N/A — maintains unposted design (ignore)", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + it("drops a finding whose title starts with 'Placeholder'", () => { + const finding = makeFinding({ + title: "Placeholder — re-evaluate after the refactor lands", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + it("drops a finding whose title ends with 'is correct' (observed on PR #10)", () => { + const finding = makeFinding({ + title: "Extensionless path handling in posix.basename is correct", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + it("drops a finding whose title ends with 'is accurate' (observed on PR #10)", () => { + const finding = makeFinding({ + title: "README doc-mention description is accurate", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + // --- Confirmation phrases: separator-anchored (observed escapes) --- + + it("drops a finding whose failure_scenario starts with 'No failure —' (observed on PR #10)", () => { + const finding = makeFinding({ + failure_scenario: + "No failure — the code handles extensionless paths correctly by falling through to the basename match.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + it("drops a finding whose failure_scenario starts with 'No concrete failure scenario —' (observed on PR #10)", () => { + const finding = makeFinding({ + failure_scenario: + "No concrete failure scenario — `fs.stat` handles forward-slash paths on Windows. This finding should be OMITTED under NOISE SUPPRESSION.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + it("drops a finding whose failure_scenario starts with 'None —'", () => { + const finding = makeFinding({ + failure_scenario: "None — the code handles this edge case.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + it("drops a finding whose confirmation phrase is separated by a plain hyphen", () => { + const finding = makeFinding({ + failure_scenario: "No bug - the guard already covers this input.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + it("drops a finding whose failure_scenario ends with 'No bug here.' (observed on PR #10)", () => { + const finding = makeFinding({ + failure_scenario: + "After re-tracing, the guard handles EOF correctly. The code is correct; my initial analysis was wrong. The comment and code are aligned. No bug here.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + // --- Suggestion signals (observed escapes) --- + + it("drops a finding whose suggestion starts with 'N/A' (observed on PR #12)", () => { + const finding = makeFinding({ + suggestion: "N/A — designed behavior per PR description.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + it("drops a finding whose suggestion starts with 'No bug —' (observed on PR #10)", () => { + const finding = makeFinding({ + suggestion: "No bug — code is correct.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + it("drops a finding whose suggestion starts with 'No action needed —' (observed on PR #10)", () => { + const finding = makeFinding({ + suggestion: + "No action needed — the code is correct. This is a documentation note: the budget subtraction correctly tracks only included files.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + it("drops a finding whose suggestion starts with 'No change needed —' (observed on PR #10)", () => { + const finding = makeFinding({ + suggestion: + "No change needed — the existing code is safe because Node.js `fs.stat` normalizes path separators on all platforms. This is a note, not a fix.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + + // --- Separator anchoring: near-misses must survive --- + + it("keeps a finding whose failure_scenario starts with 'No failure occurs until'", () => { + const finding = makeFinding({ + failure_scenario: + "No failure occurs until the third retry aborts the batch.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [finding], droppedAsNonFinding: 0 }) + }) + + it("keeps a finding whose suggestion is a concrete change", () => { + const finding = makeFinding({ + suggestion: "Add a null check before dereferencing the config value.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [finding], droppedAsNonFinding: 0 }) + }) + + it("keeps a finding with a verification-style title but real scenario and suggestion", () => { + const finding = makeFinding({ + title: "Verify the timeout budget handles overflow", + suggestion: "Clamp the timeout to the configured maximum.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [finding], droppedAsNonFinding: 0 }) + }) + + it("keeps a finding whose failure_scenario mentions 'no bug' mid-sentence", () => { + const finding = makeFinding({ + failure_scenario: + "The fallback swallows the error, so callers see no bug reports but data is lost.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [finding], droppedAsNonFinding: 0 }) + }) }) diff --git a/src/review/__tests__/prompt.test.ts b/src/review/__tests__/prompt.test.ts index 13cefd4..ad4e810 100644 --- a/src/review/__tests__/prompt.test.ts +++ b/src/review/__tests__/prompt.test.ts @@ -84,6 +84,15 @@ describe("buildSystemPrompt", () => { ) }) + it("bans declarative confirmation titles and non-action suggestions in the output discipline", () => { + const systemPrompt = buildSystemPrompt({ phase: combinedPhase }) + + expect(systemPrompt).toContain("A declarative confirmation title") + expect(systemPrompt).toContain( + '"suggestion": a concrete code change, or null', + ) + }) + it("places OUTPUT DISCIPLINE between SEVERITY_RUBRIC and ANCHORING_CONTRACT", () => { const systemPrompt = buildSystemPrompt({ phase: combinedPhase }) diff --git a/src/review/filter-non-findings.ts b/src/review/filter-non-findings.ts index 8b99ee7..e965b05 100644 --- a/src/review/filter-non-findings.ts +++ b/src/review/filter-non-findings.ts @@ -5,11 +5,44 @@ export type FilterResult = { droppedAsNonFinding: number } -/** Matches failure_scenario prefixes that signal "no real finding" — start-anchored, case-insensitive, word-bounded. */ +/** Matches field prefixes that signal "no real finding" — start-anchored, case-insensitive, word-bounded. */ const NON_FINDING_PREFIX = /^(?:n\/?a|not applicable|placeholder)\b/i -const isNonFinding = (finding: Finding): boolean => - NON_FINDING_PREFIX.test(finding.failure_scenario.trim()) +/** Confirmation phrases that signal "no real finding" when they START a + * field. The phrase must be followed by a separator or end-of-text so real + * scenarios survive — "No failure occurs until the third retry…" and "None + * of the guards catch this input" never match, while observed confirmations + * (always separator-delimited, e.g. "No failure — the code handles…") do. */ +const CONFIRMATION_PREFIX = + /^(?:none|no failure|no concrete failure scenario|no bug|no action needed|no change needed)\s*(?:[—–:.-]|$)/i + +/** Declarative confirmation titles — "…is correct" / "…is accurate" — end-anchored. */ +const CONFIRMATION_TITLE_SUFFIX = /\bis\s+(?:correct|accurate)\s*\.?\s*$/i + +/** Conclusions leaked to the end of a rambling failure_scenario + * ("…No bug here.", "…my analysis was wrong.") — self-referential verdicts + * that never legitimately end a failure description. */ +const CONFIRMATION_SCENARIO_SUFFIX = + /(?:\bno bug(?: here)?|\banalysis was wrong)\s*\.?\s*$/i + +/** Start-of-field signal shared by title, failure_scenario, and suggestion: + * an explicit non-finding prefix or a separator-anchored confirmation phrase. */ +const hasNonFindingSignal = (text: string): boolean => { + return NON_FINDING_PREFIX.test(text) || CONFIRMATION_PREFIX.test(text) +} + +const isNonFinding = (finding: Finding): boolean => { + const title = finding.title.trim() + const failureScenario = finding.failure_scenario.trim() + if (hasNonFindingSignal(title)) return true + if (CONFIRMATION_TITLE_SUFFIX.test(title)) return true + if (hasNonFindingSignal(failureScenario)) return true + if (CONFIRMATION_SCENARIO_SUFFIX.test(failureScenario)) return true + if (finding.suggestion && hasNonFindingSignal(finding.suggestion.trim())) { + return true + } + return false +} export const filterNonFindings = (findings: Finding[]): FilterResult => { const realFindings = findings.filter((finding) => !isNonFinding(finding)) diff --git a/src/review/prompt.ts b/src/review/prompt.ts index ecd1afc..b1b778e 100644 --- a/src/review/prompt.ts +++ b/src/review/prompt.ts @@ -52,10 +52,15 @@ failure scenario, do not report the finding.` const OUTPUT_DISCIPLINE = `OUTPUT DISCIPLINE — field constraints: - "title": imperative fix statement, under 80 characters (e.g. "Trim keys before inserting into the registry"). Do not start with "Issue:" or - "Bug:". + "Bug:". A declarative confirmation title ("X is correct", "X is + accurate", "N/A — …") or a verification task ("Verify X handles Y") is + not a finding — do not emit it. - "description": 1–3 sentences stating the defect and its impact. No code tracing, no call-chain walk-through, no quoting of source lines. Put traces and evidence in "analysis", not here. +- "suggestion": a concrete code change, or null when a fix is genuinely + optional. A suggestion of "no bug", "no action needed", or "N/A" means + there is no finding — do not emit it. - "failure_scenario": a concrete input or state that triggers the problem and what goes wrong. A failure_scenario starting with "N/A", "None", "Not applicable", or "Placeholder" means there is no real finding — do From 2eac0a492c35f8ef08730720ebaf44e550e2a719 Mon Sep 17 00:00:00 2001 From: Tanisha Aberdeen <32620895+aliasunder@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:56:25 -0400 Subject: [PATCH 2/5] fix(review): document optional "here" in no-bug scenario suffix The CONFIRMATION_SCENARIO_SUFFIX regex uses `\bno bug(?: here)?` which matches both "...no bug" and "...no bug here", but the README only listed the latter. Add both variants so the filter contract matches the code. Co-Authored-By: Claude Opus 4.6 (1M context) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9194cd9..271b05f 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ Models sometimes report "findings" that conclude the code is fine — titled `N/ - its **title**, **failure_scenario**, or **suggestion** starts with a non-finding signal — `N/A`, `not applicable`, `placeholder`, or a separator-delimited confirmation phrase (`none — …`, `no failure — …`, `no concrete failure scenario — …`, `no bug — …`, `no action needed — …`, `no change needed — …`) - its **title** ends with a declarative confirmation — `…is correct` or `…is accurate` -- its **failure_scenario** ends with a leaked conclusion — `…no bug here` or `…analysis was wrong` +- its **failure_scenario** ends with a leaked conclusion — `…no bug` / `…no bug here` or `…analysis was wrong` The patterns are deliberately anchored to the start or end of a field, so real findings survive: a scenario like "No failure occurs until the third retry…" or "None of the guards catch this input" never matches. Dropped counts are logged per run (`filtered non-findings`). From 563d807fdc2b9383999946509fe10b5429189f51 Mon Sep 17 00:00:00 2001 From: Tanisha Aberdeen <32620895+aliasunder@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:01:21 -0400 Subject: [PATCH 3/5] style: use truthy check for optional reason attribute Co-Authored-By: Claude Opus 4.6 (1M context) --- src/review/prompt.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/review/prompt.ts b/src/review/prompt.ts index b1b778e..cc7c158 100644 --- a/src/review/prompt.ts +++ b/src/review/prompt.ts @@ -117,10 +117,9 @@ const renderFileBlock = (file: PromptFile, delimiterNonce: string): string => { if (file.includedAs === "diff-only") { return `<${fileTag} path="${pathAttribute}" note="full content omitted: too large — see diff">\n` } - const reasonAttribute = - file.reason === undefined - ? "" - : ` reason="${escapeAttributeValue(file.reason)}"` + const reasonAttribute = file.reason + ? ` reason="${escapeAttributeValue(file.reason)}"` + : "" return `<${fileTag} path="${pathAttribute}"${reasonAttribute}>\n${file.content}\n` } From b78bcfde68f591556d5e9bccadc9e2b7a9a4c9d9 Mon Sep 17 00:00:00 2001 From: Tanisha Aberdeen <32620895+aliasunder@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:08:32 -0400 Subject: [PATCH 4/5] test: cover CONFIRMATION_SCENARIO_SUFFIX 'analysis was wrong' and title-field CONFIRMATION_PREFIX Two coverage gaps found during test audit: - The 'analysis was wrong' alternative in CONFIRMATION_SCENARIO_SUFFIX had no end-of-string test (the existing test matched 'No bug here.' instead) - CONFIRMATION_PREFIX on the title field was untested (only NON_FINDING_PREFIX was exercised on titles) Both mutation-verified: removing each regex alternative fails exactly the new test. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../__tests__/filter-non-findings.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/review/__tests__/filter-non-findings.test.ts b/src/review/__tests__/filter-non-findings.test.ts index 903118b..0355478 100644 --- a/src/review/__tests__/filter-non-findings.test.ts +++ b/src/review/__tests__/filter-non-findings.test.ts @@ -278,6 +278,17 @@ describe("filterNonFindings", () => { expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) }) + it("drops a finding whose failure_scenario ends with 'analysis was wrong.'", () => { + const finding = makeFinding({ + failure_scenario: + "On closer inspection, the boundary check is fine. My analysis was wrong.", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + // --- Suggestion signals (observed escapes) --- it("drops a finding whose suggestion starts with 'N/A' (observed on PR #12)", () => { @@ -322,6 +333,18 @@ describe("filterNonFindings", () => { expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) }) + // --- Title-field confirmation prefix (path A via CONFIRMATION_PREFIX) --- + + it("drops a finding whose title starts with 'No bug —'", () => { + const finding = makeFinding({ + title: "No bug — the fallback is intentional", + }) + + const result = filterNonFindings([finding]) + + expect(result).toEqual({ findings: [], droppedAsNonFinding: 1 }) + }) + // --- Separator anchoring: near-misses must survive --- it("keeps a finding whose failure_scenario starts with 'No failure occurs until'", () => { From 2b232c46b27d04a467bf2b57dfc44bd8c109a905 Mon Sep 17 00:00:00 2001 From: Tanisha Aberdeen <32620895+aliasunder@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:27:43 -0400 Subject: [PATCH 5/5] test: replace prompt fragment assertions with full-constant checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OUTPUT_DISCIPLINE and ANCHORING_CONTRACT assertions used multiple toContain calls with short phrases — violating the convention that deterministic output gets exact assertions. Replaced with single toContain of each full constant text. AGENTS.md test conventions now explicitly address the large-string case: assert the full section in one toContain, not multiple fragments. Co-Authored-By: Claude Opus 4.6 (1M context) --- AGENTS.md | 8 +++-- src/review/__tests__/prompt.test.ts | 45 ++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 38e445f..1404261 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,9 +117,11 @@ files. Prefer SDK-provided types over redefining shapes. - Exact assertions over loose matchers; assert whole values over substrings when output is deterministic. When fixtures and stubs produce deterministic results, assert the entire return value or call params — not just individual - fields. Asserting fragments is the cheap option; asserting the whole value - catches drift in formatting, structure, and attribution that field-level - checks miss. + fields. For large deterministic strings (prompts, rendered output), assert + the full section or constant in one `toContain` — not multiple fragments + that each check a phrase. Asserting fragments is the cheap option; + asserting the whole value catches drift in formatting, structure, and + attribution that field-level checks miss. - Two-bar rule: a test must (1) fail when the behavior breaks and (2) pass only because the intended behavior occurred. Four traps against bar 2: **silent no-op** (assert the trigger happened, not just that state was diff --git a/src/review/__tests__/prompt.test.ts b/src/review/__tests__/prompt.test.ts index ad4e810..743f504 100644 --- a/src/review/__tests__/prompt.test.ts +++ b/src/review/__tests__/prompt.test.ts @@ -68,28 +68,45 @@ describe("buildSystemPrompt", () => { expect(systemPrompt).toContain("For CI/workflow files") expect(systemPrompt).toContain('fill the "analysis" field') expect(systemPrompt).toContain("Severity rubric:") - expect(systemPrompt).toContain("OUTPUT DISCIPLINE") expect(systemPrompt).toContain( - "imperative fix statement, under 80 characters", - ) - expect(systemPrompt).toContain( - "1–3 sentences stating the defect and its impact", - ) - expect(systemPrompt).toContain( - 'starting with "N/A", "None",\n "Not applicable", or "Placeholder"', - ) - expect(systemPrompt).toContain("HARD PROHIBITION") - expect(systemPrompt).toContain( - "new-file line numbers printed in\nthe annotated diff", + [ + 'Line anchoring: "line" and "end_line" use the new-file line numbers printed in', + "the annotated diff. For inline placement, reference only numbers that appear", + "there, and keep end_line in the same hunk as line. Findings in code outside", + "the diff (traced regressions, pre-existing bugs) are still valuable — report", + "them with their real file and line; they are rendered in the review body", + "instead of inline.", + ].join("\n"), ) }) it("bans declarative confirmation titles and non-action suggestions in the output discipline", () => { const systemPrompt = buildSystemPrompt({ phase: combinedPhase }) - expect(systemPrompt).toContain("A declarative confirmation title") expect(systemPrompt).toContain( - '"suggestion": a concrete code change, or null', + [ + "OUTPUT DISCIPLINE — field constraints:", + '- "title": imperative fix statement, under 80 characters (e.g. "Trim keys', + ' before inserting into the registry"). Do not start with "Issue:" or', + ' "Bug:". A declarative confirmation title ("X is correct", "X is', + ' accurate", "N/A — …") or a verification task ("Verify X handles Y") is', + " not a finding — do not emit it.", + '- "description": 1–3 sentences stating the defect and its impact. No code', + " tracing, no call-chain walk-through, no quoting of source lines. Put", + ' traces and evidence in "analysis", not here.', + '- "suggestion": a concrete code change, or null when a fix is genuinely', + ' optional. A suggestion of "no bug", "no action needed", or "N/A" means', + " there is no finding — do not emit it.", + '- "failure_scenario": a concrete input or state that triggers the problem', + ' and what goes wrong. A failure_scenario starting with "N/A", "None",', + ' "Not applicable", or "Placeholder" means there is no real finding — do', + " not emit the finding at all.", + "", + 'HARD PROHIBITION — do not report a finding whose conclusion is "no bug",', + '"this is correct", "working as designed", "correct behavior", or any', + "equivalent. If your analysis concludes the code is correct, record that", + 'conclusion in "analysis" and move on — do not emit a finding for it.', + ].join("\n"), ) })