You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
src/review/automation-rate.ts publishes the weekly automation-rate series. Its module header states the
published definition (src/review/automation-rate.ts:12-18):
// 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
verdictShowsHumanAction implements all three (src/review/automation-rate.ts:110-114), and buildAutomationRateSeries ORs it across every row it is given (:129, existing.human = existing.human || human).
The read never gives it every verdict (src/review/automation-rate.ts:207-221):
AUTOMATION_COUNTED_ACTIONS is ['merge', 'close', 'hold'] (:54). Its own doc comment justifies the
constant as preventing exactly this class of drift (:44-47):
* "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.
But the fold consumes it for ONE question ("is this PR decided at all") and the predicate consumes an
entirely different set of columns. reevaluation_actor and reevaluation_reason live on every decision_records row regardless of action, and the header itself names label, update_branch, approve and the error/no-op classes as real, occurring actions (:11-15). A verdict whose action is one of
those, carrying reevaluation_actor = '<maintainer>' or reevaluation_reason = 'maintainer_request', is
filtered out by the SQL and never reaches verdictShowsHumanAction.
Concretely: a maintainer asks for a re-run on a PR, the re-evaluation records an update_branch verdict
with reevaluation_actor set, and the PR is subsequently auto-merged. The published series counts that PR
as automated, even though a named person is in its decision path — the exact case the definition says is manual. The hold leg still fires, so the error is one-sided: the series can only ever OVER-report
automation, on a surface whose entire purpose is to be an honest, checkable number
(src/review/automation-rate.ts:1-6).
Also affected: buildAutomationRateSeries's if (!entry.enacted && !entry.held) continue; guard (:145)
is unreachable from the production path, because the SQL already guarantees every row is one of the three.
test/unit/automation-rate.test.ts exercises buildAutomationRateSeries with hand-built rows only; no test
pairs the query's row set against the predicate's column set.
Requirements
queryAutomationRows must no longer restrict rows by action. It must select every decision_records
row in the window, so buildAutomationRateSeries sees every verdict's reevaluation_actor / reevaluation_reason.
buildAutomationRateSeries must stay unchanged in its DEFINITIONS: decided still requires entry.enacted || entry.held, automated still requires entry.enacted && !entry.human, the week is
still the week of the PR's FIRST verdict across all its rows, and verdictShowsHumanAction keeps its three
conditions exactly.
Because the first-verdict week is now derived from a wider row set, a PR whose earliest row is a
non-deciding verdict must be attributed to THAT row's week — this is the header's stated "week of its FIRST
verdict" and must be asserted by a test.
AUTOMATION_COUNTED_ACTIONS must remain exported and must remain the set the FOLD uses to decide enacted/held; only its use as a SQL WHERE filter goes away. Update its doc comment (:53-54) so it no
longer claims to be what the read consumes.
AUTOMATION_RATE_PROVENANCE_HORIZON_ISO, AUTOMATION_RATE_WEEKS, weekStartIso, ratePct and the AutomationRateSeries / AutomationRateWeek shapes must NOT change.
The read must stay bounded by the existing trailing window (created_at >= ?) — this issue must not remove
the window.
⚠️ Required pattern: keep the fold as the single place the definition lives, exactly as the module header
describes; the SQL becomes a plain windowed read. What does NOT satisfy this issue: adding 'label','update_branch','approve' to AUTOMATION_COUNTED_ACTIONS (that would silently change what decided means, since the fold reads the same constant); adding a second query that fetches only the
human-signal columns and merging the two result sets in the loader (a parallel mechanism instead of fixing
the one read); moving the human check into SQL; a test-only PR.
Deliverables
src/review/automation-rate.ts: queryAutomationRows selects every decision_records row in the
window with no action IN (...) filter, and passes only sinceIso as a bind.
src/review/automation-rate.ts: AUTOMATION_COUNTED_ACTIONS's doc comment no longer claims the read
consumes it.
A test in test/unit/automation-rate.test.ts: a PR with rows [{action:'update_branch', reevaluationActor:'maintainer'}, {action:'merge', reevaluationActor:null}]
folds to manual: 1, automated: 0 for its week.
A test in test/unit/automation-rate.test.ts: a PR with rows [{action:'label', reevaluationReason:'maintainer_request'}, {action:'close'}] folds to manual: 1, automated: 0.
A test in test/unit/automation-rate.test.ts: a PR whose ONLY rows are non-deciding
(label, update_branch) is excluded from both decided and automated — the !entry.enacted && !entry.held guard now has a reachable production shape and must be pinned.
A test in test/unit/automation-rate.test.ts: a PR whose earliest row is a non-deciding verdict in
week A and whose merge lands in week B is counted in week A.
A test in test/unit/automation-rate.test.ts driving loadAutomationRateSeries with a fetchRows
stub, asserting the loader still returns a series unchanged in shape.
A regression test at test/unit/automation-rate.test.ts named for this bug (e.g. "REGRESSION: a re-evaluation attributed to a named person counts as manual even when its verdict is non-deciding").
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example
dropping the SQL filter without pinning the now-reachable non-deciding-only exclusion, or without the
first-verdict-week test — does not resolve this issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts and packages/loopover-engine/src/**/*.ts; src/review/automation-rate.ts is measured
and gated. Both arms of every touched branch need a test: verdictShowsHumanAction's three conditions plus
its all-false fall-through; buildAutomationRateSeries's if (!existing) first-vs-subsequent row split, its !entry.enacted && !entry.held guard (both arms — now reachable), its Date.parse(row.createdAt) < Date.parse(existing.firstSeen) earlier-row arm and its else, its entry.enacted && !entry.human split, and weekStartIso's unparseable-date null arm.
Expected Outcome
The published automation rate counts a PR as manual whenever a named person touched its decision path,
regardless of which verdict class recorded that touch — so the number can no longer over-report automation by
routing human involvement through a non-deciding verdict.
Links & Resources
src/review/automation-rate.ts:1-33 — the published definition this query contradicts
src/review/automation-rate.ts:44-54 — AUTOMATION_COUNTED_ACTIONS and its one-definition claim
Context
src/review/automation-rate.tspublishes the weekly automation-rate series. Its module header states thepublished definition (
src/review/automation-rate.ts:12-18):verdictShowsHumanActionimplements all three (src/review/automation-rate.ts:110-114), andbuildAutomationRateSeriesORs it across every row it is given (:129,existing.human = existing.human || human).The read never gives it every verdict (
src/review/automation-rate.ts:207-221):AUTOMATION_COUNTED_ACTIONSis['merge', 'close', 'hold'](:54). Its own doc comment justifies theconstant as preventing exactly this class of drift (
:44-47):But the fold consumes it for ONE question ("is this PR decided at all") and the predicate consumes an
entirely different set of columns.
reevaluation_actorandreevaluation_reasonlive on everydecision_recordsrow regardless ofaction, and the header itself nameslabel,update_branch,approveand the error/no-op classes as real, occurring actions (:11-15). A verdict whose action is one ofthose, carrying
reevaluation_actor = '<maintainer>'orreevaluation_reason = 'maintainer_request', isfiltered out by the SQL and never reaches
verdictShowsHumanAction.Concretely: a maintainer asks for a re-run on a PR, the re-evaluation records an
update_branchverdictwith
reevaluation_actorset, and the PR is subsequently auto-merged. The published series counts that PRas
automated, even though a named person is in its decision path — the exact case the definition says ismanual. Theholdleg still fires, so the error is one-sided: the series can only ever OVER-reportautomation, on a surface whose entire purpose is to be an honest, checkable number
(
src/review/automation-rate.ts:1-6).Also affected:
buildAutomationRateSeries'sif (!entry.enacted && !entry.held) continue;guard (:145)is unreachable from the production path, because the SQL already guarantees every row is one of the three.
test/unit/automation-rate.test.tsexercisesbuildAutomationRateSerieswith hand-built rows only; no testpairs the query's row set against the predicate's column set.
Requirements
queryAutomationRowsmust no longer restrict rows byaction. It must select everydecision_recordsrow in the window, so
buildAutomationRateSeriessees every verdict'sreevaluation_actor/reevaluation_reason.buildAutomationRateSeriesmust stay unchanged in its DEFINITIONS:decidedstill requiresentry.enacted || entry.held,automatedstill requiresentry.enacted && !entry.human, the week isstill the week of the PR's FIRST verdict across all its rows, and
verdictShowsHumanActionkeeps its threeconditions exactly.
non-deciding verdict must be attributed to THAT row's week — this is the header's stated "week of its FIRST
verdict" and must be asserted by a test.
AUTOMATION_COUNTED_ACTIONSmust remain exported and must remain the set the FOLD uses to decideenacted/held; only its use as a SQLWHEREfilter goes away. Update its doc comment (:53-54) so it nolonger claims to be what the read consumes.
AUTOMATION_RATE_PROVENANCE_HORIZON_ISO,AUTOMATION_RATE_WEEKS,weekStartIso,ratePctand theAutomationRateSeries/AutomationRateWeekshapes must NOT change.created_at >= ?) — this issue must not removethe window.
Deliverables
src/review/automation-rate.ts:queryAutomationRowsselects everydecision_recordsrow in thewindow with no
action IN (...)filter, and passes onlysinceIsoas a bind.src/review/automation-rate.ts:AUTOMATION_COUNTED_ACTIONS's doc comment no longer claims the readconsumes it.
test/unit/automation-rate.test.ts: a PR with rows[{action:'update_branch', reevaluationActor:'maintainer'}, {action:'merge', reevaluationActor:null}]folds to
manual: 1, automated: 0for its week.test/unit/automation-rate.test.ts: a PR with rows[{action:'label', reevaluationReason:'maintainer_request'}, {action:'close'}]folds tomanual: 1, automated: 0.test/unit/automation-rate.test.ts: a PR whose ONLY rows are non-deciding(
label,update_branch) is excluded from bothdecidedandautomated— the!entry.enacted && !entry.heldguard now has a reachable production shape and must be pinned.test/unit/automation-rate.test.ts: a PR whose earliest row is a non-deciding verdict inweek A and whose
mergelands in week B is counted in week A.test/unit/automation-rate.test.tsdrivingloadAutomationRateSerieswith afetchRowsstub, asserting the loader still returns a series unchanged in shape.
test/unit/automation-rate.test.tsnamed for this bug (e.g."REGRESSION: a re-evaluation attributed to a named person counts as manual even when its verdict is non-deciding").All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example
dropping the SQL filter without pinning the now-reachable non-deciding-only exclusion, or without the
first-verdict-week test — does not resolve this issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted.
vitest.config.ts'scoverage.includecovers
src/**/*.tsandpackages/loopover-engine/src/**/*.ts;src/review/automation-rate.tsis measuredand gated. Both arms of every touched branch need a test:
verdictShowsHumanAction's three conditions plusits all-false fall-through;
buildAutomationRateSeries'sif (!existing)first-vs-subsequent row split, its!entry.enacted && !entry.heldguard (both arms — now reachable), itsDate.parse(row.createdAt) < Date.parse(existing.firstSeen)earlier-row arm and its else, itsentry.enacted && !entry.humansplit, andweekStartIso's unparseable-datenullarm.Expected Outcome
The published automation rate counts a PR as manual whenever a named person touched its decision path,
regardless of which verdict class recorded that touch — so the number can no longer over-report automation by
routing human involvement through a non-deciding verdict.
Links & Resources
src/review/automation-rate.ts:1-33— the published definition this query contradictssrc/review/automation-rate.ts:44-54—AUTOMATION_COUNTED_ACTIONSand its one-definition claimsrc/review/automation-rate.ts:110-114—verdictShowsHumanActionsrc/review/automation-rate.ts:118-176—buildAutomationRateSeriessrc/review/automation-rate.ts:207-221—queryAutomationRows, the filtered readtest/unit/automation-rate.test.ts— the existing test file