From 7df6643ecb20ab097641127d089aa435400a4306 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 26 Jul 2026 02:49:23 -0700 Subject: [PATCH] fix(orb): publish decision-grounded accuracy instead of the reversal rate The homepage 'Decision accuracy' tile published 1 - reversalRate, which overstates the gate's real accuracy two independent ways: 1. DENOMINATOR: reversalRate divides by every fleet signal including 'hold' verdicts. A hold is a deferral to a human, not a decision that can be right or wrong. Holds were ~36% of live fleet signals, dragging the rate toward zero and the published number toward 100 regardless of how the gate actually performed. 2. NUMERATOR: it counted only explicit reversal markers, so outright mispredictions never registered at all -- on the live fleet, 213 PRs where the gate said merge and the PR ended up closed, plus 21 where it said close and the PR merged, were entirely invisible. Measured on live fleet data the two differ by ~6 points: the retired formula published 99.6% where the gate's real decision accuracy is 93.6% (3,783 of 4,043 merge/close calls confirmed by outcome). Advances #8820. - analytics: InstanceMetrics/FleetAnalytics gain decisionAccuracy = (mergeConfirmed + closeConfirmed) / (merge + close verdicts), null for a holds-only instance; fleet value is the median, matching every other fleet aggregate - public-stats: publish it as fleetAccuracy.accuracyPct - UI: the hint states what the number measures rather than naming the retired formula --- .../site/proof-of-power-stats.test.tsx | 6 ++- .../components/site/proof-of-power-stats.tsx | 5 ++- src/orb/analytics.ts | 22 ++++++++++- src/review/public-stats.ts | 37 +++++++++++-------- test/unit/orb-analytics.test.ts | 32 ++++++++++++++++ test/unit/public-stats.test.ts | 24 +++++++++++- 6 files changed, 105 insertions(+), 21 deletions(-) diff --git a/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx b/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx index 5fdb423330..74fc5a9f9a 100644 --- a/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx +++ b/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx @@ -213,8 +213,12 @@ describe("ProofOfPowerStats", () => { await screen.findByText("Decision accuracy"); expect(screen.getByText("80%")).toBeTruthy(); expect(screen.queryByText("98.4%")).toBeNull(); // the own-ledger number is no longer shown + // #8820: the hint states WHAT is measured -- merge/close decisions confirmed by the realized outcome -- + // rather than the retired "reversal-grounded" formula the surface no longer publishes. expect( - screen.getByText("across 3 self-hosted instances · 2 gaming patterns flagged"), + screen.getByText( + "merge/close calls confirmed by outcome · 3 self-hosted instances · 2 gaming patterns flagged", + ), ).toBeTruthy(); // No fleet-accuracy trend exists yet -- the tile's sparkline is omitted rather than showing a mismatched one. expect(screen.getAllByRole("img", { name: "Trend over the last 8 weeks" })).toHaveLength(3); diff --git a/apps/loopover-ui/src/components/site/proof-of-power-stats.tsx b/apps/loopover-ui/src/components/site/proof-of-power-stats.tsx index 8b276cc5bf..868d0f60ff 100644 --- a/apps/loopover-ui/src/components/site/proof-of-power-stats.tsx +++ b/apps/loopover-ui/src/components/site/proof-of-power-stats.tsx @@ -162,8 +162,11 @@ export function ProofOfPowerStats({ className }: { className?: string }) { label="Decision accuracy" value={displayedAccuracyPct == null ? "—" : `${displayedAccuracyPct}%`} hint={ + // #8820: the fleet number is the share of merge/close DECISIONS the realized outcome confirmed + // (holds excluded -- they're deferrals, not decisions), so say that rather than the old + // "reversal-grounded" wording, which described a formula the surface no longer publishes. fleetEligible - ? `across ${intFmt.format(data.fleetAccuracy.instanceCount)} self-hosted instance${data.fleetAccuracy.instanceCount === 1 ? "" : "s"}${data.fleetAccuracy.gamingFlagsCaught > 0 ? ` · ${intFmt.format(data.fleetAccuracy.gamingFlagsCaught)} gaming pattern${data.fleetAccuracy.gamingFlagsCaught === 1 ? "" : "s"} flagged` : ""}` + ? `merge/close calls confirmed by outcome · ${intFmt.format(data.fleetAccuracy.instanceCount)} self-hosted instance${data.fleetAccuracy.instanceCount === 1 ? "" : "s"}${data.fleetAccuracy.gamingFlagsCaught > 0 ? ` · ${intFmt.format(data.fleetAccuracy.gamingFlagsCaught)} gaming pattern${data.fleetAccuracy.gamingFlagsCaught === 1 ? "" : "s"} flagged` : ""}` : totals.reversed > 0 ? `${intFmt.format(totals.reversed)} human-reversed` : "reversal-grounded" diff --git a/src/orb/analytics.ts b/src/orb/analytics.ts index a2daf731cd..7acfb90f1a 100644 --- a/src/orb/analytics.ts +++ b/src/orb/analytics.ts @@ -56,7 +56,18 @@ export interface InstanceMetrics { closePrecision: number | null; // P(closed & not reopened | gate said close) fpRate: number | null; // P(closed or reverted | gate said merge) — gate approved, it was wrong fnRate: number | null; // P(merged or reopened | gate said close) — gate blocked, it was wrong - reversalRate: number; // share of decided PRs a human reversed + reversalRate: number; // share of ALL signals (incl. holds) carrying an explicit reversal marker + /** Share of the gate's AUTONOMOUS decisions (merge/close verdicts) that the realized outcome confirmed — + * the number that actually answers "how often is the bot's decision right" (#8820). + * + * NOT `1 − reversalRate`, which the public surface used to publish and which overstates accuracy two ways: + * 1. its denominator counts `hold` verdicts, which are deferrals to a human, not decisions that can be + * right or wrong — on this fleet they were ~36% of all signals, diluting the rate toward zero; and + * 2. its numerator counts only EXPLICIT reversal markers, so an outright misprediction (gate said + * merge, the PR ended up closed) never registered at all. + * Measured on the live fleet the two differ by ~6 points (93.6% vs 99.6%) — the gap is real errors, not + * rounding. null when the instance made no merge/close verdicts at all (holds only). */ + decisionAccuracy: number | null; } /** #2350: one self-hosted instance whose combined volume/precision/reversal-rate pattern looks like it is @@ -81,6 +92,10 @@ export interface FleetAnalytics { closePrecision: number | null; fpRate: number | null; reversalRate: number | null; + /** Share of AUTONOMOUS decisions the realized outcome confirmed — the honest "decision accuracy" + * (#8820). See InstanceMetrics.decisionAccuracy for why this, not 1 − reversalRate, is the number to + * publish. */ + decisionAccuracy: number | null; cycleP50Ms: number | null; cycleP95Ms: number | null; }; @@ -132,6 +147,7 @@ export function foldInstance(instanceId: string, cells: Cell[]): InstanceMetrics else closeFalse += c.n; } } + const verdicts = wouldMerge + wouldClose; return { instanceId, decided, @@ -140,6 +156,7 @@ export function foldInstance(instanceId: string, cells: Cell[]): InstanceMetrics fpRate: wouldMerge > 0 ? mergeFalse / wouldMerge : null, fnRate: wouldClose > 0 ? closeFalse / wouldClose : null, reversalRate: reversals / decided, // decided ≥ 1 (the instance has at least one cell) + decisionAccuracy: verdicts > 0 ? (mergeConfirmed + closeConfirmed) / verdicts : null, }; } @@ -179,7 +196,7 @@ export async function computeFleetAnalytics(env: Env, opts: { windowDays?: numbe const reg = await env.DB.prepare(`SELECT instance_id FROM orb_instances WHERE registered = 1`).all<{ instance_id: string }>(); registered = new Set((reg.results ?? []).map((r) => r.instance_id)); } catch { - return { windowDays, instanceCount: 0, fleet: { mergePrecision: null, closePrecision: null, fpRate: null, reversalRate: null, cycleP50Ms: null, cycleP95Ms: null }, instances: [], outliers: [], gamingPatternFlags: [] }; + return { windowDays, instanceCount: 0, fleet: { mergePrecision: null, closePrecision: null, fpRate: null, reversalRate: null, decisionAccuracy: null, cycleP50Ms: null, cycleP95Ms: null }, instances: [], outliers: [], gamingPatternFlags: [] }; } // Group cells by instance, fold each. @@ -242,6 +259,7 @@ export async function computeFleetAnalytics(env: Env, opts: { windowDays?: numbe closePrecision: fleetCloseP, fpRate: median(nums((i) => i.fpRate)), reversalRate: median(nums((i) => i.reversalRate)), + decisionAccuracy: median(nums((i) => i.decisionAccuracy)), cycleP50Ms: percentile(cycle, 50), cycleP95Ms: percentile(cycle, 95), }, diff --git a/src/review/public-stats.ts b/src/review/public-stats.ts index cc3fbf8e4d..d3ecf73ff2 100644 --- a/src/review/public-stats.ts +++ b/src/review/public-stats.ts @@ -234,12 +234,12 @@ export interface PublicStatsPayload { closed: number; accuracyPct: number | null; }>; - /** Live, fleet-wide reversal-grounded accuracy across REGISTERED self-hosted ORB instances - * (computeFleetAnalytics, src/orb/analytics.ts) -- unlike totals.accuracyPct (own-ledger, frozen as of the - * self-host cutover, see the file header), this keeps growing as the fleet operates, so it's the number that - * actually reflects how ORB is treating today's contributors. accuracyPct is null until at least one - * registered instance clears computeFleetAnalytics's own minimum-volume bar -- the caller falls back to - * totals.accuracyPct in that case. */ + /** Live, fleet-wide DECISION accuracy across REGISTERED self-hosted ORB instances (computeFleetAnalytics, + * src/orb/analytics.ts): the share of the gate's own merge/close decisions that the realized outcome + * confirmed (#8820). Unlike totals.accuracyPct (own-ledger, frozen as of the self-host cutover, see the + * file header), this keeps growing as the fleet operates, so it's the number that actually reflects how + * ORB is treating today's contributors. accuracyPct is null until at least one registered instance clears + * computeFleetAnalytics's own minimum-volume bar -- the caller falls back to totals.accuracyPct then. */ fleetAccuracy: { accuracyPct: number | null; instanceCount: number; @@ -419,16 +419,21 @@ export async function getPublicStats( totals.closed += orb.closed; totals.handled += orb.total; - // computeFleetAnalytics's fleet.reversalRate is a median over ELIGIBLE instances, each of which always has a - // non-null reversalRate (InstanceMetrics.reversalRate is a plain division, never null) -- it can only be null - // when there are zero eligible instances, which is exactly the `instanceCount > 0` guard below. - let fleetAccuracyPct: number | null = null; - if (fleet.instanceCount > 0) { - /* v8 ignore next -- fleet.fleet.reversalRate is non-null whenever instanceCount > 0, per the comment above; - * the ?? 0 fallback exists only to satisfy the number|null type, not a reachable runtime case. */ - const reversalRate = fleet.fleet.reversalRate ?? 0; - fleetAccuracyPct = Math.round((1 - reversalRate) * 1000) / 10; - } + // DECISION-GROUNDED (#8820): publish the share of the gate's own merge/close decisions that the realized + // outcome CONFIRMED (fleet.decisionAccuracy), not `1 - reversalRate`. + // + // The old formula overstated accuracy two ways, and on the live fleet the two differ by ~6 points (93.6% + // vs 99.6% at the time of the fix -- real errors, not rounding): + // 1. DENOMINATOR: reversalRate divides by every signal including `hold` verdicts, which are deferrals to + // a human, not decisions that can be right or wrong. Holds were ~36% of fleet signals, dragging the + // rate toward zero and the published accuracy toward 100 no matter how the gate actually performed. + // 2. NUMERATOR: it counted only EXPLICIT reversal markers, so an outright misprediction (the gate said + // merge and the PR ended up closed) never registered at all -- hundreds of them were invisible. + // decisionAccuracy is null only when no eligible instance made a single merge/close verdict (holds only), + // which is a genuine "no signal yet" -- the caller falls back to the own-ledger number exactly as it does + // for an empty fleet. + const fleetAccuracyPct = + fleet.fleet.decisionAccuracy === null ? null : Math.round(fleet.fleet.decisionAccuracy * 1000) / 10; const reviewed = reviewedOf(totals); const w = weeklyRows[0] ?? { reviewed: 0, merged: 0 }; diff --git a/test/unit/orb-analytics.test.ts b/test/unit/orb-analytics.test.ts index 96db4d1df7..560a3d58a3 100644 --- a/test/unit/orb-analytics.test.ts +++ b/test/unit/orb-analytics.test.ts @@ -87,6 +87,38 @@ describe("computeFleetAnalytics()", () => { expect(inst.fnRate).toBeCloseTo(1 / 5); }); + it("decisionAccuracy (#8820) scores only merge/close verdicts — holds are excluded and outright mispredictions COUNT", async () => { + const env = createTestEnv(); + // The live-fleet shape that exposed the bug: a large block of holds plus real mispredictions that carry + // no reversal marker at all. + await signals(env, "i", 10, { verdict: "merge", outcome: "merged" }); // confirmed + await signals(env, "i", 2, { verdict: "merge", outcome: "closed" }); // WRONG, no reversal marker + await signals(env, "i", 6, { verdict: "close", outcome: "closed" }); // confirmed + await signals(env, "i", 2, { verdict: "close", outcome: "merged" }); // WRONG, no reversal marker + await signals(env, "i", 30, { verdict: "hold", outcome: "merged" }); // deferrals — never scored + const inst = (await computeFleetAnalytics(env)).instances[0]!; + expect(inst.decisionAccuracy).toBeCloseTo(16 / 20); // 80% over the 20 real decisions + // The old published formula would have called this ~100% — every miss here is marker-less, and the 30 + // holds dominate its denominator. This gap IS the bug. + expect(1 - inst.reversalRate).toBe(1); + }); + + it("decisionAccuracy is null for a holds-only instance (no decision to score) and drives the fleet median", async () => { + const env = createTestEnv(); + await signals(env, "holds-only", 6, { verdict: "hold", outcome: "merged" }); + const holdsOnly = (await computeFleetAnalytics(env)).instances[0]!; + expect(holdsOnly.decisionAccuracy).toBeNull(); + + const env2 = createTestEnv(); + await signals(env2, "a", 8, { verdict: "merge", outcome: "merged" }); + await signals(env2, "a", 2, { verdict: "merge", outcome: "closed" }); // 0.8 + await signals(env2, "b", 9, { verdict: "close", outcome: "closed" }); + await signals(env2, "b", 1, { verdict: "close", outcome: "merged" }); // 0.9 + await env2.DB.prepare(`INSERT INTO orb_instances (instance_id, registered) VALUES ('a', 1), ('b', 1)`).run(); + const fleet = await computeFleetAnalytics(env2); + expect(fleet.fleet.decisionAccuracy).toBeCloseTo(0.85); // median of 0.8 and 0.9 + }); + it("a superseded close (#8820) disconfirms closePrecision and counts toward reversalRate, exactly like a reopen", async () => { const env = createTestEnv(); await signals(env, "i", 3, { verdict: "close", outcome: "closed", reversal: "none" }); // confirmed diff --git a/test/unit/public-stats.test.ts b/test/unit/public-stats.test.ts index 4acf7314b8..7a8281dcee 100644 --- a/test/unit/public-stats.test.ts +++ b/test/unit/public-stats.test.ts @@ -495,10 +495,32 @@ describe("getPublicStats — live aggregate over the review ledger", () => { const out = await getPublicStats(env, NOW); - // decided=5 (>= computeFleetAnalytics's MIN_DECIDED), reversed=1 -> reversalRate 0.2 -> accuracy 80%. + // 5 merge verdicts, 4 confirmed (the 5th was reverted) -> decisionAccuracy 4/5 -> 80%. expect(out.fleetAccuracy).toEqual({ accuracyPct: 80, instanceCount: 1, windowDays: 90, gamingFlagsCaught: 0 }); }); + it("REGRESSION (#8820): the published fleet accuracy scores DECISIONS — holds are excluded and marker-less mispredictions count", async () => { + const env = createTestEnv({ LOOPOVER_PUBLIC_STATS_REPOS: "" }); + const db = env.DB; + await db.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES (?, 1)").bind("inst-1").run(); + const signal = async (n: number, verdict: string, outcome: string, tag: string) => { + for (let i = 0; i < n; i++) { + await db + .prepare(`INSERT INTO orb_signals (instance_id, repo_hash, pr_hash, gate_verdict, outcome, reversal_flag) VALUES (?, 'repo-hash', ?, ?, ?, 'none')`) + .bind("inst-1", `pr-${tag}-${i}`, verdict, outcome) + .run(); + } + }; + await signal(6, "merge", "merged", "ok"); // confirmed + await signal(2, "merge", "closed", "bad"); // WRONG, and carries no reversal marker + await signal(40, "hold", "merged", "hold"); // deferrals — must not enter the denominator + + const out = await getPublicStats(env, NOW); + // 8 real decisions, 6 confirmed -> 75%. The retired `1 - reversalRate` formula would have published + // 100% here: zero reversal markers, and 40 holds swamping its denominator. + expect(out.fleetAccuracy.accuracyPct).toBe(75); + }); + it("REGRESSION (#fairness-analytics): surfaces gamingFlagsCaught from computeFleetAnalytics's anti-farming detector", async () => { const env = createTestEnv({ LOOPOVER_PUBLIC_STATS_REPOS: "" }); const db = env.DB;