Skip to content
Merged
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
17 changes: 10 additions & 7 deletions src/review/automation-rate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,17 @@ import { safeAll } from "./public-stats";
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. Module-private: AUTOMATION_COUNTED_ACTIONS below is the exported surface, and it is
* what both the fold and the read consume -- one definition, rather than a WHERE clause and a predicate
* that can drift apart. */
* "automated" means. The FOLD classifies on this, not the read: the read (#10013) now selects every
* verdict in the window so a human signal carried on a non-deciding verdict is not filtered away before
* the fold ever sees it. */
const AUTOMATION_ENACTING_ACTIONS = ["merge", "close"] as const;

/** The action recording that the gate declined to decide and handed the PR to a person. Module-private for
* the same reason as the set above. */
const AUTOMATION_HOLD_ACTION = "hold";

/** Every action that puts a PR in the series at all -- an enacted decision, or a hold. */
/** Every action that DECIDES a PR -- an enacted decision, or a hold. This is the published set the fold
* keys `decided` on; the read no longer restricts by it (#10013), so it does not gate what rows are seen. */
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
Expand Down Expand Up @@ -214,9 +215,11 @@ async function queryAutomationRows(env: unknown, sinceIso: string): Promise<Auto
reevaluation_reason AS reevaluationReason,
reevaluation_actor AS reevaluationActor
FROM decision_records
WHERE created_at >= ?
AND action IN (${AUTOMATION_COUNTED_ACTIONS.map(() => "?").join(", ")})`,
WHERE created_at >= ?`,
// #10013: select EVERY verdict in the window, not just the enacting/hold actions. A human signal
// (reevaluation_actor / reevaluation_reason = 'maintainer_request') can ride a NON-deciding verdict (a
// `label`, an `update_branch`); the old `action IN (...)` filter dropped those rows, so a PR a human
// actually touched read as automated. buildAutomationRateSeries still keys `enacted`/`held` off `action`.
sinceIso,
...AUTOMATION_COUNTED_ACTIONS,
);
}
36 changes: 29 additions & 7 deletions test/unit/automation-rate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// would let the rate be inflated.
import { describe, expect, it } from "vitest";
import {
AUTOMATION_COUNTED_ACTIONS,
AUTOMATION_RATE_PROVENANCE_HORIZON_ISO,
buildAutomationRateSeries,
loadAutomationRateSeries,
Expand Down Expand Up @@ -108,6 +107,28 @@ describe("buildAutomationRateSeries", () => {
expect(series.automated).toBe(1);
});

it("counts a PR MANUAL when a human signal rides a NON-deciding verdict, not just a deciding one (#10013)", () => {
// The bug: `queryAutomationRows` restricted rows by `action`, so a human signal carried on a non-deciding
// verdict (a re-labelled PR, a maintainer-triggered re-run recorded as `label`/`update_branch`) never
// reached the fold. The PR's only surviving row was the clean `merge`, so a PR a person actually touched
// read as AUTOMATED. With the filter gone the fold sees the human-signal row and ORs it in.
const humanOnLabel = buildAutomationRateSeries([
row({ pullNumber: 1, action: "label", reevaluationActor: "JSONbored", createdAt: "2026-07-29T10:00:00.000Z" }),
row({ pullNumber: 1, action: "merge", createdAt: "2026-07-29T11:00:00.000Z" }),
]);
expect(humanOnLabel.decided).toBe(1);
expect(humanOnLabel.automated).toBe(0);
expect(humanOnLabel.weeks[0]?.manual).toBe(1);

// Same for a maintainer_request reason riding a non-deciding verdict.
const requestOnUpdate = buildAutomationRateSeries([
row({ pullNumber: 2, action: "update_branch", reevaluationReason: "maintainer_request", createdAt: "2026-07-29T10:00:00.000Z" }),
row({ pullNumber: 2, action: "merge", createdAt: "2026-07-29T11:00:00.000Z" }),
]);
expect(requestOnUpdate.automated).toBe(0);
expect(requestOnUpdate.weeks[0]?.manual).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);
Expand Down Expand Up @@ -217,12 +238,13 @@ describe("loadAutomationRateSeries", () => {
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]);
// #10013: the read must NOT restrict rows by action -- a human signal (reevaluation_actor /
// reevaluation_reason='maintainer_request') can ride a non-deciding verdict, and the old `action IN (...)`
// filter dropped exactly those rows, undercounting manual work. The only bind is the `created_at >= ?`
// window bound: one placeholder, one bind, and no action list.
expect(sql).not.toContain("action IN (");
expect((sql.match(/\?/g) ?? []).length).toBe(1);
expect(binds).toEqual([expect.any(String)]);
});

it("degrades to an empty series when the read throws, never failing the stats endpoint", async () => {
Expand Down