diff --git a/src/services/maintainer-recap.ts b/src/services/maintainer-recap.ts index e0d000943..37d79e5d1 100644 --- a/src/services/maintainer-recap.ts +++ b/src/services/maintainer-recap.ts @@ -14,6 +14,9 @@ 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"; +import { buildPerRepoRecapSection } from "./maintainer-recap-per-repo"; +import { buildCalibrationRecapSection } from "./maintainer-recap-calibration"; +import { buildGateOutcomesRecapSection } from "./maintainer-recap-gate-outcomes"; 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 +170,11 @@ 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: compose the dedicated section builders instead of hand-rolling inline blocks, so a live digest now + // renders the same capped/sorted Per-repo, Calibration, and Gate-outcomes sections that ship unit-tested. + const perRepo = buildPerRepoRecapSection({ windowDays: report.windowDays, repos: report.repos }); + const calibration = buildCalibrationRecapSection({ windowDays: report.windowDays, totals }); + const gateOutcomes = buildGateOutcomesRecapSection({ windowDays: report.windowDays, totals }); const lines = [ "# Maintainer recap", "", @@ -190,7 +194,15 @@ export function formatMaintainerRecap(report: RecapReport, options: { configDrif `- Reversals: ${totals.reversals}`, "", "## Per-repo", - ...recapSectionLines(perRepoLines, "_No repositories in this window._"), + ...recapSectionLines(perRepo.lines, "_No repositories in this window._"), + "", + // #8372: Calibration + Gate-outcomes are unconditional — their inputs (window + totals) always exist, so + // every digest carries both sections. Header via the builder's own title (like the configDrift render). + `## ${redactRecapLine(calibration.title)}`, + ...recapSectionLines(calibration.lines, "_No calibration data for this window._"), + "", + `## ${redactRecapLine(gateOutcomes.title)}`, + ...recapSectionLines(gateOutcomes.lines, "_No gate outcomes 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 @@ -255,6 +267,10 @@ export async function runMaintainerRecap( repos?: MaintainerRecapRepoInput[]; /** Pre-built report for test injection; skips {@link buildMaintainerRecap} when set. */ report?: RecapReport; + /** #8372: optional config-drift section forwarded straight to {@link formatMaintainerRecap}. The real cron + * caller (src/review/maintainer-recap-wire.ts) does NOT yet supply one — live digests stay unchanged for + * that path until a sentinel projection wires it in; no live drift data is sourced here. */ + configDrift?: DriftRecapSection; /** When explicitly false, short-circuits before build/format/delivery. Default: run. */ enabled?: boolean; } = {}, @@ -271,7 +287,12 @@ 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 } : {}); + // #8372: forward the caller's optional configDrift alongside the routing-shadow section. maintainer-recap- + // wire.ts passes no configDrift today, so its live digests stay unchanged until a sentinel projection wires in. + const formatted = formatMaintainerRecap(report, { + ...(options.configDrift ? { configDrift: options.configDrift } : {}), + ...(routingShadow ? { routingShadow } : {}), + }); 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 index 68800f993..3cc1657cb 100644 --- a/test/unit/maintainer-recap-format.test.ts +++ b/test/unit/maintainer-recap-format.test.ts @@ -28,17 +28,30 @@ function emptyReport(): RecapReport { describe("formatMaintainerRecap (#2240)", () => { it("renders the header and every titled section, with fallback lines and an n/a rate for an empty window", () => { const body = formatMaintainerRecap(emptyReport()); - // Header + all three titled section headers render. + // Header + every titled section header renders (Per-repo/Calibration/Gate outcomes now via the builders, #8372). expect(body).toContain("# Maintainer recap"); expect(body).toContain("## Summary"); expect(body).toContain("## Totals"); expect(body).toContain("## Per-repo"); + 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. + // The empty Summary section shows its single italic 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: an empty window now renders the per-repo BUILDER's no-activity line (not the inline fallback). + expect(body).toContain("- No repo activity in the last 7 day(s)."); + expect(body).not.toContain("_No repositories in this window._"); + // #8372: Calibration is unconditional — the zero-denominator arm still carries a section. + expect(body).toContain("- Reversals: 0"); + expect(body).toContain("- Reversal rate: 0%"); + expect(body).toContain("- Nothing auto-acted in the last 7 day(s) (0 merged + 0 closed) — reversal rate is 0 (no denominator)."); + // #8372: Gate outcomes is unconditional — the below-MIN_SAMPLE arm renders the "n/a" rate line. + expect(body).toContain("- Blocked: 0"); + expect(body).toContain("- Maintainer overrides: 0"); + expect(body).toContain("- False positives (blocked then merged): 0"); + expect(body).toContain("- False-positive rate: n/a (fewer than 5 blocks in the last 7 day(s))"); // Null rate ⇒ the "n/a" arm. expect(body).toContain("- Gate false positives: 0/0 (n/a)"); expect(body).toContain("- Repos: 0"); @@ -98,8 +111,15 @@ 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)"); + // #8372: Per-repo row rendered via the BUILDER (non-empty section arm) — the capped/sorted format. + expect(body).toContain("- acme/widgets: reviewed 5, merged 3, closed 2"); + // #8372: Calibration healthy arm (auto-acted > 0, zero reversals). + expect(body).toContain("- Calibration healthy: 0 auto-action(s) reverted over 5 merged/closed in the last 14 day(s) (reversal-rate 0%)."); + // #8372: Gate outcomes with blocked below MIN_SAMPLE ⇒ the n/a rate arm, over the window's own day count. + expect(body).toContain("- Blocked: 4"); + expect(body).toContain("- Maintainer overrides: 1"); + expect(body).toContain("- False positives (blocked then merged): 1"); + expect(body).toContain("- False-positive rate: n/a (fewer than 5 blocks in the last 14 day(s))"); // 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. @@ -110,6 +130,47 @@ describe("formatMaintainerRecap (#2240)", () => { expect(body).not.toContain("payout"); }); + it("renders Per-repo through the capped/sorted builder (the (+N more) remainder the old inline lacked), plus the drift + populated gate-rate arms (#8372)", () => { + // 10 active repos ⇒ the builder shows its top 8 and notes "(+2 more)" — the cap the old inline never applied. + const repos = Array.from({ length: 10 }, (_, i) => ({ + repoFullName: `acme/r${i}`, + reviewed: 100 - i, // strictly descending volume ⇒ deterministic order, r0 first + merged: 1, + closed: 0, + gateFalsePositives: 0, + gateOverrides: 0, + reversals: 0, + })); + const report: RecapReport = { + generatedAt: GEN, + windowDays: 7, + repos, + totals: { + reviewed: 955, + merged: 10, + closed: 5, + blocked: 8, // ≥ MIN_SAMPLE (5) ⇒ the gate rate is reported, not n/a + gateFalsePositives: 2, + gateOverrides: 3, + reversals: 2, // > 0 ⇒ the calibration DRIFT arm + gateFalsePositiveRate: 0.25, + }, + summary: [], + }; + const body = formatMaintainerRecap(report); + // Per-repo builder cap: top 8 shown, the 9th/10th collapsed into the remainder line. + expect(body).toContain("- acme/r0: reviewed 100, merged 1, closed 0"); + expect(body).toContain("- acme/r7: reviewed 93, merged 1, closed 0"); + expect(body).not.toContain("acme/r8:"); + expect(body).not.toContain("acme/r9:"); + expect(body).toContain("- (+2 more)"); + // Calibration drift arm (reversals > 0): 2/15 = 0.133 ⇒ 13%. + expect(body).toContain("- calibration drift: 2 auto-action(s) were human-reverted (reversal-rate 13%) over 15 merged/closed in the last 7 day(s). Consider reviewing confidenceFloor / close-gates for false automations."); + // Gate outcomes populated-rate arm (blocked ≥ MIN_SAMPLE): 2/8 = 25%. + expect(body).toContain("- False-positive rate: 25% (2 of 8 blocks merged anyway)"); + expect(body).not.toMatch(/\n{3,}/); + }); + it("omits cohort diagnostics from the public recap even when totals.cohorts is present", () => { const report: RecapReport = { ...emptyReport(), diff --git a/test/unit/maintainer-recap.test.ts b/test/unit/maintainer-recap.test.ts index d4dbd79fc..c47860cdb 100644 --- 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,29 @@ 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 empty window now renders the per-repo BUILDER's no-activity line, and both new sections. + expect(result.formatted).toContain("No repo activity in the last 7 day(s)."); + expect(result.formatted).toContain("## Calibration"); + expect(result.formatted).toContain("## Gate outcomes"); expect(result.formatted).toContain("(n/a)"); }); + it("forwards a configDrift section into the rendered digest, and omits the section when none is passed (#8372)", async () => { + stubRecapChannelFetch(); + const configDrift = buildDriftRecapSection({ generatedAt: GEN, sentinelEnabled: false, drifting: [], cleanKnobs: 0 }); + // configDrift PRESENT arm: the drift section reaches the formatted digest. + const withDrift = await runMaintainerRecap(envWithBothWebhooks(), { configDrift }); + expect(withDrift.skipped).toBe(false); + if (withDrift.skipped) return; + expect(withDrift.formatted).toContain("## Config drift"); + expect(withDrift.formatted).toContain("drift sentinel disabled — no drift evaluation ran this window."); + // configDrift ABSENT arm: no drift section (options.configDrift undefined ⇒ the formatter's empty-append arm). + const withoutDrift = await runMaintainerRecap(envWithBothWebhooks(), {}); + expect(withoutDrift.skipped).toBe(false); + if (withoutDrift.skipped) return; + expect(withoutDrift.formatted).not.toContain("## Config drift"); + }); + 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")] });