Skip to content

Commit 8b99983

Browse files
authored
autorevert metrics: third FN category for killswitch-active windows (#8089)
Human reverts that landed while autorevert was disabled via the `ci: disable-autorevert` label on an open issue are misclassified as False Negatives — autorevert wasn't running, it didn't miss. Surface them as a third category and exclude from the recall denominator. ## What changed - New saved query `autorevert_killswitch_windows` returns raw label add/remove events from `default.issues_label_event` plus each issue's `closed_at`. - `lib/autorevert/killswitchWindows.ts` folds the events per-issue into `[on, off]` intervals, treating a close as an implicit `unlabeled` (the lambda only honors the label on **open** issues). - `pages/api/autorevert/metrics.ts` partitions `falseNegatives` into the lambda-up subset (counted in recall) and a new `falseNegativesKillswitch` subset, linked to the killswitch issue. Weekly recall uses the lambda-up subset only. - `pages/metrics/autorevert.tsx` adds a `FN (Disabled)` StatCard, chart series (purple, stacked next to FN), legend entry, and renders FN chips that fall in a killswitch window with the `FN (disabled)` label linking to the issue. ## API additions ``` summary.false_negatives_killswitch: number weeklyMetrics[].false_negatives_killswitch: number killswitchWindows: [{ issue_number, on, off }] falseNegativesKillswitch: [{ recovery_sha, recovery_time, signals_fixed, reverted_pr_numbers, killswitch_issue }] ``` `recall` (overall and weekly) now uses `tp / (tp + fn_lambda_up)`. ## Validation Historical signal verified: issue #183016 was `labeled` 2026-05-09 01:36:48Z and `unlabeled` 2026-05-12 16:47:26Z, matching the ~79h `misc.autorevert_state` row gap. Unit test (`test/autorevertKillswitchWindows.test.ts`) covers labeled/unlabeled pairing, `closed_at` fallback, still-active intervals, multi-cycle issues, and cross-issue ordering. `yarn tsc --noEmit` and `eslint` clean on the changed files. New + existing autorevert jest tests pass.
1 parent 33f82bb commit 8b99983

6 files changed

Lines changed: 404 additions & 37 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"params": {
3+
"stopTime": "DateTime64(3)"
4+
},
5+
"tests": [
6+
{
7+
"stopTime": "2026-05-15 00:00:00.000"
8+
},
9+
{
10+
"stopTime": {
11+
"from_now": 0
12+
}
13+
}
14+
]
15+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
-- All add/remove events of the global autorevert killswitch label
2+
-- (`ci: disable-autorevert`) on pytorch/pytorch issues, plus each
3+
-- issue's `closed_at` (empty if still open). The caller folds the
4+
-- event stream per-issue into [on_ts, off_ts] active intervals,
5+
-- treating a `closed_at` as an implicit `unlabeled` (the autorevert
6+
-- lambda only honors the label on OPEN issues). Events outside the
7+
-- metrics window are needed to resolve intervals that span it.
8+
WITH issue_close AS (
9+
SELECT
10+
number AS issue_number,
11+
argMax(closed_at, updated_at) AS closed_at_str
12+
FROM default.issues
13+
WHERE
14+
repository_url = 'https://api.github.com/repos/pytorch/pytorch'
15+
GROUP BY number
16+
)
17+
18+
SELECT
19+
e.event_time AS event_time,
20+
e.action AS action,
21+
e.issue_number AS issue_number,
22+
coalesce(c.closed_at_str, '') AS issue_closed_at
23+
FROM default.issues_label_event AS e
24+
LEFT JOIN issue_close AS c ON c.issue_number = e.issue_number
25+
WHERE
26+
e.repo_name = 'pytorch/pytorch'
27+
AND e.label_name = 'ci: disable-autorevert'
28+
AND e.event_time <= {stopTime: DateTime64(3)} + INTERVAL 1 DAY
29+
ORDER BY e.issue_number, e.event_time
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Fold `ci: disable-autorevert` label add/remove events on pytorch/pytorch
2+
// issues into per-issue active intervals. The autorevert lambda only honors
3+
// the label on OPEN issues, so an issue close acts as an implicit
4+
// `unlabeled` event if no explicit one preceded it.
5+
6+
export interface KillswitchLabelEvent {
7+
event_time: string;
8+
action: "labeled" | "unlabeled" | string;
9+
issue_number: number;
10+
issue_closed_at: string;
11+
}
12+
13+
export interface KillswitchWindow {
14+
issue_number: number;
15+
on: string;
16+
off: string | null;
17+
}
18+
19+
export function foldKillswitchWindows(
20+
events: KillswitchLabelEvent[]
21+
): KillswitchWindow[] {
22+
const byIssue = new Map<number, KillswitchLabelEvent[]>();
23+
for (const e of events) {
24+
if (!byIssue.has(e.issue_number)) byIssue.set(e.issue_number, []);
25+
byIssue.get(e.issue_number)!.push(e);
26+
}
27+
const windows: KillswitchWindow[] = [];
28+
for (const [issueNumber, evs] of byIssue.entries()) {
29+
evs.sort((a, b) => a.event_time.localeCompare(b.event_time));
30+
let onTs: string | null = null;
31+
for (const e of evs) {
32+
if (e.action === "labeled" && onTs === null) {
33+
onTs = e.event_time;
34+
} else if (e.action === "unlabeled" && onTs !== null) {
35+
windows.push({
36+
issue_number: issueNumber,
37+
on: onTs,
38+
off: e.event_time,
39+
});
40+
onTs = null;
41+
}
42+
}
43+
if (onTs !== null) {
44+
const closedAt = evs[0].issue_closed_at;
45+
windows.push({
46+
issue_number: issueNumber,
47+
on: onTs,
48+
off: closedAt && closedAt !== "" ? closedAt : null,
49+
});
50+
}
51+
}
52+
windows.sort((a, b) => a.on.localeCompare(b.on));
53+
return windows;
54+
}
55+
56+
export function killswitchWindowAt(
57+
windows: KillswitchWindow[],
58+
t: string
59+
): KillswitchWindow | null {
60+
const ts = new Date(t).getTime();
61+
for (const w of windows) {
62+
const on = new Date(w.on).getTime();
63+
const off = w.off === null ? Infinity : new Date(w.off).getTime();
64+
if (ts >= on && ts <= off) return w;
65+
}
66+
return null;
67+
}

torchci/pages/api/autorevert/metrics.ts

Lines changed: 72 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
import { verifyFpForPr } from "lib/autorevert/fpVerification";
2+
import {
3+
foldKillswitchWindows,
4+
KillswitchLabelEvent,
5+
killswitchWindowAt,
6+
} from "lib/autorevert/killswitchWindows";
27
import { queryClickhouseSaved } from "lib/clickhouse";
38
import { getOctokit } from "lib/github";
49
import type { NextApiRequest, NextApiResponse } from "next";
@@ -81,6 +86,7 @@ interface WeeklyMetric {
8186
human_revert_rate: number;
8287
// New metrics
8388
false_positives: number;
89+
false_negatives_killswitch: number;
8490
precision: number;
8591
recall: number;
8692
}
@@ -148,21 +154,28 @@ export default async function handler(
148154
};
149155

150156
// Run queries in parallel
151-
const [significantReverts, autorevertEvents, weeklyMetricsRaw] =
152-
await Promise.all([
153-
queryClickhouseSaved(
154-
"autorevert_significant_reverts",
155-
queryParams
156-
) as Promise<SignificantRevert[]>,
157-
queryClickhouseSaved(
158-
"autorevert_events_with_commits",
159-
queryParams
160-
) as Promise<AutorevertEvent[]>,
161-
queryClickhouseSaved(
162-
"autorevert_weekly_metrics",
163-
queryParams
164-
) as Promise<any[]>,
165-
]);
157+
const [
158+
significantReverts,
159+
autorevertEvents,
160+
weeklyMetricsRaw,
161+
killswitchEvents,
162+
] = await Promise.all([
163+
queryClickhouseSaved(
164+
"autorevert_significant_reverts",
165+
queryParams
166+
) as Promise<SignificantRevert[]>,
167+
queryClickhouseSaved(
168+
"autorevert_events_with_commits",
169+
queryParams
170+
) as Promise<AutorevertEvent[]>,
171+
queryClickhouseSaved("autorevert_weekly_metrics", queryParams) as Promise<
172+
any[]
173+
>,
174+
queryClickhouseSaved("autorevert_killswitch_windows", {
175+
stopTime: queryParams.stopTime,
176+
}) as Promise<KillswitchLabelEvent[]>,
177+
]);
178+
const killswitchWindows = foldKillswitchWindows(killswitchEvents);
166179

167180
// Build set of recovery SHAs (reverts that fixed signals)
168181
const recoveryShaSet = new Set(
@@ -191,10 +204,30 @@ export default async function handler(
191204
}
192205
}
193206

194-
// Count False Negatives: human reverts with signal recovery
195-
const falseNegatives = significantReverts.filter(
207+
// Count False Negatives: human reverts with signal recovery. Partition
208+
// out the ones where the autorevert killswitch was active at recovery
209+
// time — those are not a lambda miss; they're an upstream_infra class
210+
// (`ci: disable-autorevert` label on an open issue, see
211+
// `default.issues_label_event`). Killswitch FNs are surfaced as a
212+
// third category and excluded from the recall denominator.
213+
const allFalseNegatives = significantReverts.filter(
196214
(r) => !r.is_autorevert && r.recovery_type === "human_revert_recovery"
197215
);
216+
const falseNegatives: SignificantRevert[] = [];
217+
const falseNegativesKillswitch: Array<
218+
SignificantRevert & { killswitch_issue: number }
219+
> = [];
220+
for (const r of allFalseNegatives) {
221+
const w = killswitchWindowAt(killswitchWindows, r.recovery_time);
222+
if (w) {
223+
falseNegativesKillswitch.push({
224+
...r,
225+
killswitch_issue: w.issue_number,
226+
});
227+
} else {
228+
falseNegatives.push(r);
229+
}
230+
}
198231

199232
// Verify false positive candidates via GitHub API
200233
let verifiedFPs: VerifiedFalsePositive[] = [];
@@ -236,22 +269,32 @@ export default async function handler(
236269
const precision = tp + fp > 0 ? (tp / (tp + fp)) * 100 : 100;
237270
const recall = tp + fn > 0 ? (tp / (tp + fn)) * 100 : 100;
238271

239-
// Enhance weekly metrics with precision/recall
240-
// Group FPs by week for weekly precision calculation
272+
// Enhance weekly metrics with precision/recall.
273+
// Group FPs and killswitch-FNs by week for the weekly aggregation.
241274
const fpByWeek = new Map<string, number>();
242275
for (const fp of confirmedFPs) {
243276
const week = getWeekStart(new Date(fp.autorevert_time));
244277
fpByWeek.set(week, (fpByWeek.get(week) || 0) + 1);
245278
}
279+
const fnKsByWeek = new Map<string, number>();
280+
for (const r of falseNegativesKillswitch) {
281+
const week = getWeekStart(new Date(r.recovery_time));
282+
fnKsByWeek.set(week, (fnKsByWeek.get(week) || 0) + 1);
283+
}
246284

247285
const weeklyMetrics: WeeklyMetric[] = weeklyMetricsRaw.map((w) => {
248286
const weekFPs = fpByWeek.get(w.week) || 0;
287+
const weekFNKs = fnKsByWeek.get(w.week) || 0;
249288
const weekTP = w.autorevert_recoveries;
250-
const weekFN = w.human_revert_recoveries;
289+
// human_revert_recoveries from the saved query counts all human
290+
// reverts with signal recovery; subtract the killswitch-attributed
291+
// ones so weekly recall matches the overall recall semantics.
292+
const weekFN = Math.max(0, w.human_revert_recoveries - weekFNKs);
251293

252294
return {
253295
...w,
254296
false_positives: weekFPs,
297+
false_negatives_killswitch: weekFNKs,
255298
precision:
256299
weekTP + weekFPs > 0
257300
? Math.round((weekTP / (weekTP + weekFPs)) * 1000) / 10
@@ -272,6 +315,7 @@ export default async function handler(
272315
tp_without_signal_recovery: tpWithoutSignalRecovery,
273316
confirmed_false_positives: fp,
274317
false_negatives: fn,
318+
false_negatives_killswitch: falseNegativesKillswitch.length,
275319
// Rates
276320
precision: Math.round(precision * 10) / 10,
277321
recall: Math.round(recall * 10) / 10,
@@ -282,6 +326,7 @@ export default async function handler(
282326
},
283327
weeklyMetrics,
284328
significantReverts,
329+
killswitchWindows,
285330
falsePositives: {
286331
candidates_checked: falsePositiveCandidates.length,
287332
confirmed: confirmedFPs,
@@ -294,6 +339,13 @@ export default async function handler(
294339
signals_fixed: r.signals_fixed,
295340
reverted_pr_numbers: r.reverted_pr_numbers,
296341
})),
342+
falseNegativesKillswitch: falseNegativesKillswitch.map((r) => ({
343+
recovery_sha: r.recovery_sha,
344+
recovery_time: r.recovery_time,
345+
signals_fixed: r.signals_fixed,
346+
reverted_pr_numbers: r.reverted_pr_numbers,
347+
killswitch_issue: r.killswitch_issue,
348+
})),
297349
};
298350

299351
setCache(cacheKey, result);

0 commit comments

Comments
 (0)