Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/review/public-stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 },
Expand Down
22 changes: 22 additions & 0 deletions test/unit/public-stats.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }];
Expand Down