pr-review has no false-negative metric and no production feedback loop — accepted defects found by advisory bots after approval are never counted or learned from #1596

The gap

We measure pr-review's operational health and its false positives. We do not measure its false negatives, and nothing feeds them back into the improvement loop.

What exists today:

So the one signal that is both free and perfectly labelled — a third-party bot found a real defect on a PR pr-review had already approved — evaporates. It is never counted, never reported, and never becomes an eval case.

Evidence from a single day (2026-08-30/31)

Every one of these is a real, accepted defect that pr-review did not raise:

Six real findings, five distinct PRs, three third-party bots — in one day. Three of them were substantive enough that they are now tracked issues.

Is pr-review approving defective PRs a bug?

Partly, and the honest answer matters for what to fix.

Not a bug: the architecture deliberately delegates depth to advisory bots (there is an advisory-review gate that waits for them) and required_review_thread_resolution: true means late findings still block the merge. On #1017 that backstop worked — the defect was raised after approval and the ruleset held the PR. So the system caught it even though the agent did not.

Genuinely a bug, in two specific places:

1. The advisory gate proceeds on partial bot coverage. Observed verbatim on .github#995:

   [advisory-gate] Only 2/3 required bots submitted; head is 1966s old — timeout fallback, proceeding
   

   pr-review approved having seen 2 of 3 advisory opinions. The timeout is a liveness protection, but it silently converts "all reviewers agreed" into "the ones that answered agreed", with no record that the verdict was taken on partial evidence.

2. Approval is never re-evaluated when a later finding lands. pr-review's approval on #1017 remained attached while CodeRabbit was raising a real defect. Only the ruleset stopped the merge. If thread-resolution enforcement were ever relaxed — or the finding arrived as a comment rather than a review thread — an approved-and-defective PR merges.

The deeper issue is that an approval from pr-review is currently read as evidence of defect-freedom, and it is not — it is an orchestration signal. Nothing quantifies how much weight it actually deserves.

Proposal

1. Measure the miss rate

Extend the existing deterministic pipeline (no LLM needed for the metric itself — reviewer_report.sh already reads reviews, threads and resolution state in one GraphQL round-trip per repo). For each PR, emit:

• missed_findings — threads opened by a trusted advisory bot after pr-review's approving review on the same head SHA, that were subsequently resolved as accepted (i.e. a fix landed or the disposition accepted it) rather than refuted.
• caught_findings — findings pr-review raised itself that were accepted.
• partial_evidence_approvals — approvals issued via the advisory-gate timeout fallback, i.e. before all registered bots reported.

Report per week alongside the #1220 scorecard, and per reviewer, so we can see which bot is catching what pr-review misses. That is directly actionable: a class of defect one bot repeatedly finds first is a prompt gap.

Distinguishing accepted-from-refuted is the crux. Roughly 3 of 4 codeant/gemini findings dispositioned this same day were false positives correctly refuted (yq claimed missing when the dedicated test workflow installs it; a registry assertion claimed unguarded when the adjacent test covers it). A naive "bot opened a thread → pr-review missed it" counter would be dominated by noise and would push the agent toward more false positives. Use the accepted/refuted disposition already recorded in the resolving reply, and treat anything ambiguous as not a miss.

2. Close the loop into the evals — dev split only

Harvest accepted misses into evals/deep-review/dev/cases.jsonl as regression cases (de-identified per evals/README.md).

Hard invariant, do not violate: harvested cases go to dev/ only. holdout/cases.jsonl is CODEOWNER-gated and holdout-guard.yml hard-fails proposer-authored PRs touching evals/; the whole point of the split is that the proposer must never see the cases it is scored against. Auto-harvesting into holdout/ would silently destroy the one control that makes #1088's success metric meaningful. Harvesting into dev/ is exactly the "merged-PR few-shot" gap #1088 already names — this issue supplies the source of truth for those cases instead of hand-authored synthetic ones.

3. Fix the two concrete holes

• Record on the PR when an approval was issued under the advisory-gate timeout, and count it (partial_evidence_approvals). Consider requiring a re-check rather than a bare approval when coverage was partial.
• When a trusted advisory bot opens an accepted finding after an approving review at the same head, dismiss or re-run that approval rather than leaving it standing on required_review_thread_resolution alone.

Acceptance criteria

• missed_findings, caught_findings, partial_evidence_approvals computed deterministically from GitHub review data — no LLM in the metric path, matching reviewer_report.sh's stated design.
• Accepted-vs-refuted classification is explicit, and ambiguous dispositions count as not a miss; the rule is documented and unit-tested with both a real miss and a refuted false positive.
• Weekly report surfaces miss rate overall and per third-party reviewer.
• Accepted misses are harvested into evals/deep-review/dev/cases.jsonl, de-identified, never into holdout/; a test asserts the harvester cannot write a holdout/ path.
• An approval issued under the advisory-gate timeout is recorded as such on the PR and counted.
• A post-approval accepted finding from a trusted bot invalidates the standing approval rather than relying solely on thread-resolution enforcement.
• Baseline the miss rate over the trailing 30 days before any prompt change, so #1088's improvements can be measured against it.

Relationship to existing work — extends, does not duplicate

• #1088 (world-class bug hunter) targets bug-finding depth and names "merged-PR few-shot — no learning from this repo's past review→merge histories" as gap 4, with a success metric of held-out eval score plus no false-positive regression. It has no false-negative metric and no production feedback path. This issue supplies both, and gives gap 4 a real case source. Treat as a companion/sub-issue.
• #1220 measures third-party reviewers' activity, never pr-review's verdict against theirs.
• #1094/#1107 gate on the held-out synthetic split; this is about production signal.
• #1552 (merged) surfaced pr-review's decision/reason — the plumbing this metric can build on.

⚠️ CI steering for PR #1604 (added 2026-08-31 after two failed auto-fix passes)

Two distinct failures on #1604. They need different fixes; the first is a design decision, not a test to relax.

1. not ok 29 Advisory gate: script is minimal — [ "$lines" -lt 500 ] (tests/dev-lead/unit/test_advisory_review_gate.bats:172)

scripts/lib/advisory-review-gate.sh has a deliberate size guard: "non-blocking means fewer lines". Adding partial-evidence recording pushed it past 500.

Do NOT raise the threshold to make this pass. The guard encodes a real property — the advisory gate runs on every review pass and is deliberately kept small and non-blocking. Growing it is the thing the test is there to catch.

Instead, extract the partial-evidence recording into its own library — e.g. scripts/lib/review-miss-evidence.sh — and have the gate call it. That matches the pattern #1567 established with scripts/lib/review-change-evidence.sh (pure, unit-tested helpers in their own lib; the caller keeps only the plumbing). It also fixes failure 2 below for free, because the new tests can source the new lib directly.

2. not ok 702-705 Partial-evidence: … — source "$SCRIPT_DIR/lib/advisory-review-gate.sh" failed (tests/dev-lead/unit/test_partial_evidence_marker.bats:16)

The setup() sources a path that does not exist, so all four tests error before asserting anything. Point it at the real location of whichever lib ends up owning these helpers (see 1 — after the extraction it should source scripts/lib/review-miss-evidence.sh). Check SCRIPT_DIR resolution against a sibling unit test that already sources a scripts/lib/*.sh correctly rather than assuming the prefix.

Reminder on scope

Everything else on #1604 is green (49 checks). Keep the change to these two items; do not touch the metric semantics already agreed in this issue — in particular the accepted-vs-refuted classification and the dev/ split only, never holdout/ invariant both still stand.