diff --git a/apps/loopover-ui/content/docs/verify-this-review.mdx b/apps/loopover-ui/content/docs/verify-this-review.mdx index d9a2296831..6612d9e6b7 100644 --- a/apps/loopover-ui/content/docs/verify-this-review.mdx +++ b/apps/loopover-ui/content/docs/verify-this-review.mdx @@ -157,6 +157,30 @@ A rate over an empty window is `null`, never `0` — "nothing was held" and "not different claims, and the fairness report renders the second as *measured zero* with its window rather than as a reassuring number. +### Automation rate + +`reviewParity`'s sibling, from the same ledger (#9727): + +```bash +curl -s "https://api.loopover.ai/v1/public/stats" | jq '.automationRate' +``` + +A pull request counts as **automated** only if every verdict for it was a merge or close with no human in +the path. Three things mark a human: + +- `action = 'hold'` — the gate declined to decide and handed it to a person, +- a `reevaluation_actor` — a named person caused a re-evaluation, +- `reevaluation_reason = 'maintainer_request'` — a human asked for the re-run. + +So a PR that was **held and later merged is manual**, not automated. Counting the final disposition instead +would let the rate be inflated by holding everything and then merging it by hand — which is exactly the +failure mode this number exists to make visible. + +Each week carries a `basis`. `holds_only` weeks start before `provenanceHorizon`, the date the bot began +recording who caused a re-evaluation; those weeks can only detect manual work that took the form of a hold, +so they **under-count** it. A week straddling that date is labelled `holds_only` too — understating +confidence rather than overstating it. + The same per-rule numbers are also published as digest-committed [EvalScoreRecords](/docs/what-you-can-verify), each independently re-derivable without trusting the transport — recompute `recordDigest` over the record's own remaining fields and compare: diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 28e8d2aed6..61c6a7ca09 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -999,6 +999,70 @@ "byAuthorClass", "byProject" ] + }, + "automationRate": { + "type": "object", + "properties": { + "weeks": { + "type": "array", + "items": { + "type": "object", + "properties": { + "weekStart": { + "type": "string" + }, + "decided": { + "type": "number" + }, + "automated": { + "type": "number" + }, + "manual": { + "type": "number" + }, + "automationRatePct": { + "type": "number", + "nullable": true + }, + "basis": { + "type": "string", + "enum": [ + "full", + "holds_only" + ] + } + }, + "required": [ + "weekStart", + "decided", + "automated", + "manual", + "automationRatePct", + "basis" + ] + } + }, + "decided": { + "type": "number" + }, + "automated": { + "type": "number" + }, + "automationRatePct": { + "type": "number", + "nullable": true + }, + "provenanceHorizon": { + "type": "string" + } + }, + "required": [ + "weeks", + "decided", + "automated", + "automationRatePct", + "provenanceHorizon" + ] } }, "required": [ @@ -1008,6 +1072,7 @@ "weekly", "rulePrecision", "reviewParity", + "automationRate", "byProject", "fleetAccuracy", "accuracyTrend", diff --git a/apps/loopover-ui/src/components/site/fairness-report-page.test.tsx b/apps/loopover-ui/src/components/site/fairness-report-page.test.tsx index 0b7e744f56..bbb6727e08 100644 --- a/apps/loopover-ui/src/components/site/fairness-report-page.test.tsx +++ b/apps/loopover-ui/src/components/site/fairness-report-page.test.tsx @@ -40,6 +40,13 @@ function renderWithClient(ui: ReactNode) { const FIXTURE: PublicStats = { // The wire always carries rulePrecision (#8230/#8231). A fixture without it is not a payload the // current backend can produce -- the one test that needs that shape strips it explicitly. + automationRate: { + weeks: [], + decided: 0, + automated: 0, + automationRatePct: null, + provenanceHorizon: "2026-07-29T00:00:00.000Z", + }, reviewParity: { windowStart: "2026-07-22T00:00:00.000Z", windowEnd: "2026-07-29T00:00:00.000Z", @@ -509,4 +516,86 @@ describe("FairnessReportPage (#fairness-analytics)", () => { expect(await screen.findByText(/No re-evaluations recorded/i)).toBeTruthy(); expect(screen.queryByText(/No verdicts recorded/i)).toBeNull(); }); + + // #9728: the automation-rate surface and its zero/reduced-basis states. + function automationFixture(over: Record) { + return { + ok: true, + durationMs: 10, + data: { + ...FIXTURE, + automationRate: { + weeks: [], + decided: 0, + automated: 0, + automationRatePct: null, + provenanceHorizon: "2026-07-29T00:00:00.000Z", + ...over, + }, + }, + }; + } + + it("renders the headline rate, the weekly table, and the definition", async () => { + apiFetch.mockResolvedValue( + automationFixture({ + decided: 10, + automated: 7, + automationRatePct: 70, + weeks: [ + { + weekStart: "2026-07-27T00:00:00.000Z", + decided: 10, + automated: 7, + manual: 3, + automationRatePct: 70, + basis: "full", + }, + ], + }), + ); + renderWithClient(); + + expect(await screen.findByText(/Automation rate/i)).toBeTruthy(); + expect(screen.getByText(/70% automated/)).toBeTruthy(); + expect(screen.getByText(/7 of 10 pull requests/)).toBeTruthy(); + // The definition must be readable without opening source -- that is #9728's acceptance. + expect(screen.getByText(/no human action/i)).toBeTruthy(); + expect(screen.getByText(/counts as manual even if it later merged/i)).toBeTruthy(); + }); + + it("labels reduced-basis weeks and explains that they UNDER-count manual work", async () => { + apiFetch.mockResolvedValue( + automationFixture({ + decided: 4, + automated: 4, + automationRatePct: 100, + weeks: [ + { + weekStart: "2026-07-06T00:00:00.000Z", + decided: 4, + automated: 4, + manual: 0, + automationRatePct: 100, + basis: "holds_only", + }, + ], + }), + ); + renderWithClient(); + + expect( + (await screen.findAllByText(/reduced basis/i)).length, + "the row badge and the footnote both say it", + ).toBeGreaterThanOrEqual(2); + expect(screen.getByText(/under-count it rather than over-count it/i)).toBeTruthy(); + }); + + it("renders an empty window as a measured zero, not as missing data", async () => { + apiFetch.mockResolvedValue(automationFixture({})); + renderWithClient(); + + expect(await screen.findByText(/No pull requests decided/i)).toBeTruthy(); + expect(screen.getByText(/measured zero, not missing data/i)).toBeTruthy(); + }); }); diff --git a/apps/loopover-ui/src/components/site/fairness-report-page.tsx b/apps/loopover-ui/src/components/site/fairness-report-page.tsx index d025c6cd74..91c28f643d 100644 --- a/apps/loopover-ui/src/components/site/fairness-report-page.tsx +++ b/apps/loopover-ui/src/components/site/fairness-report-page.tsx @@ -424,6 +424,103 @@ export function FairnessReportPage() { ) : null} + {/* #9728: the automation rate — the share of PRs decided with no human in the path. Rendered + unconditionally when present: a window that decided nothing is a MEASURED zero and says so, + which is a different claim from "we did not compute this". */} + {data.automationRate ? ( +
+

Automation rate

+

+ The share of pull requests decided and enacted with{" "} + no human action between open + and disposition. A PR that was held for a person — or that a maintainer asked to + be re-run — counts as manual even if it later merged: the question is whether + someone had to act, not how it ended. Computed from the ledger alone, so you can + recompute it:{" "} + + verify this review + + . +

+ + {data.automationRate.decided === 0 ? ( +

+ No pull requests decided in + this window — a measured zero, not missing data. +

+ ) : ( + <> +

+ {data.automationRate.automationRatePct != null + ? `${pctFmt.format(data.automationRate.automationRatePct)}% automated` + : "Rate unavailable"}{" "} + + ({intFmt.format(data.automationRate.automated)} of{" "} + {intFmt.format(data.automationRate.decided)} pull requests) + +

+ + + + + + + + + + + + + {data.automationRate.weeks.map((week) => ( + + + + + + + ))} + +
+ Automated and manual pull requests per week, with each week's + measurement basis. +
+ Week + + Decided + + Automated + + Rate +
+ {new Date(week.weekStart).toLocaleDateString()} + {week.basis === "holds_only" ? ( + + reduced basis + + ) : null} + {intFmt.format(week.decided)}{intFmt.format(week.automated)} + {week.automationRatePct != null + ? `${pctFmt.format(week.automationRatePct)}%` + : "—"} +
+ +

+ Weeks marked{" "} + reduced basis start + before {new Date(data.automationRate.provenanceHorizon).toLocaleDateString()}, + when the bot began recording who caused a re-evaluation. Those weeks can only + detect manual work that took the form of a hold, so they under-count it rather + than over-count it. +

+ + )} +
+ ) : null} + {/* #9744: re-evaluation rate + author-class parity. Rendered UNCONDITIONALLY when the block is present -- a window with no verdicts is a MEASURED zero and says so with its own bounds, which is a different claim from "we did not compute this" and must not look identical. */} diff --git a/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx b/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx index 2c763ca733..898ae5e6c3 100644 --- a/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx +++ b/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx @@ -40,6 +40,13 @@ import { const PAYLOAD: PublicStats = { // The wire always carries rulePrecision (#8230/#8231). A fixture without it is not a payload the // current backend can produce -- the one test that needs that shape strips it explicitly. + automationRate: { + weeks: [], + decided: 0, + automated: 0, + automationRatePct: null, + provenanceHorizon: "2026-07-29T00:00:00.000Z", + }, reviewParity: { windowStart: "2026-07-22T00:00:00.000Z", windowEnd: "2026-07-29T00:00:00.000Z", diff --git a/packages/loopover-contract/src/public-api.ts b/packages/loopover-contract/src/public-api.ts index 8be0164a08..932a55db75 100644 --- a/packages/loopover-contract/src/public-api.ts +++ b/packages/loopover-contract/src/public-api.ts @@ -74,6 +74,34 @@ export const ReviewParitySchema = z.object({ byProject: z.array(z.object({ project: z.string(), byAuthorClass: z.array(ParityRollupSchema) })), }); +/** + * Weekly automation-rate series (#9727): the share of pull requests decided with no human in the path. + * Computed from `decision_records` alone; definitions live beside the implementation in + * src/review/automation-rate.ts and are restated in the verifier walkthrough. + */ +export const AutomationRateWeekSchema = z.object({ + /** ISO date-time of the week's UTC Monday. */ + weekStart: z.string(), + /** Distinct pull requests with at least one verdict that week, bucketed by their FIRST verdict. */ + decided: z.number(), + automated: z.number(), + manual: z.number(), + /** Null when the week decided nothing -- an undefined ratio, never a reassuring 100%. */ + automationRatePct: z.number().nullable(), + /** `holds_only` weeks predate the re-evaluation provenance fields and can only UNDER-count manual work. + * A week straddling the horizon is labelled `holds_only` too: understating confidence, not overstating. */ + basis: z.enum(["full", "holds_only"]), +}); + +export const AutomationRateSchema = z.object({ + weeks: z.array(AutomationRateWeekSchema), + decided: z.number(), + automated: z.number(), + automationRatePct: z.number().nullable(), + /** The date the provenance fields began being written, so a reader can see which weeks are reduced-basis. */ + provenanceHorizon: z.string(), +}); + export const PublicStatsSchema = z.object({ generatedAt: z.string(), updatedAt: z.string(), @@ -94,6 +122,7 @@ export const PublicStatsSchema = z.object({ weekly: z.object({ reviewed: z.number(), merged: z.number() }), rulePrecision: PublicRulePrecisionSchema, reviewParity: ReviewParitySchema, + automationRate: AutomationRateSchema, byProject: z.array( z.object({ project: z.string(), @@ -186,4 +215,5 @@ export const PublicStatsSchema = z.object({ export type PublicStats = z.infer; export type ReviewParity = z.infer; +export type AutomationRate = z.infer; export type ParityRollup = z.infer; diff --git a/src/api/routes.ts b/src/api/routes.ts index 13b4c71562..91950ffe05 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -14,6 +14,7 @@ import { buildFindingTaxonomyDocument } from "../review/finding-taxonomy"; import { buildEnrichmentAnalyzersTaxonomyDocument } from "../review/enrichment-analyzers-taxonomy"; import { deliveryIdFor } from "../queue/delivery-id"; import { loadReviewParityRollups } from "../review/review-parity-rollups"; +import { loadAutomationRateSeries } from "../review/automation-rate"; import { GITHUB_OAUTH_STATE_COOKIE, authenticateInternalToken, @@ -672,7 +673,7 @@ export function createApp() { const publicStatsManifestOverride = await resolvePublicStatsManifestOverride(c.env); if (!isPublicStatsEnabled(c.env, publicStatsManifestOverride)) return c.json({ error: "not_found" }, 404); try { - const [stats, accuracyTrend, fleetAccuracyTrend, reuseRateTrend, reviewVolumeTrend, rulePrecision, reviewParity] = await Promise.all([ + const [stats, accuracyTrend, fleetAccuracyTrend, reuseRateTrend, reviewVolumeTrend, rulePrecision, reviewParity, automationRate] = await Promise.all([ getPublicStats(c.env), loadPublicAccuracyTrend(c.env), // #9676: the fleet-population sibling of the series above. Deliberately a SECOND series rather than a @@ -688,9 +689,12 @@ export function createApp() { // from `decision_records` alone so an outsider holding the ledger export can recompute every figure // -- the definitions live beside the code in review-parity-rollups.ts. loadReviewParityRollups(c.env), + // #9727: the weekly automation rate -- share of PRs decided with no human in the path. Same ledger, + // same reproducibility contract; definitions live beside the code in review/automation-rate.ts. + loadAutomationRateSeries(c.env), ]); c.header("Cache-Control", "public, max-age=60, stale-while-revalidate=300"); - return c.json({ ...stats, accuracyTrend, fleetAccuracyTrend, reuseRateTrend, reviewVolumeTrend, rulePrecision, reviewParity }); + return c.json({ ...stats, accuracyTrend, fleetAccuracyTrend, reuseRateTrend, reviewVolumeTrend, rulePrecision, reviewParity, automationRate }); } catch { return c.json({ error: "public_stats_unavailable" }, 503); } diff --git a/src/review/automation-rate.ts b/src/review/automation-rate.ts new file mode 100644 index 0000000000..fc6f8a023e --- /dev/null +++ b/src/review/automation-rate.ts @@ -0,0 +1,220 @@ +// Weekly automation-rate series (#9727). +// +// "How much of this is actually automated?" is the question every claim about the gate rests on, and it was +// only answerable by ad-hoc query. This publishes it as a standing series, computed from `decision_records` +// -- the anchored ledger -- and nothing else, so an outsider holding the export recomputes the same numbers. +// +// PUBLISHED DEFINITION. A pull request is counted once per week, by the week of its FIRST verdict. +// +// decided The PR reached a disposition at all: at least one verdict whose action ENACTS one +// (`merge` / `close`) or one that HOLDS. `action` also carries non-deciding classes -- +// `label`, `update_branch`, `approve`, and the error/no-op classes -- and a PR that only ever +// drew those was never decided, so it is excluded from BOTH halves rather than counted. +// Counting it would have made a PR the gate never decided read as an automated decision. +// manual A decided PR where ANY verdict shows a human in the decision path: +// - `action = 'hold'` -- the gate declined to decide and handed it to a person +// - `reevaluation_actor` -- a named person caused a re-evaluation (#9742) +// - `reevaluation_reason = 'maintainer_request'` -- a human asked for the re-run +// automated A PR that reached `merge` or `close` with none of the above: opened, decided, enacted, with +// no human action in between. Non-deciding verdicts alongside (a `label`, an `update_branch`) +// do NOT make it manual -- those are the bot acting, not a person. +// +// A PR that was held and LATER merged is manual, not automated. The question is whether a human had to act, +// not what the end state was -- counting the final disposition would let the rate be inflated by holding +// everything and then merging it by hand. +// +// BACKFILL HORIZON. The two re-evaluation fields arrived with migration 0204. Before that, only `hold` is +// observable, so a week earlier than the horizon can only UNDER-count manual work. Those weeks are published +// with `basis: "holds_only"` rather than silently mixed in with complete ones -- a series whose definition +// quietly changes partway along is worse than one that says where it changes. + +import { safeAll } from "./public-stats"; + +/** + * When the re-evaluation provenance fields (`reevaluation_reason`, `reevaluation_actor`) began being + * written -- migration 0204's ship date. Weeks starting before this can only detect manual work via `hold`. + * + * A single dated constant rather than something inferred: "the column did not exist yet" and "this was a + * first evaluation" both read as NULL, so the data genuinely cannot tell them apart. Stating the date is the + * honest option; guessing it from the rows would be a fabrication dressed as a derivation. + */ +export const AUTOMATION_RATE_PROVENANCE_HORIZON_ISO = "2026-07-29T00:00:00.000Z"; + +/** The action classes that ENACT a decision. A PR reaching one of these, with no human signal, is what + * "automated" means. Exported so the read below filters on exactly the set the fold classifies on -- one + * definition, rather than a WHERE clause and a predicate that can drift apart. */ +export const AUTOMATION_ENACTING_ACTIONS = ["merge", "close"] as const; + +/** The action recording that the gate declined to decide and handed the PR to a person. */ +export const AUTOMATION_HOLD_ACTION = "hold"; + +/** Every action that puts a PR in the series at all -- an enacted decision, or a hold. */ +export const AUTOMATION_COUNTED_ACTIONS: readonly string[] = [...AUTOMATION_ENACTING_ACTIONS, AUTOMATION_HOLD_ACTION]; + +/** How completely a week could be measured. `full` weeks see every manual signal; `holds_only` weeks predate + * the provenance fields and can only under-count manual work. */ +export type AutomationWeekBasis = "full" | "holds_only"; + +export type AutomationRateWeek = { + /** ISO date of the week's Monday, UTC. */ + weekStart: string; + /** Distinct pull requests with at least one verdict that week. */ + decided: number; + automated: number; + manual: number; + /** Null when the week decided nothing -- an undefined ratio, not a reassuring 100%. */ + automationRatePct: number | null; + basis: AutomationWeekBasis; +}; + +export type AutomationRateSeries = { + weeks: AutomationRateWeek[]; + /** Totals over every week in the series, on the same definitions. */ + decided: number; + automated: number; + automationRatePct: number | null; + /** Published so a reader can see which weeks are `holds_only` without inspecting each. */ + provenanceHorizon: string; +}; + +/** One verdict, reduced to the fields the series reads. */ +export type AutomationVerdictRow = { + repoFullName: string; + pullNumber: number; + action: string; + createdAt: string; + reevaluationReason: string | null; + reevaluationActor: string | null; +}; + +/** The UTC Monday of the week containing `iso`, as an ISO date-time. Null when unparseable. */ +export function weekStartIso(iso: string): string | null { + const parsed = Date.parse(iso); + if (!Number.isFinite(parsed)) return null; + const date = new Date(parsed); + // getUTCDay: 0=Sunday. Shift so Monday starts the week, matching the other published weekly series. + const dayOffset = (date.getUTCDay() + 6) % 7; + const monday = Date.UTC(date.getUTCFullYear(), date.getUTCMonth(), date.getUTCDate() - dayOffset); + return new Date(monday).toISOString(); +} + +/** Percent to one decimal, or null when nothing was decided. */ +function ratePct(numerator: number, denominator: number): number | null { + if (denominator <= 0) return null; + return Math.round((numerator / denominator) * 1000) / 10; +} + +/** True when this verdict shows a human in the decision path. See the published definition above. */ +export function verdictShowsHumanAction(row: Pick): boolean { + if (row.action === "hold") return true; + if (typeof row.reevaluationActor === "string" && row.reevaluationActor.trim() !== "") return true; + return row.reevaluationReason === "maintainer_request"; +} + +/** + * Build the weekly series from a window's verdict rows. PURE, so every definition above is testable against + * a hand-written table -- which is also what lets an outsider check the arithmetic without our database. + */ +export function buildAutomationRateSeries(rows: readonly AutomationVerdictRow[]): AutomationRateSeries { + // Fold verdicts to PULL REQUESTS first: the unit of the question is "did a human have to act on this PR", + // and a PR with five verdicts is still one PR. Its week is the week of its FIRST verdict, so a PR does not + // migrate between weeks as it accrues re-evaluations. + const byPull = new Map(); + for (const row of rows) { + const key = `${row.repoFullName}#${row.pullNumber}`; + const human = verdictShowsHumanAction(row); + const enacted = (AUTOMATION_ENACTING_ACTIONS as readonly string[]).includes(row.action); + const held = row.action === AUTOMATION_HOLD_ACTION; + const existing = byPull.get(key); + if (!existing) { + byPull.set(key, { firstSeen: row.createdAt, human, enacted, held }); + continue; + } + existing.human = existing.human || human; + existing.enacted = existing.enacted || enacted; + existing.held = existing.held || held; + if (Date.parse(row.createdAt) < Date.parse(existing.firstSeen)) existing.firstSeen = row.createdAt; + } + + const horizon = Date.parse(AUTOMATION_RATE_PROVENANCE_HORIZON_ISO); + const weeks = new Map(); + for (const entry of byPull.values()) { + // Never decided -- only non-deciding verdicts (a label, an update_branch, an error). Excluded from both + // halves: this PR is not evidence for OR against automation, and folding it into `automated` because it + // happens to carry no human signal is exactly the over-count this guard exists to prevent. + if (!entry.enacted && !entry.held) continue; + const week = weekStartIso(entry.firstSeen); + if (week === null) continue; + const bucket = weeks.get(week) ?? { decided: 0, automated: 0, manual: 0 }; + bucket.decided += 1; + // Enacted AND clean of every human signal. A held PR later merged by hand fails the second test, and a + // PR that only ever held fails the first -- both are manual, which is the whole point of the metric. + if (entry.enacted && !entry.human) bucket.automated += 1; + else bucket.manual += 1; + weeks.set(week, bucket); + } + + const ordered = [...weeks.entries()] + .sort(([a], [b]) => a.localeCompare(b)) + .map(([weekStart, bucket]) => ({ + weekStart, + decided: bucket.decided, + automated: bucket.automated, + manual: bucket.manual, + automationRatePct: ratePct(bucket.automated, bucket.decided), + basis: (Date.parse(weekStart) >= horizon ? "full" : "holds_only") as AutomationWeekBasis, + })); + + const decided = ordered.reduce((sum, week) => sum + week.decided, 0); + const automated = ordered.reduce((sum, week) => sum + week.automated, 0); + return { + weeks: ordered, + decided, + automated, + automationRatePct: ratePct(automated, decided), + provenanceHorizon: AUTOMATION_RATE_PROVENANCE_HORIZON_ISO, + }; +} + +/** Trailing weeks the series covers, per #9727's "at least the trailing 12 weeks where data permits". */ +const AUTOMATION_RATE_WEEKS = 12; + +/** + * Read the trailing window's verdicts and build the series (#9727). + * + * `fetchRows` is overridable so the whole path is testable without a database; it defaults to the real read, + * so the production caller passes only `env` and cannot forget to wire it. + */ +export async function loadAutomationRateSeries( + env: unknown, + options: { + weeks?: number | undefined; + nowMs?: number | undefined; + fetchRows?: ((sinceIso: string) => Promise) | undefined; + } = {}, +): Promise { + const nowMs = options.nowMs ?? Date.now(); + const requested = options.weeks; + const weeks = typeof requested === "number" && Number.isFinite(requested) && requested > 0 ? Math.trunc(requested) : AUTOMATION_RATE_WEEKS; + const sinceIso = new Date(nowMs - weeks * 7 * 86_400_000).toISOString(); + const fetchRows = options.fetchRows ?? ((since: string) => queryAutomationRows(env, since)); + const rows = await fetchRows(sinceIso).catch(() => [] as AutomationVerdictRow[]); + return buildAutomationRateSeries(rows); +} + +async function queryAutomationRows(env: unknown, sinceIso: string): Promise { + return safeAll( + env as never, + `SELECT repo_full_name AS repoFullName, + pull_number AS pullNumber, + action AS action, + created_at AS createdAt, + reevaluation_reason AS reevaluationReason, + reevaluation_actor AS reevaluationActor + FROM decision_records + WHERE created_at >= ? + AND action IN (${AUTOMATION_COUNTED_ACTIONS.map(() => "?").join(", ")})`, + sinceIso, + ...AUTOMATION_COUNTED_ACTIONS, + ); +} diff --git a/test/unit/automation-rate.test.ts b/test/unit/automation-rate.test.ts new file mode 100644 index 0000000000..a8cbcdadba --- /dev/null +++ b/test/unit/automation-rate.test.ts @@ -0,0 +1,232 @@ +// The published automation-rate definition (#9727). Every figure here is a claim an outsider is invited to +// recompute from the ledger, so these pin the DEFINITION -- especially the places where a plausible shortcut +// would let the rate be inflated. +import { describe, expect, it } from "vitest"; +import { + AUTOMATION_COUNTED_ACTIONS, + AUTOMATION_RATE_PROVENANCE_HORIZON_ISO, + buildAutomationRateSeries, + loadAutomationRateSeries, + verdictShowsHumanAction, + weekStartIso, + type AutomationVerdictRow, +} from "../../src/review/automation-rate"; + +function row(over: Partial = {}): AutomationVerdictRow { + return { + repoFullName: "o/r", + pullNumber: 1, + action: "merge", + createdAt: "2026-07-29T12:00:00.000Z", + reevaluationReason: null, + reevaluationActor: null, + ...over, + }; +} + +describe("verdictShowsHumanAction", () => { + it("counts a HOLD as human action — the gate declined to decide", () => { + expect(verdictShowsHumanAction({ action: "hold", reevaluationReason: null, reevaluationActor: null })).toBe(true); + }); + + it("counts a named re-evaluation actor, and a maintainer_request", () => { + expect(verdictShowsHumanAction({ action: "merge", reevaluationReason: null, reevaluationActor: "JSONbored" })).toBe(true); + expect(verdictShowsHumanAction({ action: "merge", reevaluationReason: "maintainer_request", reevaluationActor: null })).toBe(true); + }); + + it("does NOT count machine-paced re-evaluation causes", () => { + // A scheduled sweep or a repair is the automation working, not a human intervening. + for (const reason of ["scheduled_recheck", "pipeline_error", "upstream_state_change", "config_change"]) { + expect(verdictShowsHumanAction({ action: "merge", reevaluationReason: reason, reevaluationActor: null }), reason).toBe(false); + } + }); + + it("treats a blank actor as no actor", () => { + expect(verdictShowsHumanAction({ action: "merge", reevaluationReason: null, reevaluationActor: " " })).toBe(false); + }); +}); + +describe("weekStartIso", () => { + it("returns the UTC MONDAY of the containing week", () => { + expect(weekStartIso("2026-07-29T12:00:00.000Z")).toBe("2026-07-27T00:00:00.000Z"); // Wed -> Mon + expect(weekStartIso("2026-07-27T00:00:00.000Z")).toBe("2026-07-27T00:00:00.000Z"); // Mon -> itself + }); + + it("puts SUNDAY in the week that started six days earlier, not the next one", () => { + // getUTCDay()===0 is the off-by-one every week-bucketing bug is made of. + expect(weekStartIso("2026-08-02T23:59:59.000Z")).toBe("2026-07-27T00:00:00.000Z"); + }); + + it("is null on an unparseable timestamp rather than bucketing it somewhere wrong", () => { + expect(weekStartIso("not-a-date")).toBeNull(); + }); +}); + +describe("buildAutomationRateSeries", () => { + it("counts PULL REQUESTS once, not verdicts", () => { + const series = buildAutomationRateSeries([row({ pullNumber: 1 }), row({ pullNumber: 1 }), row({ pullNumber: 2 })]); + expect(series.decided).toBe(2); + expect(series.automated).toBe(2); + expect(series.automationRatePct).toBe(100); + }); + + it("a PR that was HELD and later merged is MANUAL — the end state is not the question", () => { + // Counting the final disposition would let the rate be inflated by holding everything and merging by hand. + const series = buildAutomationRateSeries([ + row({ pullNumber: 1, action: "hold", createdAt: "2026-07-29T10:00:00.000Z" }), + row({ pullNumber: 1, action: "merge", createdAt: "2026-07-29T11:00:00.000Z" }), + ]); + expect(series.decided).toBe(1); + expect(series.weeks[0]?.manual).toBe(1); + expect(series.automated).toBe(0); + expect(series.automationRatePct).toBe(0); + }); + + it("EXCLUDES a PR that was never actually decided, rather than counting it automated", () => { + // The regression (#9938 review): `action` also carries non-deciding classes -- `label`, `update_branch`, + // `approve`, and the error/no-op classes. None of them is `hold`, so a PR that only ever drew those + // carried no human signal and was folded in as an AUTOMATED decision, inflating the published rate with + // pull requests the gate never decided at all. + const series = buildAutomationRateSeries([ + row({ pullNumber: 1, action: "label" }), + row({ pullNumber: 2, action: "update_branch" }), + row({ pullNumber: 3, action: "approve" }), + ]); + expect(series.decided).toBe(0); + expect(series.automated).toBe(0); + expect(series.automationRatePct).toBeNull(); + }); + + it("still counts a PR that drew a non-deciding verdict AND then a real one", () => { + // The bot labelling a PR before merging it is the bot acting, not a person -- it must not flip the PR + // to manual, which would under-count automation just as wrongly as the bug over-counted it. + const series = buildAutomationRateSeries([ + row({ pullNumber: 1, action: "label", createdAt: "2026-07-29T10:00:00.000Z" }), + row({ pullNumber: 1, action: "merge", createdAt: "2026-07-29T11:00:00.000Z" }), + ]); + expect(series.decided).toBe(1); + expect(series.automated).toBe(1); + }); + + it("counts a hold-only PR as decided-and-manual, never as undecided", () => { + const series = buildAutomationRateSeries([row({ pullNumber: 1, action: "hold" })]); + expect(series.decided).toBe(1); + expect(series.automated).toBe(0); + expect(series.weeks[0]?.manual).toBe(1); + }); + + it("buckets a PR by its FIRST verdict, so re-evaluations do not migrate it between weeks", () => { + const series = buildAutomationRateSeries([ + row({ pullNumber: 1, createdAt: "2026-08-03T09:00:00.000Z" }), // a later Monday + row({ pullNumber: 1, createdAt: "2026-07-29T09:00:00.000Z" }), // the earlier week + ]); + expect(series.weeks.map((w) => w.weekStart)).toEqual(["2026-07-27T00:00:00.000Z"]); + }); + + it("separates pull numbers by REPO", () => { + const series = buildAutomationRateSeries([row({ repoFullName: "a/b", pullNumber: 1 }), row({ repoFullName: "c/d", pullNumber: 1 })]); + expect(series.decided).toBe(2); + }); + + it("orders weeks chronologically and reports a null rate for a week that decided nothing", () => { + const series = buildAutomationRateSeries([ + row({ pullNumber: 2, createdAt: "2026-08-03T09:00:00.000Z" }), + row({ pullNumber: 1, createdAt: "2026-07-29T09:00:00.000Z" }), + ]); + expect(series.weeks.map((w) => w.weekStart)).toEqual(["2026-07-27T00:00:00.000Z", "2026-08-03T00:00:00.000Z"]); + expect(buildAutomationRateSeries([]).automationRatePct).toBeNull(); + }); + + it("marks weeks before the provenance horizon as holds_only, not silently mixed in", () => { + // Before migration 0204 only `hold` was observable, so those weeks can only UNDER-count manual work. + const before = new Date(Date.parse(AUTOMATION_RATE_PROVENANCE_HORIZON_ISO) - 21 * 86_400_000).toISOString(); + const after = new Date(Date.parse(AUTOMATION_RATE_PROVENANCE_HORIZON_ISO) + 14 * 86_400_000).toISOString(); + const series = buildAutomationRateSeries([row({ createdAt: before }), row({ pullNumber: 2, createdAt: after })]); + expect(series.weeks[0]?.basis).toBe("holds_only"); + expect(series.weeks.at(-1)?.basis).toBe("full"); + expect(series.provenanceHorizon).toBe(AUTOMATION_RATE_PROVENANCE_HORIZON_ISO); + }); + + it("labels a week STRADDLING the horizon holds_only — understating confidence, not overstating it", () => { + // The horizon falls mid-week, so part of that week predates the provenance fields. Calling it `full` + // would claim completeness the data does not have; for a fairness figure the error must run the safe way. + const straddling = new Date(Date.parse(AUTOMATION_RATE_PROVENANCE_HORIZON_ISO) + 6 * 3_600_000).toISOString(); + const series = buildAutomationRateSeries([row({ createdAt: straddling })]); + expect(Date.parse(series.weeks[0]!.weekStart)).toBeLessThan(Date.parse(AUTOMATION_RATE_PROVENANCE_HORIZON_ISO)); + expect(series.weeks[0]?.basis).toBe("holds_only"); + }); + + it("drops a row with an unparseable timestamp rather than bucketing it wrong", () => { + expect(buildAutomationRateSeries([row({ createdAt: "nope" })]).decided).toBe(0); + }); +}); + +describe("loadAutomationRateSeries", () => { + it("windows on the supplied clock and defaults to 12 weeks", async () => { + let asked = ""; + await loadAutomationRateSeries({}, { + nowMs: Date.parse("2026-07-29T00:00:00.000Z"), + fetchRows: async (since) => { + asked = since; + return []; + }, + }); + expect(asked).toBe("2026-05-06T00:00:00.000Z"); // 12 weeks back + }); + + it("rejects a non-positive or fractional window rather than trusting it", async () => { + for (const weeks of [0, -3, Number.NaN, undefined]) { + let asked = ""; + await loadAutomationRateSeries({}, { nowMs: Date.parse("2026-07-29T00:00:00.000Z"), weeks, fetchRows: async (s) => { asked = s; return []; } }); + expect(asked, String(weeks)).toBe("2026-05-06T00:00:00.000Z"); + } + }); + + it("truncates a fractional window rather than rounding it up", async () => { + let asked = ""; + await loadAutomationRateSeries({}, { nowMs: Date.parse("2026-07-29T00:00:00.000Z"), weeks: 2.9, fetchRows: async (s) => { asked = s; return []; } }); + expect(asked, "2.9 weeks is a 2-week window -- never longer than the number asked for").toBe("2026-07-15T00:00:00.000Z"); + }); + + it("degrades when an INJECTED fetcher rejects, not just when the real query does", async () => { + // safeAll swallows the real query's own failures, so this outer guard is what protects a caller that + // supplies its own reader -- untested, it would look fine while being the only unprotected path. + await expect( + loadAutomationRateSeries({}, { fetchRows: async () => { throw new Error("upstream gone"); } }), + ).resolves.toMatchObject({ weeks: [], decided: 0, automationRatePct: null }); + }); + + it("reads through the real query when no fetcher is injected", async () => { + // The production caller passes only `env`; without a default the endpoint would publish an empty series + // forever while every injected-fetcher test above still passed. + let sql = ""; + let binds: unknown[] = []; + const env = { + DB: { + prepare(q: string) { + sql = q; + return { + bind: (...values: unknown[]) => { + binds = values; + return { all: async () => ({ results: [] }) }; + }, + }; + }, + }, + }; + await loadAutomationRateSeries(env); + expect(sql).toContain("FROM decision_records"); + expect(sql).toContain("reevaluation_actor"); + // The action filter is built FROM the same constant the fold classifies on, so the placeholder count and + // the bind count cannot drift apart -- a mismatch is a D1 error at runtime that no injected-rows test + // above would ever reach. + expect(sql).toContain("action IN ("); + expect((sql.match(/\?/g) ?? []).length).toBe(binds.length); + expect(binds.slice(1)).toEqual([...AUTOMATION_COUNTED_ACTIONS]); + }); + + it("degrades to an empty series when the read throws, never failing the stats endpoint", async () => { + const env = { DB: { prepare() { throw new Error("no such column"); } } }; + await expect(loadAutomationRateSeries(env)).resolves.toMatchObject({ weeks: [], decided: 0, automationRatePct: null }); + }); +});