fix(stats): bound the accuracy denominator to the window its numerator survives in

#9718 gated accuracy on whether a reversal is observable at all, and fixed the
weekly trend's Orb-folded denominator. It did not fix the third asymmetry in
#9676: `merged`/`closed` come from `github_app.pr_public_surface_published`,
the single retention-EXEMPT audit event type, so they are lifetime and never
shrink -- while `reversal_*` rows prune with the rest of audit_events at 90
days. Pairing an immortal denominator with a 90-day numerator makes
`1 - reversed/decided` drift toward 100% as the ledger ages, independent of
real reversal behavior.

Confirmed live after #9718 deployed: byProject still published 100% for all
three repos on 2377/602/508 reviewed with 0 reversals, because the reversal
signal IS observable on that deployment -- the gate passed and the ratio was
still meaningless.

The accuracy denominator is now a second, retention-windowed disposition query
that shares BOTH of the numerator's bounds: own-ledger only, and inside
audit_events' retention window. `reviewed`/`merged`/`closed` keep publishing
lifetime volume, which is measured and correct. That display-vs-denominator
split is the same shape #7449 established for the Orb fold and #9718 carried
into the weekly trend; the #7449 snapshot it supersedes is removed rather than
left dead, with its reasoning folded into the new comment.

Boundary case, stated rather than hidden: a PR published before the window but
reversed inside it contributes to the numerator and not the denominator.
accuracyPct's existing clamp already handles that (it is the same shape as the
reopened-auto-close case its comment describes), and the alternative -- an
unbounded denominator -- is the bug being fixed.

Refs #9676

Author: JSONbored

src/review/public-stats.ts

@@ -356,9 +356,10 @@ export async function getPublicStats(
   // The own-ledger side needs at least one allowlisted project to query; an empty allowlist skips these three
   // queries entirely (own-ledger totals stay zero) but still lets the Orb aggregate below run.
   const inList = projects.map(() => "?").join(", ");
-  const [dispositions, reversalRows, weeklyRows, effortRows] = projects.length === 0
+  const [dispositions, windowedDispositions, reversalRows, weeklyRows, effortRows] = projects.length === 0
     ? await Promise.all([
         Promise.resolve<DispositionRow[]>([]),
+        Promise.resolve<{ project: string; merged: number; closed: number }[]>([]),
         Promise.resolve<{ project: string; reversed: number }[]>([]),
         Promise.resolve<{ reviewed: number; merged: number }[]>([]),
         Promise.resolve<{ totalMinutes: number | null }[]>([]),
@@ -377,6 +378,25 @@ export async function getPublicStats(
         GROUP BY ev.repo`,
       ...projects,
     ),
+    // The ACCURACY denominator, bounded to the same window its numerator can survive in. `reviewed`/
+    // `merged`/`closed` above are lifetime: `github_app.pr_public_surface_published` is the single
+    // retention-EXEMPT audit event type (src/db/retention.ts's DURABLE_AUDIT_EVENT_TYPES), so those
+    // counts never shrink. `reversal_*` rows are pruned with the rest of audit_events at 90 days. Pairing
+    // an immortal denominator with a 90-day numerator makes `1 - reversed/decided` drift toward 100% as
+    // the ledger ages, independent of real reversal behavior -- the same shape #7449 fixed for the
+    // Orb-folded denominator, and confirmed live (2377/602/508 reviewed, 0 reversals, 100% each).
+    safeAll<{ project: string; merged: number; closed: number }>(
+      env,
+      `SELECT ev.repo AS project,
+              SUM(CASE WHEN pr.merged_at IS NOT NULL THEN 1 ELSE 0 END) AS merged,
+              SUM(CASE WHEN pr.state = 'closed' AND pr.merged_at IS NULL THEN 1 ELSE 0 END) AS closed
+         FROM (SELECT DISTINCT repo, number FROM (${PUBLISHED_PR_KEYS}) WHERE created_at >= ?) ev
+         LEFT JOIN pull_requests pr ON pr.repo_full_name = ev.repo AND pr.number = ev.number
+        WHERE LOWER(ev.repo) IN (${inList})
+        GROUP BY ev.repo`,
+      retentionCutoffIsoForTable("audit_events"),
+      ...projects,
+    ),
     safeAll<{ project: string; reversed: number }>(
       env,
       // A "reversal" = a human overturning a terminal engine auto-action, already detected and recorded by
@@ -466,6 +486,10 @@ export async function getPublicStats(
   // question about the same deployment, rather than one of them silently disagreeing with the others.
   const totalReversals = [...reversedByProject.values()].reduce((sum, n) => sum + n, 0);
   const reversalObservable = await loadReversalObservability(env, totalReversals);
+  // project -> the retention-windowed merged/closed pairing for `reversed`. See the query's own comment.
+  const windowedByProject = new Map(windowedDispositions.map((row) => [String(row.project).toLowerCase(), row]));
+  let windowedMerged = 0;
+  let windowedClosed = 0;
   const byProject = dispositions
     .map((d) => {
       const merged = d.merged ?? 0;
@@ -474,6 +498,11 @@ export async function getPublicStats(
       const reversed =
         reversedByProject.get(String(d.project).toLowerCase()) ?? 0;
       const reviewed = merged + closed + inReview;
+      const windowed = windowedByProject.get(String(d.project).toLowerCase());
+      const windowedRepoMerged = windowed?.merged ?? 0;
+      const windowedRepoClosed = windowed?.closed ?? 0;
+      windowedMerged += windowedRepoMerged;
+      windowedClosed += windowedRepoClosed;
       totals.handled += reviewed;
       totals.merged += merged;
       totals.closed += closed;
@@ -485,7 +514,7 @@ export async function getPublicStats(
         reviewed,
         merged,
         closed,
-        accuracyPct: accuracyPct(merged, closed, reversed, reversalObservable),
+        accuracyPct: accuracyPct(windowedRepoMerged, windowedRepoClosed, reversed, reversalObservable),
       };
     })
     .filter((r) => r.reviewed > 0)
@@ -501,13 +530,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;
+  // #7449 (superseded, same reasoning): the published global accuracyPct is NOT computed from
+  // totals.merged/closed. Those are lifetime AND fleet-folded, while totals.reversed is own-ledger-only and
+  // 90-day-pruned, so pairing them lets the denominator grow -- with every newly registered install, and with
+  // every additional day of immortal publish events -- while the numerator cannot, trending the percentage
+  // toward 100 independent of real reversal behavior. windowedMerged/windowedClosed (accumulated above) is the
+  // pairing that shares BOTH of the numerator's bounds: own-ledger only, and inside audit_events' retention
+  // window. The fleet fold still correctly inflates reviewed/handled/minutesSaved, which have no such pairing.
   // Fleet accuracy (bugfix, #fairness-analytics): independent of the own-ledger allowlist above, so it's fetched
   // unconditionally alongside the Orb global fold, matching that fold's own unscoped-regardless-of-allowlist
   // behavior (see the "skips the own-ledger queries but still queries the Orb aggregate" test).
@@ -613,7 +642,7 @@ export async function getPublicStats(
       // 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, reversalObservable),
+      accuracyPct: accuracyPct(windowedMerged, windowedClosed, totals.reversed, reversalObservable),
       minutesSaved,
     },
     weekly: { reviewed: w.reviewed ?? 0, merged: w.merged ?? 0 },

test/unit/public-stats.test.ts

@@ -737,6 +737,42 @@ describe("getPublicStats — live aggregate over the review ledger", () => {
     expect(out.byProject.map((row) => row.accuracyPct)).toEqual([null]);
   });
 
+  it("REGRESSION: the accuracy denominator is bounded to audit_events' retention window, not lifetime (real D1)", async () => {
+    // `github_app.pr_public_surface_published` is the ONE retention-exempt event type, so reviewed/merged/
+    // closed are immortal, while `reversal_*` rows prune at 90 days. Pairing them made 1 - reversed/decided
+    // drift toward 100% as the ledger aged -- live: 2377/602/508 reviewed, 0 reversals, 100% each.
+    const env = createTestEnv({ LOOPOVER_PUBLIC_STATS_REPOS: "JSONbored/loopover" });
+    const old = new Date(NOW - 200 * 86_400_000).toISOString(); // outside the 90-day window
+    const recent = new Date(NOW - 5 * 86_400_000).toISOString();
+    await upsertRepositoryFromGitHub(env, { name: "loopover", full_name: "JSONbored/loopover", private: false, owner: { login: "JSONbored" } }, 1);
+
+    // Four ancient merged PRs -- they keep counting toward `reviewed`, but must NOT pad the accuracy
+    // denominator, because a reversal of any of them would long since have been pruned.
+    for (const number of [1, 2, 3, 4]) {
+      await upsertPullRequestFromGitHub(env, "JSONbored/loopover", { number, title: `old ${number}`, state: "closed", merged_at: old, user: { login: "a" }, head: { sha: `s${number}` }, labels: [] });
+      await recordAuditEvent(env, { eventType: "github_app.pr_public_surface_published", targetKey: `JSONbored/loopover#${number}`, outcome: "completed", createdAt: old });
+    }
+    // One recent auto-closed PR, reversed by a human.
+    await upsertPullRequestFromGitHub(env, "JSONbored/loopover", { number: 5, title: "recent", state: "open", user: { login: "b" }, head: { sha: "s5" }, labels: [] });
+    await recordAuditEvent(env, { eventType: "github_app.pr_public_surface_published", targetKey: "JSONbored/loopover#5", outcome: "completed", createdAt: recent });
+    await recordAuditEvent(env, { eventType: "agent.action.close", targetKey: "JSONbored/loopover#5", outcome: "completed", createdAt: recent });
+    await recordAuditEvent(env, { eventType: "reversal_reopened", targetKey: "JSONbored/loopover#5", outcome: "completed", createdAt: recent });
+    // ...and one recent merged PR that stands, so the windowed denominator is 1 merged + 0 closed... plus
+    // PR#5 which is currently `open` and so counts as inReview, not decided.
+    await upsertPullRequestFromGitHub(env, "JSONbored/loopover", { number: 6, title: "recent ok", state: "closed", merged_at: recent, user: { login: "c" }, head: { sha: "s6" }, labels: [] });
+    await recordAuditEvent(env, { eventType: "github_app.pr_public_surface_published", targetKey: "JSONbored/loopover#6", outcome: "completed", createdAt: recent });
+
+    const out = await getPublicStats(env, NOW);
+    // Lifetime volume still reports every PR ever published -- that part is measured and unaffected.
+    expect(out.totals.reviewed).toBe(6);
+    expect(out.totals.merged).toBe(5);
+    expect(out.totals.reversed).toBe(1);
+    // Accuracy divides by the WINDOWED denominator (PR#6 merged inside the window), not the lifetime 5.
+    // 1 - 1/1 = 0%, not the 1 - 1/5 = 80% the old lifetime pairing would have published.
+    expect(out.byProject[0]?.accuracyPct).toBe(0);
+    expect(out.totals.accuracyPct).toBe(0);
+  });
+
   it("publishes a real accuracy once the deployment records the auto-actions a reversal attaches to (real D1)", async () => {
     const env = createTestEnv({ LOOPOVER_PUBLIC_STATS_REPOS: "JSONbored/loopover" });
     await upsertRepositoryFromGitHub(env, { name: "loopover", full_name: "JSONbored/loopover", private: false, owner: { login: "JSONbored" } }, 1);