diff --git a/src/services/knob-loosening-run.ts b/src/services/knob-loosening-run.ts index bcdd26e61e..ae27d66c85 100644 --- a/src/services/knob-loosening-run.ts +++ b/src/services/knob-loosening-run.ts @@ -391,7 +391,10 @@ export function isConfigDriftSentinelEnabled(env: Env): boolean { return value === "1" || value === "true" || value === "on" || value === "yes"; } -const DRIFT_FINGERPRINT_FLAG_PREFIX = "config_drift_fingerprint:"; +/** `system_flags` key prefix for a knob's drift-episode fingerprint row (exported so the maintainer-recap + * loader can read every fingerprint's `updated_at` in one bounded LIKE query rather than one query per + * knob). */ +export const DRIFT_FINGERPRINT_FLAG_PREFIX = "config_drift_fingerprint:"; export type ConfigDriftTickResult = { knobId: string; state: "alerted" | "standing" | "suppressed_looser" | "clean" }; diff --git a/src/services/maintainer-recap.ts b/src/services/maintainer-recap.ts index af7a3e5e4e..2d709a6451 100644 --- a/src/services/maintainer-recap.ts +++ b/src/services/maintainer-recap.ts @@ -14,7 +14,7 @@ import { PUBLIC_LOCAL_PATH_SCRUB_PATTERN, PUBLIC_UNSAFE_PATTERN } from "../signa import { deliverRecapToDiscord, deliverRecapToSlack } from "./notify-discord"; import type { GatePrecisionReport } from "./gate-precision"; import { MIN_GATE_PRECISION_SAMPLE } from "./gate-precision"; -import type { DriftRecapSection } from "./maintainer-recap-drift"; +import { buildDriftRecapSection, type DriftRecapKnob, 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"; @@ -22,6 +22,7 @@ import { buildGateOutcomesRecapSection } from "./maintainer-recap-gate-outcomes" import { buildPerRepoRecapSection } from "./maintainer-recap-per-repo"; import { buildTopContributorsRecapSection } from "./maintainer-recap-top-contributors"; import { buildRoutingRecapSection } from "./maintainer-recap-routing"; +import { DRIFT_FINGERPRINT_FLAG_PREFIX, isConfigDriftSentinelEnabled, loadLiveKnobStatuses } from "./knob-loosening-run"; import { REVIEWER_ROUTING_SHADOW_EVENT_TYPE, type RoutingShadowDecision } from "./reviewer-routing"; import type { OutcomeCalibration } from "./outcome-calibration"; import type { MaintainerRecapCohortCounts, MaintainerRecapContributor, MaintainerRecapRepo, RecentMergedPullRequestRecord, RecapReport } from "../types"; @@ -301,6 +302,48 @@ async function loadRoutingRecapSection(env: Env, windowDays: number, generatedAt } } +/** Source {@link DriftRecapSection} off the sentinel's live projection (#9698) and build it — the loader + * half of #8214's section, mirroring {@link loadRoutingRecapSection}: one `try`, `null` on any read error. + * `generatedAt` is always the caller's, never a fresh clock read in here. When the sentinel is off, skips + * the (expensive, per-knob) live drift evaluation entirely -- "no drift evaluation ran this window" is + * then literally true, not just the rendered copy. */ +async function loadDriftRecapSection(env: Env, generatedAt: string): Promise { + try { + const sentinelEnabled = isConfigDriftSentinelEnabled(env); + if (!sentinelEnabled) { + return buildDriftRecapSection({ generatedAt, sentinelEnabled, drifting: [], cleanKnobs: 0 }); + } + const statuses = await loadLiveKnobStatuses(env); + // Single bounded query over every fingerprint row (mirrors the per-repo-override LIKE read at + // knob-loosening-run.ts:534-537) -- not one query per knob. + const rows = await env.DB.prepare("SELECT key, updated_at FROM system_flags WHERE key LIKE ?") + .bind(`${DRIFT_FINGERPRINT_FLAG_PREFIX}%`) + .all<{ key: string; updated_at: string }>(); + const episodeSinceByKnobId = new Map(); + /* v8 ignore next -- defined-results guard, the loadKnobStatus convention */ + for (const row of rows.results ?? []) { + episodeSinceByKnobId.set(row.key.slice(DRIFT_FINGERPRINT_FLAG_PREFIX.length), row.updated_at); + } + let cleanKnobs = 0; + const drifting: DriftRecapKnob[] = []; + for (const status of statuses) { + if (!status.drift) { + cleanKnobs += 1; + continue; + } + if (status.drift.direction === "looser") continue; + const episodeSince = episodeSinceByKnobId.get(status.knobId); + // A live drift report with no persisted fingerprint row is excluded -- the section's "standing since" + // anchor doesn't exist yet for it. + if (episodeSince === undefined) continue; + drifting.push({ report: status.drift, episodeSince }); + } + return buildDriftRecapSection({ generatedAt, sentinelEnabled, drifting, cleanKnobs }); + } catch { + return null; + } +} + export async function runMaintainerRecap( env: Env, options: { @@ -313,10 +356,8 @@ export async function runMaintainerRecap( contributors?: MaintainerRecapContributor[] | undefined; /** 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. */ + /** #9698: an explicitly-passed projection (e.g. test injection) still wins over the one this function + * now loads itself via {@link loadDriftRecapSection} -- forwarded to {@link formatMaintainerRecap}. */ configDrift?: DriftRecapSection; } = {}, ): Promise { @@ -333,12 +374,15 @@ 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()); + // #9698: an explicitly-passed configDrift wins over the loaded one (test injection, same convention as + // `report` above); loadDriftRecapSection short-circuits on `??` so a caller-supplied one skips the read. + const configDrift = options.configDrift ?? (await loadDriftRecapSection(env, report.generatedAt)); // 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; + if (configDrift) recapOptions.configDrift = configDrift; const formatted = formatMaintainerRecap(report, recapOptions); const [discord, slack] = await Promise.all([ deliverRecapToDiscord(env, report, formatted), diff --git a/test/unit/maintainer-recap.test.ts b/test/unit/maintainer-recap.test.ts index bba1933dbd..a4d5cff9b4 100644 --- a/test/unit/maintainer-recap.test.ts +++ b/test/unit/maintainer-recap.test.ts @@ -2,9 +2,20 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { buildMaintainerRecap, contributorTotalsToList, foldMergedPrContributorTotals, 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 { KnobDriftReport } from "../../src/services/loosening-knobs"; +import { loadLiveKnobStatuses, type KnobStatus } from "../../src/services/knob-loosening-run"; import type { RecapReport } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; +// #9698: loadDriftRecapSection (maintainer-recap.ts) reads the sentinel's LIVE per-knob evaluation via +// loadLiveKnobStatuses -- mocked here (real corpus math is knob-loosening-run.test.ts's own suite) so these +// tests pin the WIRING: which statuses make it into the digest, not the drift evaluator's math. +vi.mock("../../src/services/knob-loosening-run", async (importOriginal) => ({ + ...(await importOriginal()), + loadLiveKnobStatuses: vi.fn(), +})); +const mockedLoadLiveKnobStatuses = vi.mocked(loadLiveKnobStatuses); + const GEN = "2026-07-08T00:00:00.000Z"; const DISCORD_HOOK = "https://discord.com/api/webhooks/123/abc"; const SLACK_HOOK = "https://hooks.slack.com/services/T00/B00/xxxyyyzzz"; @@ -323,6 +334,7 @@ function leakyRecapReport(): RecapReport { afterEach(() => { vi.unstubAllGlobals(); + mockedLoadLiveKnobStatuses.mockReset(); }); describe("runMaintainerRecap (#2252 end-to-end orchestration)", () => { @@ -465,3 +477,129 @@ describe("runMaintainerRecap (#2252 end-to-end orchestration)", () => { } }); }); + +// #9698: buildDriftRecapSection (#8214) had no production caller -- runMaintainerRecap now sources it itself +// via a private loadDriftRecapSection, mirroring loadRoutingRecapSection's fail-safe wiring exactly. +describe("runMaintainerRecap config-drift wiring (#9698)", () => { + function driftReport(overrides: Partial = {}): KnobDriftReport { + const comparison = { verdict: "improved" } as unknown as KnobDriftReport["visible"]; + return { + knobId: "ai_review_close_confidence", + ruleId: "ai_consensus_defect", + liveValue: 0.93, + dominatingValue: 0.95, + direction: "tighter", + visibleCases: 60, + heldOutCases: 15, + visible: comparison, + heldOut: comparison, + ...overrides, + }; + } + + function knobStatus(knobId: string, drift: KnobDriftReport | null): KnobStatus { + return { + knobId, + applyMode: "live", + flagEnabled: false, + tightenFlagEnabled: null, + shippedValue: 0.93, + liveValue: 0.93, + storedOverride: null, + repoOverrides: [], + drift, + reliability: null, + applied: [], + }; + } + + async function insertFingerprintRow(env: Env, knobId: string, updatedAt: string): Promise { + await env.DB.prepare("INSERT INTO system_flags (key, value, updated_at) VALUES (?, ?, ?)") + .bind(`config_drift_fingerprint:${knobId}`, `${knobId}:tighter:0.95`, updatedAt) + .run(); + } + + it("with the sentinel off, the digest carries the explicit disabled line without evaluating any knob", async () => { + stubRecapChannelFetch(); + const result = await runMaintainerRecap(envWithBothWebhooks(), { generatedAt: GEN }); + expect(result.skipped).toBe(false); + if (result.skipped) return; + expect(result.formatted).toContain("## Config drift"); + expect(result.formatted).toContain("drift sentinel disabled — no drift evaluation ran this window."); + // The (expensive, per-knob) live evaluation never runs when the sentinel is off. + expect(mockedLoadLiveKnobStatuses).not.toHaveBeenCalled(); + }); + + it("sentinel on: a knob drifting tighter with a persisted fingerprint renders its standing line; looser and unfingerprinted knobs are excluded, null-drift knobs count as clean", async () => { + stubRecapChannelFetch(); + mockedLoadLiveKnobStatuses.mockResolvedValue([ + knobStatus("clean_knob", null), + knobStatus("looser_knob", driftReport({ knobId: "looser_knob", direction: "looser" })), + knobStatus("ai_review_close_confidence", driftReport()), + knobStatus("unfingerprinted_knob", driftReport({ knobId: "unfingerprinted_knob" })), + ]); + const env = { ...envWithBothWebhooks(), CONFIG_DRIFT_SENTINEL_ENABLED: "true" as never } as Env; + // Standing 7 days: GEN is 2026-07-08T00:00:00.000Z, fingerprinted 2026-07-01T00:00:00.000Z. + await insertFingerprintRow(env, "ai_review_close_confidence", "2026-07-01T00:00:00.000Z"); + + const result = await runMaintainerRecap(env, { generatedAt: GEN }); + expect(result.skipped).toBe(false); + if (result.skipped) return; + expect(result.formatted).toContain("## Config drift"); + expect(result.formatted).toContain("ai_review_close_confidence (ai_consensus_defect): live 0.93 vs dominating 0.95 (tighter)"); + expect(result.formatted).toContain("standing 7 day(s)"); + // Only clean_knob counts as clean -- looser_knob and unfingerprinted_knob are excluded from BOTH arms. + expect(result.formatted).toContain("1 other evaluated knob(s) are clean."); + expect(result.formatted).not.toContain("looser_knob"); + expect(result.formatted).not.toContain("unfingerprinted_knob"); + }); + + it("sentinel on with nothing drifting renders the clean summary line over every evaluated knob", async () => { + stubRecapChannelFetch(); + mockedLoadLiveKnobStatuses.mockResolvedValue([knobStatus("a", null), knobStatus("b", null)]); + const env = { ...envWithBothWebhooks(), CONFIG_DRIFT_SENTINEL_ENABLED: "true" as never } as Env; + const result = await runMaintainerRecap(env, { generatedAt: GEN }); + expect(result.skipped).toBe(false); + if (result.skipped) return; + expect(result.formatted).toContain("Config drift clean: all 2 evaluated knob(s) remain their best-supported live values."); + }); + + it("a thrown system_flags read leaves the digest intact minus the config-drift section (fail-safe null arm)", async () => { + stubRecapChannelFetch(); + mockedLoadLiveKnobStatuses.mockResolvedValue([knobStatus("ai_review_close_confidence", driftReport())]); + const base = { ...envWithBothWebhooks(), CONFIG_DRIFT_SENTINEL_ENABLED: "true" as never } as Env; + 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 key, updated_at FROM system_flags")) throw new Error("fingerprint_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("## Config drift"); + // The failure costs one section, not the recap. + expect(result.formatted).toContain("## Totals"); + }); + + it("an explicitly-passed options.configDrift overrides the loaded projection and skips the live read", async () => { + stubRecapChannelFetch(); + const explicit = buildDriftRecapSection({ generatedAt: GEN, sentinelEnabled: false, drifting: [], cleanKnobs: 0 }); + const env = { ...envWithBothWebhooks(), CONFIG_DRIFT_SENTINEL_ENABLED: "true" as never } as Env; + const result = await runMaintainerRecap(env, { generatedAt: GEN, configDrift: explicit }); + expect(result.skipped).toBe(false); + if (result.skipped) return; + expect(result.formatted).toContain("drift sentinel disabled — no drift evaluation ran this window."); + // The caller-supplied projection wins WITHOUT the loader ever reading the live knob statuses. + expect(mockedLoadLiveKnobStatuses).not.toHaveBeenCalled(); + }); +});