Merge branch 'main' into fix/miner-resolve-local-store-db-path-8336

claytonlin1110 · web-flow · commit e91313eb46fd · 2026-07-24T12:49:31.000-05:00
diff --git a/packages/loopover-miner/lib/calibration.ts b/packages/loopover-miner/lib/calibration.ts
@@ -58,6 +58,33 @@ function recordKey(project: string, targetId: string): string {
   return `${project} ${targetId}`;
 }
 
+/** Build the same `(project, targetId)` → normalized-outcome map {@link buildCalibrationReport} uses internally.
+ *  Malformed outcome records are skipped. Exported so metrics-cli (#8315) can reuse the join without a second
+ *  implementation. */
+export function buildOutcomeDecisionMap(outcomes: ObservedOutcomeRecord[]): Map<string, "merge" | "close" | "hold" | ""> {
+  const outcomeByKey = new Map<string, "merge" | "close" | "hold" | "">();
+  for (const outcome of Array.isArray(outcomes) ? outcomes : []) {
+    if (!isObservedOutcomeRecord(outcome)) continue;
+    outcomeByKey.set(recordKey(outcome.project, outcome.targetId), normalizeDecision(outcome.outcomeDecision));
+  }
+  return outcomeByKey;
+}
+
+/** Resolve one prediction's `correct` flag using the same join rules as {@link buildCalibrationReport}: only
+ *  directional merge/close predictions with a realized merge/close outcome are scored; hold, pending, and
+ *  unclassifiable rows return `undefined` (unset). Malformed predictions return `undefined`. */
+export function resolvePredictionCorrectness(
+  prediction: PredictedVerdictRecord,
+  outcomeByKey: Map<string, "merge" | "close" | "hold" | "">,
+): boolean | undefined {
+  if (!isPredictedVerdictRecord(prediction)) return undefined;
+  const observed = outcomeByKey.get(recordKey(prediction.project, prediction.targetId));
+  if (observed !== "merge" && observed !== "close") return undefined;
+  const predicted = normalizeDecision(prediction.predictedDecision);
+  if (predicted !== "merge" && predicted !== "close") return undefined;
+  return predicted === observed;
+}
+
 /**
  * Join predicted-verdict records with realized-outcome records into a per-project calibration report. Pure and
  * read-only. A prediction counts as "decided" only when a realized outcome for the SAME `(project, targetId)`
@@ -70,11 +97,7 @@ export function buildCalibrationReport(
   predictions: PredictedVerdictRecord[],
   outcomes: ObservedOutcomeRecord[],
 ): CalibrationReport {
-  const outcomeByKey = new Map<string, "merge" | "close" | "hold" | "">();
-  for (const outcome of Array.isArray(outcomes) ? outcomes : []) {
-    if (!isObservedOutcomeRecord(outcome)) continue;
-    outcomeByKey.set(recordKey(outcome.project, outcome.targetId), normalizeDecision(outcome.outcomeDecision));
-  }
+  const outcomeByKey = buildOutcomeDecisionMap(outcomes);
 
   const byProject = new Map<string, CalibrationRow>();
   for (const prediction of Array.isArray(predictions) ? predictions : []) {
diff --git a/packages/loopover-miner/lib/metrics-cli.ts b/packages/loopover-miner/lib/metrics-cli.ts
@@ -1,25 +1,40 @@
 import { renderMinerPredictionMetrics } from "@loopover/engine";
 import type { MinerPredictionMetricRow } from "@loopover/engine";
+import { buildOutcomeDecisionMap, resolvePredictionCorrectness } from "./calibration.js";
+import type { ObservedOutcomeRecord } from "./calibration-types.js";
+import { toOutcomeRecords, toPredictionRecords } from "./calibration-cli.js";
+import { initEventLedger, resolveEventLedgerDbPath } from "./event-ledger.js";
+import type { EventLedger } from "./event-ledger.js";
 import { initPredictionLedger } from "./prediction-ledger.js";
 import type { PredictionLedger } from "./prediction-ledger.js";
 import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js";
 
 // `metrics` (#4838): render the miner's prediction-calibration counters as Prometheus text-exposition to stdout,
 // for a scrape wrapper or cron redirect. The counters are produced by the engine's already-built
-// renderMinerPredictionMetrics (packages/loopover-engine/src/miner-prediction-metrics.ts) -- this command only
-// reads the local prediction ledger and feeds it in, never touching the renderer itself. Strictly local + offline:
-// no network, no writes.
+// renderMinerPredictionMetrics (packages/loopover-engine/src/miner-prediction-metrics.ts) -- this command reads
+// the local prediction ledger, joins each row with realized `pr_outcome` events from the event ledger
+// (calibration-cli.js's toPredictionRecords/toOutcomeRecords + calibration.js's join), and feeds the resolved
+// rows to the renderer. Strictly local + offline: no network, no writes.
 
 const METRICS_USAGE = "Usage: loopover-miner metrics";
 
 /**
- * Project prediction-ledger rows onto the engine renderer's metric-row shape -- the predicted `conclusion` only.
- * The realized-outcome pairing (`correct`) is intentionally left unset: the miner has no outcome-join yet, so the
- * correct/incorrect counters stay zero and only `predictions_total{conclusion}` moves -- exactly how the renderer
- * is designed to degrade before outcome-pairing exists (see its header comment).
+ * Project prediction-ledger rows onto the engine renderer's metric-row shape, pairing each predicted `conclusion`
+ * with a realized outcome when one exists for the same `(repoFullName, targetId)`. Reuses calibration-cli.js's
+ * record mappers and calibration.js's {@link resolvePredictionCorrectness} so the join matches
+ * `buildCalibrationReport` exactly.
  */
-export function collectPredictionMetricRows(ledger: PredictionLedger): MinerPredictionMetricRow[] {
-  return ledger.readPredictions().map((entry) => ({ conclusion: entry.conclusion }));
+export function collectPredictionMetricRows(
+  ledger: PredictionLedger,
+  outcomes: ObservedOutcomeRecord[] = [],
+): MinerPredictionMetricRow[] {
+  const outcomeByKey = buildOutcomeDecisionMap(outcomes);
+  return toPredictionRecords(ledger.readPredictions()).map((prediction) => {
+    const correct = resolvePredictionCorrectness(prediction, outcomeByKey);
+    const row: MinerPredictionMetricRow = { conclusion: prediction.predictedDecision };
+    if (correct !== undefined) row.correct = correct;
+    return row;
+  });
 }
 
 // Open the local prediction ledger (or a test-injected one) for the duration of `run`, closing it only when we
@@ -37,19 +52,34 @@ function withPredictionLedger<T>(
   }
 }
 
-export function runMetrics(args: string[], options: { initPredictionLedger?: () => PredictionLedger } = {}): number {
+export function runMetrics(
+  args: string[],
+  options: {
+    initPredictionLedger?: () => PredictionLedger;
+    initEventLedger?: () => EventLedger;
+    env?: Record<string, string | undefined>;
+  } = {},
+): number {
   if (args.length > 0) {
     return reportCliFailure(argsWantJson(args), METRICS_USAGE);
   }
 
+  const env = options.env ?? process.env;
+  let eventLedger: EventLedger | undefined;
+  const ownsEventLedger = options.initEventLedger === undefined;
+
   try {
     return withPredictionLedger(options, (ledger) => {
+      eventLedger = (options.initEventLedger ?? (() => initEventLedger(resolveEventLedgerDbPath(env))))();
+      const outcomes = toOutcomeRecords(eventLedger.readEvents());
       // renderMinerPredictionMetrics returns a newline-terminated document; console.log re-adds the terminator, so
       // trim it to emit exactly one trailing newline.
-      console.log(renderMinerPredictionMetrics(collectPredictionMetricRows(ledger)).trimEnd());
+      console.log(renderMinerPredictionMetrics(collectPredictionMetricRows(ledger, outcomes)).trimEnd());
       return 0;
     });
   } catch (error) {
     return reportCliFailure(argsWantJson(args), describeCliError(error));
+  } finally {
+    if (ownsEventLedger) eventLedger?.close();
   }
 }
diff --git a/src/services/maintainer-recap.ts b/src/services/maintainer-recap.ts
@@ -14,6 +14,11 @@ import { PUBLIC_LOCAL_PATH_SCRUB_PATTERN, PUBLIC_UNSAFE_PATTERN } from "../signa
 import { deliverRecapToDiscord, deliverRecapToSlack } from "./notify-discord";
 import type { GatePrecisionReport } from "./gate-precision";
 import type { DriftRecapSection } from "./maintainer-recap-drift";
+// #8372: these three section builders shipped fully implemented + unit-tested but were never composed into
+// the delivered digest -- the same "built, tested, never called from production" shape as #6636.
+import { buildCalibrationRecapSection } from "./maintainer-recap-calibration";
+import { buildGateOutcomesRecapSection } from "./maintainer-recap-gate-outcomes";
+import { buildPerRepoRecapSection } from "./maintainer-recap-per-repo";
 import { buildRoutingRecapSection } from "./maintainer-recap-routing";
 import { REVIEWER_ROUTING_SHADOW_EVENT_TYPE, type RoutingShadowDecision } from "./reviewer-routing";
 import type { OutcomeCalibration } from "./outcome-calibration";
@@ -167,10 +172,12 @@ function recapSectionLines(items: string[], fallback: string): string[] {
 export function formatMaintainerRecap(report: RecapReport, options: { configDrift?: DriftRecapSection; routingShadow?: { title: string; lines: string[] } } = {}): string {
   const { totals } = report;
   const rate = totals.gateFalsePositiveRate !== null ? `${Math.round(totals.gateFalsePositiveRate * 100)}%` : "n/a";
-  const perRepoLines = report.repos.map(
-    (repo) =>
-      `${redactRecapLine(repo.repoFullName)} — ${repo.reviewed} reviewed, ${repo.merged} merged, ${repo.closed} closed, ${repo.gateFalsePositives} gate false-positive(s), ${repo.gateOverrides} override(s), ${repo.reversals} reversal(s)`,
-  );
+  // #8372: the dedicated builder replaces the inline map that duplicated it -- unlike this file's old copy,
+  // it sorts, caps the list, and emits a "(+N more)" remainder line.
+  const perRepoSection = buildPerRepoRecapSection({ windowDays: report.windowDays, repos: report.repos });
+  const perRepoLines = perRepoSection.lines.map(redactRecapLine);
+  const calibrationSection = buildCalibrationRecapSection({ windowDays: report.windowDays, totals: report.totals });
+  const gateOutcomesSection = buildGateOutcomesRecapSection({ windowDays: report.windowDays, totals: report.totals });
   const lines = [
     "# Maintainer recap",
     "",
@@ -191,6 +198,14 @@ export function formatMaintainerRecap(report: RecapReport, options: { configDrif
     "",
     "## Per-repo",
     ...recapSectionLines(perRepoLines, "_No repositories in this window._"),
+    "",
+    // #8372: unconditional (not behind an options flag) -- both sections read only report.totals/windowDays,
+    // which every RecapReport always carries, so there is nothing for a caller to opt into.
+    `## ${redactRecapLine(calibrationSection.title)}`,
+    ...recapSectionLines(calibrationSection.lines.map(redactRecapLine), "_No calibration lines for this window._"),
+    "",
+    `## ${redactRecapLine(gateOutcomesSection.title)}`,
+    ...recapSectionLines(gateOutcomesSection.lines.map(redactRecapLine), "_No gate-outcome lines for this window._"),
     // #8214: optional config-drift section (maintainer-recap-drift.ts) — appended only when the caller has a
     // sentinel projection to render, so every existing digest stays byte-identical until the sentinel wires in.
     ...(options.configDrift
@@ -257,6 +272,11 @@ export async function runMaintainerRecap(
     report?: RecapReport;
     /** When explicitly false, short-circuits before build/format/delivery. Default: run. */
     enabled?: boolean;
+    /** #8372: forwarded to {@link formatMaintainerRecap} so a caller holding a drift projection can have it
+     *  rendered. Deliberately NOT sourced here -- reading the knob-loosening sentinel state is its own
+     *  data-sourcing concern; this is only the plumbing, so the section stays absent until a caller passes it
+     *  and every existing digest is unaffected. */
+    configDrift?: DriftRecapSection;
   } = {},
 ): Promise<RunMaintainerRecapResult> {
   if (options.enabled === false) return { skipped: true, reason: "disabled" };
@@ -271,7 +291,13 @@ export async function runMaintainerRecap(
   // #8229 stage 1: the routing-shadow section reads the window's recorded decisions straight from the
   // audit trail — fail-safe to an absent section (the recap must never break on a read blip).
   const routingShadow = await loadRoutingRecapSection(env, report.windowDays, options.generatedAt ?? nowIso());
-  const formatted = formatMaintainerRecap(report, routingShadow ? { routingShadow } : {});
+  // Built up key-by-key rather than passed as a conditional-spread literal: exactOptionalPropertyTypes
+  // forbids handing either key an explicit `undefined`, and #8229's routingShadow and #8372's configDrift
+  // are independent — each is present or absent on its own, so a single ternary can't express all four cases.
+  const recapOptions: { routingShadow?: { title: string; lines: string[] }; configDrift?: DriftRecapSection } = {};
+  if (routingShadow) recapOptions.routingShadow = routingShadow;
+  if (options.configDrift) recapOptions.configDrift = options.configDrift;
+  const formatted = formatMaintainerRecap(report, recapOptions);
   const [discord, slack] = await Promise.all([
     deliverRecapToDiscord(env, report, formatted),
     deliverRecapToSlack(env, report, formatted),
diff --git a/test/unit/maintainer-recap-format.test.ts b/test/unit/maintainer-recap-format.test.ts
@@ -33,12 +33,18 @@ describe("formatMaintainerRecap (#2240)", () => {
     expect(body).toContain("## Summary");
     expect(body).toContain("## Totals");
     expect(body).toContain("## Per-repo");
+    // #8372: both builders read only totals/windowDays, so their sections are unconditional.
+    expect(body).toContain("## Calibration");
+    expect(body).toContain("## Gate outcomes");
     // #8214: without a sentinel projection the drift section is entirely absent — the digest stays
     // byte-identical to the pre-drift shape, not a dangling empty header.
     expect(body).not.toContain("## Config drift");
     // Empty sections show a single fallback line instead of dangling under the header.
     expect(body).toContain("_No summary lines for this window._");
-    expect(body).toContain("_No repositories in this window._");
+    // #8372: the ## Per-repo body now comes from buildPerRepoRecapSection, which emits its own
+    // windowed empty-state line, so the section is never empty and the generic fallback never fires.
+    expect(body).toContain("No repo activity in the last 7 day(s).");
+    expect(body).not.toContain("_No repositories in this window._");
     // Null rate ⇒ the "n/a" arm.
     expect(body).toContain("- Gate false positives: 0/0 (n/a)");
     expect(body).toContain("- Repos: 0");
@@ -98,8 +104,10 @@ describe("formatMaintainerRecap (#2240)", () => {
     // Numeric / non-null rate arm.
     expect(body).toContain("- Gate false positives: 1/4 (25%)");
     expect(body).toContain("- Repos: 1");
-    // Per-repo row rendered (non-empty section arm).
-    expect(body).toContain("acme/widgets — 5 reviewed, 3 merged, 2 closed, 1 gate false-positive(s), 1 override(s), 0 reversal(s)");
+    // Per-repo row rendered (non-empty section arm), now in buildPerRepoRecapSection's row format (#8372).
+    // The gate/override/reversal counts this row used to carry are unchanged in ## Totals above, and are
+    // broken out per-dimension by the ## Gate outcomes section this digest now composes.
+    expect(body).toContain("acme/widgets: reviewed 5, merged 3, closed 2");
     // Clean summary line survives verbatim (redaction no-op arm).
     expect(body).toContain("- Normal recap line about resolved reviews.");
     // Arm 1: local path scrubbed to the placeholder, raw path gone.
diff --git a/test/unit/maintainer-recap.test.ts b/test/unit/maintainer-recap.test.ts
@@ -1,5 +1,6 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { buildMaintainerRecap, runMaintainerRecap, type MaintainerRecapRepoInput } from "../../src/services/maintainer-recap";
+import { buildDriftRecapSection } from "../../src/services/maintainer-recap-drift";
 import type { OutcomeCalibration } from "../../src/services/outcome-calibration";
 import type { RecapReport } from "../../src/types";
 import { createTestEnv } from "../helpers/d1";
@@ -229,10 +230,75 @@ describe("runMaintainerRecap (#2252 end-to-end orchestration)", () => {
     expect(result.skipped).toBe(false);
     if (result.skipped) return;
     expect(result.report.repos).toEqual([]);
-    expect(result.formatted).toContain("_No repositories in this window._");
+    // #8372: the ## Per-repo body is buildPerRepoRecapSection's, which carries its own empty-state line.
+    expect(result.formatted).toContain("No repo activity in the last 7 day(s).");
     expect(result.formatted).toContain("(n/a)");
   });
 
+  it("forwards a caller-supplied configDrift projection into the delivered digest (#8372 present arm)", async () => {
+    const calls = stubRecapChannelFetch();
+    const configDrift = buildDriftRecapSection({ generatedAt: GEN, sentinelEnabled: false, drifting: [], cleanKnobs: 0 });
+    const result = await runMaintainerRecap(envWithBothWebhooks(), { configDrift });
+    expect(result.skipped).toBe(false);
+    if (result.skipped) return;
+    expect(result.formatted).toContain("## Config drift");
+    // Reaches the actual delivered payload, not just the returned string.
+    expect(calls.some((c) => c.body.includes("## Config drift"))).toBe(true);
+  });
+
+  // #8372: runMaintainerRecap now assembles its formatter options key-by-key, so the routingShadow-present
+  // arm needs a real recorded decision. #8229 shipped that path with no test that produced one.
+  it("includes the #8229 routing-shadow section when the window has recorded decisions (routingShadow present arm)", async () => {
+    stubRecapChannelFetch();
+    const env = envWithBothWebhooks();
+    await env.DB.prepare(
+      "INSERT INTO audit_events (id, event_type, actor, target_key, outcome, detail, metadata_json, created_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?)",
+    )
+      .bind(
+        "ae-routing-1",
+        "reviewer_routing_shadow",
+        "loopover",
+        "acme/widgets#1",
+        "completed",
+        "shadow",
+        JSON.stringify({ repoFullName: "acme/widgets", preferredProvider: "claude-code", basis: ["evidence"] }),
+        GEN, // pinned to the same instant as generatedAt below, so the since-filter keeps it deterministically
+      )
+      .run();
+    const result = await runMaintainerRecap(env, { generatedAt: GEN });
+    expect(result.skipped).toBe(false);
+    if (result.skipped) return;
+    expect(result.formatted).toContain("Reviewer routing shadow");
+  });
+
+  // The absent arm: loadRoutingRecapSection is fail-safe (returns null on any read error), so a routing
+  // read blip must leave the digest intact minus that one section rather than breaking the whole recap.
+  it("omits the routing-shadow section when its audit read fails (routingShadow absent arm)", async () => {
+    stubRecapChannelFetch();
+    const base = envWithBothWebhooks();
+    const env = new Proxy(base, {
+      get(target, prop, receiver) {
+        if (prop !== "DB") return Reflect.get(target, prop, receiver);
+        return new Proxy(target.DB, {
+          get(dbTarget, dbProp, dbReceiver) {
+            if (dbProp !== "prepare") return Reflect.get(dbTarget, dbProp, dbReceiver);
+            return (sql: string) => {
+              if (sql.includes("SELECT metadata_json FROM audit_events")) throw new Error("routing_read_blip");
+              return dbTarget.prepare(sql);
+            };
+          },
+        });
+      },
+    }) as Env;
+
+    const result = await runMaintainerRecap(env, { generatedAt: GEN });
+    expect(result.skipped).toBe(false);
+    if (result.skipped) return;
+    expect(result.formatted).not.toContain("Reviewer routing shadow");
+    // The rest of the digest is unaffected — the failure costs one section, not the recap.
+    expect(result.formatted).toContain("## Totals");
+  });
+
   it("short-circuits when enabled is false — no build/format/fetch (flag-OFF arm)", async () => {
     const calls = stubRecapChannelFetch();
     const result = await runMaintainerRecap(envWithBothWebhooks(), { enabled: false, repos: [repoInput("owner/repo")] });
diff --git a/test/unit/miner-calibration.test.ts b/test/unit/miner-calibration.test.ts
diff --git a/test/unit/miner-metrics-cli.test.ts b/test/unit/miner-metrics-cli.test.ts
