From fdbe4ff0fe93952783e7042b555524f499d13d41 Mon Sep 17 00:00:00 2001 From: xfodev Date: Mon, 20 Jul 2026 13:30:19 +0200 Subject: [PATCH] fix(review): compute public-stats accuracyPct from own-ledger merged/closed only getPublicStats computed the global totals.accuracyPct from the Orb-fleet-folded totals.merged/closed (denominator) against the own-ledger-only totals.reversed (numerator). getOrbGlobalStats has no reversal concept, so the denominator grew with every newly registered install while the numerator stayed own-ledger-scoped, trending the published accuracy toward 100% independent of real reversal behavior. Snapshot the pre-fold own-ledger merged/closed (beside the existing ownLedgerReviewed snapshot) and compute the global accuracyPct from those, so numerator and denominator are drawn from the same population (option 1 of the issue). The fleet fold still inflates reviewed/handled/minutesSaved, which have no numerator/denominator pairing; accuracyPct's formula and byProject's per-project values are unchanged. Adds a regression test asserting a huge Orb-fleet fold no longer pulls accuracyPct toward 100. Closes #7449 --- src/review/public-stats.ts | 12 +++++++++++- test/unit/public-stats.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/review/public-stats.ts b/src/review/public-stats.ts index d3b6852449..23334c3a72 100644 --- a/src/review/public-stats.ts +++ b/src/review/public-stats.ts @@ -377,6 +377,13 @@ export async function getPublicStats( // Snapshot before Orb merge: effort SQL only covers allowlisted own-ledger publishes, while `reviewed` // below includes Orb fleet outcomes folded into totals.merged/closed. const ownLedgerReviewed = reviewedOf(totals); + // #7449: also snapshot the pre-fold own-ledger merged/closed. totals.reversed stays own-ledger-only (the Orb + // aggregate has no reversal concept), so the published global accuracyPct below is computed from THESE, not the + // fleet-folded totals.merged/closed -- otherwise the denominator would grow with every newly registered install + // while the numerator stayed own-ledger-scoped, trending the percentage toward 100 independent of real reversal + // behavior. The fleet fold still (correctly) inflates reviewed/handled/minutesSaved, which have no such pairing. + const ownLedgerMerged = totals.merged; + const ownLedgerClosed = totals.closed; const orb = await getOrbGlobalStats(env); totals.merged += orb.merged; totals.closed += orb.closed; @@ -398,7 +405,10 @@ export async function getPublicStats( ...totals, reviewed, filteredPct: filteredPct(reviewed, totals.merged), - accuracyPct: accuracyPct(totals.merged, totals.closed, totals.reversed), + // Option 1 of #7449: compute the global accuracy from the OWN-LEDGER merged/closed snapshot (not the + // fleet-folded totals.merged/closed), so its numerator (own-ledger reversed) and denominator are drawn + // from the same population. See the ownLedgerMerged/ownLedgerClosed snapshot above the Orb fold for why. + accuracyPct: accuracyPct(ownLedgerMerged, ownLedgerClosed, totals.reversed), minutesSaved, }, weekly: { reviewed: w.reviewed ?? 0, merged: w.merged ?? 0 }, diff --git a/test/unit/public-stats.test.ts b/test/unit/public-stats.test.ts index 8b43d56ba6..52e03e9f73 100644 --- a/test/unit/public-stats.test.ts +++ b/test/unit/public-stats.test.ts @@ -311,6 +311,28 @@ describe("getPublicStats — live aggregate over the review ledger", () => { expect(out.totals.minutesSaved).toBe(2742 * MINUTES_SAVED_PER_PR + 80 * MINUTES_SAVED_PER_PR); }); + it("REGRESSION (#7449): global accuracyPct reflects the own-ledger population, not the Orb-fleet-inflated merged/closed denominator", async () => { + // Own-ledger: 100 decided (all merged), 10 real reversals -> a true 90% accuracy. A huge registered Orb fleet + // (6000 merged + 4000 closed, with no reversal data at all) must NOT dilute the denominator toward 100. + const handler = (sql: string): Row[] => { + if (isDispositions(sql)) return [{ project: "JSONbored/loopover", reviewed: 100, merged: 100, closed: 0, inReview: 0 }]; + if (isReversal(sql)) return [{ project: "JSONbored/loopover", reversed: 10 }]; + if (sql.includes("orb_pr_outcomes")) return [{ merged: 6000, closed: 4000, total: 10000 }]; + return []; + }; + const out = await getPublicStats(stubEnv(handler), NOW); + // The fleet fold still (correctly) inflates the raw aggregate counts... + expect(out.totals.merged).toBe(100 + 6000); + expect(out.totals.closed).toBe(0 + 4000); + expect(out.totals.handled).toBe(100 + 10000); + // ...but accuracy is computed from own-ledger only: 1 - 10/(100 + 0) = 90.0%. + expect(out.totals.accuracyPct).toBe(90); + // The pre-fix fleet-inflated denominator would have produced 1 - 10/(6100 + 4000) = 99.0% -- guard against it. + expect(out.totals.accuracyPct).not.toBe(99); + // Per-project accuracy is already same-scope and stays unchanged: 1 - 10/100 = 90. + expect(out.byProject[0]!.accuracyPct).toBe(90); + }); + it("keeps own-ledger per-PR effort sum separate from Orb fleet flat credit", async () => { const withOrbAndEffort = (sql: string): Row[] => { if (sql.includes("orb_pr_outcomes")) return [{ merged: 10, closed: 5, total: 15 }];