From 259bf0960972e91751c00a73545cff103f42ae07 Mon Sep 17 00:00:00 2001 From: Codevena Date: Wed, 5 Aug 2026 22:13:07 +0200 Subject: [PATCH 01/14] =?UTF-8?q?docs:=20spec=20for=20closing=20the=20true?= =?UTF-8?q?-positive=20hole=20=E2=80=94=20anchor=20integrity=20+=20critic?= =?UTF-8?q?=20severity=20floor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2026-08-05-true-positive-hole-design.md | 263 ++++++++++++++++++ 1 file changed, 263 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-05-true-positive-hole-design.md diff --git a/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md b/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md new file mode 100644 index 0000000..17e7e1b --- /dev/null +++ b/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md @@ -0,0 +1,263 @@ +# Closing the true-positive hole pilot-02 exposed + +_2026-08-05. Task (b) from `NEXT_SESSION.md`, following +`docs/dev/2026-08-05-pilot-02-result.md` and the measurement fixes in +`docs/superpowers/plans/2026-08-05-rig-measurement-fixes.md` (task (a), `5a1f94f`)._ + +## The evidence this design starts from + +pilot-02's turn 2 seeded a path traversal. The panel **detected it twice**. Both detections +ended at INFO, the turn recorded 0 blocking findings, and M3 scored a miss. + +Read the recorded findings yourself: + +```bash +bun -e 'const j = JSON.parse(await Bun.file("rig/results/pilot-02/turns/2/.reviewgate/pending.json").text()); +for (const f of j.findings) console.log(f.id, f.rule_id, f.severity, f.category, f.file, f.line_start, f.confidence, f.consensus, f.critic_verdict ?? "", f.scope_demoted ?? "")' +sed -n '1,10p' rig/results/pilot-02/turns/2/diff.patch +``` + +| | rule_id | line_start | conf | consensus | what demoted it | +|---|---|---|---:|---|---| +| F-001 | `path-traversal-readtemplate` | **25** | 0.55 | singleton | critic → `likely_fp` | +| F-002 | `path-traversal` | **67** | 0.90 | singleton | `scope_demoted` | + +`src/store.ts` is a **brand-new 27-line file** in that turn's diff (`@@ -0,0 +1,27 @@`), so +every line of it is inside the changed ranges. **F-002 is anchored at line 67 — past EOF of a +file that is entirely in-diff.** Nothing in `src/providers/review-output.ts` or +`src/schemas/finding.ts` validates a line anchor against the file it names; `FindingSchema` only +requires `z.number().int().positive()`. + +That single bad anchor produced all three of the failures the pilot-02 write-up attributes to +independent mechanisms: + +1. **F-002 was scope-demoted** (`aggregator.ts:352`) as "outside the changed lines". It was never + out-of-diff — it was mis-anchored. +2. **The two findings never merged.** 25 and 67 are 42 lines apart, past both `REGION_WINDOW` + (5, `:192`) and `WORDING_MERGE_MAX_LINE_DISTANCE` (25, `:229`). +3. **No merge meant no corroboration.** `computeConsensus(1, 2)` is `"singleton"`; merged, it + would be `computeConsensus(2, 2)` → `"majority"` → `isCorroborated` at `:610` → **the critic + could not have demoted F-001 at all.** + +So the handoff's candidate (2) — "merge same-file/same-category detections before the critic" — +addresses a symptom. These two findings did not need a broader merge rule; they needed one +reviewer not to invent a line number. + +The handoff's candidate (1) remains a real defect on its own merit, independent of turn 2: the +critic's exemption is keyed to CRITICAL (`:604`), while the sibling `deltaScoped` pass exempts +`touchesSecurityOrCorrectness` at **any** severity (`:664`). A WARN-severity security finding is +demotable by a single adversarial critic, and that demote crosses the blocking boundary. + +## Scope + +Two independent slices, both in `src/core/aggregator.ts`, neither touching the reviewer or +provider layer. Both are **always-on** — they are defect corrections, and the sibling +protections they mirror (G0's clamp at `:155`, `isCriticalSecurity` at `:604`, delta-scope's +exemption at `:664`) are all unflagged. Observability comes from markers a pilot can count, not +from config toggles; no new config key, and therefore no second TTY control-plane approval. + +### Explicitly out of scope + +- **Merge/clustering changes.** Widening the merge to same-file/same-category would bundle + genuinely separate security bugs under one decision, which `isHighStakesCategory` (`:279`) + exists to prevent. +- **An invalid anchor still participates in region clustering**, so a bogus line 67 could merge + with a real finding at line 65. This risk exists today, did not fire in turn 2, and removing + it means changing merge behaviour — a third change with its own blast radius. +- **M5 critic cost attribution** (`orchestrator.ts:2300`) stays scoped out; it is a + provider-contract change, as recorded in the task-(a) plan. + +## Slice A — anchor integrity + +Policy, chosen deliberately: **an out-of-range anchor is treated exactly like a missing one.** +`scopeFindings` already carries that precedent in the same function — +`if (!f.line_start) return f; // no usable line → keep (conservative)` (`:330`). A reviewer that +cites line 67 of a 27-line file has given us no usable location, and the fail-safe direction in +this codebase is that a suppressor must never silently soften a finding it cannot place. + +The rejected alternative is re-anchoring the finding into the nearest changed range. It would +additionally recover the merge path, but the gate would then be asserting a location the +reviewer never gave — the gate would be lying about where the bug is. + +### Detect + +A new pass immediately after the path-normalize / `demoteRedaction` map (`:429-432`), before the +deterministic sort and clustering. For each finding whose file has a **known** line count: + +```ts +if (lineCount !== undefined && f.line_start > lineCount) → { ...f, anchor_invalid: true } +``` + +Stamp only. No severity change here, so the pass is inert on its own and its effect is entirely +attributable to the consumer below. + +The pass runs **unconditionally**, not inside `scopeFindings` — that function returns early when +`scopeToDiff` is false or `changedRanges` is absent (`:317`). Detecting separately means a +mis-anchoring reviewer is still visible in `pending.md` on a repo that has diff-scoping turned +off, where the flag changes nothing else. + +**`line_end` does not participate.** Reviewers routinely over-estimate `line_end` as a sloppy +range hint; if `line_start` names a real line the finding *is* anchored somewhere real, and +`rangeOverlapsChanged` (`src/diff/hunks.ts:116`) already handles the range. Stamping on +`line_end` would fire on ordinary findings and switch off scope-demoting broadly — a fail-open, +which is the exact direction this slice exists to close. Guard test 2 pins this. + +### Act + +`scopeFindings:330` becomes: + +```ts +if (!f.line_start || f.anchor_invalid) return f; // no usable line → keep (conservative) +``` + +### Input + +`AggregateInput` gains: + +```ts +/** Line counts of the files this diff changed, for anchor validation. An entry is present + * ONLY when the file was read successfully. Absent map, or an absent entry, means UNKNOWN — + * no anchor on that file is ever stamped, and behaviour is byte-identical to before this + * field existed. Same optional-input compatibility rule `seedLanded` and `criticRuns` follow. */ +fileLineCounts?: Map; +``` + +The orchestrator builds it at the `aggregate()` call site (`orchestrator.ts:2451`) from the +changed-file set, using `readFileSync` under `this.input.repoRoot` (already imported at +`orchestrator.ts:7`). Keys are `normalizeRepoPath`-canonical, matching `changedRanges`. + +### Schema and rendering + +`FindingSchema` gains `anchor_invalid: z.boolean().optional()`, alongside `scope_demoted` +(`finding.ts:81`) and `protected_high_precision` (`:191`). + +`report-writer.ts` gains one badge next to the existing `📍 outside changed lines` (`:57`), e.g. +`⚑ the reviewer cited a line that does not exist in this file — location unverified, kept +blocking`. The badge is the diagnostic that made this whole slice findable; without it a +mis-anchoring reviewer is invisible. + +## Slice B — critic severity floor + +`aggregator.ts:604` gains a sibling to `isCriticalSecurity`: + +```ts +const isCriticalSecurity = f.severity === "CRITICAL" && touchesSecurityOrCorrectness(f); +// The critic may not push a security/correctness finding BELOW WARN — that is the one demote +// that crosses the blocking boundary. An already-INFO one stays droppable, so the critic keeps +// its FP-filtering power exactly where reviewers are noisiest. +const isBlockingSecurity = f.severity === "WARN" && touchesSecurityOrCorrectness(f); +``` + +Both feed the same two branches at `:615` and `:619`. A protected finding therefore takes the +existing `survivors.push({ ...f, critic_verdict: "keep" })` path at `:635`, which already renders +honestly — no new marker needed. + +**Why the floor stops at WARN and not lower.** The stated harm is a demote that crosses the +blocking boundary (`isBlocking` is `CRITICAL || WARN`). WARN → INFO crosses it; INFO → drop does +not, and low-confidence INFO security chatter is the noisiest thing the critic filters. Making +security wholly critic-immune would re-inflate FP burden in a way pilot-02 has **zero data** on +(`rejectedAsFp` was 0 on every turn, `known_fp.jsonl` ended the run empty). Guard test 5 pins +that the floor does not over-apply. + +**Blast radius.** `demoteOneStep` (`:155`) is unchanged — a WARN security finding simply never +reaches it from the critic pass. The reputation and confidence-floor passes are untouched; they +already carry their own hard security veto (`touchesSecurity`, `:285`). + +## Data flow + +``` +reviewers → findings (line_start unvalidated) +orchestrator:2451 changedRanges = parseChangedRanges(diff) + fileLineCounts = line counts of changed files ← new +aggregate(): normalize paths + demoteRedaction + markInvalidAnchors ← Slice A detect + sort → cluster → dedupe → consensus + critic pass ← Slice B floor + scopeFindings ← Slice A act + deltaScope → fp-ledger → reputation → verdict +report-writer: anchor_invalid badge +``` + +## Fail-safety + +Each row states its failure direction, not just its behaviour. + +| Condition | Behaviour | Why | +|---|---|---| +| `fileLineCounts` absent entirely | Both slices inert; output byte-identical to today | Optional-input compatibility, as `seedLanded`/`criticRuns` | +| File missing, unreadable, binary, or over a byte cap | **No entry** → anchors on that file never stamped | Unknown must stay unknown; never guess a line count | +| Finding on a file the diff did not change | No entry → demotes on the file-absent branch (`:332`) as today | Anchor validation must not rescue findings on unchanged files | +| `line_start` valid, `line_end` past EOF | **Not stamped** | Stamping here would fail open; see Slice A | +| Anchor invalid **and** severity already INFO | Kept and flagged, and **`scope_demoted` is no longer stamped** on it | `demote()` (`:325`) stamps the marker even on INFO, and `report-writer.ts:498` buckets on it. Such a finding moves from the "outside changed lines" list to the in-scope advisory list — correct, since we do not know it is outside. A deliberate, visible consequence, not an oversight | + +## Measurability, stated honestly + +`SUPPRESSION_LAYERS` (`src/rig/ablate.ts:44`) is `["critic","reputation","fp-ledger","lore"]` — +there is no `scope` or `anchor` layer, and the ablation reconstructs a layer's counterfactual by +reading the demotion markers it left behind. + +- **Slice B stays fully ablatable.** A protected finding carries `critic_verdict: "keep"`, and + `−critic` shows no recall delta where the floor held. +- **Slice A is observable, not ablatable.** pilot-03 can count `anchor_invalid` findings and how + many stayed blocking; it cannot produce a counterfactual matrix row for the slice. The pilot-03 + write-up must say this rather than let the matrix imply coverage it does not have. + +## Testing + +Every guard test carries the two numbers of the quantity it guards. A test whose two values match +is vacuous **on paper** and is rewritten before it is written. + +| # | Guards | WITHOUT the mechanism | WITH it | +|---|---|---|---| +| 1 | Invalid anchor is not scope-demoted (turn-2 shape: 27-line new file, finding at line 67) | INFO + `scope_demoted` → **0 blocking** | WARN + `anchor_invalid` → **1 blocking** | +| 2 | Does not over-apply — a *valid* out-of-diff anchor still demotes. Mutation: stamp on `line_end > lineCount` | over-broad predicate → **1 blocking** | correct predicate → **0 blocking** | +| 3 | Inert without `fileLineCounts` — one fixture, map present vs absent | map absent → **0 blocking** | map present → **1 blocking** | +| 4 | WARN + security + `likely_fp` survives the critic | INFO + `critic_verdict: likely_fp` → **0 blocking** | WARN + `critic_verdict: keep` → **1 blocking** | +| 5 | Floor does not over-apply — INFO + security + `likely_fp` is still dropped | "exempt at every severity" variant → `criticDropped` **0** | correct floor → `criticDropped` **1** | +| 6 | Floor is category-keyed, not severity-keyed — WARN + *quality* still demotes | severity-only variant → **1 blocking** | correct floor → **0 blocking** | + +Each is seen **red** first, in a copy of the repo; the original is confirmed unmodified with +`git diff` after each copy is discarded. + +**Acceptance on real data, labelled honestly.** A fixture reconstructed from turn 2's recorded +`pending.json` — both findings, their actual lines, categories, confidences and consensus — +showing the turn flip from 0 blocking to blocking. This is a **reconstruction, not a replay**: +the archived findings are post-aggregation, so the pre-aggregation input is inferred from the +demotion markers, exactly as `ablate.ts` does. The write-up must not call it a replay. + +**Static gates.** `bunx tsc --noEmit`, `bun run lint`, and the full `bun test` — `FindingSchema` +changes, so the persisted-artifact suite runs whole. + +**Reviews.** A plan gate with an **executing** reviewer before implementation, then the +post-implementation pipeline with two independent slots. Codex is quota-blocked until +**2026-08-08T11:07Z**, so Slot A is `agy`/Gemini or a Claude reviewer subagent, and Slot B a +second, different voice. + +## Sequencing for pilot-03 + +In this order. The trap is that (b) changes gate behaviour, so it only reaches a pilot through a +rebuild — and the rebuild re-pins the binary. + +1. Implement, pass both review gates, commit. +2. `bun run build`; record the new `sha256`. **This deploys to every repo via the + `~/.local/bin/reviewgate` symlink** — the whole machine's gate behaviour changes at that + moment, not just the sandbox's. +3. Preregister pilot-03 against the **new** hash, with every floor written as a **rate**, never a + count (pilot-02's M3 floor was miswritten as a count). +4. Run. Never rebuild mid-run. +5. Expect a landed-seed denominator of **3**, not 5 — the agent declined the SQL-injection and + hardcoded-secret prompts in both pilots. + +The preregistration is a separate artifact, written after the build against the new hash. It is +not part of this spec. + +## Risks + +| Risk | Handling | +|---|---| +| Slice A rescues findings it should not, weakening diff-scoping generally | Only `line_start` past a **known** EOF stamps; unknown stays unknown. Guard tests 2 and 3 pin both over-application modes | +| Slice B re-inflates FP burden | Floor stops at WARN; already-INFO security stays droppable (guard 5). pilot-03 reports FP burden, though pilot-02 showed M2 has no signal in this rig | +| `fileLineCounts` costs I/O on every gate run | Bounded to the changed-file set, one `readFileSync` each, byte-capped; the research phase already reads these files | +| `FindingSchema` change breaks older persisted artifacts | Field is `.optional()`, mirroring `scope_demoted`/`protected_high_precision` | +| pilot-03 conflates the two slices | Slice B is isolated by the existing `−critic` ablation; Slice A is reported as a count of `anchor_invalid`. Stated as a limitation, not papered over | +| n = 3 landed seeds cannot establish an effect size | Same limit as pilot-02, restated in the write-up. This detects signal, not magnitude | From 0a32bf74d03748c51394545f7f039fa47ce0dd88 Mon Sep 17 00:00:00 2001 From: Codevena Date: Wed, 5 Aug 2026 22:29:09 +0200 Subject: [PATCH 02/14] =?UTF-8?q?docs:=20revise=20spec=20=E2=80=94=20the?= =?UTF-8?q?=20real=20defect=20is=20fact-check=20calling=20a=20mis-anchored?= =?UTF-8?q?=20finding=20fabricated?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2026-08-05-true-positive-hole-design.md | 317 ++++++++++-------- 1 file changed, 186 insertions(+), 131 deletions(-) diff --git a/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md b/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md index 17e7e1b..505041b 100644 --- a/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md +++ b/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md @@ -4,6 +4,12 @@ _2026-08-05. Task (b) from `NEXT_SESSION.md`, following `docs/dev/2026-08-05-pilot-02-result.md` and the measurement fixes in `docs/superpowers/plans/2026-08-05-rig-measurement-fixes.md` (task (a), `5a1f94f`)._ +> **Revision note.** A first version of this spec proposed a new anchor-validation pass in +> `aggregator.ts`. That was wrong: `src/core/fact-check.ts` already validates line anchors, it +> already fired on the finding in question, and the new pass would have silently reversed a +> deliberate, documented policy. The design below replaces it. What survives from the first +> version is Slice B, unchanged. + ## The evidence this design starts from pilot-02's turn 2 seeded a path traversal. The panel **detected it twice**. Both detections @@ -13,128 +19,163 @@ Read the recorded findings yourself: ```bash bun -e 'const j = JSON.parse(await Bun.file("rig/results/pilot-02/turns/2/.reviewgate/pending.json").text()); -for (const f of j.findings) console.log(f.id, f.rule_id, f.severity, f.category, f.file, f.line_start, f.confidence, f.consensus, f.critic_verdict ?? "", f.scope_demoted ?? "")' +for (const f of j.findings) console.log(f.id, f.rule_id, f.severity, f.line_start, f.confidence, f.consensus, f.critic_verdict ?? "-", f.fact_invalid ?? "-", f.scope_demoted ?? "-")' sed -n '1,10p' rig/results/pilot-02/turns/2/diff.patch ``` | | rule_id | line_start | conf | consensus | what demoted it | |---|---|---|---:|---|---| | F-001 | `path-traversal-readtemplate` | **25** | 0.55 | singleton | critic → `likely_fp` | -| F-002 | `path-traversal` | **67** | 0.90 | singleton | `scope_demoted` | +| F-002 | `path-traversal` | **67** | 0.90 | singleton | **`fact_invalid`**, then `scope_demoted` | + +`src/store.ts` is a **brand-new 27-line file** in that turn's diff (`@@ -0,0 +1,27 @@`). +F-002 is anchored at **line 67 — past EOF**. + +### The gate caught the bad anchor, and drew the wrong conclusion + +`validateFindingFacts` (`src/core/fact-check.ts`, called at `orchestrator.ts:2226`, before +aggregation) detected it exactly and demoted the finding, writing this into its details: + +``` +[reviewgate fact-check] cited location src/store.ts:67 does not exist in the working tree +(file has 27 lines) — almost certainly hallucinated; demoted to advisory. +``` + +That pass exists for good reason — two production field reports where a lone reviewer emitted a +0.97 CRITICAL citing content in an **empty** file. Its stated premise is: + +> _"if a file has 3 lines, a finding on line 99 is unambiguously fabricated"_ (`fact-check.ts:18`) -`src/store.ts` is a **brand-new 27-line file** in that turn's diff (`@@ -0,0 +1,27 @@`), so -every line of it is inside the changed ranges. **F-002 is anchored at line 67 — past EOF of a -file that is entirely in-diff.** Nothing in `src/providers/review-output.ts` or -`src/schemas/finding.ts` validates a line anchor against the file it names; `FindingSchema` only -requires `z.number().int().positive()`. +**F-002 falsifies that premise.** It carries the reviewer's own quoted evidence: -That single bad anchor produced all three of the failures the pilot-02 write-up attributes to -independent mechanisms: +``` +evidence_line: " return readFileSync(`./templates/${name}`, 'utf8')" +``` + +which is **verbatim line 26** of that 27-line file. The reviewer read the real code, quoted it +correctly, and mis-numbered the line. The finding was **mis-anchored, not fabricated** — and the +gate told the agent the opposite, at 0.90 confidence, about a real path traversal. + +The discriminator already exists **in the same file**: `attestEvidence:154` computes +`lines.some((l) => normalizeLine(l) === evN)` — "does this quote match any line of the file". It +runs at `orchestrator.ts:2573`, render-only, *after* aggregation, and never feeds the demote. + +### Scale, measured across both pilots -1. **F-002 was scope-demoted** (`aggregator.ts:352`) as "outside the changed lines". It was never - out-of-diff — it was mis-anchored. +```bash +bun -e 'let tot=0,ev=0,fi=0; const g=new Bun.Glob("rig/results/pilot-0*/turns/*/reports/*-pending.json"); +for await (const p of g.scan(".")) { const j=JSON.parse(await Bun.file(p).text()); +for (const f of j.findings ?? []) { tot++; if (f.evidence_line) ev++; if (f.fact_invalid) fi++; } } +console.log({tot, ev, fi})' +``` + +| | | +|---|---| +| findings across both pilots | 68 | +| carrying `evidence_line` | **55 (81 %)** | +| distinct findings `fact_invalid` fired on, in 24 turns | **1** | +| ...whose evidence line matched a real line of the cited file | **1 of 1** | + +The one time this pass fired, it labelled a real security finding a hallucination. **n = 1**: the +change below is justified by the mechanism, not by that sample size. + +### Why this one anchor cost the whole turn + +The bad anchor caused all three failures the pilot-02 write-up attributes to independent layers: + +1. **`fact_invalid`** demoted F-002 to INFO as fabricated (above). 2. **The two findings never merged.** 25 and 67 are 42 lines apart, past both `REGION_WINDOW` - (5, `:192`) and `WORDING_MERGE_MAX_LINE_DISTANCE` (25, `:229`). -3. **No merge meant no corroboration.** `computeConsensus(1, 2)` is `"singleton"`; merged, it - would be `computeConsensus(2, 2)` → `"majority"` → `isCorroborated` at `:610` → **the critic + (5, `aggregator.ts:192`) and `WORDING_MERGE_MAX_LINE_DISTANCE` (25, `:229`). +3. **No merge meant no corroboration.** `computeConsensus(1, 2)` is `"singleton"`; merged it + would be `computeConsensus(2, 2)` → `"majority"` → `isCorroborated` (`:610`) → **the critic could not have demoted F-001 at all.** So the handoff's candidate (2) — "merge same-file/same-category detections before the critic" — -addresses a symptom. These two findings did not need a broader merge rule; they needed one -reviewer not to invent a line number. +targets a symptom. These findings did not need a broader merge rule; they needed the anchor +repaired, after which they are **one line apart** and merge under the existing window. -The handoff's candidate (1) remains a real defect on its own merit, independent of turn 2: the -critic's exemption is keyed to CRITICAL (`:604`), while the sibling `deltaScoped` pass exempts -`touchesSecurityOrCorrectness` at **any** severity (`:664`). A WARN-severity security finding is -demotable by a single adversarial critic, and that demote crosses the blocking boundary. +The handoff's candidate (1) is a real defect independent of turn 2: the critic's exemption is +keyed to CRITICAL (`:604`) while the sibling `deltaScoped` pass exempts +`touchesSecurityOrCorrectness` at **any** severity (`:664`). That is Slice B. ## Scope -Two independent slices, both in `src/core/aggregator.ts`, neither touching the reviewer or -provider layer. Both are **always-on** — they are defect corrections, and the sibling -protections they mirror (G0's clamp at `:155`, `isCriticalSecurity` at `:604`, delta-scope's -exemption at `:664`) are all unflagged. Observability comes from markers a pilot can count, not -from config toggles; no new config key, and therefore no second TTY control-plane approval. +Two slices. Both **always-on** — they are defect corrections, and the sibling protections they +sit beside are all unflagged. Observability comes from markers a pilot can count, not from config +toggles; no new config key, and so no second TTY control-plane approval. ### Explicitly out of scope - **Merge/clustering changes.** Widening the merge to same-file/same-category would bundle genuinely separate security bugs under one decision, which `isHighStakesCategory` (`:279`) - exists to prevent. -- **An invalid anchor still participates in region clustering**, so a bogus line 67 could merge - with a real finding at line 65. This risk exists today, did not fire in turn 2, and removing - it means changing merge behaviour — a third change with its own blast radius. + exists to prevent. Slice A reaches the merge by fixing the input, not the rule. - **M5 critic cost attribution** (`orchestrator.ts:2300`) stays scoped out; it is a provider-contract change, as recorded in the task-(a) plan. +- **Findings with no `evidence_line`** (19 % of the corpus) get no new protection. Their + behaviour is unchanged. -## Slice A — anchor integrity - -Policy, chosen deliberately: **an out-of-range anchor is treated exactly like a missing one.** -`scopeFindings` already carries that precedent in the same function — -`if (!f.line_start) return f; // no usable line → keep (conservative)` (`:330`). A reviewer that -cites line 67 of a 27-line file has given us no usable location, and the fail-safe direction in -this codebase is that a suppressor must never silently soften a finding it cannot place. +## Slice A — distinguish mis-anchored from fabricated -The rejected alternative is re-anchoring the finding into the nearest changed range. It would -additionally recover the merge path, but the gate would then be asserting a location the -reviewer never gave — the gate would be lying about where the bug is. - -### Detect - -A new pass immediately after the path-normalize / `demoteRedaction` map (`:429-432`), before the -deterministic sort and clustering. For each finding whose file has a **known** line count: +The whole change is inside `validateFindingFacts`, at the point where it has already read the +file and decided the line is out of range (`fact-check.ts:116-119`): ```ts -if (lineCount !== undefined && f.line_start > lineCount) → { ...f, anchor_invalid: true } +const lines = lineCount(text); +if (f.line_start <= lines) return f; // cited line exists → untouched (unchanged) +// Out of range. Before calling it a fabrication, consult the reviewer's OWN quoted evidence: +// if evidence_line matches a real line of THIS file, the reviewer read real code and +// mis-numbered it. That is mis-anchored, not fabricated, and demoting it as a hallucination +// is a false accusation against a finding we can prove is grounded. +const repaired = reanchorByEvidence(f, text); +if (repaired) return repaired; +return demote(f, note); // no quote, or quote matches nothing → unchanged ``` -Stamp only. No severity change here, so the pass is inert on its own and its effect is entirely -attributable to the consumer below. +`reanchorByEvidence(f, text)` returns `null` unless **all** of these hold: -The pass runs **unconditionally**, not inside `scopeFindings` — that function returns early when -`scopeToDiff` is false or `changedRanges` is absent (`:317`). Detecting separately means a -mis-anchoring reviewer is still visible in `pending.md` on a repo that has diff-scoping turned -off, where the flag changes nothing else. +1. `f.evidence_line` is a non-empty string, and non-empty after `normalizeLine` (`:129`, already + in this file — defangs injection markers, collapses whitespace); +2. at least one line of `text` equals it under the same normalization. -**`line_end` does not participate.** Reviewers routinely over-estimate `line_end` as a sloppy -range hint; if `line_start` names a real line the finding *is* anchored somewhere real, and -`rangeOverlapsChanged` (`src/diff/hunks.ts:116`) already handles the range. Stamping on -`line_end` would fire on ordinary findings and switch off scope-demoting broadly — a fail-open, -which is the exact direction this slice exists to close. Guard test 2 pins this. +On a match it returns the finding re-anchored to that line, with `line_end` collapsed to it, a +`anchor_repaired: true` marker, and a details note recording the original number. -### Act +**Disambiguation, stated as a rule rather than left to chance:** when the quote matches several +lines, re-anchor to the occurrence **nearest the cited line**; ties break to the **lower** line +number. Both candidates are real occurrences of the reviewer's own quote, so this chooses among +facts rather than inventing one, and it is fully deterministic — which the aggregator's clustering +requires (`:433-444` sorts precisely to keep clustering order-independent). -`scopeFindings:330` becomes: +**This is not the gate inventing a location.** It re-anchors to a line the reviewer itself +quoted. The rejected alternative — clamping to the nearest changed hunk — would have been. -```ts -if (!f.line_start || f.anchor_invalid) return f; // no usable line → keep (conservative) -``` +**Cost: zero extra I/O.** The re-anchor reuses the `text` the pass already read under its +existing O_NOFOLLOW-contained, 5 MB-capped reader. No new file access, no new pass, no new +`AggregateInput` field, and **no orchestrator change at all**. -### Input +### Why placing it here makes the rest fall out -`AggregateInput` gains: +`validateFindingFacts` runs **pre-aggregation** (`orchestrator.ts:2226`), so the repaired line is +what clustering sees. For turn 2 that means 67 → 26, one line from F-001's anchor at 25, inside +`REGION_WINDOW` → the two merge → `"majority"` → `isCorroborated` → the critic is barred, and the +finding is in-diff so nothing scope-demotes it. -```ts -/** Line counts of the files this diff changed, for anchor validation. An entry is present - * ONLY when the file was read successfully. Absent map, or an absent entry, means UNKNOWN — - * no anchor on that file is ever stamped, and behaviour is byte-identical to before this - * field existed. Same optional-input compatibility rule `seedLanded` and `criticRuns` follow. */ -fileLineCounts?: Map; -``` - -The orchestrator builds it at the `aggregate()` call site (`orchestrator.ts:2451`) from the -changed-file set, using `readFileSync` under `this.input.repoRoot` (already imported at -`orchestrator.ts:7`). Keys are `normalizeRepoPath`-canonical, matching `changedRanges`. +It also makes the two evidence passes agree: `attestEvidence` (`:2573`) reads +`lines[line_start - 1]`, which after the repair is the quoted line, so it stops treating the +finding as ambiguous. ### Schema and rendering -`FindingSchema` gains `anchor_invalid: z.boolean().optional()`, alongside `scope_demoted` -(`finding.ts:81`) and `protected_high_precision` (`:191`). +`FindingSchema` gains `anchor_repaired: z.boolean().optional()`, alongside `fact_invalid` +(`finding.ts:234`) and `scope_demoted` (`:81`). + +`report-writer.ts` gains one badge beside the existing `🔎 cited location not found` (`:43`): -`report-writer.ts` gains one badge next to the existing `📍 outside changed lines` (`:57`), e.g. -`⚑ the reviewer cited a line that does not exist in this file — location unverified, kept -blocking`. The badge is the diagnostic that made this whole slice findable; without it a -mis-anchoring reviewer is invisible. +``` +⚑ reviewer cited a line that does not exist — re-anchored to the source line it quoted +``` + +The badge is what makes a mis-anchoring reviewer visible instead of silently corrected. ## Slice B — critic severity floor @@ -152,81 +193,94 @@ Both feed the same two branches at `:615` and `:619`. A protected finding theref existing `survivors.push({ ...f, critic_verdict: "keep" })` path at `:635`, which already renders honestly — no new marker needed. -**Why the floor stops at WARN and not lower.** The stated harm is a demote that crosses the -blocking boundary (`isBlocking` is `CRITICAL || WARN`). WARN → INFO crosses it; INFO → drop does -not, and low-confidence INFO security chatter is the noisiest thing the critic filters. Making -security wholly critic-immune would re-inflate FP burden in a way pilot-02 has **zero data** on -(`rejectedAsFp` was 0 on every turn, `known_fp.jsonl` ended the run empty). Guard test 5 pins -that the floor does not over-apply. +**Why the floor stops at WARN.** The stated harm is a demote crossing the blocking boundary +(`isBlocking` is `CRITICAL || WARN`). WARN → INFO crosses it; INFO → drop does not, and +low-confidence INFO security chatter is the noisiest thing the critic filters. Making security +wholly critic-immune would re-inflate FP burden in a way pilot-02 has **zero data** on +(`rejectedAsFp` was 0 on every turn; `known_fp.jsonl` ended the run empty). + +**Kept even though Slice A alone rescues turn 2.** Slice A works through the merge, which needs +two detections. A lone WARN security finding — the common case — still faces a critic whose +exemption is keyed to a severity it does not have. The two slices protect different populations. -**Blast radius.** `demoteOneStep` (`:155`) is unchanged — a WARN security finding simply never -reaches it from the critic pass. The reputation and confidence-floor passes are untouched; they -already carry their own hard security veto (`touchesSecurity`, `:285`). +**Blast radius.** `demoteOneStep` (`:155`) is unchanged — a WARN security finding never reaches +it from the critic pass. The reputation and confidence-floor passes are untouched; they already +carry their own hard security veto (`touchesSecurity`, `:285`). ## Data flow ``` -reviewers → findings (line_start unvalidated) -orchestrator:2451 changedRanges = parseChangedRanges(diff) - fileLineCounts = line counts of changed files ← new -aggregate(): normalize paths + demoteRedaction - markInvalidAnchors ← Slice A detect - sort → cluster → dedupe → consensus - critic pass ← Slice B floor - scopeFindings ← Slice A act - deltaScope → fp-ledger → reputation → verdict -report-writer: anchor_invalid badge +reviewers → findings (line_start unvalidated, evidence_line usually present) +orchestrator:2226 validateFindingFacts + ├─ line in range → untouched + ├─ out of range, quote matches → RE-ANCHOR + anchor_repaired ← Slice A + └─ out of range, no match/quote → demote INFO + fact_invalid (unchanged) +orchestrator:2451 aggregate() + cluster (now sees the repaired line → the two findings merge) + consensus → "majority" + critic pass (isCorroborated bars it; Slice B bars WARN+security too) + scopeFindings → deltaScope → fp-ledger → reputation → verdict +orchestrator:2573 attestEvidence (now agrees; no evidence_mismatch) +report-writer: anchor_repaired badge ``` ## Fail-safety -Each row states its failure direction, not just its behaviour. +Each row states its failure direction, not merely its behaviour. | Condition | Behaviour | Why | |---|---|---| -| `fileLineCounts` absent entirely | Both slices inert; output byte-identical to today | Optional-input compatibility, as `seedLanded`/`criticRuns` | -| File missing, unreadable, binary, or over a byte cap | **No entry** → anchors on that file never stamped | Unknown must stay unknown; never guess a line count | -| Finding on a file the diff did not change | No entry → demotes on the file-absent branch (`:332`) as today | Anchor validation must not rescue findings on unchanged files | -| `line_start` valid, `line_end` past EOF | **Not stamped** | Stamping here would fail open; see Slice A | -| Anchor invalid **and** severity already INFO | Kept and flagged, and **`scope_demoted` is no longer stamped** on it | `demote()` (`:325`) stamps the marker even on INFO, and `report-writer.ts:498` buckets on it. Such a finding moves from the "outside changed lines" list to the in-scope advisory list — correct, since we do not know it is outside. A deliberate, visible consequence, not an oversight | +| No `evidence_line` on the finding | Demoted exactly as today | 19 % of the corpus; the empty-file fabrication case that motivated the pass is untouched | +| `evidence_line` present, matches **no** line | Demoted exactly as today | A quote that is in no line of the file is the fabrication signal, now positively established rather than inferred from a number | +| `evidence_line` empty after `normalizeLine` | Treated as absent → demoted | A whitespace/marker-only quote carries no signal | +| Quote matches **several** lines | Re-anchor to the nearest, ties → lower line | Deterministic; clustering must not depend on iteration order | +| File unreadable, oversize, symlinked, absent | Untouched, as today | The existing reader already fails safe here; Slice A adds no new access | +| Finding on a path in `deletedPaths` | Skipped, as today | Commentary on removed code, not a fabrication | +| Cited line **in** range | Untouched — re-anchor never runs | The pass stays demote-or-repair-only on out-of-range findings; it can never move a valid anchor | ## Measurability, stated honestly -`SUPPRESSION_LAYERS` (`src/rig/ablate.ts:44`) is `["critic","reputation","fp-ledger","lore"]` — -there is no `scope` or `anchor` layer, and the ablation reconstructs a layer's counterfactual by -reading the demotion markers it left behind. +`SUPPRESSION_LAYERS` (`src/rig/ablate.ts:44`) is `["critic","reputation","fp-ledger","lore"]`, and +`ablate.ts:88` already treats `fact_invalid` as a non-ablatable other-suppressor. - **Slice B stays fully ablatable.** A protected finding carries `critic_verdict: "keep"`, and `−critic` shows no recall delta where the floor held. -- **Slice A is observable, not ablatable.** pilot-03 can count `anchor_invalid` findings and how - many stayed blocking; it cannot produce a counterfactual matrix row for the slice. The pilot-03 - write-up must say this rather than let the matrix imply coverage it does not have. +- **Slice A is observable, not ablatable.** pilot-03 can count `anchor_repaired` findings and how + many stayed blocking; it cannot produce a counterfactual matrix row. The write-up must say so + rather than let the matrix imply coverage it does not have. +- **Expect a small count.** The pass fired once in 24 turns. A pilot-03 that shows 0 + `anchor_repaired` findings has not refuted the fix — it has not exercised it, and the write-up + must say that instead of reporting a null result. ## Testing Every guard test carries the two numbers of the quantity it guards. A test whose two values match -is vacuous **on paper** and is rewritten before it is written. +is vacuous **on paper** and gets rewritten before it is written. | # | Guards | WITHOUT the mechanism | WITH it | |---|---|---|---| -| 1 | Invalid anchor is not scope-demoted (turn-2 shape: 27-line new file, finding at line 67) | INFO + `scope_demoted` → **0 blocking** | WARN + `anchor_invalid` → **1 blocking** | -| 2 | Does not over-apply — a *valid* out-of-diff anchor still demotes. Mutation: stamp on `line_end > lineCount` | over-broad predicate → **1 blocking** | correct predicate → **0 blocking** | -| 3 | Inert without `fileLineCounts` — one fixture, map present vs absent | map absent → **0 blocking** | map present → **1 blocking** | -| 4 | WARN + security + `likely_fp` survives the critic | INFO + `critic_verdict: likely_fp` → **0 blocking** | WARN + `critic_verdict: keep` → **1 blocking** | -| 5 | Floor does not over-apply — INFO + security + `likely_fp` is still dropped | "exempt at every severity" variant → `criticDropped` **0** | correct floor → `criticDropped` **1** | -| 6 | Floor is category-keyed, not severity-keyed — WARN + *quality* still demotes | severity-only variant → **1 blocking** | correct floor → **0 blocking** | +| 1 | Out-of-range + quote matches → re-anchored, not demoted | INFO + `fact_invalid` → **0 blocking** | CRITICAL kept, `line_start` = matched line, `anchor_repaired` → **1 blocking** | +| 2 | No `evidence_line` → still demoted (the empty-file case). Mutation: re-anchor unconditionally | unconditional repair → `fact_invalid` **absent** | correct gate → `fact_invalid` **true** | +| 3 | Quote matching **no** line → still demoted. Mutation: skip the match test | skipped test → `fact_invalid` **absent** | correct gate → `fact_invalid` **true** | +| 4 | Multiple matches → nearest-to-cited wins, deterministic | first-match rule → `line_start` **2** | nearest rule → `line_start` **8** | +| 5 | In-range finding is never moved. Mutation: run the repair before the range check | repair-first → `line_start` **moves** | correct order → `line_start` **unchanged** | +| 6 | Cascade, reconstructed from turn 2: repair → merge → majority → blocking | 2 findings, consensus `singleton`, **0 blocking** | 1 merged finding, consensus `majority`, **1 blocking** | +| 7 | WARN + security + `likely_fp` survives the critic | INFO + `critic_verdict: likely_fp` → **0 blocking** | WARN + `critic_verdict: keep` → **1 blocking** | +| 8 | Floor does not over-apply — INFO + security + `likely_fp` still dropped | "exempt at every severity" variant → `criticDropped` **0** | correct floor → `criticDropped` **1** | +| 9 | Floor is category-keyed, not severity-keyed — WARN + *quality* still demotes | severity-only variant → **1 blocking** | correct floor → **0 blocking** | Each is seen **red** first, in a copy of the repo; the original is confirmed unmodified with `git diff` after each copy is discarded. -**Acceptance on real data, labelled honestly.** A fixture reconstructed from turn 2's recorded -`pending.json` — both findings, their actual lines, categories, confidences and consensus — -showing the turn flip from 0 blocking to blocking. This is a **reconstruction, not a replay**: -the archived findings are post-aggregation, so the pre-aggregation input is inferred from the -demotion markers, exactly as `ablate.ts` does. The write-up must not call it a replay. +**Test 6 is the acceptance test, and it is a reconstruction, not a replay.** It is built from +turn 2's recorded `pending.json` — both findings, their real lines, categories, confidences, +messages and the actual `evidence_line` — plus a temp-dir copy of the 27-line `src/store.ts` from +`turns/2/diff.patch`. The archived findings are post-aggregation, so the pre-aggregation input is +inferred from the demotion markers, exactly as `ablate.ts` does. The write-up must not call it a +replay. -**Static gates.** `bunx tsc --noEmit`, `bun run lint`, and the full `bun test` — `FindingSchema` -changes, so the persisted-artifact suite runs whole. +**Static gates.** `bunx tsc --noEmit`, `bun run lint`, and the full `bun test` — +`FindingSchema` changes, so the persisted-artifact suite runs whole. **Reviews.** A plan gate with an **executing** reviewer before implementation, then the post-implementation pipeline with two independent slots. Codex is quota-blocked until @@ -255,9 +309,10 @@ not part of this spec. | Risk | Handling | |---|---| -| Slice A rescues findings it should not, weakening diff-scoping generally | Only `line_start` past a **known** EOF stamps; unknown stays unknown. Guard tests 2 and 3 pin both over-application modes | -| Slice B re-inflates FP burden | Floor stops at WARN; already-INFO security stays droppable (guard 5). pilot-03 reports FP burden, though pilot-02 showed M2 has no signal in this rig | -| `fileLineCounts` costs I/O on every gate run | Bounded to the changed-file set, one `readFileSync` each, byte-capped; the research phase already reads these files | -| `FindingSchema` change breaks older persisted artifacts | Field is `.optional()`, mirroring `scope_demoted`/`protected_high_precision` | -| pilot-03 conflates the two slices | Slice B is isolated by the existing `−critic` ablation; Slice A is reported as a count of `anchor_invalid`. Stated as a limitation, not papered over | -| n = 3 landed seeds cannot establish an effect size | Same limit as pilot-02, restated in the write-up. This detects signal, not magnitude | +| Re-anchoring rescues a genuinely fabricated finding | Only when the reviewer's quote is **found verbatim in the cited file**. A model that fabricates a line number *and* quotes real source has still read the file — that is the definition of mis-anchored. Guards 2 and 3 pin both no-quote and no-match paths | +| The repair moves a finding onto unrelated code | It moves it onto a line the reviewer quoted; multi-match resolves to the nearest occurrence, deterministically. Guard 4 pins the rule | +| Weakening the empty-file protection the pass was built for | An empty file has no lines, so no quote can match → the motivating case can never be re-anchored. Guard 2 uses exactly that fixture | +| Slice B re-inflates FP burden | Floor stops at WARN; already-INFO security stays droppable (guard 8). pilot-03 reports FP burden, though pilot-02 showed M2 has no signal in this rig | +| `FindingSchema` change breaks older persisted artifacts | Field is `.optional()`, mirroring `fact_invalid`/`scope_demoted` | +| pilot-03 conflates the two slices | Slice B is isolated by the existing `−critic` ablation; Slice A is reported as a count of `anchor_repaired`. Stated as a limitation, not papered over | +| n = 1 observed mis-anchor, n = 3 landed seeds | The fix is justified by mechanism, not sample size, and the write-up says so. A pilot-03 with 0 repairs is an unexercised path, not a refutation | From dbc2911fb8996f32b28cd05f6b24d18b13dc637f Mon Sep 17 00:00:00 2001 From: Codevena Date: Wed, 5 Aug 2026 22:33:59 +0200 Subject: [PATCH 03/14] docs: implementation plan for the true-positive hole + spec note on merge propagation --- .../plans/2026-08-05-true-positive-hole.md | 816 ++++++++++++++++++ .../2026-08-05-true-positive-hole-design.md | 9 + 2 files changed, 825 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-05-true-positive-hole.md diff --git a/docs/superpowers/plans/2026-08-05-true-positive-hole.md b/docs/superpowers/plans/2026-08-05-true-positive-hole.md new file mode 100644 index 0000000..baab74c --- /dev/null +++ b/docs/superpowers/plans/2026-08-05-true-positive-hole.md @@ -0,0 +1,816 @@ +# True-Positive Hole Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Stop Reviewgate demoting a real security finding twice over — once by calling a +mis-anchored finding "almost certainly hallucinated", once by letting a lone critic push a WARN +security finding below the blocking boundary. + +**Architecture:** Two independent slices. **Slice A** teaches `validateFindingFacts` to +distinguish *mis-anchored* from *fabricated* by consulting the reviewer's own `evidence_line`, +and re-anchors instead of demoting when the quote is a real line of the cited file. Because that +pass runs pre-aggregation, the repaired line feeds clustering, which is what lets two detections +of the same bug merge and gain corroboration. **Slice B** extends the critic's security exemption +from CRITICAL-only to CRITICAL-and-WARN. + +**Tech Stack:** TypeScript on Bun. `bun test` (not jest/vitest), `biome` for lint, +`tsc --noEmit` for types. Zod schemas are the source of truth for every persisted artifact. + +## Global Constraints + +- Spec of record: `docs/superpowers/specs/2026-08-05-true-positive-hole-design.md`. +- Runtime is **Bun**. Use `bun`/`bunx`, never `npm`/`node`/`npx`. +- `bunx tsc --noEmit` and `bun run lint` must both be clean before any task is "done". +- `FindingSchema` changes in Task 1, so the **full** `bun test` runs from Task 1 onward. +- Every new schema field is `.optional()` — older persisted `pending.json` must still parse. +- No new config key, no `reviewgate.config.ts` change: both slices are always-on. Touching the + config would arm the control plane and demand a TTY approval. +- **Do NOT run `bun run build`.** It deploys to every repo via the `~/.local/bin/reviewgate` + symlink. The rebuild is step 2 of the pilot-03 sequencing, after all review gates pass. +- Never `git add -A` at the repo root — it tracks `.reviewgate/` runtime state. Add named paths. +- Guard tests that already pass against current code are **vacuous until mutation-checked**. + Every such test below carries an explicit mutation step: apply the mutation **in a copy of the + repo**, confirm RED, discard the copy, confirm `git diff` shows the original unchanged. + +--- + +## File Structure + +| File | Responsibility | Change | +|---|---|---| +| `src/core/fact-check.ts` | Deterministic finding fact-check | Add `reanchorByEvidence`; consult it before demoting | +| `src/schemas/finding.ts` | Persisted finding shape | Add `anchor_repaired` to the finding and to `members[]` | +| `src/core/aggregator.ts` | Dedup, merge, suppression passes, verdict | Propagate `anchor_repaired` across a merge; add the critic's WARN-security floor | +| `src/core/report-writer.ts` | Renders `pending.md` | One badge | +| `tests/unit/fact-check-reanchor.test.ts` | Slice A guards | **Create** | +| `tests/unit/aggregator-critic.test.ts` | Critic-pass guards | Append Slice B guards | +| `tests/unit/anchor-repair-cascade.test.ts` | End-to-end acceptance | **Create** | + +`src/core/orchestrator.ts` is deliberately **not** touched — the whole change sits inside a pass +it already calls. + +--- + +## Task 1: Slice A — re-anchor a mis-anchored finding instead of demoting it + +**Files:** +- Modify: `src/schemas/finding.ts:234` (beside `fact_invalid`) +- Modify: `src/core/fact-check.ts:116-119` (the demote decision) and add a helper above it +- Test: `tests/unit/fact-check-reanchor.test.ts` (create) + +**Interfaces:** +- Consumes: `normalizeLine(s: string): string` — already in `fact-check.ts:129`, a hoisted + function declaration, so it is callable from a function defined above it. It defangs injection + markers, collapses whitespace runs to one space, and trims. +- Consumes: `lineCount(text: string): number` — already in `fact-check.ts:32`. +- Produces: `Finding.anchor_repaired?: boolean` — Task 2 propagates it, Task 3 renders it. + +- [ ] **Step 1: Write the failing tests** + +Create `tests/unit/fact-check-reanchor.test.ts`: + +```ts +// tests/unit/fact-check-reanchor.test.ts +// +// pilot-02 turn 2: the fact-check demoted a 0.90 path-traversal finding as "almost certainly +// hallucinated" because it cited line 67 of a 27-line file — while the reviewer's OWN +// evidence_line matched line 26 of that file verbatim. Mis-anchored, not fabricated. These +// guards pin the distinction, and pin that the fabrication protection the pass was built for +// (a CRITICAL citing a line in an EMPTY file) is untouched. +import { describe, expect, it } from "bun:test"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { validateFindingFacts } from "../../src/core/fact-check.ts"; +import type { Finding } from "../../src/schemas/finding.ts"; + +const EVIDENCE = " return readFileSync(`./templates/${name}`, 'utf8')"; + +function mkFinding(over: Partial = {}): Finding { + return { + id: "F-001", + signature: "sig1", + severity: "CRITICAL", + category: "security", + rule_id: "path-traversal", + file: "store.ts", + line_start: 67, + line_end: 67, + message: "Path traversal vulnerability in readTemplate.", + details: "details", + reviewer: { provider: "openrouter", model: "deepseek/deepseek-v3.2", persona: "security" }, + confidence: 0.9, + consensus: "singleton", + ...over, + }; +} + +function repo(content: string): string { + const dir = mkdtempSync(join(tmpdir(), "rg-reanchor-")); + mkdirSync(join(dir, "src"), { recursive: true }); + writeFileSync(join(dir, "store.ts"), content); + writeFileSync(join(dir, "empty.yaml"), ""); + return dir; +} + +// 5 lines, the quoted evidence at line 3. +const FIVE_LINES = ["const a = 1", "const b = 2", EVIDENCE, "const d = 4", "const e = 5"].join("\n") + "\n"; + +describe("validateFindingFacts — mis-anchored vs fabricated", () => { + // GUARD 1. WITHOUT the mechanism: severity INFO + fact_invalid -> 0 blocking. + // WITH it: CRITICAL kept at the quoted line 3 + anchor_repaired -> 1 blocking. + it("re-anchors an out-of-range finding whose evidence_line matches a real line", () => { + const dir = repo(FIVE_LINES); + const out = validateFindingFacts( + [mkFinding({ line_start: 67, line_end: 67, evidence_line: EVIDENCE })], + dir, + new Set(), + ); + expect(out[0]?.severity).toBe("CRITICAL"); + expect(out[0]?.line_start).toBe(3); + expect(out[0]?.line_end).toBe(3); + expect(out[0]?.anchor_repaired).toBe(true); + expect(out[0]?.fact_invalid).toBeUndefined(); + expect(out[0]?.details).toContain("re-anchored"); + }); + + // GUARD 2 (passes on current code — MUTATION-CHECKED in Step 3). + // WITHOUT the evidence gate (repair unconditionally): fact_invalid absent. + // WITH it: fact_invalid true. This is the empty-file field-report case. + it("still demotes an out-of-range finding that carries NO evidence_line", () => { + const dir = repo(FIVE_LINES); + const out = validateFindingFacts( + [mkFinding({ file: "empty.yaml", line_start: 2, line_end: 2 })], + dir, + new Set(), + ); + expect(out[0]?.severity).toBe("INFO"); + expect(out[0]?.fact_invalid).toBe(true); + expect(out[0]?.anchor_repaired).toBeUndefined(); + }); + + // GUARD 3 (passes on current code — MUTATION-CHECKED in Step 3). + // WITHOUT the match test: fact_invalid absent. WITH it: fact_invalid true. + it("still demotes when the evidence_line matches NO line of the cited file", () => { + const dir = repo(FIVE_LINES); + const out = validateFindingFacts( + [mkFinding({ line_start: 67, evidence_line: "this line is nowhere in the file" })], + dir, + new Set(), + ); + expect(out[0]?.severity).toBe("INFO"); + expect(out[0]?.fact_invalid).toBe(true); + expect(out[0]?.anchor_repaired).toBeUndefined(); + }); + + // GUARD 4. WITHOUT the nearest-match rule (first match wins): line_start 2. + // WITH it: line_start 8. Deterministic either way, but only one is the rule. + it("resolves a multi-match to the occurrence NEAREST the cited line", () => { + const dup = ["a", EVIDENCE, "c", "d", "e", "f", "g", EVIDENCE, "i", "j"].join("\n") + "\n"; + const dir = repo(dup); + const out = validateFindingFacts( + [mkFinding({ line_start: 20, line_end: 20, evidence_line: EVIDENCE })], + dir, + new Set(), + ); + expect(out[0]?.line_start).toBe(8); + expect(out[0]?.anchor_repaired).toBe(true); + }); + + // GUARD 5 (passes on current code — MUTATION-CHECKED in Step 3). + // WITHOUT the range check first (repair before it): line_start moves to 3. + // WITH the correct order: line_start stays 1. The pass must never move a VALID anchor. + it("never touches a finding whose cited line is IN range", () => { + const dir = repo(FIVE_LINES); + const out = validateFindingFacts( + [mkFinding({ line_start: 1, line_end: 1, evidence_line: EVIDENCE })], + dir, + new Set(), + ); + expect(out[0]?.line_start).toBe(1); + expect(out[0]?.anchor_repaired).toBeUndefined(); + expect(out[0]?.fact_invalid).toBeUndefined(); + }); + + // Robustness: the match runs under normalizeLine, so indentation/whitespace differences in + // the reviewer's quote must not defeat it. WITHOUT normalization: no match -> demoted. + // WITH it: re-anchored to line 3. + it("matches the quote whitespace-insensitively", () => { + const dir = repo(FIVE_LINES); + const out = validateFindingFacts( + [mkFinding({ line_start: 67, evidence_line: "return readFileSync(`./templates/${name}`, 'utf8')" })], + dir, + new Set(), + ); + expect(out[0]?.line_start).toBe(3); + expect(out[0]?.anchor_repaired).toBe(true); + }); +}); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `bun test tests/unit/fact-check-reanchor.test.ts` + +Expected: guards 1, 4 and the whitespace test FAIL (the finding comes back `INFO` + +`fact_invalid`, `line_start` still 67). Guards 2, 3, 5 PASS — they pin behaviour that already +exists, and Step 3 proves they are not vacuous. + +- [ ] **Step 3: Mutation-check the three tests that already pass** + +Do this in a **copy**, never in the working repo: + +```bash +cp -R /Users/markus/Developer/reviewgate /private/tmp/claude-501/-Users-markus-Developer-reviewgate/4d33f15e-1cc6-4490-b511-a3a6d3b10443/scratchpad/mut1 +``` + +In the copy's `src/core/fact-check.ts`, apply each mutation and record the result: + +| Mutation | Expected RED | +|---|---| +| Delete the `if (f.line_start <= lines) return f;` early return (repair before the range check) | GUARD 5 red — `line_start` 3, expected 1 | +| Make `reanchorByEvidence` return a repaired finding even when `evidence_line` is absent (anchor to line 1) | GUARD 2 red — `fact_invalid` undefined, expected true | +| Make `reanchorByEvidence` skip the `normalizeLine` equality test and take line 1 | GUARD 3 red — `fact_invalid` undefined, expected true | + +Then: `rm -rf ` and `git -C /Users/markus/Developer/reviewgate diff --stat` → must show the +original untouched. + +(Steps 1–3 are written against the finished implementation; if a mutation cannot be applied +because the code does not exist yet, do Step 4 first and then Step 3.) + +- [ ] **Step 4: Add the schema field** + +In `src/schemas/finding.ts`, immediately after `fact_invalid: z.boolean().optional(),` (`:234`): + +```ts + // Slice A (pilot-02 turn 2): set true when the fact-check found the cited line OUT OF RANGE + // but the reviewer's own evidence_line matched a real line of that file — the finding was + // MIS-ANCHORED, not fabricated, so it is re-anchored to the quoted line instead of demoted. + // Severity is untouched; this is a provenance/render marker. Mutually exclusive with + // fact_invalid by construction (the repair returns before the demote). + anchor_repaired: z.boolean().optional(), +``` + +- [ ] **Step 5: Add the helper to `src/core/fact-check.ts`** + +Insert directly above `export function validateFindingFacts` (`:58`): + +```ts +/** + * A cited line that does not exist is NOT automatically a fabrication. When the reviewer quoted + * its own evidence and that quote is a real line of the cited file, the reviewer read real code + * and mis-numbered it — mis-anchored, not hallucinated. Demoting that as a hallucination is a + * false accusation against a finding we can PROVE is grounded (pilot-02 turn 2: a 0.90 path + * traversal, quote verbatim at line 26, cited at 67, demoted with "almost certainly + * hallucinated"). + * + * Returns the finding re-anchored to the quoted line, or null when nothing can be proven — in + * which case the caller demotes exactly as before. This can only ever be reached for an + * out-of-range anchor, so it can never move a valid one. + * + * Multiple matches resolve to the occurrence NEAREST the cited line, ties to the LOWER line + * number. Both are real occurrences of the reviewer's own quote, so this chooses among facts + * rather than inventing one — and it must be deterministic, because the aggregator's clustering + * is deliberately order-independent and keys on line_start. + */ +function reanchorByEvidence(f: Finding, text: string): Finding | null { + const ev = typeof f.evidence_line === "string" ? f.evidence_line : null; + if (ev === null || ev.length === 0) return null; + const evN = normalizeLine(ev); + if (evN.length === 0) return null; // quote was only whitespace/markers → no signal + const lines = text.split("\n"); + const cited = f.line_start - 1; + let best = -1; + for (let i = 0; i < lines.length; i++) { + const raw = lines[i]; + if (raw === undefined) continue; + if (normalizeLine(raw) !== evN) continue; + // Strict `<` with an ascending scan makes ties keep the LOWER line number. + if (best === -1 || Math.abs(i - cited) < Math.abs(best - cited)) best = i; + } + if (best === -1) return null; // quote matches no line → the fabrication signal stands + const line = best + 1; + const total = lineCount(text); + const note = `\n\n[reviewgate fact-check] the reviewer cited ${f.file}:${f.line_start}, which does not exist (the file has ${total} line${total === 1 ? "" : "s"}), but the evidence it quoted matches line ${line} verbatim — MIS-ANCHORED, not fabricated, so it has been re-anchored there. Treat the defect as real and check line ${line}.`; + return { + ...f, + line_start: line, + line_end: line, + anchor_repaired: true, + details: `${f.details.slice(0, 2000 - note.length)}${note}`, + }; +} +``` + +- [ ] **Step 6: Consult the helper before demoting** + +In `validateFindingFacts`, replace lines `116-119`: + +```ts + const lines = lineCount(text); + if (f.line_start <= lines) return f; // cited line exists → real finding, untouched + const note = `\n\n[reviewgate fact-check] cited location ${file}:${f.line_start} does not exist in the working tree (file has ${lines} line${lines === 1 ? "" : "s"}) — almost certainly hallucinated; demoted to advisory. Verify before treating as real.`; + return demote(f, note); +``` + +with: + +```ts + const lines = lineCount(text); + if (f.line_start <= lines) return f; // cited line exists → real finding, untouched + // Out of range. Before calling it a fabrication, consult the reviewer's OWN quoted evidence: + // a quote that matches a real line of THIS file proves the reviewer read the code and + // mis-numbered it, which is not what this pass exists to catch. + const repaired = reanchorByEvidence(f, text); + if (repaired !== null) return repaired; + const note = `\n\n[reviewgate fact-check] cited location ${file}:${f.line_start} does not exist in the working tree (file has ${lines} line${lines === 1 ? "" : "s"}) — almost certainly hallucinated; demoted to advisory. Verify before treating as real.`; + return demote(f, note); +``` + +- [ ] **Step 7: Run the tests to verify they pass** + +Run: `bun test tests/unit/fact-check-reanchor.test.ts tests/unit/fact-check.test.ts tests/unit/evidence-attestation.test.ts` +Expected: PASS, including the pre-existing fact-check and evidence suites. + +- [ ] **Step 8: Full gates** + +```bash +bunx tsc --noEmit +bun run lint +bun test +``` +Expected: all clean. `FindingSchema` changed, so the whole suite matters here. + +- [ ] **Step 9: Commit** + +```bash +git add src/schemas/finding.ts src/core/fact-check.ts tests/unit/fact-check-reanchor.test.ts +git commit -m "fix(fact-check): re-anchor a mis-anchored finding instead of calling it fabricated" +``` + +--- + +## Task 2: Carry `anchor_repaired` through a dedup merge + +**Why this task exists:** Slice A's whole point is that the repaired line lets two detections +merge. But `memberOf` (`aggregator.ts:295`) projects a member down to six fields and drops +everything else, and the representative is chosen by severity with **ties keeping the first**. In +the turn-2 shape both findings are WARN and the repaired one sorts second, so it becomes a +*member* — and the marker, the badge and pilot-03's count all vanish in exactly the case the +slice was built for. `demoted_from_critical` already solved this problem in the same code +(`:524-531`); this mirrors it. + +**Files:** +- Modify: `src/schemas/finding.ts:254` (inside the `members[]` object) +- Modify: `src/core/aggregator.ts:295-307` (`memberOf`) and `:529-538` (the dedupe push) +- Test: `tests/unit/anchor-repair-cascade.test.ts` (create; Task 5 adds the second test to it) + +**Interfaces:** +- Consumes: `Finding.anchor_repaired` from Task 1. +- Produces: a deduped finding whose `anchor_repaired` is `OR(representative, all members)`. + +- [ ] **Step 1: Write the failing test** + +Create `tests/unit/anchor-repair-cascade.test.ts`: + +```ts +// tests/unit/anchor-repair-cascade.test.ts +import { describe, expect, it } from "bun:test"; +import { aggregate } from "../../src/core/aggregator.ts"; +import type { Finding } from "../../src/schemas/finding.ts"; + +function fin(over: Partial): Finding { + return { + id: "F-x", + signature: "s", + severity: "WARN", + category: "security", + rule_id: "r", + file: "src/store.ts", + line_start: 25, + line_end: 25, + message: "m", + details: "d", + reviewer: { provider: "ollama", model: "glm-5.2:cloud", persona: "correctness" }, + confidence: 0.55, + consensus: "singleton", + ...over, + }; +} + +describe("anchor_repaired survives a dedup merge", () => { + // WITHOUT the OR-propagation: anchor_repaired is undefined on the merged finding (it lived on + // the member, and ties-keep-first made the UNrepaired finding the representative) -> the badge + // and pilot-03's count both disappear in exactly the cascade case. + // WITH it: anchor_repaired is true on the single merged finding. + it("OR-propagates anchor_repaired from a merged member to the representative", () => { + const r = aggregate({ + findings: [ + fin({ signature: "sigA", line_start: 25, line_end: 25, rule_id: "path-traversal-readtemplate" }), + fin({ + signature: "sigB", + line_start: 26, + line_end: 26, + rule_id: "path-traversal", + anchor_repaired: true, + reviewer: { provider: "openrouter", model: "deepseek/deepseek-v3.2", persona: "security" }, + confidence: 0.9, + }), + ], + reviewersTotal: 2, + }); + expect(r.dedupedFindings.length).toBe(1); + expect(r.dedupedFindings[0]?.anchor_repaired).toBe(true); + expect(r.dedupedFindings[0]?.consensus).toBe("majority"); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `bun test tests/unit/anchor-repair-cascade.test.ts` +Expected: FAIL — `anchor_repaired` is `undefined`, expected `true`. (`consensus` already passes; +the merge itself is existing behaviour.) + +- [ ] **Step 3: Add the field to the member schema** + +In `src/schemas/finding.ts`, inside the `members[]` object right after +`demoted_from_critical: z.boolean().optional(),` (`:254`): + +```ts + // Slice A: per-member mis-anchor provenance, so a repaired member merged under an + // unrepaired equal-severity representative (ties-keep-first) does not silently lose + // the marker — the badge and the pilot count both key on it. + anchor_repaired: z.boolean().optional(), +``` + +- [ ] **Step 4: Carry it in `memberOf`** + +In `src/core/aggregator.ts`, inside `memberOf` after the `demoted_from_critical` spread (`:305`): + +```ts + ...(f.anchor_repaired === true ? { anchor_repaired: true } : {}), +``` + +- [ ] **Step 5: OR it into the representative** + +In `src/core/aggregator.ts`, after the `demotedFromCritical` computation (`:529-531`): + +```ts + // Slice A mirror of the G0 OR above: the repaired finding is often NOT the representative + // (equal severity + ties-keep-first), and losing the marker would hide the mis-anchor in + // exactly the merge the repair made possible. + const anchorRepaired = + sample.anchor_repaired === true || members.some((m) => m.anchor_repaired === true); +``` + +and add to the `deduped.push({...})` object after the `demoted_from_critical` spread (`:538`): + +```ts + ...(anchorRepaired ? { anchor_repaired: true } : {}), +``` + +- [ ] **Step 6: Run tests to verify they pass** + +Run: `bun test tests/unit/anchor-repair-cascade.test.ts tests/unit/aggregator-dedup-category.test.ts` +Expected: PASS. + +- [ ] **Step 7: Gates and commit** + +```bash +bunx tsc --noEmit && bun run lint && bun test +git add src/schemas/finding.ts src/core/aggregator.ts tests/unit/anchor-repair-cascade.test.ts +git commit -m "fix(aggregator): carry anchor_repaired across a dedup merge" +``` + +--- + +## Task 3: Render the mis-anchor badge + +**Files:** +- Modify: `src/core/report-writer.ts:43` (beside the `fact_invalid` badge) +- Test: `tests/unit/anchor-repair-cascade.test.ts` (append) + +**Interfaces:** +- Consumes: `Finding.anchor_repaired` (Task 1), `findingBadges(f: Finding): string | null` + (`report-writer.ts:39`, already exported). + +- [ ] **Step 1: Write the failing test** + +Append to `tests/unit/anchor-repair-cascade.test.ts` (and add +`import { findingBadges } from "../../src/core/report-writer.ts";` to the imports): + +```ts +describe("anchor_repaired badge", () => { + // WITHOUT the badge: findingBadges returns null for a repaired finding -> the agent sees a + // silently corrected line number and no signal that a reviewer mis-anchored. + // WITH it: the badge text is present. + it("renders a badge naming the repair", () => { + const out = findingBadges(fin({ anchor_repaired: true })); + expect(out).toContain("re-anchored"); + }); + + it("renders no such badge for an ordinary finding", () => { + expect(findingBadges(fin({}))).toBeNull(); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `bun test tests/unit/anchor-repair-cascade.test.ts` +Expected: FAIL — `findingBadges` returns `null`, expected a string containing "re-anchored". + +- [ ] **Step 3: Add the badge** + +In `src/core/report-writer.ts`, directly after the `fact_invalid` badge (`:43`): + +```ts + // Slice A: the counterpart to the badge above — the cited line was wrong, but the reviewer's + // quoted evidence proved the finding is grounded, so it was moved rather than demoted. + if (f.anchor_repaired) + badges.push("⚑ reviewer cited a line that does not exist — re-anchored to the source line it quoted"); +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `bun test tests/unit/anchor-repair-cascade.test.ts tests/unit/report-writer.test.ts` +Expected: PASS — the new badge tests plus the existing report-writer suite. + +- [ ] **Step 5: Gates and commit** + +```bash +bunx tsc --noEmit && bun run lint && bun test +git add src/core/report-writer.ts tests/unit/anchor-repair-cascade.test.ts +git commit -m "feat(report): badge a finding whose anchor was repaired from its quoted evidence" +``` + +--- + +## Task 4: Slice B — the critic's WARN-security floor + +**Files:** +- Modify: `src/core/aggregator.ts:604` (add the floor), `:615` and `:619` (use it) +- Test: `tests/unit/aggregator-critic.test.ts` (append) + +**Interfaces:** +- Consumes: `touchesSecurityOrCorrectness(f: Finding): boolean` (`aggregator.ts:255`). +- Produces: nothing new — a protected finding takes the existing + `survivors.push({ ...f, critic_verdict: "keep" })` path at `:635`. + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/unit/aggregator-critic.test.ts`: + +```ts +describe("critic — security/correctness floor (pilot-02 turn 2)", () => { + // GUARD 7. WITHOUT the floor: INFO + critic_verdict "likely_fp" -> 0 blocking. + // WITH it: WARN + critic_verdict "keep" -> 1 blocking. + it("does not demote a WARN security finding below the blocking boundary", () => { + const f = fin({ signature: "sigSecWarn", severity: "WARN", category: "security" }); + const r = aggregate({ + findings: [f], + reviewersTotal: 2, + critic: new Map([["sigSecWarn", { verdict: "likely_fp" }]]), + }); + expect(r.dedupedFindings[0]?.severity).toBe("WARN"); + expect(r.dedupedFindings[0]?.critic_verdict).toBe("keep"); + }); + + it("does not demote a WARN correctness finding below the blocking boundary", () => { + const f = fin({ signature: "sigCorrWarn", severity: "WARN", category: "correctness" }); + const r = aggregate({ + findings: [f], + reviewersTotal: 2, + critic: new Map([["sigCorrWarn", { verdict: "likely_fp" }]]), + }); + expect(r.dedupedFindings[0]?.severity).toBe("WARN"); + expect(r.dedupedFindings[0]?.critic_verdict).toBe("keep"); + }); + + // GUARD 8 (passes on current code — MUTATION-CHECKED in Step 3). + // WITH an "exempt at every severity" floor: criticDroppedCount 0. + // WITH the correct WARN floor: criticDroppedCount 1. The critic keeps its FP-filtering + // power exactly where reviewers are noisiest (low-confidence INFO security chatter). + it("still drops an already-INFO security likely_fp", () => { + const f = fin({ signature: "sigSecInfo", severity: "INFO", category: "security" }); + const r = aggregate({ + findings: [f], + reviewersTotal: 2, + critic: new Map([["sigSecInfo", { verdict: "likely_fp" }]]), + }); + expect(r.criticDroppedCount).toBe(1); + expect(r.dedupedFindings.length).toBe(0); + }); + + // GUARD 9 (passes on current code — MUTATION-CHECKED in Step 3). + // WITH a severity-only floor (all WARN exempt): the finding stays WARN -> 1 blocking. + // WITH the correct category-keyed floor: INFO -> 0 blocking. + it("still demotes a WARN quality finding", () => { + const f = fin({ signature: "sigQualWarn", severity: "WARN", category: "quality" }); + const r = aggregate({ + findings: [f], + reviewersTotal: 2, + critic: new Map([["sigQualWarn", { verdict: "likely_fp" }]]), + }); + expect(r.dedupedFindings[0]?.severity).toBe("INFO"); + expect(r.dedupedFindings[0]?.critic_verdict).toBe("likely_fp"); + }); +}); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `bun test tests/unit/aggregator-critic.test.ts` +Expected: the two "does not demote a WARN …" tests FAIL (severity comes back `INFO`, +`critic_verdict` `likely_fp`). Guards 8 and 9 PASS — Step 3 proves they are not vacuous. + +- [ ] **Step 3: Mutation-check guards 8 and 9** + +In a **copy** of the repo: + +| Mutation in `aggregator.ts` | Expected RED | +|---|---| +| `const isBlockingSecurity = touchesSecurityOrCorrectness(f);` (drop the severity test → exempt at every severity) | GUARD 8 red — `criticDroppedCount` 0, expected 1 | +| `const isBlockingSecurity = f.severity === "WARN";` (drop the category test) | GUARD 9 red — severity `WARN`, expected `INFO` | + +Then `rm -rf ` and confirm `git diff --stat` shows the original untouched. + +- [ ] **Step 4: Implement the floor** + +In `src/core/aggregator.ts`, replace line `604`: + +```ts + const isCriticalSecurity = f.severity === "CRITICAL" && touchesSecurityOrCorrectness(f); +``` + +with: + +```ts + const isCriticalSecurity = f.severity === "CRITICAL" && touchesSecurityOrCorrectness(f); + // pilot-02 turn 2: the exemption above is keyed to CRITICAL, so a WARN-severity security + // finding was demotable by a single adversarial critic — and WARN→INFO is the one demote + // that crosses the blocking boundary (isBlocking = CRITICAL || WARN). The sibling + // delta-scope pass already exempts security/correctness at ANY severity; mirror that floor + // here. An already-INFO security finding stays droppable, so the critic keeps its + // FP-filtering power where reviewers are noisiest. + const isBlockingSecurity = f.severity === "WARN" && touchesSecurityOrCorrectness(f); + const isSecurityProtected = isCriticalSecurity || isBlockingSecurity; +``` + +Then replace `!isCriticalSecurity` with `!isSecurityProtected` in **both** conditions — `:615` +(`if (!isCriticalSecurity && !isCorroborated && isProtected(f))`) and `:619` +(`if (!isCriticalSecurity && !isCorroborated)`). + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `bun test tests/unit/aggregator-critic.test.ts` +Expected: PASS. + +- [ ] **Step 6: Gates and commit** + +```bash +bunx tsc --noEmit && bun run lint && bun test +git add src/core/aggregator.ts tests/unit/aggregator-critic.test.ts +git commit -m "fix(aggregator): the critic may not demote a security finding below WARN" +``` + +--- + +## Task 5: End-to-end acceptance — the turn-2 cascade + +**Files:** +- Test: `tests/unit/anchor-repair-cascade.test.ts` (append) + +No production code changes. If this test does not pass on the strength of Tasks 1–4, one of them +is wrong. + +**Interfaces:** +- Consumes: `validateFindingFacts` (Task 1), `aggregate` (Tasks 2 and 4). + +**What it reconstructs:** turn 2's two findings with their real lines, categories, providers, +confidences and the actual `evidence_line`, over a temp-dir copy of the 27-line `src/store.ts` +taken from `rig/results/pilot-02/turns/2/diff.patch`. This is a **reconstruction, not a replay** — +the archived findings are post-aggregation, so the pre-aggregation severities are inferred from +the demotion markers, exactly as `ablate.ts` does. + +- [ ] **Step 1: Write the failing test** + +Append to `tests/unit/anchor-repair-cascade.test.ts` (add `mkdirSync`, `mkdtempSync`, +`writeFileSync`, `tmpdir`, `join` and `validateFindingFacts` to the imports): + +```ts +// The 27-line src/store.ts exactly as turn 2's diff created it. Line 26 is the quoted evidence. +const STORE_TS = [ + "import { readFileSync } from 'node:fs'", + "", + "export interface KVStore {", + " get(key: K): V | undefined", + " set(key: K, value: V): void", + " has(key: K): boolean", + "}", + "", + "export function createStore(): KVStore {", + " const entries = new Map()", + "", + " return {", + " get(key) {", + " return entries.get(key)", + " },", + " set(key, value) {", + " entries.set(key, value)", + " },", + " has(key) {", + " return entries.has(key)", + " },", + " }", + "}", + "", + "export function readTemplate(name: string): string {", + " return readFileSync(`./templates/${name}`, 'utf8')", + "}", +].join("\n") + "\n"; + +describe("pilot-02 turn 2 — the full cascade", () => { + // WITHOUT the fix: F-002 is demoted as fabricated (INFO + fact_invalid) and stays 42 lines + // from F-001, so nothing merges, both stay `singleton`, and the critic demotes F-001 to INFO + // -> 2 findings, 0 blocking. This is what pilot-02 recorded. + // WITH the fix: F-002 re-anchors 67 -> 26, merges with F-001 at 25, consensus becomes + // `majority`, the critic is barred -> 1 finding, blocking WARN. + it("repairs the anchor, merges, corroborates, and stays blocking", () => { + const dir = mkdtempSync(join(tmpdir(), "rg-turn2-")); + mkdirSync(join(dir, "src"), { recursive: true }); + writeFileSync(join(dir, "src", "store.ts"), STORE_TS); + + const evidence = " return readFileSync(`./templates/${name}`, 'utf8')"; + const raw: Finding[] = [ + fin({ + signature: "sigF001", + rule_id: "path-traversal-readtemplate", + line_start: 25, + line_end: 27, + confidence: 0.55, + evidence_line: evidence, + message: "readTemplate interpolates 'name' directly into a filesystem path", + }), + fin({ + signature: "sigF002", + rule_id: "path-traversal", + line_start: 67, + line_end: 67, + confidence: 0.9, + evidence_line: evidence, + message: "Path traversal vulnerability in readTemplate.", + reviewer: { provider: "openrouter", model: "deepseek/deepseek-v3.2", persona: "security" }, + }), + ]; + + const checked = validateFindingFacts(raw, dir, new Set()); + expect(checked.find((f) => f.signature === "sigF002")?.line_start).toBe(26); + + const r = aggregate({ + findings: checked, + reviewersTotal: 2, + changedRanges: new Map([["src/store.ts", [[1, 27]] as Array<[number, number]>]]), + scopeToDiff: true, + // The critic called the weaker detection a likely FP, exactly as it did in the pilot. + critic: new Map([["sigF001", { verdict: "likely_fp" }]]), + }); + + expect(r.dedupedFindings.length).toBe(1); + expect(r.dedupedFindings[0]?.consensus).toBe("majority"); + expect(r.dedupedFindings[0]?.severity).toBe("WARN"); + expect(r.dedupedFindings[0]?.critic_verdict).toBe("keep"); + expect(r.dedupedFindings[0]?.anchor_repaired).toBe(true); + expect(r.dedupedFindings[0]?.scope_demoted).toBeUndefined(); + expect(r.dedupedFindings[0]?.fact_invalid).toBeUndefined(); + }); +}); +``` + +- [ ] **Step 2: Run it** + +Run: `bun test tests/unit/anchor-repair-cascade.test.ts` +Expected: PASS on the strength of Tasks 1–4. If it fails, do **not** patch the test — find which +task's behaviour is wrong. + +- [ ] **Step 3: Gates and commit** + +```bash +bunx tsc --noEmit && bun run lint && bun test +git add tests/unit/anchor-repair-cascade.test.ts +git commit -m "test: pilot-02 turn-2 cascade — repair, merge, corroborate, stay blocking" +``` + +--- + +## After the tasks + +1. **Review pipeline** — the post-implementation gate, two independent slots, both must return + `VERDICT: PASS`. Codex is quota-blocked until **2026-08-08T11:07Z**, so Slot A (the executing + reviewer) is `agy`/Gemini or a Claude reviewer subagent, and Slot B is a second, different + voice. Tell the reviewer explicitly to mutation-check the new guard tests in a copy. +2. **`rm -rf .review/`** before the final commit. +3. **Stop and ask before pushing.** Do not push to `origin`. +4. **Do not `bun run build` yet** — that is step 2 of the pilot-03 sequencing in the spec, and it + re-pins the binary for every repo on the machine. diff --git a/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md b/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md index 505041b..2735762 100644 --- a/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md +++ b/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md @@ -177,6 +177,15 @@ finding as ambiguous. The badge is what makes a mis-anchoring reviewer visible instead of silently corrected. +**The marker must survive the merge it enables.** Found while planning, and load-bearing: the +repair's whole purpose is to let two detections cluster, but `memberOf` (`aggregator.ts:295`) +projects a member down to six fields, and the representative is chosen by severity with **ties +keeping the first**. In the turn-2 shape both findings are WARN and the repaired one sorts second, +so it becomes a *member* — and the marker, the badge and the pilot count would all vanish in +exactly the case the slice was built for. `anchor_repaired` is therefore carried in `members[]` +and OR-propagated to the representative, mirroring what `demoted_from_critical` already does at +`:524-531` for precisely the same reason. + ## Slice B — critic severity floor `aggregator.ts:604` gains a sibling to `isCriticalSecurity`: From 68e218042c5c64fc1d9c86ec9315dd79609fff65 Mon Sep 17 00:00:00 2001 From: Codevena Date: Wed, 5 Aug 2026 22:48:03 +0200 Subject: [PATCH 04/14] fix(fact-check): re-anchor a mis-anchored finding instead of calling it fabricated --- src/core/fact-check.ts | 50 +++++++++ src/schemas/finding.ts | 6 ++ tests/unit/fact-check-reanchor.test.ts | 141 +++++++++++++++++++++++++ 3 files changed, 197 insertions(+) create mode 100644 tests/unit/fact-check-reanchor.test.ts diff --git a/src/core/fact-check.ts b/src/core/fact-check.ts index 8c92007..4c69ac5 100644 --- a/src/core/fact-check.ts +++ b/src/core/fact-check.ts @@ -45,6 +45,51 @@ function demote(f: Finding, note: string): Finding { }; } +/** + * A cited line that does not exist is NOT automatically a fabrication. When the reviewer quoted + * its own evidence and that quote is a real line of the cited file, the reviewer read real code + * and mis-numbered it — mis-anchored, not hallucinated. Demoting that as a hallucination is a + * false accusation against a finding we can PROVE is grounded (pilot-02 turn 2: a 0.90 path + * traversal, quote verbatim at line 26, cited at 67, demoted with "almost certainly + * hallucinated"). + * + * Returns the finding re-anchored to the quoted line, or null when nothing can be proven — in + * which case the caller demotes exactly as before. This can only ever be reached for an + * out-of-range anchor, so it can never move a valid one. + * + * Multiple matches resolve to the occurrence NEAREST the cited line, ties to the LOWER line + * number. Both are real occurrences of the reviewer's own quote, so this chooses among facts + * rather than inventing one — and it must be deterministic, because the aggregator's clustering + * is deliberately order-independent and keys on line_start. + */ +function reanchorByEvidence(f: Finding, text: string): Finding | null { + const ev = typeof f.evidence_line === "string" ? f.evidence_line : null; + if (ev === null || ev.length === 0) return null; + const evN = normalizeLine(ev); + if (evN.length === 0) return null; // quote was only whitespace/markers → no signal + const lines = text.split("\n"); + const cited = f.line_start - 1; + let best = -1; + for (let i = 0; i < lines.length; i++) { + const raw = lines[i]; + if (raw === undefined) continue; + if (normalizeLine(raw) !== evN) continue; + // Strict `<` with an ascending scan makes ties keep the LOWER line number. + if (best === -1 || Math.abs(i - cited) < Math.abs(best - cited)) best = i; + } + if (best === -1) return null; // quote matches no line → the fabrication signal stands + const line = best + 1; + const total = lineCount(text); + const note = `\n\n[reviewgate fact-check] the reviewer cited ${f.file}:${f.line_start}, which does not exist (the file has ${total} line${total === 1 ? "" : "s"}), but the evidence it quoted matches line ${line} verbatim — MIS-ANCHORED, not fabricated, so it has been re-anchored there. Treat the defect as real and check line ${line}.`; + return { + ...f, + line_start: line, + line_end: line, + anchor_repaired: true, + details: `${f.details.slice(0, 2000 - note.length)}${note}`, + }; +} + /** * Demote findings whose cited file:line provably does not exist (file present but the * line is out of range / the file is empty). Pure, synchronous, fail-safe. @@ -115,6 +160,11 @@ export function validateFindingFacts( } const lines = lineCount(text); if (f.line_start <= lines) return f; // cited line exists → real finding, untouched + // Out of range. Before calling it a fabrication, consult the reviewer's OWN quoted evidence: + // a quote that matches a real line of THIS file proves the reviewer read the code and + // mis-numbered it, which is not what this pass exists to catch. + const repaired = reanchorByEvidence(f, text); + if (repaired !== null) return repaired; const note = `\n\n[reviewgate fact-check] cited location ${file}:${f.line_start} does not exist in the working tree (file has ${lines} line${lines === 1 ? "" : "s"}) — almost certainly hallucinated; demoted to advisory. Verify before treating as real.`; return demote(f, note); }); diff --git a/src/schemas/finding.ts b/src/schemas/finding.ts index 4a7ce47..20155e9 100644 --- a/src/schemas/finding.ts +++ b/src/schemas/finding.ts @@ -232,6 +232,12 @@ export const FindingSchema = z.object({ // a fabrication regardless of category, and demoting (vs blocking on a phantom) is // strictly safer. Fail-safe: any fs uncertainty leaves the finding untouched. fact_invalid: z.boolean().optional(), + // Slice A (pilot-02 turn 2): set true when the fact-check found the cited line OUT OF RANGE + // but the reviewer's own evidence_line matched a real line of that file — the finding was + // MIS-ANCHORED, not fabricated, so it is re-anchored to the quoted line instead of demoted. + // Severity is untouched; this is a provenance/render marker. Mutually exclusive with + // fact_invalid by construction (the repair returns before the demote). + anchor_repaired: z.boolean().optional(), // M5 Part B0: per-member provenance of a merged cluster. The aggregator clusters // findings (possibly different rule_id/category/signature) under one // representative; this records each member's own signature + trusted base diff --git a/tests/unit/fact-check-reanchor.test.ts b/tests/unit/fact-check-reanchor.test.ts new file mode 100644 index 0000000..31b2c70 --- /dev/null +++ b/tests/unit/fact-check-reanchor.test.ts @@ -0,0 +1,141 @@ +// tests/unit/fact-check-reanchor.test.ts +// +// pilot-02 turn 2: the fact-check demoted a 0.90 path-traversal finding as "almost certainly +// hallucinated" because it cited line 67 of a 27-line file — while the reviewer's OWN +// evidence_line matched line 26 of that file verbatim. Mis-anchored, not fabricated. These +// guards pin the distinction, and pin that the fabrication protection the pass was built for +// (a CRITICAL citing a line in an EMPTY file) is untouched. +import { describe, expect, it } from "bun:test"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { validateFindingFacts } from "../../src/core/fact-check.ts"; +import type { Finding } from "../../src/schemas/finding.ts"; + +const EVIDENCE = " return readFileSync(`./templates/${name}`, 'utf8')"; + +function mkFinding(over: Partial = {}): Finding { + return { + id: "F-001", + signature: "sig1", + severity: "CRITICAL", + category: "security", + rule_id: "path-traversal", + file: "store.ts", + line_start: 67, + line_end: 67, + message: "Path traversal vulnerability in readTemplate.", + details: "details", + reviewer: { provider: "openrouter", model: "deepseek/deepseek-v3.2", persona: "security" }, + confidence: 0.9, + consensus: "singleton", + ...over, + }; +} + +function repo(content: string): string { + const dir = mkdtempSync(join(tmpdir(), "rg-reanchor-")); + mkdirSync(join(dir, "src"), { recursive: true }); + writeFileSync(join(dir, "store.ts"), content); + writeFileSync(join(dir, "empty.yaml"), ""); + return dir; +} + +// 5 lines, the quoted evidence at line 3. +const FIVE_LINES = `${["const a = 1", "const b = 2", EVIDENCE, "const d = 4", "const e = 5"].join("\n")}\n`; + +describe("validateFindingFacts — mis-anchored vs fabricated", () => { + // GUARD 1. WITHOUT the mechanism: severity INFO + fact_invalid -> 0 blocking. + // WITH it: CRITICAL kept at the quoted line 3 + anchor_repaired -> 1 blocking. + it("re-anchors an out-of-range finding whose evidence_line matches a real line", () => { + const dir = repo(FIVE_LINES); + const out = validateFindingFacts( + [mkFinding({ line_start: 67, line_end: 67, evidence_line: EVIDENCE })], + dir, + new Set(), + ); + expect(out[0]?.severity).toBe("CRITICAL"); + expect(out[0]?.line_start).toBe(3); + expect(out[0]?.line_end).toBe(3); + expect(out[0]?.anchor_repaired).toBe(true); + expect(out[0]?.fact_invalid).toBeUndefined(); + expect(out[0]?.details).toContain("re-anchored"); + }); + + // GUARD 2 (passes on current code — MUTATION-CHECKED in Step 3). + // WITHOUT the evidence gate (repair unconditionally): fact_invalid absent. + // WITH it: fact_invalid true. This is the empty-file field-report case. + it("still demotes an out-of-range finding that carries NO evidence_line", () => { + const dir = repo(FIVE_LINES); + const out = validateFindingFacts( + [mkFinding({ file: "empty.yaml", line_start: 2, line_end: 2 })], + dir, + new Set(), + ); + expect(out[0]?.severity).toBe("INFO"); + expect(out[0]?.fact_invalid).toBe(true); + expect(out[0]?.anchor_repaired).toBeUndefined(); + }); + + // GUARD 3 (passes on current code — MUTATION-CHECKED in Step 3). + // WITHOUT the match test: fact_invalid absent. WITH it: fact_invalid true. + it("still demotes when the evidence_line matches NO line of the cited file", () => { + const dir = repo(FIVE_LINES); + const out = validateFindingFacts( + [mkFinding({ line_start: 67, evidence_line: "this line is nowhere in the file" })], + dir, + new Set(), + ); + expect(out[0]?.severity).toBe("INFO"); + expect(out[0]?.fact_invalid).toBe(true); + expect(out[0]?.anchor_repaired).toBeUndefined(); + }); + + // GUARD 4. WITHOUT the nearest-match rule (first match wins): line_start 2. + // WITH it: line_start 8. Deterministic either way, but only one is the rule. + it("resolves a multi-match to the occurrence NEAREST the cited line", () => { + const dup = `${["a", EVIDENCE, "c", "d", "e", "f", "g", EVIDENCE, "i", "j"].join("\n")}\n`; + const dir = repo(dup); + const out = validateFindingFacts( + [mkFinding({ line_start: 20, line_end: 20, evidence_line: EVIDENCE })], + dir, + new Set(), + ); + expect(out[0]?.line_start).toBe(8); + expect(out[0]?.anchor_repaired).toBe(true); + }); + + // GUARD 5 (passes on current code — MUTATION-CHECKED in Step 3). + // WITHOUT the range check first (repair before it): line_start moves to 3. + // WITH the correct order: line_start stays 1. The pass must never move a VALID anchor. + it("never touches a finding whose cited line is IN range", () => { + const dir = repo(FIVE_LINES); + const out = validateFindingFacts( + [mkFinding({ line_start: 1, line_end: 1, evidence_line: EVIDENCE })], + dir, + new Set(), + ); + expect(out[0]?.line_start).toBe(1); + expect(out[0]?.anchor_repaired).toBeUndefined(); + expect(out[0]?.fact_invalid).toBeUndefined(); + }); + + // Robustness: the match runs under normalizeLine, so indentation/whitespace differences in + // the reviewer's quote must not defeat it. WITHOUT normalization: no match -> demoted. + // WITH it: re-anchored to line 3. + it("matches the quote whitespace-insensitively", () => { + const dir = repo(FIVE_LINES); + const out = validateFindingFacts( + [ + mkFinding({ + line_start: 67, + evidence_line: "return readFileSync(`./templates/${name}`, 'utf8')", + }), + ], + dir, + new Set(), + ); + expect(out[0]?.line_start).toBe(3); + expect(out[0]?.anchor_repaired).toBe(true); + }); +}); From 58c985fe8aa518cb7061b2f59d2f967d506618e4 Mon Sep 17 00:00:00 2001 From: Codevena Date: Wed, 5 Aug 2026 22:58:27 +0200 Subject: [PATCH 05/14] fix(aggregator): carry anchor_repaired across a dedup merge --- src/core/aggregator.ts | 7 +++ src/schemas/finding.ts | 4 ++ tests/unit/anchor-repair-cascade.test.ts | 59 ++++++++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 tests/unit/anchor-repair-cascade.test.ts diff --git a/src/core/aggregator.ts b/src/core/aggregator.ts index 06b545c..4b5a8da 100644 --- a/src/core/aggregator.ts +++ b/src/core/aggregator.ts @@ -303,6 +303,7 @@ function memberOf(f: Finding): NonNullable[number] { // can OR it in (a flagged member merged under an unflagged equal-severity rep must not // silently lose the flag). Only set when true so members[] stays minimal otherwise. ...(f.demoted_from_critical === true ? { demoted_from_critical: true } : {}), + ...(f.anchor_repaired === true ? { anchor_repaired: true } : {}), }; } @@ -529,6 +530,11 @@ export function aggregate(input: AggregateInput): AggregateResult { const demotedFromCritical = sample.demoted_from_critical === true || members.some((m) => m.demoted_from_critical === true); + // Slice A mirror of the G0 OR above: the repaired finding is often NOT the representative + // (equal severity + ties-keep-first), and losing the marker would hide the mis-anchor in + // exactly the merge the repair made possible. + const anchorRepaired = + sample.anchor_repaired === true || members.some((m) => m.anchor_repaired === true); deduped.push({ ...sample, details: details.slice(0, 2000), @@ -536,6 +542,7 @@ export function aggregate(input: AggregateInput): AggregateResult { consensus, members, ...(demotedFromCritical ? { demoted_from_critical: true } : {}), + ...(anchorRepaired ? { anchor_repaired: true } : {}), }); } diff --git a/src/schemas/finding.ts b/src/schemas/finding.ts index 20155e9..a0e590e 100644 --- a/src/schemas/finding.ts +++ b/src/schemas/finding.ts @@ -258,6 +258,10 @@ export const FindingSchema = z.object({ // merge can OR-propagate it to the representative (a demoted member merged under // an unflagged equal-severity representative must not silently lose the flag). demoted_from_critical: z.boolean().optional(), + // Slice A: per-member mis-anchor provenance, so a repaired member merged under an + // unrepaired equal-severity representative (ties-keep-first) does not silently lose + // the marker — the badge and the pilot count both key on it. + anchor_repaired: z.boolean().optional(), }), ) .optional(), diff --git a/tests/unit/anchor-repair-cascade.test.ts b/tests/unit/anchor-repair-cascade.test.ts new file mode 100644 index 0000000..c52b23b --- /dev/null +++ b/tests/unit/anchor-repair-cascade.test.ts @@ -0,0 +1,59 @@ +// tests/unit/anchor-repair-cascade.test.ts +import { describe, expect, it } from "bun:test"; +import { aggregate } from "../../src/core/aggregator.ts"; +import type { Finding } from "../../src/schemas/finding.ts"; + +function fin(over: Partial): Finding { + return { + id: "F-x", + signature: "s", + severity: "WARN", + category: "security", + rule_id: "r", + file: "src/store.ts", + line_start: 25, + line_end: 25, + message: "m", + details: "d", + reviewer: { provider: "ollama", model: "glm-5.2:cloud", persona: "correctness" }, + confidence: 0.55, + consensus: "singleton", + ...over, + }; +} + +describe("anchor_repaired survives a dedup merge", () => { + // WITHOUT the OR-propagation: anchor_repaired is undefined on the merged finding (it lived on + // the member, and ties-keep-first made the UNrepaired finding the representative) -> the badge + // and pilot-03's count both disappear in exactly the cascade case. + // WITH it: anchor_repaired is true on the single merged finding. + it("OR-propagates anchor_repaired from a merged member to the representative", () => { + const r = aggregate({ + findings: [ + fin({ + signature: "sigA", + line_start: 25, + line_end: 25, + rule_id: "path-traversal-readtemplate", + }), + fin({ + signature: "sigB", + line_start: 26, + line_end: 26, + rule_id: "path-traversal", + anchor_repaired: true, + reviewer: { + provider: "openrouter", + model: "deepseek/deepseek-v3.2", + persona: "security", + }, + confidence: 0.9, + }), + ], + reviewersTotal: 2, + }); + expect(r.dedupedFindings.length).toBe(1); + expect(r.dedupedFindings[0]?.anchor_repaired).toBe(true); + expect(r.dedupedFindings[0]?.consensus).toBe("majority"); + }); +}); From 2c7066fd1c4e6fec7da0b282210c5664aa754a0d Mon Sep 17 00:00:00 2001 From: Codevena Date: Wed, 5 Aug 2026 23:07:00 +0200 Subject: [PATCH 06/14] feat(report): badge a finding whose anchor was repaired from its quoted evidence --- src/core/report-writer.ts | 6 ++++++ tests/unit/anchor-repair-cascade.test.ts | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/core/report-writer.ts b/src/core/report-writer.ts index a0c2aeb..13c985b 100644 --- a/src/core/report-writer.ts +++ b/src/core/report-writer.ts @@ -41,6 +41,12 @@ export function findingBadges(f: Finding): string | null { if (f.deterministic) badges.push("🔒 deterministic check — fix it (re-runs automatically; not rejectable)"); if (f.fact_invalid) badges.push("🔎 cited location not found — likely hallucinated"); + // Slice A: the counterpart to the badge above — the cited line was wrong, but the reviewer's + // quoted evidence proved the finding is grounded, so it was moved rather than demoted. + if (f.anchor_repaired) + badges.push( + "⚑ reviewer cited a line that does not exist — re-anchored to the source line it quoted", + ); if (f.grounding_demoted) badges.push("🌫 cited token absent from corpus — likely fabricated"); if (f.hypothetical_demoted) badges.push( diff --git a/tests/unit/anchor-repair-cascade.test.ts b/tests/unit/anchor-repair-cascade.test.ts index c52b23b..1d25af8 100644 --- a/tests/unit/anchor-repair-cascade.test.ts +++ b/tests/unit/anchor-repair-cascade.test.ts @@ -1,6 +1,7 @@ // tests/unit/anchor-repair-cascade.test.ts import { describe, expect, it } from "bun:test"; import { aggregate } from "../../src/core/aggregator.ts"; +import { findingBadges } from "../../src/core/report-writer.ts"; import type { Finding } from "../../src/schemas/finding.ts"; function fin(over: Partial): Finding { @@ -57,3 +58,17 @@ describe("anchor_repaired survives a dedup merge", () => { expect(r.dedupedFindings[0]?.consensus).toBe("majority"); }); }); + +describe("anchor_repaired badge", () => { + // WITHOUT the badge: findingBadges returns null for a repaired finding -> the agent sees a + // silently corrected line number and no signal that a reviewer mis-anchored. + // WITH it: the badge text is present. + it("renders a badge naming the repair", () => { + const out = findingBadges(fin({ anchor_repaired: true })); + expect(out).toContain("re-anchored"); + }); + + it("renders no such badge for an ordinary finding", () => { + expect(findingBadges(fin({}))).toBeNull(); + }); +}); From 82b23c6d684ddd5b8b4ec81d619c2ec9fdb12cbc Mon Sep 17 00:00:00 2001 From: Codevena Date: Wed, 5 Aug 2026 23:22:31 +0200 Subject: [PATCH 07/14] fix(aggregator): the critic may not demote a security finding below WARN --- src/core/aggregator.ts | 12 +++++- tests/unit/aggregator-critic.test.ts | 55 ++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/core/aggregator.ts b/src/core/aggregator.ts index 4b5a8da..530a206 100644 --- a/src/core/aggregator.ts +++ b/src/core/aggregator.ts @@ -609,6 +609,14 @@ export function aggregate(input: AggregateInput): AggregateResult { const cv = critic && critSigs.map((s) => critic.get(s)).find((v) => v?.verdict === "likely_fp"); if (cv?.verdict === "likely_fp") { const isCriticalSecurity = f.severity === "CRITICAL" && touchesSecurityOrCorrectness(f); + // pilot-02 turn 2: the exemption above is keyed to CRITICAL, so a WARN-severity security + // finding was demotable by a single adversarial critic — and WARN→INFO is the one demote + // that crosses the blocking boundary (isBlocking = CRITICAL || WARN). The sibling + // delta-scope pass already exempts security/correctness at ANY severity; mirror that floor + // here. An already-INFO security finding stays droppable, so the critic keeps its + // FP-filtering power where reviewers are noisiest. + const isBlockingSecurity = f.severity === "WARN" && touchesSecurityOrCorrectness(f); + const isSecurityProtected = isCriticalSecurity || isBlockingSecurity; // A single adversarial critic must not override GROUP agreement. Both // unanimous AND majority are corroborated consensus — the verdict gate // treats them identically (warnFail), and the confidence- and reputation- @@ -619,11 +627,11 @@ export function aggregate(input: AggregateInput): AggregateResult { // the critic calls it likely_fp — the dangerous direction is a demoted TRUE positive // (field report F-005). Tag it so the agent sees WHY it stayed blocking; do NOT set // critic_verdict (that renders the dismissive "likely FP" badge). - if (!isCriticalSecurity && !isCorroborated && isProtected(f)) { + if (!isSecurityProtected && !isCorroborated && isProtected(f)) { survivors.push({ ...f, protected_high_precision: true }); continue; } - if (!isCriticalSecurity && !isCorroborated) { + if (!isSecurityProtected && !isCorroborated) { const demoted = demoteOneStep(f); if (demoted.severity === "drop") { criticDropped.push(f); // INFO likely_fp dropped entirely — keep it attributable diff --git a/tests/unit/aggregator-critic.test.ts b/tests/unit/aggregator-critic.test.ts index 6780187..d82cd33 100644 --- a/tests/unit/aggregator-critic.test.ts +++ b/tests/unit/aggregator-critic.test.ts @@ -203,3 +203,58 @@ describe("aggregate with critic", () => { expect(r.dedupedFindings[0]?.consensus).toBe("unanimous"); }); }); + +describe("critic — security/correctness floor (pilot-02 turn 2)", () => { + // GUARD 7. WITHOUT the floor: INFO + critic_verdict "likely_fp" -> 0 blocking. + // WITH it: WARN + critic_verdict "keep" -> 1 blocking. + it("does not demote a WARN security finding below the blocking boundary", () => { + const f = fin({ signature: "sigSecWarn", severity: "WARN", category: "security" }); + const r = aggregate({ + findings: [f], + reviewersTotal: 2, + critic: new Map([["sigSecWarn", { verdict: "likely_fp" }]]), + }); + expect(r.dedupedFindings[0]?.severity).toBe("WARN"); + expect(r.dedupedFindings[0]?.critic_verdict).toBe("keep"); + }); + + it("does not demote a WARN correctness finding below the blocking boundary", () => { + const f = fin({ signature: "sigCorrWarn", severity: "WARN", category: "correctness" }); + const r = aggregate({ + findings: [f], + reviewersTotal: 2, + critic: new Map([["sigCorrWarn", { verdict: "likely_fp" }]]), + }); + expect(r.dedupedFindings[0]?.severity).toBe("WARN"); + expect(r.dedupedFindings[0]?.critic_verdict).toBe("keep"); + }); + + // GUARD 8 (passes on current code — MUTATION-CHECKED in Step 3). + // WITH an "exempt at every severity" floor: criticDroppedCount 0. + // WITH the correct WARN floor: criticDroppedCount 1. The critic keeps its FP-filtering + // power exactly where reviewers are noisiest (low-confidence INFO security chatter). + it("still drops an already-INFO security likely_fp", () => { + const f = fin({ signature: "sigSecInfo", severity: "INFO", category: "security" }); + const r = aggregate({ + findings: [f], + reviewersTotal: 2, + critic: new Map([["sigSecInfo", { verdict: "likely_fp" }]]), + }); + expect(r.criticDroppedCount).toBe(1); + expect(r.dedupedFindings.length).toBe(0); + }); + + // GUARD 9 (passes on current code — MUTATION-CHECKED in Step 3). + // WITH a severity-only floor (all WARN exempt): the finding stays WARN -> 1 blocking. + // WITH the correct category-keyed floor: INFO -> 0 blocking. + it("still demotes a WARN quality finding", () => { + const f = fin({ signature: "sigQualWarn", severity: "WARN", category: "quality" }); + const r = aggregate({ + findings: [f], + reviewersTotal: 2, + critic: new Map([["sigQualWarn", { verdict: "likely_fp" }]]), + }); + expect(r.dedupedFindings[0]?.severity).toBe("INFO"); + expect(r.dedupedFindings[0]?.critic_verdict).toBe("likely_fp"); + }); +}); From aadfebc5920a71f9e5f45f139b624063a5bb6d08 Mon Sep 17 00:00:00 2001 From: Codevena Date: Wed, 5 Aug 2026 23:33:59 +0200 Subject: [PATCH 08/14] =?UTF-8?q?test:=20pilot-02=20turn-2=20cascade=20?= =?UTF-8?q?=E2=80=94=20repair,=20merge,=20corroborate,=20stay=20blocking?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/unit/anchor-repair-cascade.test.ts | 91 ++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/tests/unit/anchor-repair-cascade.test.ts b/tests/unit/anchor-repair-cascade.test.ts index 1d25af8..c6a3631 100644 --- a/tests/unit/anchor-repair-cascade.test.ts +++ b/tests/unit/anchor-repair-cascade.test.ts @@ -1,6 +1,10 @@ // tests/unit/anchor-repair-cascade.test.ts import { describe, expect, it } from "bun:test"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { aggregate } from "../../src/core/aggregator.ts"; +import { validateFindingFacts } from "../../src/core/fact-check.ts"; import { findingBadges } from "../../src/core/report-writer.ts"; import type { Finding } from "../../src/schemas/finding.ts"; @@ -72,3 +76,90 @@ describe("anchor_repaired badge", () => { expect(findingBadges(fin({}))).toBeNull(); }); }); + +// The 27-line src/store.ts exactly as turn 2's diff created it. Line 26 is the quoted evidence. +const STORE_TS = `${[ + "import { readFileSync } from 'node:fs'", + "", + "export interface KVStore {", + " get(key: K): V | undefined", + " set(key: K, value: V): void", + " has(key: K): boolean", + "}", + "", + "export function createStore(): KVStore {", + " const entries = new Map()", + "", + " return {", + " get(key) {", + " return entries.get(key)", + " },", + " set(key, value) {", + " entries.set(key, value)", + " },", + " has(key) {", + " return entries.has(key)", + " },", + " }", + "}", + "", + "export function readTemplate(name: string): string {", + " return readFileSync(`./templates/${name}`, 'utf8')", + "}", +].join("\n")}\n`; + +describe("pilot-02 turn 2 — the full cascade", () => { + // WITHOUT the fix: F-002 is demoted as fabricated (INFO + fact_invalid) and stays 42 lines + // from F-001, so nothing merges, both stay `singleton`, and the critic demotes F-001 to INFO + // -> 2 findings, 0 blocking. This is what pilot-02 recorded. + // WITH the fix: F-002 re-anchors 67 -> 26, merges with F-001 at 25, consensus becomes + // `majority`, the critic is barred -> 1 finding, blocking WARN. + it("repairs the anchor, merges, corroborates, and stays blocking", () => { + const dir = mkdtempSync(join(tmpdir(), "rg-turn2-")); + mkdirSync(join(dir, "src"), { recursive: true }); + writeFileSync(join(dir, "src", "store.ts"), STORE_TS); + + const evidence = " return readFileSync(`./templates/${name}`, 'utf8')"; + const raw: Finding[] = [ + fin({ + signature: "sigF001", + rule_id: "path-traversal-readtemplate", + line_start: 25, + line_end: 27, + confidence: 0.55, + evidence_line: evidence, + message: "readTemplate interpolates 'name' directly into a filesystem path", + }), + fin({ + signature: "sigF002", + rule_id: "path-traversal", + line_start: 67, + line_end: 67, + confidence: 0.9, + evidence_line: evidence, + message: "Path traversal vulnerability in readTemplate.", + reviewer: { provider: "openrouter", model: "deepseek/deepseek-v3.2", persona: "security" }, + }), + ]; + + const checked = validateFindingFacts(raw, dir, new Set()); + expect(checked.find((f) => f.signature === "sigF002")?.line_start).toBe(26); + + const r = aggregate({ + findings: checked, + reviewersTotal: 2, + changedRanges: new Map([["src/store.ts", [[1, 27]] as Array<[number, number]>]]), + scopeToDiff: true, + // The critic called the weaker detection a likely FP, exactly as it did in the pilot. + critic: new Map([["sigF001", { verdict: "likely_fp" }]]), + }); + + expect(r.dedupedFindings.length).toBe(1); + expect(r.dedupedFindings[0]?.consensus).toBe("majority"); + expect(r.dedupedFindings[0]?.severity).toBe("WARN"); + expect(r.dedupedFindings[0]?.critic_verdict).toBe("keep"); + expect(r.dedupedFindings[0]?.anchor_repaired).toBe(true); + expect(r.dedupedFindings[0]?.scope_demoted).toBeUndefined(); + expect(r.dedupedFindings[0]?.fact_invalid).toBeUndefined(); + }); +}); From ad6c03f88de2f8c319961b8e53803908d335e314 Mon Sep 17 00:00:00 2001 From: Codevena Date: Wed, 5 Aug 2026 23:41:17 +0200 Subject: [PATCH 09/14] test: clean up temp dirs created by the anchor-repair tests --- tests/unit/anchor-repair-cascade.test.ts | 11 +++++++++-- tests/unit/fact-check-reanchor.test.ts | 11 +++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/tests/unit/anchor-repair-cascade.test.ts b/tests/unit/anchor-repair-cascade.test.ts index c6a3631..c9b7620 100644 --- a/tests/unit/anchor-repair-cascade.test.ts +++ b/tests/unit/anchor-repair-cascade.test.ts @@ -1,6 +1,6 @@ // tests/unit/anchor-repair-cascade.test.ts -import { describe, expect, it } from "bun:test"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { afterAll, describe, expect, it } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { aggregate } from "../../src/core/aggregator.ts"; @@ -8,6 +8,12 @@ import { validateFindingFacts } from "../../src/core/fact-check.ts"; import { findingBadges } from "../../src/core/report-writer.ts"; import type { Finding } from "../../src/schemas/finding.ts"; +// Temp dirs created by this file's tests, removed after the run whether tests pass or fail. +const createdDirs: string[] = []; +afterAll(() => { + for (const dir of createdDirs) rmSync(dir, { recursive: true, force: true }); +}); + function fin(over: Partial): Finding { return { id: "F-x", @@ -116,6 +122,7 @@ describe("pilot-02 turn 2 — the full cascade", () => { // `majority`, the critic is barred -> 1 finding, blocking WARN. it("repairs the anchor, merges, corroborates, and stays blocking", () => { const dir = mkdtempSync(join(tmpdir(), "rg-turn2-")); + createdDirs.push(dir); mkdirSync(join(dir, "src"), { recursive: true }); writeFileSync(join(dir, "src", "store.ts"), STORE_TS); diff --git a/tests/unit/fact-check-reanchor.test.ts b/tests/unit/fact-check-reanchor.test.ts index 31b2c70..ddb1502 100644 --- a/tests/unit/fact-check-reanchor.test.ts +++ b/tests/unit/fact-check-reanchor.test.ts @@ -5,13 +5,19 @@ // evidence_line matched line 26 of that file verbatim. Mis-anchored, not fabricated. These // guards pin the distinction, and pin that the fabrication protection the pass was built for // (a CRITICAL citing a line in an EMPTY file) is untouched. -import { describe, expect, it } from "bun:test"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { afterAll, describe, expect, it } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { validateFindingFacts } from "../../src/core/fact-check.ts"; import type { Finding } from "../../src/schemas/finding.ts"; +// Temp dirs created by repo(), removed after the run whether tests pass or fail. +const createdDirs: string[] = []; +afterAll(() => { + for (const dir of createdDirs) rmSync(dir, { recursive: true, force: true }); +}); + const EVIDENCE = " return readFileSync(`./templates/${name}`, 'utf8')"; function mkFinding(over: Partial = {}): Finding { @@ -35,6 +41,7 @@ function mkFinding(over: Partial = {}): Finding { function repo(content: string): string { const dir = mkdtempSync(join(tmpdir(), "rg-reanchor-")); + createdDirs.push(dir); mkdirSync(join(dir, "src"), { recursive: true }); writeFileSync(join(dir, "store.ts"), content); writeFileSync(join(dir, "empty.yaml"), ""); From 11c1701cbfcea7974b8425a28efdf74a96e325c4 Mon Sep 17 00:00:00 2001 From: Codevena Date: Wed, 5 Aug 2026 23:47:14 +0200 Subject: [PATCH 10/14] test: state honestly what the cascade test proves, and assert the badge --- tests/unit/anchor-repair-cascade.test.ts | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tests/unit/anchor-repair-cascade.test.ts b/tests/unit/anchor-repair-cascade.test.ts index c9b7620..3925a0f 100644 --- a/tests/unit/anchor-repair-cascade.test.ts +++ b/tests/unit/anchor-repair-cascade.test.ts @@ -115,11 +115,19 @@ const STORE_TS = `${[ ].join("\n")}\n`; describe("pilot-02 turn 2 — the full cascade", () => { - // WITHOUT the fix: F-002 is demoted as fabricated (INFO + fact_invalid) and stays 42 lines - // from F-001, so nothing merges, both stay `singleton`, and the critic demotes F-001 to INFO - // -> 2 findings, 0 blocking. This is what pilot-02 recorded. - // WITH the fix: F-002 re-anchors 67 -> 26, merges with F-001 at 25, consensus becomes - // `majority`, the critic is barred -> 1 finding, blocking WARN. + // WITHOUT the fix (Task 1): F-002 is demoted as fabricated (INFO + fact_invalid) and stays 42 + // lines from F-001, so nothing merges, both stay `singleton` -> 2 findings, 0 blocking. This is + // what pilot-02 recorded. + // WITH the fix: F-002 re-anchors 67 -> 26, merges with F-001 at 25, and the merge lifts + // consensus to `majority` -> 1 finding. + // What bars the critic here is pre-existing corroboration (consensus === "majority"), NOT + // Task 4's WARN-security floor — with Task 4 reverted this scenario still passes, because the + // repair-driven merge already makes the finding corroborated before the critic runs. Task 4's + // floor covers the DIFFERENT, uncorroborated case: a single WARN security finding with no + // second detection to merge with. That case is exercised separately in + // tests/unit/aggregator-critic.test.ts, describe "critic — security/correctness floor + // (pilot-02 turn 2)". This test's job is to prove the repair -> merge -> corroboration chain + // (Tasks 1-2) composes and ends up blocking WARN, badge included (Task 3). it("repairs the anchor, merges, corroborates, and stays blocking", () => { const dir = mkdtempSync(join(tmpdir(), "rg-turn2-")); createdDirs.push(dir); @@ -162,10 +170,13 @@ describe("pilot-02 turn 2 — the full cascade", () => { }); expect(r.dedupedFindings.length).toBe(1); + const merged = r.dedupedFindings[0]; + if (!merged) throw new Error("expected one merged finding"); expect(r.dedupedFindings[0]?.consensus).toBe("majority"); expect(r.dedupedFindings[0]?.severity).toBe("WARN"); expect(r.dedupedFindings[0]?.critic_verdict).toBe("keep"); expect(r.dedupedFindings[0]?.anchor_repaired).toBe(true); + expect(findingBadges(merged)).toContain("re-anchored"); expect(r.dedupedFindings[0]?.scope_demoted).toBeUndefined(); expect(r.dedupedFindings[0]?.fact_invalid).toBeUndefined(); }); From 591367c45b0125036bfc9a5dadad2bfd569b2a29 Mon Sep 17 00:00:00 2001 From: Codevena Date: Thu, 6 Aug 2026 00:16:33 +0200 Subject: [PATCH 11/14] fix(fact-check): require an identifier-like token in the evidence quote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A punctuation-only evidence_line ("}", " }", fullwidth "｝") normalizes to a string that matches many lines, so it could re-anchor and rescue a wholly fabricated CRITICAL that the pre-branch code correctly demoted. Require the quote to carry at least one identifier-like token before it counts as a repair key; anything else falls through to the existing demote. Also: correct the now-backwards evidence_line trust-status comments in the finding schema and the review-output schema shown to the model, fix an off-by-one in the acceptance test's simulated hunk range, reword the tie-break doc comment on reanchorByEvidence (unreachable given the range-checked call site — last-match and nearest-match are the same occurrence), rename a label collision in report-writer.ts, and correct two overclaims plus record one deliberate seam in the design doc. --- .../2026-08-05-true-positive-hole-design.md | 25 +++++++++++++------ src/core/fact-check.ts | 18 +++++++++---- src/core/report-writer.ts | 4 +-- src/providers/review-output.ts | 10 +++++--- src/schemas/finding.ts | 7 ++++-- tests/unit/anchor-repair-cascade.test.ts | 2 +- tests/unit/fact-check-reanchor.test.ts | 20 +++++++++++++++ 7 files changed, 66 insertions(+), 20 deletions(-) diff --git a/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md b/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md index 2735762..73156f2 100644 --- a/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md +++ b/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md @@ -76,8 +76,10 @@ console.log({tot, ev, fi})' | distinct findings `fact_invalid` fired on, in 24 turns | **1** | | ...whose evidence line matched a real line of the cited file | **1 of 1** | -The one time this pass fired, it labelled a real security finding a hallucination. **n = 1**: the -change below is justified by the mechanism, not by that sample size. +Running the command above prints `fi: 2`, not 1 — both numbers are right. It is the **same** +signature (`8deca571…`) recurring in `turns/3`, so the raw count is 2 rows but 1 distinct finding. +The one time this pass fired (on a distinct finding), it labelled a real security finding a +hallucination. **n = 1**: the change below is justified by the mechanism, not by that sample size. ### Why this one anchor cost the whole turn @@ -113,6 +115,13 @@ toggles; no new config key, and so no second TTY control-plane approval. provider-contract change, as recorded in the task-(a) plan. - **Findings with no `evidence_line`** (19 % of the corpus) get no new protection. Their behaviour is unchanged. +- **Signature reordering.** `applySymbolSignatures` runs at `orchestrator.ts:2219`, *before* + `validateFindingFacts` at `:2226`, so a repaired finding keeps a signature derived from the + phantom pre-repair line rather than the symbol-relative form a correctly-anchored duplicate + would get. Deliberate, not an oversight: reordering the two passes would invalidate every + persisted signature and cache entry. Accepted because the location-keyed guard closes exactly + this gap and, thanks to the repair, now sees a *stable* region instead of a churning bogus one + (final review, M-6). ## Slice A — distinguish mis-anchored from fabricated @@ -141,10 +150,12 @@ On a match it returns the finding re-anchored to that line, with `line_end` coll `anchor_repaired: true` marker, and a details note recording the original number. **Disambiguation, stated as a rule rather than left to chance:** when the quote matches several -lines, re-anchor to the occurrence **nearest the cited line**; ties break to the **lower** line -number. Both candidates are real occurrences of the reviewer's own quote, so this chooses among -facts rather than inventing one, and it is fully deterministic — which the aggregator's clustering -requires (`:433-444` sorts precisely to keep clustering order-independent). +lines, re-anchor to the **LAST** matching occurrence — which, because this only ever runs on a +citation past EOF, is also the occurrence **nearest** the cited line, so a tie cannot occur given +the range-checked call site. Any matching occurrence is a real instance of the reviewer's own +quote, so this chooses among facts rather than inventing one, and it is fully deterministic — +which the aggregator's clustering requires (`:433-444` sorts precisely to keep clustering +order-independent). **This is not the gate inventing a location.** It re-anchors to a line the reviewer itself quoted. The rejected alternative — clamping to the nearest changed hunk — would have been. @@ -318,7 +329,7 @@ not part of this spec. | Risk | Handling | |---|---| -| Re-anchoring rescues a genuinely fabricated finding | Only when the reviewer's quote is **found verbatim in the cited file**. A model that fabricates a line number *and* quotes real source has still read the file — that is the definition of mis-anchored. Guards 2 and 3 pin both no-quote and no-match paths | +| Re-anchoring rescues a genuinely fabricated finding | Only when the reviewer's quote is **found verbatim in the cited file** AND carries at least one identifier-like token (3+ word characters) — a punctuation-only quote like `}` matches dozens of lines and was demonstrated to rescue a fabricated CRITICAL, so the quote alone is not proof the finding is real (final review, I-1). This bounds but does not eliminate the residual: a fabricator that additionally invents a plausible identifier-bearing line, or that simply cites an IN-RANGE line, was never covered by this pass at all — `validateFindingFacts` only ever runs on out-of-range citations. Guards 2 and 3 pin both no-quote and no-match paths | | The repair moves a finding onto unrelated code | It moves it onto a line the reviewer quoted; multi-match resolves to the nearest occurrence, deterministically. Guard 4 pins the rule | | Weakening the empty-file protection the pass was built for | An empty file has no lines, so no quote can match → the motivating case can never be re-anchored. Guard 2 uses exactly that fixture | | Slice B re-inflates FP burden | Floor stops at WARN; already-INFO security stays droppable (guard 8). pilot-03 reports FP burden, though pilot-02 showed M2 has no signal in this rig | diff --git a/src/core/fact-check.ts b/src/core/fact-check.ts index 4c69ac5..0789170 100644 --- a/src/core/fact-check.ts +++ b/src/core/fact-check.ts @@ -57,16 +57,22 @@ function demote(f: Finding, note: string): Finding { * which case the caller demotes exactly as before. This can only ever be reached for an * out-of-range anchor, so it can never move a valid one. * - * Multiple matches resolve to the occurrence NEAREST the cited line, ties to the LOWER line - * number. Both are real occurrences of the reviewer's own quote, so this chooses among facts - * rather than inventing one — and it must be deterministic, because the aggregator's clustering - * is deliberately order-independent and keys on line_start. + * Multiple matches resolve to the LAST matching occurrence — which, because this only ever runs on + * a citation past EOF (the range check above always fires first), is also the occurrence NEAREST + * the cited line; a tie cannot occur given the range-checked call site. Any matching occurrence is + * a real instance of the reviewer's own quote, so this chooses among facts rather than inventing + * one — and it must be deterministic, because the aggregator's clustering is deliberately + * order-independent and keys on line_start. */ function reanchorByEvidence(f: Finding, text: string): Finding | null { const ev = typeof f.evidence_line === "string" ? f.evidence_line : null; if (ev === null || ev.length === 0) return null; const evN = normalizeLine(ev); if (evN.length === 0) return null; // quote was only whitespace/markers → no signal + // A quote with no identifier-like token ("}", "},", "});") names no code: it matches dozens of + // lines and proves nothing about WHICH line the reviewer read. Falling through to the demote is + // fail-safe — it restores exactly the pre-repair behaviour for a quote that carries no signal. + if (!/[A-Za-z_$][A-Za-z0-9_$]{2,}/.test(evN)) return null; const lines = text.split("\n"); const cited = f.line_start - 1; let best = -1; @@ -74,7 +80,9 @@ function reanchorByEvidence(f: Finding, text: string): Finding | null { const raw = lines[i]; if (raw === undefined) continue; if (normalizeLine(raw) !== evN) continue; - // Strict `<` with an ascending scan makes ties keep the LOWER line number. + // Strict `<` with an ascending scan keeps the LAST match. Since `cited` is always past EOF + // here, every match index is strictly below it and distance is monotonic in i, so "last + // match" and "nearest match" are the same occurrence — no tie can arise to break. if (best === -1 || Math.abs(i - cited) < Math.abs(best - cited)) best = i; } if (best === -1) return null; // quote matches no line → the fabrication signal stands diff --git a/src/core/report-writer.ts b/src/core/report-writer.ts index 13c985b..447c2cd 100644 --- a/src/core/report-writer.ts +++ b/src/core/report-writer.ts @@ -41,8 +41,8 @@ export function findingBadges(f: Finding): string | null { if (f.deterministic) badges.push("🔒 deterministic check — fix it (re-runs automatically; not rejectable)"); if (f.fact_invalid) badges.push("🔎 cited location not found — likely hallucinated"); - // Slice A: the counterpart to the badge above — the cited line was wrong, but the reviewer's - // quoted evidence proved the finding is grounded, so it was moved rather than demoted. + // Anchor repair: the counterpart to the badge above — the cited line was wrong, but the + // reviewer's quoted evidence proved the finding is grounded, so it was moved rather than demoted. if (f.anchor_repaired) badges.push( "⚑ reviewer cited a line that does not exist — re-anchored to the source line it quoted", diff --git a/src/providers/review-output.ts b/src/providers/review-output.ts index 24e0bb0..4f85f53 100644 --- a/src/providers/review-output.ts +++ b/src/providers/review-output.ts @@ -60,9 +60,13 @@ export const REVIEW_OUTPUT_SCHEMA = { details: { type: "string" }, confidence: { type: "number" }, // S4 (field report 2026-06-23): the exact source line the finding relies on, verbatim, or - // null if the deciding line/artifact was not provided to the reviewer. RENDER-ONLY: a - // deterministic cross-check (fact-check.ts) badges a CLEAR mismatch vs the working-tree - // line; it never changes severity. Nullable per strict-mode (express optional via type). + // null if the deciding line/artifact was not provided to the reviewer. UNTRUSTED + // reviewer-supplied input: a deterministic cross-check (fact-check.ts) badges a CLEAR + // mismatch vs the working-tree line (render-only, never changes severity), AND — added by + // the true-positive-hole slice — `validateFindingFacts` consults this quote to tell a + // mis-anchored finding (out-of-range line, quote matches real source) from a fabricated + // one, which DOES decide blocking vs advisory. Nullable per strict-mode (express optional + // via type). evidence_line: { type: ["string", "null"] }, }, }, diff --git a/src/schemas/finding.ts b/src/schemas/finding.ts index a0e590e..44d1fb8 100644 --- a/src/schemas/finding.ts +++ b/src/schemas/finding.ts @@ -122,8 +122,11 @@ export const FindingSchema = z.object({ // unavailable). NEVER changes severity — purely an ownership tag, unlike foreign_to_session. session_attributable: z.boolean().optional(), // S4 (field report 2026-06-23): the exact source line the reviewer self-attests it relied on, - // verbatim (capped). RENDER-ONLY: fact-check badges a CLEAR mismatch vs the working-tree line. - // Never changes severity. + // verbatim (capped). UNTRUSTED reviewer-supplied input. Two consumers: `attestEvidence` badges a + // CLEAR mismatch vs the working-tree line (render-only, never changes severity); and — added by + // the true-positive-hole slice — `validateFindingFacts` (fact-check.ts) consults it to tell a + // MIS-ANCHORED finding (an out-of-range line whose quote matches real source) from a fabricated + // one, which DOES decide blocking vs advisory (see `anchor_repaired`). evidence_line: z.string().optional(), // S4: set true (render-only) when the reviewer's quoted evidence_line matches NO line in the cited // file — a strong signal it reasoned on stale/absent/fabricated context. Advisory badge ONLY; the diff --git a/tests/unit/anchor-repair-cascade.test.ts b/tests/unit/anchor-repair-cascade.test.ts index 3925a0f..7b9b02b 100644 --- a/tests/unit/anchor-repair-cascade.test.ts +++ b/tests/unit/anchor-repair-cascade.test.ts @@ -163,7 +163,7 @@ describe("pilot-02 turn 2 — the full cascade", () => { const r = aggregate({ findings: checked, reviewersTotal: 2, - changedRanges: new Map([["src/store.ts", [[1, 27]] as Array<[number, number]>]]), + changedRanges: new Map([["src/store.ts", [[1, 28]] as Array<[number, number]>]]), scopeToDiff: true, // The critic called the weaker detection a likely FP, exactly as it did in the pilot. critic: new Map([["sigF001", { verdict: "likely_fp" }]]), diff --git a/tests/unit/fact-check-reanchor.test.ts b/tests/unit/fact-check-reanchor.test.ts index ddb1502..bce0436 100644 --- a/tests/unit/fact-check-reanchor.test.ts +++ b/tests/unit/fact-check-reanchor.test.ts @@ -145,4 +145,24 @@ describe("validateFindingFacts — mis-anchored vs fabricated", () => { expect(out[0]?.line_start).toBe(3); expect(out[0]?.anchor_repaired).toBe(true); }); + + // GUARD 6 (I-1, final review 2026-08-05). A punctuation-only quote names no code: "}", " }", + // "\t}" and fullwidth "｝" all normalize to "}", which matches many lines and proves nothing + // about WHICH line the reviewer read. + // WITHOUT the identifier-token guard: the quote "}" matches the file's brace line and the + // out-of-range CRITICAL re-anchors -> severity CRITICAL, anchor_repaired true. + // WITH it: a quote with no identifier-like token falls through to the demote exactly as before + // -> severity INFO, fact_invalid true, anchor_repaired undefined. + it("does not repair on a punctuation-only quote, even when it matches a real line", () => { + const braceLines = `${["function f() {", " return 1", "}", "const b = 2", "const c = 3"].join("\n")}\n`; + const dir = repo(braceLines); + const out = validateFindingFacts( + [mkFinding({ line_start: 999, line_end: 999, evidence_line: "}" })], + dir, + new Set(), + ); + expect(out[0]?.severity).toBe("INFO"); + expect(out[0]?.fact_invalid).toBe(true); + expect(out[0]?.anchor_repaired).toBeUndefined(); + }); }); From 2a287f50a89f65c7ada11ba91e0b3fbffe88fca2 Mon Sep 17 00:00:00 2001 From: Codevena Date: Thu, 6 Aug 2026 00:29:34 +0200 Subject: [PATCH 12/14] fix(fact-check): let a two-character identifier qualify as an evidence quote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The identifier-token guard required a 3-character minimum ({2,}), which rejected genuine 2-character quotes ("if", "do", "in", "of", "as" and other 2-char identifiers) as evidence for an anchor repair. Lower the bound to {1,} (2-char minimum): punctuation-only quotes ("}", "});", fullwidth "｝") stay rejected at any bound since they contain no identifier character at all, so this only fixes the false-negative on real code without reopening the punctuation hole closed in the previous commit. Also label f.evidence_line as UNTRUSTED reviewer-supplied input at both reanchorByEvidence's doc comment and its call site in validateFindingFacts, now that it decides blocking vs advisory rather than being purely render-only. --- src/core/fact-check.ts | 26 +++++++++++++++++++------- tests/unit/fact-check-reanchor.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/core/fact-check.ts b/src/core/fact-check.ts index 0789170..8fddd2b 100644 --- a/src/core/fact-check.ts +++ b/src/core/fact-check.ts @@ -53,6 +53,11 @@ function demote(f: Finding, note: string): Finding { * traversal, quote verbatim at line 26, cited at 67, demoted with "almost certainly * hallucinated"). * + * `f.evidence_line` is UNTRUSTED reviewer-supplied input — it is never executed or taken on + * trust by itself. It only ever earns a repair by matching, byte-for-byte after normalization, + * real content already read from the working tree; a caller relaxing that normalization or the + * match/identifier-token guards below would let the reviewer's own text decide the outcome. + * * Returns the finding re-anchored to the quoted line, or null when nothing can be proven — in * which case the caller demotes exactly as before. This can only ever be reached for an * out-of-range anchor, so it can never move a valid one. @@ -69,10 +74,15 @@ function reanchorByEvidence(f: Finding, text: string): Finding | null { if (ev === null || ev.length === 0) return null; const evN = normalizeLine(ev); if (evN.length === 0) return null; // quote was only whitespace/markers → no signal - // A quote with no identifier-like token ("}", "},", "});") names no code: it matches dozens of - // lines and proves nothing about WHICH line the reviewer read. Falling through to the demote is - // fail-safe — it restores exactly the pre-repair behaviour for a quote that carries no signal. - if (!/[A-Za-z_$][A-Za-z0-9_$]{2,}/.test(evN)) return null; + // A quote with no identifier-like token at all ("}", "},", "});" — zero [A-Za-z_$] characters + // present) names no code: it matches dozens of lines and proves nothing about WHICH line the + // reviewer read. That rejection holds at ANY length bound, since these quotes contain no + // identifier character to begin with. The {1,} bound (2-character minimum) exists on top of + // that only to exclude a bare single-letter quote ("x", "a") — it still accepts a genuine + // 2-char token, including common JS keywords ("if", "do", "in", "of", "as"). Falling through to + // the demote is fail-safe — it restores exactly the pre-repair behaviour for a quote that + // carries no signal. + if (!/[A-Za-z_$][A-Za-z0-9_$]{1,}/.test(evN)) return null; const lines = text.split("\n"); const cited = f.line_start - 1; let best = -1; @@ -168,9 +178,11 @@ export function validateFindingFacts( } const lines = lineCount(text); if (f.line_start <= lines) return f; // cited line exists → real finding, untouched - // Out of range. Before calling it a fabrication, consult the reviewer's OWN quoted evidence: - // a quote that matches a real line of THIS file proves the reviewer read the code and - // mis-numbered it, which is not what this pass exists to catch. + // Out of range. Before calling it a fabrication, consult the reviewer's OWN quoted evidence — + // f.evidence_line is UNTRUSTED reviewer-supplied input, so this is a match against real file + // content already read above, never a trust decision made from the quote alone: a quote that + // matches a real line of THIS file proves the reviewer read the code and mis-numbered it, + // which is not what this pass exists to catch. const repaired = reanchorByEvidence(f, text); if (repaired !== null) return repaired; const note = `\n\n[reviewgate fact-check] cited location ${file}:${f.line_start} does not exist in the working tree (file has ${lines} line${lines === 1 ? "" : "s"}) — almost certainly hallucinated; demoted to advisory. Verify before treating as real.`; diff --git a/tests/unit/fact-check-reanchor.test.ts b/tests/unit/fact-check-reanchor.test.ts index bce0436..13c5372 100644 --- a/tests/unit/fact-check-reanchor.test.ts +++ b/tests/unit/fact-check-reanchor.test.ts @@ -165,4 +165,28 @@ describe("validateFindingFacts — mis-anchored vs fabricated", () => { expect(out[0]?.fact_invalid).toBe(true); expect(out[0]?.anchor_repaired).toBeUndefined(); }); + + // GUARD 7 (F-001, gate finding on this branch). The identifier bound must not be so strict that + // a genuine 2-character token — a JS keyword ("if", "do", "in", "of", "as") or any 2-char + // identifier — fails to qualify as a repair key, even though no individual token in the quoted + // line reaches 3 characters. + // WITHOUT the {1,} bound (the old {2,}, a 3-char minimum): "if (a || b) {" contains no token of + // 3+ chars, so the guard rejects it and the out-of-range CRITICAL falls through to the demote + // -> severity INFO, fact_invalid true, anchor_repaired undefined. + // WITH it: "if" (2 chars) qualifies as an identifier-like token, so the finding repairs -> + // severity CRITICAL, line_start moved to the matched line, anchor_repaired true. + it("repairs on a genuine 2-character identifier token, not just 3+", () => { + const twoCharLine = "if (a || b) {"; + const content = `${["const a = 1", "const b = 2", twoCharLine, "const d = 4", "const e = 5"].join("\n")}\n`; + const dir = repo(content); + const out = validateFindingFacts( + [mkFinding({ line_start: 999, line_end: 999, evidence_line: twoCharLine })], + dir, + new Set(), + ); + expect(out[0]?.severity).toBe("CRITICAL"); + expect(out[0]?.line_start).toBe(3); + expect(out[0]?.anchor_repaired).toBe(true); + expect(out[0]?.fact_invalid).toBeUndefined(); + }); }); From 4f117a0d585782e6a1d9496fc17f02d0ae9c62eb Mon Sep 17 00:00:00 2001 From: Codevena Date: Thu, 6 Aug 2026 01:01:56 +0200 Subject: [PATCH 13/14] chore(lore): re-verify review-output-schema-strict after the evidence_line doc change --- .reviewgate/lore/review-output-schema-strict.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.reviewgate/lore/review-output-schema-strict.md b/.reviewgate/lore/review-output-schema-strict.md index 7c72e59..18108dc 100644 --- a/.reviewgate/lore/review-output-schema-strict.md +++ b/.reviewgate/lore/review-output-schema-strict.md @@ -4,8 +4,8 @@ id: review-output-schema-strict status: canon anchors: - "src/providers/review-output.ts" -verified_at: 2026-07-10 -verified_tree: "3d6c09350b1a2dcede15eb0917e011310c0c327b409895dc00c469491a1a5068" +verified_at: 2026-08-06 +verified_tree: "5388991bd6aebe97b8fad3437f141401be49649d18cbc2a772761f301ef94487" tags: [] --- Why REVIEW_OUTPUT_SCHEMA is shaped the way it is: codex's `--output-schema` runs From 6f3bee9bbad4b339ab867e90a2f8c1a8175be0cf Mon Sep 17 00:00:00 2001 From: Codevena Date: Thu, 6 Aug 2026 08:35:18 +0200 Subject: [PATCH 14/14] docs: align the evidence-guard wording with the shipped two-char bound --- .../superpowers/specs/2026-08-05-true-positive-hole-design.md | 4 ++-- src/core/report-writer.ts | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md b/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md index 73156f2..23ae56b 100644 --- a/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md +++ b/docs/superpowers/specs/2026-08-05-true-positive-hole-design.md @@ -329,8 +329,8 @@ not part of this spec. | Risk | Handling | |---|---| -| Re-anchoring rescues a genuinely fabricated finding | Only when the reviewer's quote is **found verbatim in the cited file** AND carries at least one identifier-like token (3+ word characters) — a punctuation-only quote like `}` matches dozens of lines and was demonstrated to rescue a fabricated CRITICAL, so the quote alone is not proof the finding is real (final review, I-1). This bounds but does not eliminate the residual: a fabricator that additionally invents a plausible identifier-bearing line, or that simply cites an IN-RANGE line, was never covered by this pass at all — `validateFindingFacts` only ever runs on out-of-range citations. Guards 2 and 3 pin both no-quote and no-match paths | -| The repair moves a finding onto unrelated code | It moves it onto a line the reviewer quoted; multi-match resolves to the nearest occurrence, deterministically. Guard 4 pins the rule | +| Re-anchoring rescues a genuinely fabricated finding | Only when the reviewer's quote is **found verbatim in the cited file** AND carries at least one identifier-like token (2+ word characters) — a punctuation-only quote like `}` matches dozens of lines and was demonstrated to rescue a fabricated CRITICAL, so the quote alone is not proof the finding is real (final review, I-1). The bound was lowered from a 3-character to a 2-character minimum after this repo's own gate flagged that a real line whose longest token is 2 chars (e.g. `if (a || b) {`) would otherwise be wrongly demoted; measured over all 35 distinct `evidence_line` values recorded across both pilots, both bounds reject **0**, so the change is neutral on real data and strictly better on the short-token case. This bounds but does not eliminate the residual: a fabricator that additionally invents a plausible identifier-bearing line, or that simply cites an IN-RANGE line, was never covered by this pass at all — `validateFindingFacts` only ever runs on out-of-range citations. Guards 2 and 3 pin both no-quote and no-match paths | +| The repair moves a finding onto unrelated code | It moves it onto a line the reviewer quoted; multi-match resolves to the LAST matching occurrence — which, because the citation is always past EOF, is also the nearest, so a tie cannot arise, deterministically. Guard 4 pins the rule | | Weakening the empty-file protection the pass was built for | An empty file has no lines, so no quote can match → the motivating case can never be re-anchored. Guard 2 uses exactly that fixture | | Slice B re-inflates FP burden | Floor stops at WARN; already-INFO security stays droppable (guard 8). pilot-03 reports FP burden, though pilot-02 showed M2 has no signal in this rig | | `FindingSchema` change breaks older persisted artifacts | Field is `.optional()`, mirroring `fact_invalid`/`scope_demoted` | diff --git a/src/core/report-writer.ts b/src/core/report-writer.ts index 447c2cd..1c553a6 100644 --- a/src/core/report-writer.ts +++ b/src/core/report-writer.ts @@ -42,7 +42,9 @@ export function findingBadges(f: Finding): string | null { badges.push("🔒 deterministic check — fix it (re-runs automatically; not rejectable)"); if (f.fact_invalid) badges.push("🔎 cited location not found — likely hallucinated"); // Anchor repair: the counterpart to the badge above — the cited line was wrong, but the - // reviewer's quoted evidence proved the finding is grounded, so it was moved rather than demoted. + // reviewer's quoted evidence (carrying an identifier-like token) matched a real line of this + // file, showing the reviewer read real code rather than fabricating one, so it was moved rather + // than demoted. That is weaker than proof the defect itself is real — see finding.ts. if (f.anchor_repaired) badges.push( "⚑ reviewer cited a line that does not exist — re-anchored to the source line it quoted",