Skip to content

Commit 01d2b5a

Browse files
authored
feat(review): per-rule (not just per-project) gate-decision precision tracking (#8099)
Closes #7984. computeGateEval (parity.ts) scores prediction-vs-ground-truth AGGREGATED PER PROJECT — one systematically wrong deterministic rule (like the 2026-07-21/22 hotkey/coldkey regex bug, #7981) can sit at effectively 0% precision while hiding inside an otherwise-healthy project-wide close-precision number, diluted by every OTHER correct close reason the same project produces. The precision-over-time circuit breaker (auto-tune.ts) can never isolate and react to a single broken rule this way, even in principle. New src/review/rule-gate-eval.ts adds that missing dimension by RE-AGGREGATING data already recorded — review_audit's gate_decision rows already carry a reason code (`summary`) — no new collection pipeline, no new table. Mirrors contributor-gate-eval.ts's own established "new dimension, same fold" pattern exactly: - computeRuleGateEval: per-(project, ruleCode) rows, so a maintainer can see "rule X: 0/4 correct" on one repo even while that repo's own aggregate still looks healthy. - computeBlendedRuleGateEval: the SAME cells pooled ACROSS every project a rule has fired on, volume-weighted — a rule's trustworthiness is a property of the rule, not of any one repo it happened to trip. This is the report #7986 will read. - rulesBelowClosePrecisionFloor: which blended rows have cleared enough sample (>= AUTOTUNE_MIN_DECIDED) but sit below the SAME AUTOTUNE_CLOSE_PRECISION_FLOOR the project-level breaker uses — the exact lookup #7986 needs, and the exact "rule X: 0/4 correct" signal this issue exists to surface. Wired into operator-dashboard.ts (an existing operator-facing read path, #7984's own stated deliverable) as a new "Rules below close-precision floor" metric card, alongside the existing contributor-fairness tiles. Read/reporting only — no gate/disposition decision changes here; that's #7986's job. Validated against a replay of the incident shape (an isolated 0/12-correct rule on an otherwise-healthy project with 20 correct closes on other reasons) at both the per-rule and dashboard levels.
1 parent d07b057 commit 01d2b5a

4 files changed

Lines changed: 689 additions & 0 deletions

File tree

src/review/rule-gate-eval.ts

Lines changed: 260 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,260 @@
1+
// Per-rule (not just per-project) gate-decision accuracy (#7984, epic #7980).
2+
//
3+
// computeGateEval (parity.ts) scores prediction-vs-ground-truth AGGREGATED PER PROJECT — one systematically
4+
// wrong deterministic rule (like the 2026-07-21/22 hotkey/coldkey regex bug, #7981) can sit at effectively 0%
5+
// precision while hiding inside an otherwise-healthy project-wide close-precision number, diluted by every
6+
// OTHER correct close reason the SAME project produces. The precision-over-time circuit breaker (auto-tune.ts)
7+
// can never isolate and react to a single broken RULE this way, even in principle. This module adds that
8+
// missing dimension, by RE-AGGREGATING data that's already recorded — review_audit's gate_decision rows
9+
// already carry a reason code (`summary`, the disposition's blockerClass/first blocker code, or the gate's own
10+
// conclusion for a clean merge) — no new collection pipeline, no new table.
11+
//
12+
// Structure mirrors contributor-gate-eval.ts EXACTLY (that file's own header names this same "new dimension on
13+
// the same fold" pattern as the established convention for extending computeGateEval): one function keyed by
14+
// (project, ruleCode) for a "which rule is broken on which repo" view, and a BLENDED counterpart keyed by
15+
// ruleCode ALONE, pooling raw counts across every project — because a rule's trustworthiness is a property of
16+
// the rule itself, not of any one repo it happened to fire in, and that's the exact question #7986 (which
17+
// consumes this module) needs to ask when deciding whether to still exempt a concrete-evidence close from the
18+
// breaker.
19+
//
20+
// READ/REPORTING ONLY (#7984's own stated boundary): nothing here changes any gate/disposition decision.
21+
// #7986 is what actually reads this data to change breaker behavior.
22+
23+
import { AUTOTUNE_CLOSE_PRECISION_FLOOR, AUTOTUNE_MIN_DECIDED } from "./auto-tune";
24+
import { REVERSAL_DISCOUNT_WEIGHT } from "./parity";
25+
26+
export interface RuleGateEvalRow {
27+
project: string;
28+
ruleCode: string;
29+
wouldMerge: number;
30+
mergeConfirmed: number;
31+
mergeFalse: number;
32+
wouldClose: number;
33+
closeConfirmed: number;
34+
closeFalse: number;
35+
decided: number;
36+
mergePrecision: number | null;
37+
closePrecision: number | null;
38+
weightedMergeConfirmed: number;
39+
weightedCloseConfirmed: number;
40+
weightedMergePrecision: number | null;
41+
weightedClosePrecision: number | null;
42+
}
43+
44+
export interface RuleGateEvalReport {
45+
rows: RuleGateEvalRow[];
46+
hasSignal: boolean;
47+
}
48+
49+
export interface BlendedRuleGateEvalRow {
50+
ruleCode: string;
51+
/** Distinct projects this rule has decided rows on, contributing to the blend. */
52+
projectCount: number;
53+
wouldMerge: number;
54+
mergeConfirmed: number;
55+
mergeFalse: number;
56+
wouldClose: number;
57+
closeConfirmed: number;
58+
closeFalse: number;
59+
decided: number;
60+
mergePrecision: number | null;
61+
closePrecision: number | null;
62+
weightedMergeConfirmed: number;
63+
weightedCloseConfirmed: number;
64+
weightedMergePrecision: number | null;
65+
weightedClosePrecision: number | null;
66+
}
67+
68+
export interface BlendedRuleGateEvalReport {
69+
rows: BlendedRuleGateEvalRow[];
70+
hasSignal: boolean;
71+
}
72+
73+
const MIN_DECIDED_FOR_SIGNAL = 10;
74+
75+
/** Storage seam matching parity.ts's own `storage(env)`. */
76+
function storage(env: Env): D1Database {
77+
return env.DB;
78+
}
79+
80+
type RuleGateCell = { project: string; ruleCode: string; pred: string; truth: string; reversed: number; n: number };
81+
82+
/**
83+
* Shared read: review_audit's latest gate_decision per target joined to the latest pr_outcome (ground truth),
84+
* grouped down to one row per (project, ruleCode, pred, truth, reversed) cell — the finest grain both
85+
* computeRuleGateEval (folds by project+ruleCode) and computeBlendedRuleGateEval (folds by ruleCode alone,
86+
* pooling projects) need. Keeping the SQL in one place guarantees both consumers see the exact same underlying
87+
* facts; only the in-memory fold differs. Pure read; fail-safe -> [].
88+
*
89+
* `ruleCode` is `review_audit.summary` — the SAME single reason-code string computeGateEval's own query reads
90+
* (via `decision`/`pred`) but does NOT currently select (parity.ts's `gd` CTE only selects `project`/`pred`).
91+
* For a MERGE decision this is typically the gate's own conclusion (e.g. "success"), not a "rule" in the
92+
* #7986 sense — those rows are harmless to include (they just aren't interesting) and are included here rather
93+
* than filtered out, so this stays a faithful, complete re-aggregation of the same underlying data
94+
* computeGateEval reads, not a second, narrower read with its own selection bias.
95+
*/
96+
async function queryRuleGateCells(env: Env, opts: { days: number; nowMs: number; source?: string; minerOnly?: boolean }): Promise<RuleGateCell[]> {
97+
const days = Number.isFinite(opts.days) && opts.days > 0 ? Math.min(opts.days, 730) : 90;
98+
const fromIso = new Date(opts.nowMs - days * 86_400_000).toISOString().slice(0, 10);
99+
const sourceFilter = opts.source ? "AND source = ?" : "";
100+
const minerFilter = opts.minerOnly ? "AND miner_authored = 1" : "";
101+
// Latest row per target_id via ROW_NUMBER()+rn=1 -- NOT SQLite's "bare column with MAX()" trick, which
102+
// Postgres rejects outright ("column must appear in the GROUP BY clause") -- mirrors computeGateEval's own
103+
// identical portability note (parity.ts) and contributor-gate-eval.ts's queryContributorGateCells.
104+
const sql = `
105+
WITH gd AS (
106+
SELECT target_id, project, decision AS pred, summary AS rule_code, created_at,
107+
ROW_NUMBER() OVER (PARTITION BY target_id ORDER BY created_at DESC) AS rn
108+
FROM review_audit WHERE event_type = 'gate_decision' AND decision IS NOT NULL AND created_at >= ? ${sourceFilter} ${minerFilter}
109+
),
110+
po AS (
111+
SELECT target_id, decision AS truth, created_at,
112+
ROW_NUMBER() OVER (PARTITION BY target_id ORDER BY created_at DESC) AS rn
113+
FROM review_audit WHERE event_type = 'pr_outcome' AND decision IS NOT NULL
114+
),
115+
rev AS (
116+
SELECT DISTINCT target_id FROM review_audit WHERE event_type IN ('reversal_reverted', 'reversal_reopened')
117+
)
118+
SELECT gd.project AS project, COALESCE(gd.rule_code, 'unknown') AS ruleCode, gd.pred AS pred, po.truth AS truth,
119+
CASE WHEN rev.target_id IS NOT NULL THEN 1 ELSE 0 END AS reversed, COUNT(*) AS n
120+
FROM gd JOIN po ON gd.target_id = po.target_id
121+
LEFT JOIN rev ON gd.target_id = rev.target_id
122+
WHERE gd.rn = 1 AND po.rn = 1
123+
GROUP BY gd.project, ruleCode, gd.pred, po.truth, reversed`;
124+
125+
try {
126+
const stmt = storage(env).prepare(sql);
127+
const bound = opts.source ? stmt.bind(fromIso, opts.source) : stmt.bind(fromIso);
128+
const res = await bound.all<RuleGateCell>();
129+
return res.results ?? [];
130+
} catch {
131+
return [];
132+
}
133+
}
134+
135+
function foldCell(
136+
target: { wouldMerge: number; mergeConfirmed: number; mergeFalse: number; wouldClose: number; closeConfirmed: number; closeFalse: number; decided: number; weightedMergeConfirmed: number; weightedCloseConfirmed: number },
137+
c: RuleGateCell,
138+
): void {
139+
target.decided += c.n;
140+
const weightedN = c.reversed ? c.n * REVERSAL_DISCOUNT_WEIGHT : c.n;
141+
if (c.pred === "merge") {
142+
target.wouldMerge += c.n;
143+
if (c.truth === "merged") {
144+
target.mergeConfirmed += c.n;
145+
target.weightedMergeConfirmed += weightedN;
146+
} else if (c.truth === "closed") target.mergeFalse += c.n;
147+
} else if (c.pred === "close") {
148+
target.wouldClose += c.n;
149+
if (c.truth === "closed") {
150+
target.closeConfirmed += c.n;
151+
target.weightedCloseConfirmed += weightedN;
152+
} else if (c.truth === "merged") target.closeFalse += c.n;
153+
}
154+
}
155+
156+
/**
157+
* Per-(project, ruleCode) gate accuracy over review_audit's existing gate_decision predictions vs the realized
158+
* pr_outcome. Pure read; fail-safe -> empty report. Mirrors computeGateEval (parity.ts) exactly, with
159+
* `ruleCode` (review_audit.summary) added to both the GROUP BY and the fold key — so a maintainer can see
160+
* "rule X: 0/4 correct" on a repo even while that repo's OWN project-wide aggregate still looks healthy.
161+
*/
162+
export async function computeRuleGateEval(env: Env, opts: { days: number; nowMs: number; source?: string; minerOnly?: boolean }): Promise<RuleGateEvalReport> {
163+
const cells = await queryRuleGateCells(env, opts);
164+
if (cells.length === 0) return { rows: [], hasSignal: false };
165+
166+
const byKey = new Map<string, RuleGateEvalRow>();
167+
const row = (project: string, ruleCode: string): RuleGateEvalRow => {
168+
const key = `${project}:${ruleCode}`;
169+
let r = byKey.get(key);
170+
if (!r) {
171+
r = {
172+
project, ruleCode, wouldMerge: 0, mergeConfirmed: 0, mergeFalse: 0, wouldClose: 0, closeConfirmed: 0, closeFalse: 0, decided: 0,
173+
mergePrecision: null, closePrecision: null, weightedMergeConfirmed: 0, weightedCloseConfirmed: 0, weightedMergePrecision: null, weightedClosePrecision: null,
174+
};
175+
byKey.set(key, r);
176+
}
177+
return r;
178+
};
179+
180+
for (const c of cells) foldCell(row(c.project, c.ruleCode), c);
181+
182+
const rows = [...byKey.values()]
183+
.map((r) => ({
184+
...r,
185+
mergePrecision: r.wouldMerge > 0 ? r.mergeConfirmed / r.wouldMerge : null,
186+
closePrecision: r.wouldClose > 0 ? r.closeConfirmed / r.wouldClose : null,
187+
weightedMergePrecision: r.wouldMerge > 0 ? r.weightedMergeConfirmed / r.wouldMerge : null,
188+
weightedClosePrecision: r.wouldClose > 0 ? r.weightedCloseConfirmed / r.wouldClose : null,
189+
}))
190+
.sort((a, b) => a.project.localeCompare(b.project) || a.ruleCode.localeCompare(b.ruleCode));
191+
return { rows, hasSignal: rows.some((r) => r.decided >= MIN_DECIDED_FOR_SIGNAL) };
192+
}
193+
194+
/**
195+
* The global, cross-repo blended counterpart to computeRuleGateEval: one row per ruleCode, POOLING raw
196+
* prediction/outcome counts across every project that code has fired on before computing a single precision
197+
* ratio -- volume-weighted, not an average of each project's own precision, so a code with 40 decided
198+
* instances on one repo and 2 on another isn't distorted toward a 50/50 blend. This is the report #7986
199+
* actually consumes: a rule's own track record, independent of which repo happened to trip it.
200+
*/
201+
export async function computeBlendedRuleGateEval(env: Env, opts: { days: number; nowMs: number; source?: string; minerOnly?: boolean }): Promise<BlendedRuleGateEvalReport> {
202+
const cells = await queryRuleGateCells(env, opts);
203+
if (cells.length === 0) return { rows: [], hasSignal: false };
204+
205+
const byRuleCode = new Map<string, BlendedRuleGateEvalRow>();
206+
const projectsByRuleCode = new Map<string, Set<string>>();
207+
const row = (ruleCode: string): BlendedRuleGateEvalRow => {
208+
let r = byRuleCode.get(ruleCode);
209+
if (!r) {
210+
r = {
211+
ruleCode, projectCount: 0, wouldMerge: 0, mergeConfirmed: 0, mergeFalse: 0, wouldClose: 0, closeConfirmed: 0, closeFalse: 0, decided: 0,
212+
mergePrecision: null, closePrecision: null, weightedMergeConfirmed: 0, weightedCloseConfirmed: 0, weightedMergePrecision: null, weightedClosePrecision: null,
213+
};
214+
byRuleCode.set(ruleCode, r);
215+
}
216+
return r;
217+
};
218+
219+
for (const c of cells) {
220+
let projects = projectsByRuleCode.get(c.ruleCode);
221+
if (!projects) {
222+
projects = new Set<string>();
223+
projectsByRuleCode.set(c.ruleCode, projects);
224+
}
225+
projects.add(c.project);
226+
foldCell(row(c.ruleCode), c);
227+
}
228+
229+
const rows = [...byRuleCode.values()]
230+
.map((r) => ({
231+
...r,
232+
// Every ruleCode in byRuleCode was inserted into projectsByRuleCode in the SAME loop iteration above --
233+
// the two maps always have identical keysets, so this lookup can never miss.
234+
projectCount: projectsByRuleCode.get(r.ruleCode)!.size,
235+
mergePrecision: r.wouldMerge > 0 ? r.mergeConfirmed / r.wouldMerge : null,
236+
closePrecision: r.wouldClose > 0 ? r.closeConfirmed / r.wouldClose : null,
237+
weightedMergePrecision: r.wouldMerge > 0 ? r.weightedMergeConfirmed / r.wouldMerge : null,
238+
weightedClosePrecision: r.wouldClose > 0 ? r.weightedCloseConfirmed / r.wouldClose : null,
239+
}))
240+
.sort((a, b) => a.ruleCode.localeCompare(b.ruleCode));
241+
return { rows, hasSignal: rows.some((r) => r.decided >= MIN_DECIDED_FOR_SIGNAL) };
242+
}
243+
244+
/**
245+
* Blended rows whose close-side sample has cleared enough volume to trust (`wouldClose >= minDecided`, default
246+
* {@link AUTOTUNE_MIN_DECIDED} — the SAME floor the project-level close-precision breaker in auto-tune.ts
247+
* uses) but whose weighted close precision sits below `floor` (default {@link AUTOTUNE_CLOSE_PRECISION_FLOOR})
248+
* — exactly the "rule X: 0/4 correct" signal #7984 exists to surface, pure fold over an already-fetched
249+
* {@link BlendedRuleGateEvalReport}'s rows (no I/O). This is also the exact lookup #7986 reads to decide
250+
* whether a rule's concrete-evidence breaker exemption should still hold: a rule NOT in this list either has
251+
* an insufficient sample (stays exempt, per #7986's own "insufficient sample defaults to keeping the
252+
* exemption" rule) or a healthy track record (stays exempt, correctly).
253+
*/
254+
export function rulesBelowClosePrecisionFloor(
255+
rows: readonly BlendedRuleGateEvalRow[],
256+
floor: number = AUTOTUNE_CLOSE_PRECISION_FLOOR,
257+
minDecided: number = AUTOTUNE_MIN_DECIDED,
258+
): BlendedRuleGateEvalRow[] {
259+
return rows.filter((r) => r.wouldClose >= minDecided && r.weightedClosePrecision != null && r.weightedClosePrecision < floor);
260+
}

src/services/operator-dashboard.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { computeFleetAnalytics, getFleetHealthSummary, type FleetAnalytics, type
3434
import { computeAgentHealth, computeCalibration, type AgentHealth, type Calibration } from "../review/ops";
3535
import { computeGateEval, type GateEvalReport } from "../review/parity";
3636
import { computeContributorGateEval, contributorFairnessFlags, computeBlendedContributorGateEval, contributorGlobalFairnessFlags } from "../review/contributor-gate-eval";
37+
import { computeBlendedRuleGateEval, rulesBelowClosePrecisionFloor } from "../review/rule-gate-eval";
3738
import { computeCycleTimeAggregate, computeFindingAcceptance, type CycleTimeAggregate } from "../review/stats";
3839
import { loadUpstreamStatus, type UpstreamStatus } from "../upstream/ruleset";
3940
import { nowIso } from "../utils/json";
@@ -139,6 +140,7 @@ export async function buildOperatorDashboardPayload(
139140
gateEval,
140141
contributorGateEval,
141142
blendedContributorGateEval,
143+
blendedRuleGateEval,
142144
cycleTime,
143145
calibration,
144146
agentHealth,
@@ -170,6 +172,10 @@ export async function buildOperatorDashboardPayload(
170172
computeContributorGateEval(env, { days: GATE_ANALYTICS_WINDOW_DAYS, nowMs: Date.now() }),
171173
// #global-contributor-trust: the SAME data pooled cross-repo into one blended figure per login, same window.
172174
computeBlendedContributorGateEval(env, { days: GATE_ANALYTICS_WINDOW_DAYS, nowMs: Date.now() }),
175+
// #7984: the SAME review_audit data re-aggregated by RULE CODE (pooled cross-repo) instead of by project —
176+
// isolates a single systematically-wrong deterministic rule even while its host project's own aggregate
177+
// still looks healthy. Fails safe to an empty report on any read error.
178+
computeBlendedRuleGateEval(env, { days: GATE_ANALYTICS_WINDOW_DAYS, nowMs: Date.now() }),
173179
// #2194: cycle-time percentiles from the stats feed; fails safe to an empty aggregate.
174180
computeCycleTimeAggregate(env, { days: GATE_ANALYTICS_WINDOW_DAYS, nowMs: Date.now() }),
175181
computeCalibration(env, operatorAgentConfig(env)),
@@ -205,6 +211,9 @@ export async function buildOperatorDashboardPayload(
205211
const contributorFairnessFlagCount = contributorFairnessFlags(contributorGateEval.rows).length;
206212
// #global-contributor-trust: same fold, but over the blended (cross-repo) rows.
207213
const globalContributorFairnessFlagCount = contributorGlobalFairnessFlags(blendedContributorGateEval.rows).length;
214+
// #7984: rules whose sample has cleared enough volume to trust but whose weighted close precision sits
215+
// below the SAME floor auto-tune.ts's project-level close-precision breaker uses -- pure fold, no extra I/O.
216+
const rulesBelowFloor = rulesBelowClosePrecisionFloor(blendedRuleGateEval.rows);
208217
const installedRepos = repositories.filter((repo: RepositoryRecord) => repo.isInstalled).length;
209218
const registeredRepos = repositories.filter((repo: RepositoryRecord) => repo.isRegistered).length;
210219
// #1967: map FindingAcceptanceAggregate's field names onto the AcceptanceRateCard's expected shape.
@@ -286,6 +295,15 @@ export async function buildOperatorDashboardPayload(
286295
value: String(globalContributorFairnessFlagCount),
287296
delta: globalContributorFairnessFlagCount > 0 ? `${blendedContributorGateEval.rows.length} contributor(s) evaluated` : "no outliers detected",
288297
},
298+
{
299+
// #7984: a rule code (unlike a contributor login) is not personally identifying, so the flagged codes
300+
// themselves are surfaced directly here -- same "name the specific thing" posture as the fleet
301+
// gaming-pattern-flags tile above, not the privacy-conscious count-only posture the contributor tiles
302+
// use for logins.
303+
label: "Rules below close-precision floor",
304+
value: String(rulesBelowFloor.length),
305+
delta: rulesBelowFloor.length > 0 ? rulesBelowFloor.map((r) => r.ruleCode).join(", ") : "no rule below floor",
306+
},
289307
],
290308
noiseReduction: [
291309
{

test/unit/operator-dashboard.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,32 @@ describe("operator dashboard payload", () => {
224224
expect(JSON.stringify(payload)).not.toContain("flagged-login");
225225
});
226226

227+
it("surfaces a rule below the close-precision floor (#7984), replaying the incident shape: an isolated bad rule on an otherwise-healthy project", async () => {
228+
const env = createTestEnv();
229+
const seedClose = async (id: string, ruleCode: string | null, truth: "closed" | "merged"): Promise<void> => {
230+
await env.DB.prepare(
231+
`INSERT INTO review_audit (id, project, target_id, event_type, decision, summary, source, created_at) VALUES (?, 'metagraphed/metagraphed', ?, 'gate_decision', 'close', ?, 'gittensory-native', ?)`,
232+
)
233+
.bind(`gd-${id}`, `metagraphed/metagraphed#${id}`, ruleCode, new Date().toISOString())
234+
.run();
235+
await env.DB.prepare(
236+
`INSERT INTO review_audit (id, project, target_id, event_type, decision, source, created_at) VALUES (?, 'metagraphed/metagraphed', ?, 'pr_outcome', ?, 'github', ?)`,
237+
)
238+
.bind(`po-${id}`, `metagraphed/metagraphed#${id}`, truth, new Date().toISOString())
239+
.run();
240+
};
241+
// The buggy rule: 12 closes (clears AUTOTUNE_MIN_DECIDED), every single one later merged -- 0% precision.
242+
for (let i = 1; i <= 12; i++) await seedClose(`bad-${i}`, "surface_lane_reject", "merged");
243+
// The same project's every OTHER close reason: perfectly healthy, would dilute a project-wide number.
244+
for (let i = 1; i <= 20; i++) await seedClose(`good-${i}`, "missing_linked_issue", "closed");
245+
246+
const payload = await buildOperatorDashboardPayload(env);
247+
expect(payload.metrics).toEqual(
248+
expect.arrayContaining([expect.objectContaining({ label: "Rules below close-precision floor", value: "1", delta: "surface_lane_reject" })]),
249+
);
250+
expect(JSON.stringify(payload)).not.toContain("missing_linked_issue"); // the healthy rule never appears
251+
});
252+
227253
it("wires computeFindingAcceptance into the dashboard's acceptance card shape (#1967/#5213)", async () => {
228254
const env = createTestEnv();
229255
await env.DB.prepare(

0 commit comments

Comments
 (0)