feat(guardian): per-finding skeptic judgement + impact scoring (#246)

[zaebee]

Implements docs/specs/2026-07-29-guardian-skeptic-scoring-design.md (merged in #268) via its 11-task plan. Closes the mechanism half of #246; the model matrix is phase 2 and needs a budget decision.

What was broken

The recall-lean finder (#249) delegates precision to the skeptic, and the skeptic could not deliver. Live evidence from this repo today, PR #263: 8 findings, all confirmed, all at one severity — one real defect (Path.is_file() raises on EACCES/ENAMETOOLONG) sitting indistinguishable from two factually wrong claims (for size: int in range(...) is not valid Python; as_posix() would have broken the very Windows-path case the line exists for).

Two causes, only one of which is the model:

• Mechanism — one call carried the whole findings list, verdicts mapped back by finding_index. Everything competed for one attention window, and a single parse failure discarded every verdict in the pass.
• Contract — Literal["confirmed","refuted","uncertain"] can say "this is false" but not "this is true and worthless", and cannot be sorted or thresholded.

What changed

Two orthogonal axes. verdict (is it true — only explicit refuted drops) and the new impact_score: int 0–10 (does it matter — below threshold hides, never drops, and stays in the JSONL so the benchmark can still see what was cut).

One call per finding, carrying only that finding's own hunks, concurrent behind a semaphore (default 3 — mistral's free tier caps tokens per minute). finding_index is gone, and with it the out-of-range and duplicate failure modes that previously had to be handled.

Partial degradation. skeptic_status gains partial with judged/total in the footer and metrics. One failed call now costs one verdict instead of the whole pass.

A previously-reverted lever became usable. Few-shot false-positive examples once swung mistral into refute-all (pr-143 recall 0.7 → 0.0) because with one axis, refutation was the only way to express "bad finding". Calibration now lands on the importance axis and cannot reach the refutation logic. The truth-axis prompt is reused verbatim — it was bought by reverting a refute-by-default version that killed 7/7 findings including 2 GT matches.

"If ruff/ruff format/mypy --strict would catch it, impact ≤ 2" — the Gemini-review advice, placed on the axis where it is safe. In the finder prompt the same instruction re-introduces the precision bias that caused recall 0/5.

Benchmark can now answer the question. --record-finder / --replay-finder freeze a finder pass so every skeptic variant judges an identical set; annotate_matches records each finding's score and whether it matched ground truth; score_separation is the gate metric — median(score | GT) − median(score | non-GT), pooled across PRs. A flat distribution scores 0, which is exactly what the #263 pathology would produce.

Two deliberate deviations from the plan

1. T3 was not a straight move. split_diff_by_file dragged along _block_path and a _OLD_FILE_RE/_NEW_FILE_RE pair — and diff_index.py already had its own _NEW_FILE_RE with a different pattern (the chunker pair is git-C-quote aware, the indexer pair is not). Merged blindly, one would have silently replaced the other and changed inline-comment anchoring. They now coexist as _QUOTED_*, with the module docstring stating the difference is deliberate.

2. T10 records the diff alongside the findings. The plan replayed a bare ReviewResult, which would have forced a worktree + ingest to reconstruct the diff for the skeptic — paying exactly the setup cost replay exists to avoid. A replay run now touches no git at all.

Self-measurement moved, and is re-pinned honestly

diff_index became a shared leaf (chunker, github_poster, skeptic), and a shared leaf adds cross-community edges: guardian's modularity Q fell 0.3516 → 0.3320, flipping suggest-packages from split to borderline. The metric that actually tracks the layout improved: divergence 0.7224 → 0.6705. tests/self_parsing/test_suggest.py is re-pinned with that reasoning; its load-bearing claim (nested reads below the flat 1.0) holds and strengthened. Same precedent as the #255 re-pin.

Verification

• 1107 tests pass (2 skipped), mypy strict, ruff, doc-coverage 99.5% — run after every one of the 11 tasks, each of which leaves the suite green (the batch API stays alive until the last task for exactly that reason).
• Self-drift within tolerance, unresolved 21.6% (< 30% gate).
• The deleted batch tests were replaced, not dropped: every behaviour they protected has an equivalent against apply_judgements. The two index-mapping tests have no equivalent by design — without indices those failures cannot occur.

Not in this PR

The phase-1 A/B benchmark run (batch arm vs per-finding arm, same codestral model, frozen finder output) — it needs API spend, and the gate is noise down + zero lost GT + separation ≥ 3. Phase 2 (gemini / local ollama matrix) needs an explicit budget go; the prior gemini run hit the 30 PLN cap.

GUARDIAN_IMPACT_THRESHOLD ships at 0 — nothing is hidden until that benchmark shows the score distribution.

🤖 Generated with Claude Code

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[zaebee]

/guardian review

[zaebee]

Phase 1 + 2 measured — gate passes, with two caveats

Protocol per spec §4: one frozen finder pass per PR (mistral-medium), every skeptic variant judging that same set, so the only variable is the skeptic. Raw rows committed in benchmarks/guardian/results.jsonl (tagged bench_arm / bench_phase).

Results across 3 PRs

PR	finder (frozen)	arm B — per-finding	arm A — batch
143	r 1.00 / noise 22	r 1.00 / noise 15	r 0.67 / noise 10
144	r 0.60 / noise 8	r 0.60 / noise 5	r 0.40 / noise 2
140	r 0.40 / noise 16	r 0.20 / noise 11	r 0.20 / noise 9
mean	r 0.667 / 15.3	r 0.600 / 10.3	r 0.422 / 7.0

Per-finding: −33% noise for −10% recall. Batch: −54% noise for −37% recall. Batch buys its lower noise by destroying findings — on pr-143 it refuted 14 of 28 and took two ground-truth findings with them, the exact over-refutation pathology that was reverted once before. Isolation is the better operating point on all three PRs.

Phase 1 (same-family control, codestral on pr-143): judged 28/28, refuted 0, noise unchanged at 22. A same-family skeptic contributes nothing in either arm — the cross-family choice is where the entire effect lives, which is what #246 originally proposed.

Gate 2 passes at exactly the line: pooled separation 3.0 vs the required ≥3 (GT n=12 median 7.0; non-GT n=58 median 4.0). Per-PR it swings 1.5 / 7.0 / 2.0 — precisely the small-sample instability that made the spec require pooling. 3.0 against a 3.0 bar is not margin.

Caveat 1 — a flaw in my own analysis, corrected

I first reported "7/7 refutations correct, zero GT killed". That evidence was tautological: annotate_matches derives matched_gt from the visible list, which excludes refuted findings, so a refuted finding can never carry the flag. That metric cannot detect a GT-killing refutation at all.

The sound evidence is recall before vs after: unchanged on pr-143 (1.00) and pr-144 (0.60), but pr-140 fell 0.40 → 0.20 — per-finding gemini did kill a real finding there. The comparative claim survives (isolation holds recall better than batch on 2 of 3 PRs, ties on the third); the absolute claim "isolation protects recall" does not.

matched_gt remains correct for what it was added for — the score distribution among survivors. Measuring what the skeptic killed needs a separate metric against the frozen set. Filed as a follow-up.

Caveat 2 — the impact axis under-delivers

Gate 2 passes only via pooling. Per-PR the axis is unstable, and on pr-143 the separation among survivors is 0.0: gemini's low scores sit exactly on the findings it already refuted, while every surviving noise finding scores the same as the GT ones (medians 7.0 vs 7.0). The two axes were designed orthogonal; for both models tested they are largely collinear.

Cost: per-finding took 221k skeptic prompt tokens across the three PRs vs ~131k for batch — +69%, not the reduction the spec assumed (the spec's cost claim is refuted: findings cluster by file, so a file's hunks are re-sent once per finding in it).

What this changes

• The mechanism is worth keeping, but for a different reason than the spec argued. Not ranking — recall protection under a discriminating skeptic.
• GUARDIAN_IMPACT_THRESHOLD stays 0. Separation at the gate line and unstable per PR means the knob would cut blind.
• Prod config should move to GUARDIAN_SKEPTIC=gemini — cross-family carries the whole effect. That's a repo-variable change, separate from this PR.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud