fix(review): gate the zero-error label floor at the Bonferroni split delta (#9922)

calibrateActThreshold compared the label count against a floor computed at
the raw delta, but the scan itself tests each candidate at delta/K after
#9066's Bonferroni split. A zero-error calibration set that clears the raw
floor but falls short of the split-delta floor fell through the whole scan
and was misreported as no_certifiable_threshold (an error-rate shortfall)
instead of insufficient_labels (a label-count shortfall), losing #9048's
status split. Also stop reporting a recalibration infra failure as
insufficient_labels in runRiskControlRecalibration's catch.

Co-authored-by: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>

Author: bitfathers94

src/review/risk-control-wire.ts

@@ -178,8 +178,8 @@ async function recalibrateArm(env: Env, arm: "close" | "merge", verdict: "close"
  *  (#8835's "per-repo where sample size permits, global fallback otherwise"). A repo key certifies or is
  *  retracted independently of the global one; the actuation read prefers the repo key. Returns per-arm
  *  global statuses for the caller's log line. */
-export async function runRiskControlRecalibration(env: Env): Promise<Record<string, CalibrationResult["status"]>> {
-  const summary: Record<string, CalibrationResult["status"]> = {};
+export async function runRiskControlRecalibration(env: Env): Promise<Record<string, CalibrationResult["status"] | "error">> {
+  const summary: Record<string, CalibrationResult["status"] | "error"> = {};
   for (const { arm, verdict, alpha } of riskControlArms(env)) {
     try {
       summary[arm] = (await recalibrateArm(env, arm, verdict, alpha, null)).status;
@@ -195,7 +195,10 @@ export async function runRiskControlRecalibration(env: Env): Promise<Record<stri
       }
     } catch (error) {
       console.warn(JSON.stringify({ event: "risk_control_recalibrate_error", arm, message: errorMessage(error).slice(0, 160) }));
-      summary[arm] = "insufficient_labels";
+      // #9637: an infrastructure failure (DB read/write, JSON parse) is not a statistical shortfall — reporting
+      // it as insufficient_labels told an operator to go collect labels for a problem collecting labels never
+      // caused, in the same daily-tick log line #9048 split for the opposite reason.
+      summary[arm] = "error";
     }
   }
   return summary;

src/review/risk-control.ts

@@ -22,7 +22,11 @@
 // HONESTY GUARDS, all load-bearing:
 //   • INSUFFICIENT LABELS IS A REFUSAL, never a degraded guess. Even a zero-error calibration set cannot
 //     certify α until n ≥ ln(δ)/ln(1−α) (the exact rule-of-three bound) — at α=0.005, δ=0.05 that is 598
-//     clean labels. Below it this module refuses and the caller keeps the static floor.
+//     clean labels. Below it this module refuses and the caller keeps the static floor. #9637: this floor is
+//     evaluated at the δ the scan actually TESTS (δ/K, the Bonferroni split above), not the raw δ — a set that
+//     clears the raw floor can still be short of what the split scan needs, and reporting that as
+//     `no_certifiable_threshold` instead would misdirect an operator toward investigating precision when the
+//     real gap is label count.
 //   • NO CERTIFIABLE THRESHOLD IS ALSO A REFUSAL, distinct from insufficient labels (#9048): a repo can have
 //     plenty of labels and still refuse when no λ's bound clears α — a fundamentally different shortfall
 //     ("the error rate is too high", not "there aren't enough labels"). `no_certifiable_threshold` carries the
@@ -146,19 +150,29 @@ export function calibrateActThreshold(pairs: CalibrationPair[], alpha: number, d
   // #9066: Bonferroni-split δ across the K candidates this scan actually tests — see the module header for
   // why this, not literal fixed-sequence stopping, is the chosen fix.
   const testDelta = delta / candidates.length;
+  // #9637: the label floor must be re-checked at the δ the scan actually tests (testDelta), not the raw δ
+  // above — a zero-error set can clear the raw-δ floor yet still be short of what the split-δ scan needs,
+  // and without this re-check every candidate below fails its n >= effectiveNeeded gate, falling through to
+  // `no_certifiable_threshold` (an error-rate shortfall) when the true shortfall is labels. candidates.length
+  // is always ≥ 1 here (the raw-δ floor check above already returned for an empty `pairs`), so this divides
+  // safely and, at K=1, effectiveNeeded === needed — identical to pre-#9637 behavior.
+  const effectiveNeeded = minimumCalibrationLabels(alpha, testDelta);
+  if (sorted.length < effectiveNeeded) {
+    return { status: "insufficient_labels", needed: effectiveNeeded, have: sorted.length, alpha, delta };
+  }
   let dropped = 0;
   let droppedErrors = 0;
   let index = 0;
   // Tracks the closest-to-certifying candidate for `no_certifiable_threshold`'s message. The very first
-  // candidate always computes a bound (n = sorted.length ≥ needed, guaranteed by the floor check above), so
-  // these are always overwritten before any caller can observe the initial values.
+  // candidate always computes a bound (n = sorted.length ≥ effectiveNeeded, guaranteed by the floor check
+  // above), so these are always overwritten before any caller can observe the initial values.
   let bestLambda = candidates[0]!;
   let bestN = sorted.length;
   let bestUpperBound = Infinity;
   for (const lambda of candidates) {
     const n = sorted.length - dropped;
     const errors = totalErrors - droppedErrors;
-    if (n >= needed) {
+    if (n >= effectiveNeeded) {
       const bound = clopperPearsonUpperBound(errors, n, testDelta);
       if (bound < bestUpperBound) {
         bestUpperBound = bound;
@@ -206,6 +220,11 @@ export function validateCalibrationPayload(value: unknown): { alpha: number; lam
   if (typeof v.coverageAtLambda !== "number" || !(v.coverageAtLambda >= 0 && v.coverageAtLambda <= 1)) return null;
   if (typeof v.delta !== "number" || !(v.delta > 0 && v.delta <= 1)) return null;
   if (typeof v.nAtLambda !== "number" || !Number.isFinite(v.nAtLambda)) return null;
+  // #9637: deliberately the UNSPLIT delta, not delta/K — the stored payload only ever records the originally-
+  // requested delta (candidates.length is not persisted), and nAtLambda is the coverage at the ALREADY-
+  // CERTIFIED lambda, not a candidate count mid-scan. A payload legitimately certified by the split-delta scan
+  // always clears this looser, unsplit-delta floor too (effectiveNeeded ≥ needed), so this stays the coarser,
+  // always-safe check calibrateActThreshold itself no longer uses for its own scan.
   if (v.nAtLambda < minimumCalibrationLabels(v.alpha, v.delta)) return null;
   return { alpha: v.alpha, lambda: v.lambda, coverageAtLambda: v.coverageAtLambda, nAtLambda: v.nAtLambda, delta: v.delta };
 }

test/unit/risk-control-wire.test.ts

@@ -123,18 +123,19 @@ describe("runRiskControlRecalibration", () => {
 
   it("REGRESSION (#9048): NO CERTIFIABLE THRESHOLD — ample labels but no λ clears α reports the true total, not a residual stratum, under a distinct event_type", async () => {
     const env = createTestEnv({ LOOPOVER_RISK_CONTROL_CLOSE_ALPHA: "0.005" }); // fixed alpha; floor = 598
-    // 600 dirty pairs (all wrong) at 0.5, then 50 clean pairs at 0.99 — 650 total, well over the 598 floor,
-    // but no candidate certifies: 0.5 has ruinous error, 0.99 falls below the floor once 0.5 is dropped.
-    for (let i = 1; i <= 600; i += 1) await seedLabeledDecision(env, i, "close", "incorrect", 0.5);
-    for (let i = 601; i <= 650; i += 1) await seedLabeledDecision(env, i, "close", "correct", 0.99);
+    // 700 dirty pairs (all wrong) at 0.5, then 100 clean pairs at 0.99 — 800 total, over the split-delta floor
+    // for these 2 candidates (#9637: 736, not the raw 598), but no candidate certifies: 0.5 has ruinous error,
+    // 0.99 falls below the split-delta floor once 0.5 is dropped.
+    for (let i = 1; i <= 700; i += 1) await seedLabeledDecision(env, i, "close", "incorrect", 0.5);
+    for (let i = 701; i <= 800; i += 1) await seedLabeledDecision(env, i, "close", "correct", 0.99);
     const summary = await runRiskControlRecalibration(env);
     expect(summary.close).toBe("no_certifiable_threshold");
     const flag = await env.DB.prepare(`SELECT value FROM system_flags WHERE key = ?`).bind(riskControlFlagKey("close")).first();
     expect(flag).toBeFalsy(); // no certifiable guarantee — retracted, same as insufficient_labels
     const audit = await env.DB.prepare(
       `SELECT detail FROM audit_events WHERE event_type = 'risk_control_no_certifiable_threshold' AND target_key = 'riskcontrol:close'`,
     ).first<{ detail: string }>();
-    expect(audit!.detail).toContain("650 labels available but no threshold achieves α=0.005");
+    expect(audit!.detail).toContain("800 labels available but no threshold achieves α=0.005");
     // The label-shortfall event must NOT also fire for this scope — it is a distinct outcome (#9048).
     const shortfall = await env.DB.prepare(
       `SELECT detail FROM audit_events WHERE event_type = 'risk_control_insufficient' AND target_key = 'riskcontrol:close'`,
@@ -157,7 +158,7 @@ describe("fail-safe arms", () => {
     vi.restoreAllMocks();
   });
 
-  it("a throwing arm recalibration is contained: the OTHER arm still runs and the summary reports insufficient", async () => {
+  it("REGRESSION (#9637): a throwing arm recalibration reports 'error', never a statistical status — an infra failure is not a label shortfall", async () => {
     const env = createTestEnv();
     const { vi } = await import("vitest");
     const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
@@ -169,7 +170,7 @@ describe("fail-safe arms", () => {
       return realPrepare(sql);
     });
     const summary = await runRiskControlRecalibration(env);
-    expect(summary).toEqual({ close: "insufficient_labels", merge: "insufficient_labels" });
+    expect(summary).toEqual({ close: "error", merge: "error" });
     expect(warn).toHaveBeenCalled();
     vi.restoreAllMocks();
   });

test/unit/risk-control.test.ts

@@ -71,31 +71,67 @@ describe("calibrateActThreshold", () => {
   });
 
   it("REGRESSION (#9048): a repo with AMPLE labels that cannot certify reports its true total, not a residual stratum size", () => {
-    // 650 total labels (well over the 598-label floor for alpha=0.005) — but every candidate either has a
-    // ruinous error rate (the 600 pairs at 0.5, all wrong) or falls below the sample-size floor once that
-    // dirty stratum is dropped (only 50 remain at 0.99). Before #9048 this reported `have: 50` under
-    // `insufficient_labels` — exactly the "N usable labels of 59/598 needed" bug: a residual stratum size
-    // misreported as the repo's total label supply.
-    const pairs = [...Array.from({ length: 600 }, () => pair(0.5, false)), ...Array.from({ length: 50 }, () => pair(0.99, true))];
+    // 800 total labels (well over the 736-label split-delta floor for these 2 candidates at alpha=0.005,
+    // #9637) — but every candidate either has a ruinous error rate (the 700 pairs at 0.5, all wrong) or falls
+    // below the split-delta sample-size floor once that dirty stratum is dropped (only 100 remain at 0.99).
+    // Before #9048 this reported `have: 100` under `insufficient_labels` — exactly the "N usable labels of
+    // 59/598 needed" bug: a residual stratum size misreported as the repo's total label supply.
+    const pairs = [...Array.from({ length: 700 }, () => pair(0.5, false)), ...Array.from({ length: 100 }, () => pair(0.99, true))];
     const result = calibrateActThreshold(pairs, 0.005, 0.05);
     expect(result.status).toBe("no_certifiable_threshold");
     if (result.status === "no_certifiable_threshold") {
-      expect(result.totalPairs).toBe(650); // the TRUE total, never a residual stratum size
-      expect(result.bestN).toBe(650); // the only candidate that cleared the sample-size floor
+      expect(result.totalPairs).toBe(800); // the TRUE total, never a residual stratum size
+      expect(result.bestN).toBe(800); // the only candidate that cleared the split-delta sample-size floor
       expect(result.bestLambda).toBe(0.5);
       expect(result.bestUpperBound).toBeGreaterThan(0.005); // could not certify alpha
       expect(result.bestUpperBound).toBeLessThanOrEqual(1);
     }
   });
 
   it("REGRESSION (#9066): does not over-certify across many observed confidence candidates — Bonferroni-splits delta across the K distinct thresholds actually tested", () => {
-    // Same shape the pre-fix code certified at lambda=0.9 (700 clean pairs, 10 distinct confidences, zero
+    // Same shape the pre-#9066 code certified at lambda=0.9 (700 clean pairs, 10 distinct confidences, zero
     // errors, alpha=0.005): the RAW zero-error bound at n=700, delta=0.05 is ≈0.0043 (<=0.005, would certify),
-    // but split across K=10 candidates (delta/10=0.005) the bound is ≈0.0075 (>0.005) — correctly refuses,
-    // because reporting whichever of 10 tested candidates passes first is a selection, not a single test.
+    // but split across K=10 candidates (delta/10=0.005) the bound is ≈0.0075 (>0.005) — over-certification is
+    // refused. #9637: the label floor at that split delta (1058) is now also enforced up front, so 700 pairs —
+    // genuinely short of what 10 candidates need — refuses as `insufficient_labels`, not the pre-#9637
+    // `no_certifiable_threshold` (which would have wrongly implied the error rate, not the label count, was
+    // the shortfall). Either way `calibrated` must never be reached.
     const pairs = Array.from({ length: 700 }, (_, i) => pair(0.9 + (i % 10) / 100, true));
     const result = calibrateActThreshold(pairs, 0.005, 0.05);
+    expect(result.status).toBe("insufficient_labels");
+    expect(result.status).not.toBe("calibrated");
+    if (result.status === "insufficient_labels") {
+      expect(result.needed).toBe(1058);
+      expect(result.have).toBe(700);
+    }
+  });
+
+  it("REGRESSION (#9637): the split-delta floor, not the raw floor, gates a zero-error set — 59 clean labels across 59 distinct confidences is insufficient, not uncertifiable", () => {
+    // Exactly the raw floor (minimumCalibrationLabels(0.05, 0.05) = 59), so the pre-scan check alone would
+    // let this through — but every pair is a distinct confidence (K=59 candidates), so the scan actually
+    // tests each candidate at delta/59. The effective floor at that split delta is 138: below it, a
+    // ZERO-ERROR set must still refuse as insufficient_labels, not fall through the whole scan into a
+    // misleading no_certifiable_threshold (#9048's status split, reintroduced by #9066's Bonferroni
+    // correction and the reason this issue exists).
+    const pairs = Array.from({ length: 59 }, (_, i) => pair(0.4 + i / 1000, true));
+    const result = calibrateActThreshold(pairs, 0.05, 0.05);
+    expect(result).toMatchObject({ status: "insufficient_labels", needed: 138, have: 59 });
+  });
+
+  it("REGRESSION (#9637): no_certifiable_threshold remains reachable once labels clear the split-delta floor", () => {
+    // A single confidence bucket (K=1 candidate, so the split delta equals the raw delta — this is also the
+    // `candidates.length === 1` edge the fix must leave behaviorally identical to before #9637) isolates a
+    // genuine error-rate shortfall from the label-count shortfall the tests above exercise: 1000 pairs, 12%
+    // wrong — comfortably over alpha=0.05's own floor (59) but nowhere near a certifiable error rate.
+    const pairs = [...Array.from({ length: 880 }, () => pair(0.9, true)), ...Array.from({ length: 120 }, () => pair(0.9, false))];
+    const result = calibrateActThreshold(pairs, 0.05, 0.05);
     expect(result.status).toBe("no_certifiable_threshold");
+    if (result.status === "no_certifiable_threshold") {
+      expect(result.totalPairs).toBe(1000);
+      expect(result.bestN).toBe(1000);
+      expect(result.bestLambda).toBe(0.9);
+      expect(result.bestUpperBound).toBeGreaterThan(0.05);
+    }
   });
 
   it("is deterministic and input-order independent", () => {