Skip to content
Closed
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
33 changes: 27 additions & 6 deletions src/services/maintainer-recap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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",
"",
Expand All @@ -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
Expand Down Expand Up @@ -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;
} = {},
Expand All @@ -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),
Expand Down
71 changes: 66 additions & 5 deletions test/unit/maintainer-recap-format.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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.
Expand All @@ -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(),
Expand Down
22 changes: 21 additions & 1 deletion test/unit/maintainer-recap.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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")] });
Expand Down