Skip to content

Commit 3eaad0f

Browse files
committed
ci(drift): the shard timing-drift check exit-grades on schedule alone
`--check-drift` takes the run's event (`--event`), required. On `schedule`, the profile scripts/test-shard-timings.json is measured on, a DRIFT exits 1 as before; on every other event it prints the same text, raises one `::warning::` annotation and exits 0. A missing or empty event is refused. Claude-Session: https://claude.ai/code/session_012yK1ddAdAqfzwigRHZq3r2 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0c6a5ee commit 3eaad0f

1 file changed

Lines changed: 188 additions & 30 deletions

File tree

‎scripts/partition-test-shards.mjs‎

Lines changed: 188 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,16 @@
7575
// node scripts/partition-test-shards.mjs <turbo-ls.json> --shard N/M \
7676
// [--exclude <pkg>]...
7777
// node scripts/partition-test-shards.mjs --check-drift <run-summary.json>... \
78-
// [--label <text>]
78+
// --event <github.event_name> [--label <text>]
7979
// node scripts/partition-test-shards.mjs --self-test
8080
//
8181
// `--check-drift` is the half that keeps the dataset honest AFTER it is
8282
// written (#16173). Every Test Core shard already passes `--summarize`, so the
8383
// run it just finished has written the measured truth to `.turbo/runs/`; this
8484
// mode reads that back, compares it to what this script PREDICTED for the same
8585
// packages, raises a `::warning::` past WARN_MEASURED_OVER_PREDICTED, and reds
86-
// past MAX_MEASURED_OVER_PREDICTED. Without it the dataset
86+
// past MAX_MEASURED_OVER_PREDICTED -- on a `schedule` run; on every other event
87+
// that red is a `::warning::` and exits 0 (TIMING_GRADED_EVENT, C2). Without it the dataset
8788
// rots silently in one direction and the only instrument that notices is a
8889
// shard killed by the job timeout -- which is a shard that produced NO reading
8990
// while the rollup read green.
@@ -200,6 +201,54 @@ export const MAX_MEASURED_OVER_PREDICTED = 1.5;
200201
// the warning cannot be made unreachable by moving either end.
201202
export const WARN_MEASURED_OVER_PREDICTED = 1.3;
202203

204+
// ── WHERE THE TWO TIMING GATES EXIT-GRADE: `schedule` ALONE (C2) ──────────
205+
//
206+
// Ruling (letter C2, for both timing steps):
207+
// https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511
208+
// "a timing gate exit-grades only on the run profile its baseline was measured
209+
// on; everywhere else it reports."
210+
//
211+
// Both timing gates read one baseline: the weights `--check-drift` predicts
212+
// from, and the ceilings scripts/check-test-suite-ceilings.mjs grades against.
213+
// The refresh lane measures it on ci.yml's hourly `schedule` runs and no others
214+
// (DEFAULT_RUN_EVENT in scripts/ci/select-shard-timings-run.mjs). A
215+
// `merge_group` or `pull_request` build executes far more of the workspace on
216+
// the same runners. On the morning the refreshed dataset went live this step
217+
// redded 3 of 19 queue builds and the heads of three PRs (#22953, #22954,
218+
// #22959, filed as #22966) at 1.56-1.62x, on packages none of them touched,
219+
// while every hourly run read green. So both gates take the run's event
220+
// (`--event "$GITHUB_EVENT_NAME"` in ci.yml) and:
221+
//
222+
// schedule exit-graded, exactly as before: a red exits 1
223+
// every other REPORT ONLY (merge_group, pull_request, push,
224+
// event workflow_dispatch): the same table prints, each red becomes a
225+
// `::warning::` annotation, and the step exits 0
226+
//
227+
// A real slowdown merged to `main` reds the next hourly run. A missing or empty
228+
// `--event` is refused, never defaulted: a step that lost its event would
229+
// otherwise grade everywhere or nowhere, and say nothing either way.
230+
export const TIMING_GRADED_EVENT = 'schedule';
231+
232+
/** 'grade' on TIMING_GRADED_EVENT, 'report' on any other event; a missing event is refused. */
233+
export function timingGateMode(event) {
234+
if (typeof event !== 'string' || event.trim() === '') {
235+
throw new Error(
236+
`the run's event is ${JSON.stringify(event ?? null)}. The timing gates exit-grade on ` +
237+
`\`${TIMING_GRADED_EVENT}\` alone and report on every other event, so they need it: pass ` +
238+
'`--event "$GITHUB_EVENT_NAME"`. A missing event is refused rather than guessed.'
239+
);
240+
}
241+
return event === TIMING_GRADED_EVENT ? 'grade' : 'report';
242+
}
243+
244+
/** The one line both timing gates print to say which of the two modes this run is in. */
245+
export function timingModeNote(event) {
246+
return timingGateMode(event) === 'grade'
247+
? `exit-graded: a \`${event}\` run is the profile the baseline was measured on`
248+
: `report only: a \`${event}\` run is not the profile the baseline was measured on ` +
249+
`(\`${TIMING_GRADED_EVENT}\`), so a red reading is a warning here and the step exits 0`;
250+
}
251+
203252
// ── SHARDING ONE PACKAGE BELOW PACKAGE GRANULARITY (#16173) ────────────────
204253
//
205254
// The floor argument at the top of this file is not a caveat, it is a wall: a
@@ -1082,7 +1131,12 @@ export function escapeWorkflowCommandMessage(text) {
10821131
return String(text).replace(/%/g, '%25').replace(/\r/g, '%0D').replace(/\n/g, '%0A');
10831132
}
10841133

1085-
export function renderDriftVerdict(report, label) {
1134+
//
1135+
// `event` is the run's (ci.yml passes `$GITHUB_EVENT_NAME`): off
1136+
// TIMING_GRADED_EVENT a DRIFT prints the same text, becomes a `::warning::`
1137+
// annotation, and exits 0 (C2, see timingGateMode).
1138+
export function renderDriftVerdict(report, label, event) {
1139+
const mode = timingGateMode(event);
10861140
const skipped =
10871141
report.unpredicted.length === 0
10881142
? ''
@@ -1115,25 +1169,37 @@ export function renderDriftVerdict(report, label) {
11151169
`(${ratioOf(r)}${r.overshoot >= 0 ? '+' : ''}${r.overshoot.toFixed(1)}s)`
11161170
)
11171171
.join('\n');
1172+
const note = `\n ${timingModeNote(event)}.`;
11181173

11191174
if (report.drifted) {
1120-
return {
1121-
verdict: 'DRIFT',
1122-
exitCode: 1,
1123-
out: [],
1124-
err: [
1125-
`shard-timing-drift: DRIFT -- ${label}, ${head}.${skipped}\n` +
1126-
' Heaviest overshoots:\n' +
1127-
`${worst}\n` +
1128-
' scripts/test-shard-timings.json no longer describes this workspace, so the shard split\n' +
1129-
' is balancing a quantity that is not the runtime. Refresh it -- see\n' +
1130-
' scripts/measure-test-shard-timings.mjs for the two refresh paths -- and ⛔ do NOT\n' +
1131-
' hand-edit the dataset or raise this bound to absorb the gap. Expect the refresh to red\n' +
1132-
" this script's own balance pins if a single suite has outgrown the acceptance bound:\n" +
1133-
' that is those pins working, and the remedy they name is splitting that suite below\n' +
1134-
' package granularity, never a different shard count.',
1135-
],
1136-
};
1175+
const text =
1176+
`shard-timing-drift: DRIFT -- ${label}, ${head}.${skipped}\n` +
1177+
' Heaviest overshoots:\n' +
1178+
`${worst}\n` +
1179+
' scripts/test-shard-timings.json no longer describes this workspace, so the shard split\n' +
1180+
' is balancing a quantity that is not the runtime. Refresh it -- see\n' +
1181+
' scripts/measure-test-shard-timings.mjs for the two refresh paths -- and ⛔ do NOT\n' +
1182+
' hand-edit the dataset or raise this bound to absorb the gap. Expect the refresh to red\n' +
1183+
" this script's own balance pins if a single suite has outgrown the acceptance bound:\n" +
1184+
' that is those pins working, and the remedy they name is splitting that suite below\n' +
1185+
` package granularity, never a different shard count.${note}`;
1186+
if (mode === 'report') {
1187+
const top = report.rows[0];
1188+
return {
1189+
verdict: 'DRIFT',
1190+
exitCode: 0,
1191+
out: [
1192+
`::warning title=Test Core shard timing drift::${escapeWorkflowCommandMessage(
1193+
`${label}: ${head}, past the red bound. Heaviest overshoot: ${top.name}, predicted ` +
1194+
`${top.predicted.toFixed(1)}s, measured ${top.measured.toFixed(1)}s. Report only on a \`${event}\` run: ` +
1195+
`this step exit-grades on \`${TIMING_GRADED_EVENT}\` runs alone, the profile ` +
1196+
'scripts/test-shard-timings.json was measured on, so a drift that is real reds the next hourly run on main.'
1197+
)}`,
1198+
],
1199+
err: [text],
1200+
};
1201+
}
1202+
return { verdict: 'DRIFT', exitCode: 1, out: [], err: [text] };
11371203
}
11381204

11391205
if (report.warned) {
@@ -1149,15 +1215,15 @@ export function renderDriftVerdict(report, label) {
11491215
'Not a failure: this step stays green until the red bound.'
11501216
)}`,
11511217
],
1152-
err: [`shard-timing-drift: WARN -- ${label}, ${head}.${skipped}\n Heaviest overshoots:\n${worst}`],
1218+
err: [`shard-timing-drift: WARN -- ${label}, ${head}.${skipped}\n Heaviest overshoots:\n${worst}${note}`],
11531219
};
11541220
}
11551221

11561222
return {
11571223
verdict: 'OK',
11581224
exitCode: 0,
11591225
out: [],
1160-
err: [`shard-timing-drift: OK -- ${label}, ${head}.${skipped}`],
1226+
err: [`shard-timing-drift: OK -- ${label}, ${head}.${skipped}${note}`],
11611227
};
11621228
}
11631229

@@ -1249,11 +1315,12 @@ const SELF_TEST_BATTERIES = Object.freeze({
12491315
'file-level slice items (#16173)': 20,
12501316
'file-level slices reach vitest through OS_TEST_SHARD (#19278)': 11,
12511317
'density cap and the per-run slice decision (#22075)': 14,
1318+
'the drift step exit-grades on `schedule` alone, and reports everywhere else (C2)': 8,
12521319
});
12531320

12541321
// DELETING an entry silences that battery's floor exactly as effectively as
12551322
// zeroing it, so the roster's own size is pinned too.
1256-
const SELF_TEST_BATTERY_FLOOR = 12;
1323+
const SELF_TEST_BATTERY_FLOOR = 13;
12571324

12581325
// The key an assertion is filed under when no battery is open. It is not a
12591326
// declared battery, so it reds by the same set difference rather than silently
@@ -1898,7 +1965,7 @@ function selfTest() {
18981965
// with a workflow command would mint a real annotation on every lint run).
18991966
const label = 'Test Core (9/9)';
19001967
check(() => {
1901-
const v = renderDriftVerdict(tierOf(131), label);
1968+
const v = renderDriftVerdict(tierOf(131), label, TIMING_GRADED_EVENT);
19021969
const commands = v.out.filter((line) => line.startsWith('::'));
19031970
if (v.verdict !== 'WARN' || v.exitCode !== 0 || commands.length !== 1) {
19041971
throw new Error(
@@ -1910,19 +1977,19 @@ function selfTest() {
19101977
}
19111978
});
19121979
check(() => {
1913-
const v = renderDriftVerdict(tierOf(151), label);
1980+
const v = renderDriftVerdict(tierOf(151), label, TIMING_GRADED_EVENT);
19141981
if (v.verdict !== 'DRIFT' || v.exitCode !== 1 || v.out.some((line) => line.startsWith('::'))) {
19151982
throw new Error(`drift warning: a red rendered ${v.verdict} exit ${v.exitCode}, ${v.out.length} stdout line(s)`);
19161983
}
19171984
});
19181985
check(() => {
19191986
for (const [seconds, expected] of [[118, 'OK'], [WARN_MEASURED_OVER_PREDICTED * 100, 'OK']]) {
1920-
const v = renderDriftVerdict(tierOf(seconds), label);
1987+
const v = renderDriftVerdict(tierOf(seconds), label, TIMING_GRADED_EVENT);
19211988
if (v.verdict !== expected || v.exitCode !== 0 || v.out.length !== 0) {
19221989
throw new Error(`drift warning: ${seconds / 100}x rendered ${v.verdict} with ${v.out.length} stdout line(s)`);
19231990
}
19241991
}
1925-
const nm = renderDriftVerdict(driftReport(replayed.samples, driftTimings), label);
1992+
const nm = renderDriftVerdict(driftReport(replayed.samples, driftTimings), label, TIMING_GRADED_EVENT);
19261993
if (nm.verdict !== 'NOT MEASURED' || nm.exitCode !== 0 || nm.out.length !== 0) {
19271994
throw new Error(`drift warning: a replay-only shard rendered ${nm.verdict}`);
19281995
}
@@ -1936,6 +2003,85 @@ function selfTest() {
19362003
}
19372004
});
19382005

2006+
// -- EXIT-GRADED ON `schedule` ALONE (C2) --------------------------------
2007+
//
2008+
// The baseline is measured on hourly `schedule` runs, so only those exit 1
2009+
// on a DRIFT; every other event prints the same text, annotates a warning
2010+
// and exits 0. Read as rendered text, like the tier above: printing a
2011+
// `::warning::` from here would mint a real annotation on every lint run.
2012+
battery('the drift step exit-grades on `schedule` alone, and reports everywhere else (C2)');
2013+
const offSchedule = ['merge_group', 'pull_request', 'push', 'workflow_dispatch'];
2014+
2015+
check(() => {
2016+
if (TIMING_GRADED_EVENT !== 'schedule' || timingGateMode('schedule') !== 'grade') {
2017+
throw new Error(`C2: the graded event is ${TIMING_GRADED_EVENT}, and schedule reads ${timingGateMode('schedule')}`);
2018+
}
2019+
});
2020+
check(() => {
2021+
const graded = offSchedule.filter((e) => timingGateMode(e) !== 'report');
2022+
if (graded.length) throw new Error(`C2: ${graded.join(', ')} exit-grade, but only schedule may`);
2023+
});
2024+
// A step that lost its event is refused, never defaulted to either mode.
2025+
check(() => {
2026+
const accepted = [undefined, null, '', ' '].filter((e) => !threw(() => timingGateMode(e)));
2027+
if (accepted.length) throw new Error(`C2: a missing event was read as a mode: ${JSON.stringify(accepted)}`);
2028+
if (!threw(() => renderDriftVerdict(tierOf(151), label))) throw new Error('C2: a render with no event was graded');
2029+
});
2030+
check(() => {
2031+
const v = renderDriftVerdict(tierOf(151), label, 'schedule');
2032+
if (v.verdict !== 'DRIFT' || v.exitCode !== 1 || v.out.length !== 0 || !v.err[0].includes('exit-graded')) {
2033+
throw new Error(`C2: a schedule DRIFT rendered ${v.verdict} exit ${v.exitCode}, ${v.out.length} stdout line(s)`);
2034+
}
2035+
});
2036+
check(() => {
2037+
for (const event of offSchedule) {
2038+
const v = renderDriftVerdict(tierOf(151), label, event);
2039+
const commands = v.out.filter((line) => line.startsWith('::'));
2040+
if (v.verdict !== 'DRIFT' || v.exitCode !== 0 || commands.length !== 1) {
2041+
throw new Error(`C2: a ${event} DRIFT rendered ${v.verdict} exit ${v.exitCode} with ${commands.length} command(s)`);
2042+
}
2043+
if (!commands[0].startsWith('::warning ') || !commands[0].includes(label) || !commands[0].includes(event) || commands[0].includes('\n')) {
2044+
throw new Error(`C2: a ${event} DRIFT is not one ::warning line naming the shard and the event: ${commands[0]}`);
2045+
}
2046+
}
2047+
});
2048+
// The SAME table on every event: only the mode line differs.
2049+
check(() => {
2050+
const body = (v) => v.err[0].split('\n').slice(0, -1).join('\n');
2051+
const graded = renderDriftVerdict(tierOf(151), label, 'schedule');
2052+
const reported = renderDriftVerdict(tierOf(151), label, 'merge_group');
2053+
if (body(graded) !== body(reported) || !body(graded).includes('Heaviest overshoots:')) {
2054+
throw new Error(`C2: the report-only text is not the graded text:\n${graded.err[0]}\n---\n${reported.err[0]}`);
2055+
}
2056+
if (!reported.err[0].endsWith('so a red reading is a warning here and the step exits 0.')) {
2057+
throw new Error(`C2: the report-only text does not say so:\n${reported.err[0]}`);
2058+
}
2059+
});
2060+
check(() => {
2061+
// The tiers under the red are unchanged off schedule: WARN annotates, OK and NOT MEASURED say nothing on stdout.
2062+
const warn = renderDriftVerdict(tierOf(131), label, 'pull_request');
2063+
const ok = renderDriftVerdict(tierOf(118), label, 'push');
2064+
const nm = renderDriftVerdict(driftReport(replayed.samples, driftTimings), label, 'merge_group');
2065+
if (warn.verdict !== 'WARN' || warn.exitCode !== 0 || warn.out.length !== 1 || !warn.out[0].startsWith('::warning ')) {
2066+
throw new Error(`C2: a pull_request WARN rendered ${warn.verdict} exit ${warn.exitCode}`);
2067+
}
2068+
if (ok.verdict !== 'OK' || ok.exitCode !== 0 || ok.out.length !== 0 || !ok.err[0].includes('report only')) {
2069+
throw new Error(`C2: a push OK rendered ${ok.verdict} exit ${ok.exitCode}:\n${ok.err[0]}`);
2070+
}
2071+
if (nm.verdict !== 'NOT MEASURED' || nm.exitCode !== 0 || nm.out.length !== 0) {
2072+
throw new Error(`C2: a merge_group replay-only shard rendered ${nm.verdict}`);
2073+
}
2074+
});
2075+
// The CLI: `--event` is read, and its absence is refused before any summary is.
2076+
check(() => {
2077+
const parsed = parseDriftArgs(['--check-drift', 'a.json', '--label', 'L', '--event', 'merge_group']);
2078+
if (parsed.event !== 'merge_group' || parsed.inputs.join() !== 'a.json' || parsed.label !== 'L') {
2079+
throw new Error(`C2: --check-drift parsed as ${JSON.stringify(parsed)}`);
2080+
}
2081+
if (!threw(() => parseDriftArgs(['--check-drift', 'a.json']))) throw new Error('C2: --check-drift with no --event was accepted');
2082+
if (!threw(() => parseDriftArgs(['--check-drift', 'a.json', '--event', '']))) throw new Error('C2: an empty --event was accepted');
2083+
});
2084+
19392085
// -- FILE-LEVEL SLICE ITEMS (#16173) ------------------------------------
19402086
//
19412087
// The grammar, its refusals, and the two joins that read it. The balancing
@@ -2474,19 +2620,31 @@ function selfTest() {
24742620
// test tasks were all cache replays has said nothing about the dataset, and
24752621
// reporting that as OK is the #4690 shape -- a check that read nothing
24762622
// reporting as a check that found nothing wrong.
2477-
function checkDrift(argv) {
2623+
//
2624+
// `--event` is required (C2): the run's event decides whether a DRIFT exits 1
2625+
// (`schedule`) or is reported as a warning (every other event), and a missing
2626+
// one is refused by timingGateMode rather than defaulted either way.
2627+
export function parseDriftArgs(argv) {
24782628
const inputs = [];
24792629
let label = 'this shard';
2630+
let event = null;
24802631
for (let i = 0; i < argv.length; i++) {
24812632
const arg = argv[i];
24822633
if (arg === '--check-drift') continue;
24832634
else if (arg === '--label') label = argv[++i];
2635+
else if (arg === '--event') event = argv[++i];
24842636
else if (arg.startsWith('--')) throw new Error(`unrecognized argument: ${arg}`);
24852637
else inputs.push(arg);
24862638
}
2639+
timingGateMode(event);
2640+
return { inputs, label, event };
2641+
}
2642+
2643+
function checkDrift(argv) {
2644+
const { inputs, label, event } = parseDriftArgs(argv);
24872645
if (inputs.length === 0) {
24882646
console.error(
2489-
'usage: partition-test-shards.mjs --check-drift <run-summary.json>... [--label <text>]'
2647+
'usage: partition-test-shards.mjs --check-drift <run-summary.json>... --event <github.event_name> [--label <text>]'
24902648
);
24912649
process.exit(1);
24922650
}
@@ -2515,7 +2673,7 @@ function checkDrift(argv) {
25152673
}
25162674
const timings = loadTimings();
25172675
const report = driftReport(merged, timings, MAX_MEASURED_OVER_PREDICTED, observedSlices);
2518-
const rendered = renderDriftVerdict(report, label);
2676+
const rendered = renderDriftVerdict(report, label, event);
25192677
for (const line of rendered.out) console.log(line);
25202678
for (const line of rendered.err) console.error(line);
25212679
if (rendered.exitCode !== 0) process.exit(rendered.exitCode);

0 commit comments

Comments
 (0)