feat(review): surface aggregate review-effort in maintainer stats (#2155)

src/review/review-effort.ts

@@ -46,6 +46,11 @@ function bandForEffort(effort: number): 1 | 2 | 3 | 4 | 5 {
   return 5;
 }
 
+/** Map a persisted minutes estimate back to its complexity band (inverse of `estimateReviewEffort`'s minutes step). */
+export function bandFromMinutes(minutes: number): 1 | 2 | 3 | 4 | 5 {
+  return bandForEffort(Math.max(0, minutes) / MINUTES_PER_EFFORT);
+}
+
 /** Estimate the review effort of a change set. Pure and deterministic. */
 export function estimateReviewEffort(files: ReviewEffortFile[]): ReviewEffort {
   let weighted = 0;

src/review/stats.ts

@@ -16,19 +16,11 @@
 // deps so the core decision/reversal/gate-action aggregation is fully native here. The host wires its own
 // implementations (or the defaults below, which emit empty/no-signal reports, keeping the payload shape).
 //
-// SCOPE NOTE (#1955 — deterministic review-effort score): this feed's tables (`review_targets`, `review_audit`,
-// and the injected eval/parity engine's own source, `review_audit`'s `gate_decision`/`pr_outcome` rows) are the
-// LEGACY reviewbot ledger — nothing writes new rows to `review_targets` since the self-host convergence cutover
-// (see public-stats.ts's file header), and every write into `review_audit` (outcomes-wire.ts's `pr_outcome`,
-// the gate's `gate_decision`) carries only decision/outcome metadata, never a PR's changed files or patches.
-// There is therefore NO live source this module could aggregate a per-PR review-effort estimate FROM today —
-// unlike public-stats.ts's `getPublicStats`, which reads the ACTIVE `audit_events` ledger the live review
-// pipeline (src/queue/processors.ts) still writes to every publish, and where `estimateReviewEffort`'s minutes
-// are now persisted (`reviewEffortMinutes` in `github_app.pr_public_surface_published` metadata) and averaged.
-// Wiring a same-shaped aggregate here would only ever read back a permanently-null placeholder — scaffolding
-// with no live behavior — so it is deliberately left out of this change rather than faked. A REAL maintainer-
-// dashboard effort aggregate needs its own persisted source (e.g. this module reading `audit_events` the way
-// public-stats.ts now does), which is a genuine follow-up, not a one-line addition to this file.
+// REVIEW EFFORT (#2155): decision/reversal/gate-action aggregates still read the legacy `review_targets` /
+// `review_audit` ledgers above, but the maintainer dashboard's complexity read comes from the ACTIVE `audit_events`
+// ledger (same `github_app.pr_public_surface_published` rows + `reviewEffortMinutes` metadata public-stats.ts uses).
+// Bearer-gated here only — never folded into the public homepage counter.
+import { bandFromMinutes } from "./review-effort";
 
 // ── Inlined report types (ported shapes from reviewbot src/core/{eval,tuning}.ts) ────────────────
 
@@ -176,6 +168,13 @@ const BUCKET_SQL: Record<string, string> = {
   month: "strftime('%Y-%m', created_at)",
 };
 
+export interface ReviewEffortAggregate {
+  /** Rounded average complexity band across distinct reviewed PRs in the window; null when no samples. */
+  avgBand: number | null;
+  /** Sum of per-PR estimated review minutes in the window; 0 when no samples. */
+  totalEstimatedMinutes: number;
+}
+
 export interface StatsPayload {
   generatedAt: string;
   window: { fromIso: string; days: number; bucket: string };
@@ -187,6 +186,8 @@ export interface StatsPayload {
   reversals: Array<{ bucket: string; project: string; n: number }>;
   /** Non-content gate decisions (incl. SHADOW would-actions), per project+action. */
   gateActions: Array<{ project: string; action: string; n: number }>;
+  /** Aggregate review-effort signal for maintainer triage (#2155); reads `audit_events`, not the legacy ledgers. */
+  reviewEffort: ReviewEffortAggregate;
   /** Gate eval: prediction scored against the PR's real outcome — merge/close precision per project. */
   gateEval: GateEvalReport;
   /** Ranked tuning recommendations derived from the eval (ready-to-flip / tighten / loosen). */
@@ -195,6 +196,18 @@ export interface StatsPayload {
   gateParity: GateParityReport & { cutoverReady: Array<{ project: string; ready: boolean }> };
 }
 
+/** Fold per-PR persisted minutes into the maintainer aggregate (avg band + total minutes). */
+export function aggregateReviewEffort(perPrMinutes: number[]): ReviewEffortAggregate {
+  if (perPrMinutes.length === 0) {
+    return { avgBand: null, totalEstimatedMinutes: 0 };
+  }
+  const bands = perPrMinutes.map((minutes) => bandFromMinutes(minutes));
+  return {
+    avgBand: Math.round(bands.reduce((sum, band) => sum + band, 0) / bands.length),
+    totalEstimatedMinutes: perPrMinutes.reduce((sum, minutes) => sum + minutes, 0),
+  };
+}
+
 /** Aggregate the decision ledger for the dashboard. Pure-ish (reads D1 only); no GitHub I/O. */
 export async function computeStats(
   env: Env,
@@ -211,7 +224,7 @@ export async function computeStats(
   const bucketExpr = BUCKET_SQL[bucket] ?? BUCKET_SQL.day;
   const fromIso = new Date(opts.nowMs - days * 86_400_000).toISOString().slice(0, 10); // YYYY-MM-DD
 
-  const [decisionRows, reversalRows] = await Promise.all([
+  const [decisionRows, reversalRows, effortRows] = await Promise.all([
     storage(env).prepare(
       `SELECT ${bucketExpr} AS bucket, project, COALESCE(verdict, status) AS verdict, COUNT(*) AS n
        FROM review_targets
@@ -226,6 +239,25 @@ export async function computeStats(
        GROUP BY bucket, project
        ORDER BY bucket ASC`,
     ).bind(fromIso).all<{ bucket: string; project: string; n: number }>(),
+    // review-effort (#2155): same persisted `reviewEffortMinutes` public-stats averages, scoped to this window.
+    // Repeated publish events for one PR collapse to one sample (per-PR AVG) before the global fold.
+    storage(env).prepare(
+      `SELECT minutes FROM (
+         SELECT repo, number, AVG(minutes) AS minutes
+           FROM (
+             SELECT LOWER(substr(target_key, 1, instr(target_key, '#') - 1)) AS repo,
+                    CAST(substr(target_key, instr(target_key, '#') + 1) AS INTEGER) AS number,
+                    json_extract(metadata_json, '$.reviewEffortMinutes') AS minutes
+               FROM audit_events
+              WHERE event_type = 'github_app.pr_public_surface_published'
+                AND created_at >= ?
+                AND instr(target_key, '#') > 0
+           )
+          WHERE minutes IS NOT NULL
+          GROUP BY repo, number
+       )`,
+    ).bind(fromIso).all<{ minutes: number }>()
+      .catch(() => ({ results: [] as Array<{ minutes: number }> })),
   ]);
 
   // Non-content gate decisions (incl. SHADOW would-actions) — recorded as `gate_decision` audit rows with
@@ -247,6 +279,9 @@ export async function computeStats(
 
   const rows = decisionRows.results ?? [];
   const reversals = reversalRows.results ?? [];
+  const reviewEffort = aggregateReviewEffort(
+    (effortRows.results ?? []).map((row) => row.minutes ?? 0).filter((minutes) => minutes > 0),
+  );
   return {
     generatedAt: new Date(opts.nowMs).toISOString(),
     window: { fromIso, days, bucket },
@@ -255,6 +290,7 @@ export async function computeStats(
     rows,
     reversals,
     gateActions: gateRows.results ?? [],
+    reviewEffort,
     gateEval,
     recommendations,
     gateParity: { ...parity, cutoverReady: parity.rows.map((r) => ({ project: r.project, ready: isParityCutoverReady(r) })) },

test/unit/review-effort.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { estimateReviewEffort, type ReviewEffortFile } from "../../src/review/review-effort";
+import { bandFromMinutes, estimateReviewEffort, type ReviewEffortFile } from "../../src/review/review-effort";
 
 // A patch with exactly `added` added lines (each `+`), so a test can dial the effort precisely.
 function srcPatch(added: number): string {
@@ -56,3 +56,17 @@ describe("estimateReviewEffort (#2151)", () => {
     expect(estimateReviewEffort(files)).toEqual({ band: 5, minutes: 258 });
   });
 });
+
+describe("bandFromMinutes (#2155)", () => {
+  it("maps persisted minutes back to the same band the estimator would have produced", () => {
+    const samples = [
+      estimateReviewEffort([]),
+      estimateReviewEffort([file("src/a.ts", 20)]),
+      estimateReviewEffort([file("src/a.ts", 200)]),
+      estimateReviewEffort([file("src/a.ts", 400)]),
+    ];
+    for (const sample of samples) {
+      expect(bandFromMinutes(sample.minutes)).toBe(sample.band);
+    }
+  });
+});

test/unit/stats.test.ts

@@ -1,5 +1,7 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
+import { createTestEnv } from "../helpers/d1";
 import {
+  aggregateReviewEffort,
   computeStats,
   handleParity,
   handleStats,
@@ -22,6 +24,10 @@ function stubEnv(extra: Record<string, unknown> = {}): Env {
     { project: "metagraphed", action: "merge", n: 7 },
     { project: "metagraphed", action: "hold", n: 2 },
   ];
+  const effortMinutes = [
+    { minutes: 4 },
+    { minutes: 96 },
+  ];
   let lastSql = "";
   return {
     ...extra,
@@ -31,9 +37,15 @@ function stubEnv(extra: Record<string, unknown> = {}): Env {
         return {
           bind: () => ({
             all: async () => ({
-              // gate_decision breakdown → gateActions; other review_audit reads → reversals;
+              // review-effort read → effortMinutes; gate_decision → gateActions; other review_audit → reversals;
               // everything else → decision rows. (The eval/parity engine is the default no-op deps.)
-              results: lastSql.includes("gate_decision") ? gateActions : lastSql.includes("review_audit") ? reversals : decisions,
+              results: lastSql.includes("reviewEffortMinutes")
+                ? effortMinutes
+                : lastSql.includes("gate_decision")
+                  ? gateActions
+                  : lastSql.includes("review_audit")
+                    ? reversals
+                    : decisions,
             }),
           }),
         };
@@ -59,6 +71,7 @@ describe("computeStats — D1 aggregate for the dashboard", () => {
     expect(out.gateEval).toEqual({ rows: [], hasSignal: false });
     expect(out.recommendations).toEqual([]);
     expect(out.gateParity.cutoverReady).toEqual([]);
+    expect(out.reviewEffort).toEqual({ avgBand: 3, totalEstimatedMinutes: 100 });
   });
 
   it("clamps an absurd window and falls back to a safe bucket", async () => {
@@ -130,6 +143,96 @@ describe("handleStats — bearer-gated, CORS-open feed", () => {
   });
 });
 
+describe("aggregateReviewEffort — maintainer complexity fold (#2155)", () => {
+  it("returns null avgBand and 0 total minutes for an empty sample", () => {
+    expect(aggregateReviewEffort([])).toEqual({ avgBand: null, totalEstimatedMinutes: 0 });
+  });
+
+  it("averages bands and sums minutes across per-PR samples", () => {
+    // minutes 4 -> band 1; minutes 96 -> band 4 -> rounded avg 3; total 100.
+    expect(aggregateReviewEffort([4, 96])).toEqual({ avgBand: 3, totalEstimatedMinutes: 100 });
+  });
+});
+
+describe("computeStats — review-effort read is fail-safe", () => {
+  function effortThrowingEnv(): Env {
+    let lastSql = "";
+    return {
+      DB: {
+        prepare: (s: string) => {
+          lastSql = s;
+          return {
+            bind: () => ({
+              all: async () => {
+                if (lastSql.includes("reviewEffortMinutes")) throw new Error("effort read down");
+                return { results: [] };
+              },
+            }),
+          };
+        },
+      },
+    } as unknown as Env;
+  }
+
+  it("falls back to reviewEffort null/0 when the audit_events effort query rejects", async () => {
+    const out = await computeStats(effortThrowingEnv(), { days: 30, bucket: "day", nowMs: NOW });
+    expect(out.reviewEffort).toEqual({ avgBand: null, totalEstimatedMinutes: 0 });
+  });
+
+  it("averages real reviewEffortMinutes out of audit_events via json_extract (real D1)", async () => {
+    const env = createTestEnv();
+    const db = env.DB;
+    await db
+      .prepare(
+        `INSERT INTO audit_events (id, event_type, target_key, outcome, metadata_json, created_at)
+         VALUES (?, ?, ?, ?, ?, ?), (?, ?, ?, ?, ?, ?)`,
+      )
+      .bind(
+        "published-a",
+        "github_app.pr_public_surface_published",
+        "JSONbored/gittensory#10",
+        "completed",
+        JSON.stringify({ reviewEffortMinutes: 4 }),
+        "2026-06-10T00:00:00.000Z",
+        "published-b",
+        "github_app.pr_public_surface_published",
+        "JSONbored/gittensory#11",
+        "completed",
+        JSON.stringify({ reviewEffortMinutes: 96 }),
+        "2026-06-11T00:00:00.000Z",
+      )
+      .run();
+
+    const out = await computeStats(env, { days: 90, bucket: "day", nowMs: NOW });
+    expect(out.reviewEffort).toEqual({ avgBand: 3, totalEstimatedMinutes: 100 });
+  });
+
+  it("skips nullish/zero minute rows when folding reviewEffort (the ?? 0 + > 0 filter branches)", async () => {
+    function mixedEffortEnv(): Env {
+      let lastSql = "";
+      return {
+        DB: {
+          prepare: (s: string) => {
+            lastSql = s;
+            return {
+              bind: () => ({
+                all: async () => ({
+                  results: lastSql.includes("reviewEffortMinutes")
+                    ? [{ minutes: null }, { minutes: 0 }, { minutes: 10 }]
+                    : [],
+                }),
+              }),
+            };
+          },
+        },
+      } as unknown as Env;
+    }
+
+    const out = await computeStats(mixedEffortEnv(), { days: 30, bucket: "day", nowMs: NOW });
+    expect(out.reviewEffort).toEqual({ avgBand: 2, totalEstimatedMinutes: 10 });
+  });
+});
+
 describe("computeStats — gate-decision read is fail-safe", () => {
   // A stubEnv whose gate_decision query rejects: computeStats should still resolve with gateActions: [].
   function gateThrowingEnv(): Env {
@@ -300,6 +403,7 @@ describe("computeStats — NaN window + null D1 results (the ?? [] fallbacks)",
     expect(out.gateActions).toEqual([]);
     expect(out.projects).toEqual([]);
     expect(out.verdicts).toEqual([]);
+    expect(out.reviewEffort).toEqual({ avgBand: null, totalEstimatedMinutes: 0 });
   });
 });