Skip to content

Commit 14db2b8

Browse files
authored
fix(review): compute public-stats accuracyPct from own-ledger merged/closed only (#7474)
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
1 parent 4314944 commit 14db2b8

2 files changed

Lines changed: 33 additions & 1 deletion

File tree

src/review/public-stats.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,13 @@ export async function getPublicStats(
377377
// Snapshot before Orb merge: effort SQL only covers allowlisted own-ledger publishes, while `reviewed`
378378
// below includes Orb fleet outcomes folded into totals.merged/closed.
379379
const ownLedgerReviewed = reviewedOf(totals);
380+
// #7449: also snapshot the pre-fold own-ledger merged/closed. totals.reversed stays own-ledger-only (the Orb
381+
// aggregate has no reversal concept), so the published global accuracyPct below is computed from THESE, not the
382+
// fleet-folded totals.merged/closed -- otherwise the denominator would grow with every newly registered install
383+
// while the numerator stayed own-ledger-scoped, trending the percentage toward 100 independent of real reversal
384+
// behavior. The fleet fold still (correctly) inflates reviewed/handled/minutesSaved, which have no such pairing.
385+
const ownLedgerMerged = totals.merged;
386+
const ownLedgerClosed = totals.closed;
380387
const orb = await getOrbGlobalStats(env);
381388
totals.merged += orb.merged;
382389
totals.closed += orb.closed;
@@ -398,7 +405,10 @@ export async function getPublicStats(
398405
...totals,
399406
reviewed,
400407
filteredPct: filteredPct(reviewed, totals.merged),
401-
accuracyPct: accuracyPct(totals.merged, totals.closed, totals.reversed),
408+
// Option 1 of #7449: compute the global accuracy from the OWN-LEDGER merged/closed snapshot (not the
409+
// fleet-folded totals.merged/closed), so its numerator (own-ledger reversed) and denominator are drawn
410+
// from the same population. See the ownLedgerMerged/ownLedgerClosed snapshot above the Orb fold for why.
411+
accuracyPct: accuracyPct(ownLedgerMerged, ownLedgerClosed, totals.reversed),
402412
minutesSaved,
403413
},
404414
weekly: { reviewed: w.reviewed ?? 0, merged: w.merged ?? 0 },

test/unit/public-stats.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,28 @@ describe("getPublicStats — live aggregate over the review ledger", () => {
311311
expect(out.totals.minutesSaved).toBe(2742 * MINUTES_SAVED_PER_PR + 80 * MINUTES_SAVED_PER_PR);
312312
});
313313

314+
it("REGRESSION (#7449): global accuracyPct reflects the own-ledger population, not the Orb-fleet-inflated merged/closed denominator", async () => {
315+
// Own-ledger: 100 decided (all merged), 10 real reversals -> a true 90% accuracy. A huge registered Orb fleet
316+
// (6000 merged + 4000 closed, with no reversal data at all) must NOT dilute the denominator toward 100.
317+
const handler = (sql: string): Row[] => {
318+
if (isDispositions(sql)) return [{ project: "JSONbored/loopover", reviewed: 100, merged: 100, closed: 0, inReview: 0 }];
319+
if (isReversal(sql)) return [{ project: "JSONbored/loopover", reversed: 10 }];
320+
if (sql.includes("orb_pr_outcomes")) return [{ merged: 6000, closed: 4000, total: 10000 }];
321+
return [];
322+
};
323+
const out = await getPublicStats(stubEnv(handler), NOW);
324+
// The fleet fold still (correctly) inflates the raw aggregate counts...
325+
expect(out.totals.merged).toBe(100 + 6000);
326+
expect(out.totals.closed).toBe(0 + 4000);
327+
expect(out.totals.handled).toBe(100 + 10000);
328+
// ...but accuracy is computed from own-ledger only: 1 - 10/(100 + 0) = 90.0%.
329+
expect(out.totals.accuracyPct).toBe(90);
330+
// The pre-fix fleet-inflated denominator would have produced 1 - 10/(6100 + 4000) = 99.0% -- guard against it.
331+
expect(out.totals.accuracyPct).not.toBe(99);
332+
// Per-project accuracy is already same-scope and stays unchanged: 1 - 10/100 = 90.
333+
expect(out.byProject[0]!.accuracyPct).toBe(90);
334+
});
335+
314336
it("keeps own-ledger per-PR effort sum separate from Orb fleet flat credit", async () => {
315337
const withOrbAndEffort = (sql: string): Row[] => {
316338
if (sql.includes("orb_pr_outcomes")) return [{ merged: 10, closed: 5, total: 15 }];

0 commit comments

Comments
 (0)