From 83231e391906c668e37aebc6f201c8d0204d0010 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 14:30:27 +0000 Subject: [PATCH 1/2] ci(timing gates): both timing gates report on every event, schedule included (WIP: scripts) The grading mode goes: TIMING_GRADED_EVENT is removed, timingGateMode reads report for every event and still refuses a missing one, and a package over its ceiling or a shard past its drift bound is one ::warning:: annotation and exit 0 on every event. Headers carry the ruling URL and the two-sentence diagnosis; the graded-mode self-test cases became report-mode cases, the refusal cases are unchanged. The ci.yml step comments follow in the next commit. Claude-Session: https://claude.ai/code/session_01LqNgdPZB2Jnr5HpGbaqLeB Co-authored-by: Claude --- scripts/check-test-suite-ceilings.mjs | 296 ++++++++++++++------------ scripts/partition-test-shards.mjs | 276 +++++++++++++----------- 2 files changed, 315 insertions(+), 257 deletions(-) diff --git a/scripts/check-test-suite-ceilings.mjs b/scripts/check-test-suite-ceilings.mjs index cc855740c0d..9b3bb91fc37 100644 --- a/scripts/check-test-suite-ceilings.mjs +++ b/scripts/check-test-suite-ceilings.mjs @@ -2,10 +2,10 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * Per-package suite-duration ratchet (#16468): a `schedule` run in which a - * package's test suite took longer than its measured ceiling is RED, naming the - * package and its slowest test files; every other run reports the same reading - * as a warning; and a ceiling rises only by a ruling. + * Per-package suite-duration ceilings (#16468): a run in which a package's test + * suite took longer than its measured ceiling is a WARNING, naming the package + * and its slowest test files, on every event and `schedule` included -- a + * reading, never a red (ruling E, below); and a ceiling rises only by a ruling. * * node scripts/check-test-suite-ceilings.mjs --captures --event [--label ] * node scripts/check-test-suite-ceilings.mjs --self-test @@ -40,37 +40,40 @@ * - A ceiling rises only by a ruling: RULED_CEILING_RAISES, each entry naming * the ruling's comment URL. Nothing else in this file moves a ceiling. * - * ## Exit-graded on `schedule` alone; every other event reports (C2) + * ## Report-only on every event, `schedule` included (ruling E, superseding C2) * - * Ruling (letter C2, for both timing steps, plus an absolute floor): - * https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511 - * "a timing gate exit-grades only on the run profile its baseline was measured - * on; everywhere else it reports." The ceilings come from the refresh window, - * which reads ci.yml's hourly `schedule` runs alone; those replay part of the - * workspace from the turbo cache, while a merge-queue or PR build executes far - * more of it on the same runners. Graded against that window, 9 of the 19 queue - * builds on the morning the table went live redded on packages no PR in them - * touched, and PR heads did too, while the hourly run read green. + * Ruling (#22949, letter E, item 1; the maintainer's word 「22949 同意」): + * https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6125775640 + * It retires the `schedule`-only red that letter C2 + * (https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511) + * left in place. The diagnosis, in two sentences. A wall-clock reading on a + * shared runner measures the run's load as much as the code: the same package + * reads 82 s on a quiet hour (most packages replayed) and 209 s on a busy one + * (the whole workspace executing), 2.56x, while the ceiling formula (window max + * x 1.25) allows 2.0x of weight. The ceilings come from a window that was mostly + * quiet-density and every event, `schedule` included, meets both densities, so + * busy hours red the same near-ceiling packages whatever their code does: + * density, not code. * - * So `--event` is required, and the mode is TIMING_GRADED_EVENT's, shared with - * the drift step (timingGateMode in partition-test-shards.mjs): + * So `--event` is required, and the mode is timingGateMode's, shared with the + * drift step (partition-test-shards.mjs) and 'report' on every event: * - * schedule exit-graded, as before: a package over its ceiling exits 1 - * with an `::error::` annotation naming it - * every other REPORT ONLY (merge_group, pull_request, push, - * event workflow_dispatch): the same table prints, each package over - * its ceiling is a `::warning::` annotation, and it exits 0 + * every event REPORT ONLY (schedule, merge_group, pull_request, push, + * workflow_dispatch): the table prints, each package over its + * ceiling is a `::warning::` annotation, and it exits 0 * - * A real slowdown merged to `main` reds the next hourly run, which names the - * package and its slowest files. A refusal -- a malformed raise record or - * ceiling table, or a missing `--event` -- is not a timing reading, does not - * vary with runner load, and exits 1 on every event. + * A refusal -- a malformed raise record or ceiling table, or a missing + * `--event` -- is not a timing reading, does not vary with runner load, and + * exits 1 on every event. The table stays a reading: the formula, the floor and + * RULED_CEILING_RAISES are untouched, and regression detection for the wall + * moves to the dataset refresh's diff (the refresh lane, E2, #23059) and to the + * per-file budget, neither of which is this file's. * - * ⚠ The hourly run grades only the packages it EXECUTES: a turbo cache replay - * carries no window, so it is never graded. A package the hourly run replays on - * every run (ci.yml's `timeout-minutes` note: "scheduled runs mostly replay the - * cache their commit's push run saved") is graded on any hourly run that does - * execute it and reported everywhere else; nothing here forces one to. + * ⚠ A run reads only the packages it EXECUTES: a turbo cache replay carries no + * window, so it is never read. A package the hourly run replays on every run + * (ci.yml's `timeout-minutes` note: "scheduled runs mostly replay the cache + * their commit's push run saved") is read on any run that does execute it; + * nothing here forces one to. * * ## The absolute floor: CEILING_FLOOR_SECONDS, 20 s * @@ -135,17 +138,17 @@ * ratchet's ceilings. The ceilings are measured at the density the dataset's * own split runs at, which the live split holds under its density cap * (`densityCap()` in partition-test-shards.mjs). A future split that raises - * the density will read red here on packages it did not touch; that red is - * the split making suites slower, and the remedy is the split, not a raise. + * the density will read OVER here on packages it did not touch; that warning + * is the split making suites slower, and the remedy is the split, not a raise. * * ## Exit codes * - * 0 OK, or NOT MEASURED (no table yet; nothing executed; no captures); a - * new package or one below the floor never makes it 1; and on any event - * but `schedule`, a package over its ceiling (reported as a warning) - * 1 a package over its ceiling on a `schedule` run, or a refusal on any - * event (a malformed ceiling table -- a package the dataset knows that - * lost its row among them -- or raise record, or no `--event`) + * 0 OK, NOT MEASURED (no table yet; nothing executed; no captures), or a + * package over its ceiling (reported as a warning), on every event; a new + * package or one below the floor never makes it 1 + * 1 a refusal, on any event (a malformed ceiling table -- a package the + * dataset knows that lost its row among them -- or raise record, or no + * `--event`) */ import { readFileSync, readdirSync, statSync } from 'node:fs'; @@ -154,18 +157,13 @@ import { fileURLToPath } from 'node:url'; import process from 'node:process'; import { isEntrypoint } from './invoked-as.mjs'; -import { - TIMING_GRADED_EVENT, - escapeWorkflowCommandMessage, - timingGateMode, - timingModeNote, -} from './partition-test-shards.mjs'; +import { escapeWorkflowCommandMessage, timingGateMode, timingModeNote } from './partition-test-shards.mjs'; const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); const DATASET_PATH = path.join(REPO_ROOT, 'scripts', 'test-shard-timings.json'); const DATASET_REL = 'scripts/test-shard-timings.json'; -/** How many of a red package's slowest test files the verdict names. */ +/** How many of an over-ceiling package's slowest test files the verdict names. */ export const SLOWEST_FILES_NAMED = 5; /** @@ -301,7 +299,7 @@ export function readCeilingTable(dataset) { * Returns `{ table, rows, problems }`; each row is one executed package with a * `status`: * - * over graded, over its effective ceiling RED on `schedule`, a warning elsewhere + * over graded, over its effective ceiling a warning, on every event * ok graded, within its effective ceiling * below-floor effective ceiling under CEILING_FLOOR_SECONDS never red * new not in the dataset, and no ruled raise no ceiling, not red @@ -372,9 +370,9 @@ export function gradeCeilings({ merged, dataset, raises = RULED_CEILING_RAISES } // ── Rendering ───────────────────────────────────────────────────────────── -// A new package is not among them (ruling Q2 B): it is red only once a ceiling -// exists for it and it runs over. -const RED_STATUSES = new Set(['over']); +// A new package is not among them (ruling Q2 B): it is reported only once a +// ceiling exists for it and it runs over. +const OVER_STATUSES = new Set(['over']); function sliceNote(row) { if (!row.slices) return ''; @@ -402,12 +400,12 @@ function ceilingText(row) { * self-test can read it: `out` is stdout (workflow-command annotations), * `err` the human log. * - * `event` is the run's (`--event`, required). Off TIMING_GRADED_EVENT a - * package over its ceiling prints the same table, is a `::warning::` - * annotation, and exits 0 (C2); a refusal exits 1 on every event. + * `event` is the run's (`--event`, required) and is named in the mode line. On + * every event a package over its ceiling prints the table, is a `::warning::` + * annotation, and exits 0; a refusal exits 1 on every event. */ export function renderCeilingVerdict(report, label, event) { - const mode = timingGateMode(event); + timingGateMode(event); const head = `suite-ceiling`; if (report.problems.length > 0) { return { @@ -436,7 +434,7 @@ export function renderCeilingVerdict(report, label, event) { } const graded = report.rows.filter((r) => r.status === 'ok' || r.status === 'over'); - const red = report.rows.filter((r) => RED_STATUSES.has(r.status)); + const overs = report.rows.filter((r) => OVER_STATUSES.has(r.status)); const lines = report.rows.map((r) => { switch (r.status) { case 'ok': @@ -476,7 +474,7 @@ export function renderCeilingVerdict(report, label, event) { const floorNote = belowFloor ? `; ${belowFloor} below the ${CEILING_FLOOR_SECONDS}s floor, never red` : ''; const note = `\n ${timingModeNote(event)}.`; - if (red.length === 0) { + if (overs.length === 0) { const verdict = graded.length === 0 ? 'NOT MEASURED' : 'OK'; const summaryLine = graded.length === 0 @@ -491,7 +489,7 @@ export function renderCeilingVerdict(report, label, event) { }; } - const detail = red + const detail = overs .map( (r) => ` ${r.name}: ${r.seconds.toFixed(2)}s measured against its ${r.ceiling.toFixed(2)}s ceiling ` + @@ -502,32 +500,23 @@ export function renderCeilingVerdict(report, label, event) { `${r.name}: ${r.seconds.toFixed(2)}s, over its ${r.ceiling.toFixed(2)}s ceiling. Slowest file: ` + `${r.files[0] ? `${r.files[0].file} ${(r.files[0].ms / 1000).toFixed(2)}s` : '(none captured)'}.`; const text = - `${head}: OVER -- ${label}: ${red.length} package(s) over their suite-duration ceiling.\n` + + `${head}: OVER -- ${label}: ${overs.length} package(s) over their suite-duration ceiling.\n` + `${detail}\n` + ` ${basis}.\n` + - ' Make the suite faster, or take the raise to a ruling: a ceiling rises only by an entry in\n' + + ' If this is a real slowdown, make the suite faster, or take the raise to a ruling: a ceiling rises only by an entry in\n' + ' RULED_CEILING_RAISES (scripts/check-test-suite-ceilings.mjs) naming the ruling comment. ⛔ Never\n' + ` hand-edit \`ceilings\` in ${DATASET_REL} -- it is generated, and the next refresh holds what it finds.\n` + ` Every executed package this run:\n${lines.join('\n')}${stale}${note}`; - if (mode === 'report') { - return { - verdict: 'OVER', - exitCode: 0, - out: red.map( - (r) => - `::warning title=Suite-duration ceiling::${escapeWorkflowCommandMessage( - `${reading(r)} Report only on a \`${event}\` run: this gate exit-grades on \`${TIMING_GRADED_EVENT}\` ` + - 'runs alone, the profile its ceilings were measured on, so a slowdown that is real reds the next ' + - 'hourly run on main.' - )}` - ), - err: [text], - }; - } return { verdict: 'OVER', - exitCode: 1, - out: red.map((r) => `::error title=Suite-duration ceiling::${escapeWorkflowCommandMessage(reading(r))}`), + exitCode: 0, + out: overs.map( + (r) => + `::warning title=Suite-duration ceiling::${escapeWorkflowCommandMessage( + `${reading(r)} Report only, on every event (this is a \`${event}\` run): a suite's wall on a shared ` + + "runner measures the run's load as much as the package, so this is a reading, not a failure." + )}` + ), err: [text], }; } @@ -627,7 +616,7 @@ let selfTestReachedVerdict = false; // adding cases is ordinary work. A battery below its floor means cases stopped // running; the remedy is to find what stopped registering, never to lower it. const SELF_TEST_BATTERIES = Object.freeze({ - 'over the ceiling is red, naming the package and its slowest files': 6, + 'over the ceiling is a warning, naming the package and its slowest files': 6, 'provisional prints "no ceiling: provisional" and is not red': 3, 'a new package prints "no ceiling: new package" and is not red; a known one that lost its row is refused': 10, 'no ceiling table yet is NOT MEASURED, never red': 5, @@ -635,7 +624,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'a ruled raise lifts a ceiling, and only a well-formed one does': 8, 'a malformed ceiling table is refused, never read as a pass': 4, 'the absolute floor: a ceiling under CEILING_FLOOR_SECONDS is never red, and prints "below floor"': 8, - 'off `schedule` the gate reports, never reds (C2)': 8, + 'on every event, `schedule` included, the gate reports and never reds (ruling E)': 8, }); // Deleting an entry silences that battery's floor as effectively as zeroing @@ -689,21 +678,21 @@ function selfTest() { provenance: { measuredAt: '2026-10-12', ceilingHeadroom: 1.25 }, ...extra, }); - // `event` defaults to the graded one, so every battery above C2's reads the - // exit-graded verdict it was written against. - const run = (captures, ds = dataset(), raises = {}, event = TIMING_GRADED_EVENT) => { + // `event` defaults to `schedule`: every event reads the same, and `schedule` is + // the one that used to exit-grade, so the batteries render with the strictest case. + const run = (captures, ds = dataset(), raises = {}, event = 'schedule') => { const report = gradeCeilings({ merged: foldCaptures(captures), dataset: ds, raises }); return { report, ...renderCeilingVerdict(report, 'Test Core', event) }; }; const status = (r, name) => r.report.rows.find((row) => row.name === name)?.status; const raiseUrl = 'https://github.com/objectstack-ai/objectstack/issues/1#issuecomment-2'; - battery('over the ceiling is red, naming the package and its slowest files'); + battery('over the ceiling is a warning, naming the package and its slowest files'); const over = run([ capture('1/6', [['a', 151]], [file('a', 'fast.test.ts', 1_000), file('a', 'slow.test.ts', 90_000), file('a', 'mid.test.ts', 30_000)]), ]); check(() => { - if (over.exitCode !== 1 || over.verdict !== 'OVER') throw new Error(`over: exit ${over.exitCode}, verdict ${over.verdict}`); + if (over.exitCode !== 0 || over.verdict !== 'OVER') throw new Error(`over: exit ${over.exitCode}, verdict ${over.verdict}`); }); check(() => { if (!over.err[0].includes('a: 151.00s measured against its 150.00s ceiling')) throw new Error(`over: the package is not named:\n${over.err[0]}`); @@ -715,8 +704,8 @@ function selfTest() { if (!(i >= 0 && i < j && j < k)) throw new Error(`over: the slowest files are not named slowest first:\n${text}`); }); check(() => { - if (!over.out.some((l) => l.startsWith('::error title=Suite-duration ceiling::a: 151.00s'))) { - throw new Error(`over: no error annotation names the package: ${JSON.stringify(over.out)}`); + if (over.out.length !== 1 || !over.out[0].startsWith('::warning title=Suite-duration ceiling::a: 151.00s')) { + throw new Error(`over: no single warning annotation names the package: ${JSON.stringify(over.out)}`); } }); check(() => { @@ -725,7 +714,7 @@ function selfTest() { if (at.exitCode !== 0 || at.verdict !== 'OK' || status(at, 'a') !== 'ok') throw new Error(`over: a run AT its ceiling read ${at.verdict}`); }); check(() => { - // Only the files of the red package are named, at most SLOWEST_FILES_NAMED. + // Only the files of the over-ceiling package are named, at most SLOWEST_FILES_NAMED. const many = Array.from({ length: 9 }, (_, i) => file('a', `f${i}.test.ts`, 1000 * (i + 1))); const r = run([capture('1/6', [['a', 999], ['b', 1]], [...many, file('b', 'other.test.ts', 999_999)])]); const row = r.report.rows.find((x) => x.name === 'a'); @@ -761,15 +750,16 @@ function selfTest() { if (!fresh.err[0].includes('brand-new: 5.00s -- no ceiling: new package')) throw new Error(`new: the line is wrong:\n${fresh.err[0]}`); }); check(() => { - // Beside a package over its ceiling, only that one is red and named. + // Beside a package over its ceiling, only that one is reported and named. const both = run([capture('2/6', [['brand-new', 9999], ['a', 151]])]); if ( - both.exitCode !== 1 || + both.exitCode !== 0 || + both.verdict !== 'OVER' || !both.err[0].includes('1 package(s) over their suite-duration ceiling') || both.out.length !== 1 || both.out.some((l) => l.includes('brand-new')) ) { - throw new Error(`new: a new package was counted red beside an over one:\n${both.err[0]}\n${JSON.stringify(both.out)}`); + throw new Error(`new: a new package was counted over beside an over one:\n${both.err[0]}\n${JSON.stringify(both.out)}`); } }); check(() => { @@ -862,10 +852,10 @@ function selfTest() { if (r.exitCode !== 0 || status(r, 'cli') !== 'ok') throw new Error(`slices: a skewed slice redded a suite within its ceiling (${r.verdict})`); }); check(() => { - // The sum over the whole ceiling is red, said as a summed set. + // The sum over the whole ceiling is reported, said as a summed set. const r = run([capture('2/6', [['cli', 700, [1, 3]]]), capture('3/6', [['cli', 700, [2, 3]]]), capture('4/6', [['cli', 700, [3, 3]]])]); - if (r.exitCode !== 1 || !r.err[0].includes('cli: 2100.00s measured against its 2000.00s ceiling') || !r.err[0].includes('all 3 slices, summed across shards')) { - throw new Error(`slices: an over-ceiling sum was not red, or not said as a sum:\n${r.err[0]}`); + if (r.exitCode !== 0 || r.verdict !== 'OVER' || !r.err[0].includes('cli: 2100.00s measured against its 2000.00s ceiling') || !r.err[0].includes('all 3 slices, summed across shards')) { + throw new Error(`slices: an over-ceiling sum was not reported, or not said as a sum:\n${r.err[0]}`); } }); check(() => { @@ -895,7 +885,7 @@ function selfTest() { }); check(() => { const r = run([capture('1/6', [['a', 201]])], dataset(), { a: { seconds: 200, ruling: raiseUrl } }); - if (r.exitCode !== 1 || !r.err[0].includes(raiseUrl)) throw new Error(`raise: over the raise was not red, or the ruling not named:\n${r.err[0]}`); + if (r.exitCode !== 0 || r.verdict !== 'OVER' || !r.err[0].includes(raiseUrl)) throw new Error(`raise: over the raise was not reported, or the ruling not named:\n${r.err[0]}`); }); check(() => { // A ruled ceiling for a new package grades it before the first refresh after it lands could. @@ -905,14 +895,14 @@ function selfTest() { check(() => { // ...and one it can run over: a ruled ceiling is a ceiling, new package or not. const r = run([capture('1/6', [['brand-new', 31]])], dataset(), { 'brand-new': { seconds: 30, ruling: raiseUrl } }); - if (r.exitCode !== 1 || status(r, 'brand-new') !== 'over' || !r.err[0].includes('brand-new: 31.00s measured against its 30.00s ceiling')) { - throw new Error(`raise: a new package over its ruled ceiling was not red (${status(r, 'brand-new')}):\n${r.err[0]}`); + if (r.exitCode !== 0 || status(r, 'brand-new') !== 'over' || !r.err[0].includes('brand-new: 31.00s measured against its 30.00s ceiling')) { + throw new Error(`raise: a new package over its ruled ceiling was not reported (${status(r, 'brand-new')}):\n${r.err[0]}`); } }); check(() => { // A raise at or under the generated ceiling changes nothing, and says so. const r = run([capture('1/6', [['a', 151]])], dataset(), { a: { seconds: 120, ruling: raiseUrl } }); - if (r.exitCode !== 1 || !r.err[0].includes('changing nothing')) throw new Error(`raise: a stale raise lowered or silently stayed:\n${r.err[0]}`); + if (r.exitCode !== 0 || r.verdict !== 'OVER' || !r.err[0].includes('changing nothing')) throw new Error(`raise: a stale raise lowered or silently stayed:\n${r.err[0]}`); }); check(() => { // No ruling, no raise: the record is refused and the run is red. @@ -969,30 +959,33 @@ function selfTest() { } }); check(() => { - // Off schedule too: below the floor is never a warning either. + // On any event: below the floor is never a warning either. const r = run([capture('1/6', [['tiny', 4.2], ['a', 10]])], floorSet, {}, 'merge_group'); if (r.exitCode !== 0 || status(r, 'tiny') !== 'below-floor' || r.out.length !== 0) { - throw new Error(`floor: a below-floor package annotated off schedule: ${JSON.stringify(r.out)}`); + throw new Error(`floor: a below-floor package annotated on merge_group: ${JSON.stringify(r.out)}`); } }); check(() => { const r = run([capture('1/6', [['small', 25]])], floorSet); - if (r.exitCode !== 1 || r.verdict !== 'OVER' || !r.out.some((l) => l.startsWith('::error title=Suite-duration ceiling::small: 25.00s, over its 22.71s ceiling'))) { - throw new Error(`floor: a 22.71s ceiling run at 25s on schedule was not red: ${r.verdict} ${JSON.stringify(r.out)}`); + if (r.exitCode !== 0 || r.verdict !== 'OVER' || !r.out.some((l) => l.startsWith('::warning title=Suite-duration ceiling::small: 25.00s, over its 22.71s ceiling'))) { + throw new Error(`floor: a 22.71s ceiling run at 25s on schedule was not a warning: ${r.verdict} ${JSON.stringify(r.out)}`); } }); check(() => { - const r = run([capture('1/6', [['small', 25]])], floorSet, {}, 'merge_group'); - const errors = r.out.filter((l) => l.startsWith('::error')); - if (r.exitCode !== 0 || errors.length || r.out.length !== 1 || !r.out[0].startsWith('::warning title=Suite-duration ceiling::small: 25.00s')) { - throw new Error(`floor: a 22.71s ceiling run at 25s off schedule was not a warning alone: exit ${r.exitCode} ${JSON.stringify(r.out)}`); + // The same on every event: one warning, never an error, exit 0. + for (const event of ['schedule', 'merge_group', 'pull_request', 'push', 'workflow_dispatch']) { + const r = run([capture('1/6', [['small', 25]])], floorSet, {}, event); + const errors = r.out.filter((l) => l.startsWith('::error')); + if (r.exitCode !== 0 || errors.length || r.out.length !== 1 || !r.out[0].startsWith('::warning title=Suite-duration ceiling::small: 25.00s')) { + throw new Error(`floor: a 22.71s ceiling run at 25s on ${event} was not a warning alone: exit ${r.exitCode} ${JSON.stringify(r.out)}`); + } } }); check(() => { // AT the floor is graded (strict `<`), and so is a below-floor ceiling a ruled raise lifts to it. const at = run([capture('1/6', [['edge', CEILING_FLOOR_SECONDS + 1]])], floorSet); const raised = run([capture('1/6', [['tiny', 26]])], floorSet, { tiny: { seconds: 25, ruling: raiseUrl } }); - if (status(at, 'edge') !== 'over' || at.exitCode !== 1 || status(raised, 'tiny') !== 'over' || raised.exitCode !== 1) { + if (status(at, 'edge') !== 'over' || at.exitCode !== 0 || status(raised, 'tiny') !== 'over' || raised.exitCode !== 0) { throw new Error(`floor: AT the floor read ${status(at, 'edge')}, a raised tiny read ${status(raised, 'tiny')}`); } }); @@ -1004,71 +997,97 @@ function selfTest() { } }); - battery('off `schedule` the gate reports, never reds (C2)'); - const offSchedule = ['merge_group', 'pull_request', 'push', 'workflow_dispatch']; + battery('on every event, `schedule` included, the gate reports and never reds (ruling E)'); + const events = ['schedule', 'merge_group', 'pull_request', 'push', 'workflow_dispatch']; const twoOver = [capture('1/6', [['a', 151], ['b', 90]], [file('a', 'slow.test.ts', 90_000)])]; check(() => { - for (const event of offSchedule) { + // `schedule` is the case that used to exit 1: it is two warnings now, like the rest. + for (const event of events) { const r = run(twoOver, dataset(), {}, event); const warnings = r.out.filter((l) => l.startsWith('::warning title=Suite-duration ceiling::')); if (r.exitCode !== 0 || r.verdict !== 'OVER' || warnings.length !== 2 || r.out.length !== 2) { - throw new Error(`C2: two packages over on ${event} read ${r.verdict}, exit ${r.exitCode}, ${JSON.stringify(r.out)}`); + throw new Error(`ruling E: two packages over on ${event} read ${r.verdict}, exit ${r.exitCode}, ${JSON.stringify(r.out)}`); } } }); check(() => { - const r = run(twoOver, dataset(), {}, 'merge_group'); - if (r.out.some((l) => l.startsWith('::error')) || r.out.some((l) => l.includes('\n')) || !r.out.every((l) => l.includes('merge_group'))) { - throw new Error(`C2: an off-schedule annotation is an error, spans lines, or does not name the event: ${JSON.stringify(r.out)}`); + // Each annotation is one line, names the event it was raised on, and is never an error. + for (const event of events) { + const r = run(twoOver, dataset(), {}, event); + if (r.out.some((l) => l.startsWith('::error')) || r.out.some((l) => l.includes('\n')) || !r.out.every((l) => l.includes(event))) { + throw new Error(`ruling E: a ${event} annotation is an error, spans lines, or does not name the event: ${JSON.stringify(r.out)}`); + } } }); check(() => { - const r = run(twoOver); - if (r.exitCode !== 1 || r.out.length !== 2 || !r.out.every((l) => l.startsWith('::error title=Suite-duration ceiling::'))) { - throw new Error(`C2: two packages over on schedule were not red: exit ${r.exitCode} ${JSON.stringify(r.out)}`); + // The sweep that would catch a grade creeping back in: no scenario, on no event, raises an error or exits non-zero. + const { ceilings: _x, uncapped: _y, ...noTable } = dataset(); + const scenarios = [ + [twoOver, dataset()], + [[capture('1/6', [['a', 10]])], dataset()], + [[capture('1/6', [['small', 25], ['tiny', 400]])], dataset({ packages: { ...dataset().packages, small: 18, tiny: 3 }, ceilings: { ...dataset().ceilings, small: 22.71, tiny: 3.4 } })], + [[capture('1/6', [['a', 9999]])], noTable], + [[capture('2/6', [['cli', 9999, [1, 3]]])], dataset()], + ]; + const graded = []; + for (const [captures, ds] of scenarios) { + for (const event of events) { + const r = run(captures, ds, {}, event); + if (r.exitCode !== 0 || r.out.some((l) => l.startsWith('::error'))) graded.push(`${event}: ${r.verdict} exit ${r.exitCode}`); + } } + if (graded.length) throw new Error(`ruling E: a timing reading graded: ${graded.join('; ')}`); }); check(() => { - // The SAME table on every event: only the mode line differs. + // The SAME table on every event: only the event the mode line names differs. const body = (r) => r.err[0].split('\n').slice(0, -1).join('\n'); - const graded = run(twoOver); - const reported = run(twoOver, dataset(), {}, 'pull_request'); - if (body(graded) !== body(reported) || !body(graded).includes('Every executed package this run')) { - throw new Error(`C2: the report-only table is not the graded one:\n${graded.err[0]}\n---\n${reported.err[0]}`); + const first = run(twoOver, dataset(), {}, events[0]); + for (const event of events.slice(1)) { + const other = run(twoOver, dataset(), {}, event); + if (body(first) !== body(other) || !body(first).includes('Every executed package this run')) { + throw new Error(`ruling E: the ${event} table is not the ${events[0]} table:\n${first.err[0]}\n---\n${other.err[0]}`); + } } - if (!graded.err[0].endsWith('the profile the baseline was measured on.') || !reported.err[0].endsWith('the step exits 0.')) { - throw new Error(`C2: the mode line is missing:\n${graded.err[0]}\n---\n${reported.err[0]}`); + for (const event of events) { + const r = run(twoOver, dataset(), {}, event); + if (!r.err[0].endsWith(`report only on every event, this \`${event}\` run included: a wall-clock reading on a shared runner measures the run's load as much as the code, so a reading past its bound is a warning and the step exits 0.`)) { + throw new Error(`ruling E: the ${event} mode line is missing or reads graded:\n${r.err[0]}`); + } } }); check(() => { - // Within every ceiling, off schedule: OK, no annotation, the mode said. + // Within every ceiling: OK, no annotation, the mode said. const r = run([capture('1/6', [['a', 10]])], dataset(), {}, 'push'); - if (r.exitCode !== 0 || r.verdict !== 'OK' || r.out.length !== 0 || !r.err[0].includes('report only: a `push` run')) { - throw new Error(`C2: an OK push run read ${r.verdict}:\n${r.err[0]}`); + if (r.exitCode !== 0 || r.verdict !== 'OK' || r.out.length !== 0 || !r.err[0].includes('report only on every event, this `push` run included')) { + throw new Error(`ruling E: an OK push run read ${r.verdict}:\n${r.err[0]}`); } }); check(() => { - // A refusal is not a timing reading: red on every event. - const r = run([capture('1/6', [['a', 180]])], dataset(), { a: { seconds: 200, ruling: 'no' } }, 'merge_group'); - if (r.exitCode !== 1 || r.verdict !== 'REFUSED') throw new Error(`C2: a malformed raise record off schedule read ${r.verdict}`); + // A refusal is not a timing reading: red on every event, `schedule` among them. + for (const event of events) { + const r = run([capture('1/6', [['a', 180]])], dataset(), { a: { seconds: 200, ruling: 'no' } }, event); + if (r.exitCode !== 1 || r.verdict !== 'REFUSED') throw new Error(`ruling E: a malformed raise record on ${event} read ${r.verdict}, exit ${r.exitCode}`); + } }); check(() => { - // No table yet is unchanged off schedule: NOT MEASURED with its warning. + // No table yet is unchanged on any event: NOT MEASURED with its warning. const { ceilings: _x, uncapped: _y, ...noTable } = dataset(); - const r = run([capture('1/6', [['a', 9999]])], noTable, {}, 'merge_group'); - if (r.exitCode !== 0 || r.verdict !== 'NOT MEASURED' || !r.out.some((l) => l.startsWith('::warning title=Suite-duration ceilings::NOT MEASURED'))) { - throw new Error(`C2: no table off schedule read ${r.verdict}`); + for (const event of events) { + const r = run([capture('1/6', [['a', 9999]])], noTable, {}, event); + if (r.exitCode !== 0 || r.verdict !== 'NOT MEASURED' || !r.out.some((l) => l.startsWith('::warning title=Suite-duration ceilings::NOT MEASURED'))) { + throw new Error(`ruling E: no table on ${event} read ${r.verdict}`); + } } }); check(() => { // A missing event is refused, in the renderer and at the command line, never defaulted. const report = gradeCeilings({ merged: foldCaptures([capture('1/6', [['a', 151]])]), dataset: dataset(), raises: {} }); - if (!threw(() => renderCeilingVerdict(report, 'Test Core'))) throw new Error('C2: a render with no event was graded'); - if (!threw(() => parseCeilingArgs(['--captures', 'd']))) throw new Error('C2: --captures with no --event was accepted'); - if (!threw(() => parseCeilingArgs(['--captures', 'd', '--event', '']))) throw new Error('C2: an empty --event was accepted'); + if (!threw(() => renderCeilingVerdict(report, 'Test Core'))) throw new Error('ruling E: a render with no event was accepted'); + if (!threw(() => parseCeilingArgs(['--captures', 'd']))) throw new Error('ruling E: --captures with no --event was accepted'); + if (!threw(() => parseCeilingArgs(['--captures', 'd', '--event', '']))) throw new Error('ruling E: an empty --event was accepted'); const parsed = parseCeilingArgs(['--captures', 'd', '--label', 'L', '--event', 'merge_group']); if (parsed.event !== 'merge_group' || parsed.capturesDir !== 'd' || parsed.label !== 'L') { - throw new Error(`C2: the arguments parsed as ${JSON.stringify(parsed)}`); + throw new Error(`ruling E: the arguments parsed as ${JSON.stringify(parsed)}`); } }); @@ -1107,9 +1126,10 @@ function selfTest() { // ── Entry point ─────────────────────────────────────────────────────────── /** - * The grading run's arguments. `--event` is required (C2): it decides whether - * a package over its ceiling exits 1 (`schedule`) or is a warning (any other - * event), and a missing one is refused by timingGateMode, never defaulted. + * The grading run's arguments. `--event` is required: it is named in the mode + * line every verdict prints (no event changes the verdict: a package over its + * ceiling is a warning on all of them), and a missing one is refused by + * timingGateMode, never defaulted. */ export function parseCeilingArgs(argv) { let capturesDir = null; diff --git a/scripts/partition-test-shards.mjs b/scripts/partition-test-shards.mjs index 2c1bcf62605..c6aad9682c9 100644 --- a/scripts/partition-test-shards.mjs +++ b/scripts/partition-test-shards.mjs @@ -82,9 +82,10 @@ // written (#16173). Every Test Core shard already passes `--summarize`, so the // run it just finished has written the measured truth to `.turbo/runs/`; this // mode reads that back, compares it to what this script PREDICTED for the same -// packages, raises a `::warning::` past WARN_MEASURED_OVER_PREDICTED, and reds -// past MAX_MEASURED_OVER_PREDICTED -- on a `schedule` run; on every other event -// that red is a `::warning::` and exits 0 (TIMING_GRADED_EVENT, C2). Without it the dataset +// packages, raises a `::warning::` past WARN_MEASURED_OVER_PREDICTED, and a +// second one -- DRIFT -- past MAX_MEASURED_OVER_PREDICTED. It is a READING and +// never a red, on every event, `schedule` included: "TIMING IS A READING" +// below carries the ruling and the diagnosis. Without it the dataset // rots silently in one direction and the only instrument that notices is a // shard killed by the job timeout -- which is a shard that produced NO reading // while the rollup read green. @@ -130,7 +131,8 @@ export const SHARD_COUNT = 6; export const MAX_SHARD_OVER_MEAN = 1.3; // The factor a shard's MEASURED test total may exceed its PREDICTED total by -// before `--check-drift` reds. The durable half of #16173. +// before `--check-drift` names it DRIFT -- a `::warning::`, never a red (see +// "TIMING IS A READING" below). The durable half of #16173. // // Everything above balances a GENERATED dataset, and a generated file that // nothing re-measures rots in one direction only: suites get slower, the @@ -201,52 +203,60 @@ export const MAX_MEASURED_OVER_PREDICTED = 1.5; // the warning cannot be made unreachable by moving either end. export const WARN_MEASURED_OVER_PREDICTED = 1.3; -// ── WHERE THE TWO TIMING GATES EXIT-GRADE: `schedule` ALONE (C2) ────────── -// -// Ruling (letter C2, for both timing steps): -// https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511 -// "a timing gate exit-grades only on the run profile its baseline was measured -// on; everywhere else it reports." -// -// Both timing gates read one baseline: the weights `--check-drift` predicts -// from, and the ceilings scripts/check-test-suite-ceilings.mjs grades against. -// The refresh lane measures it on ci.yml's hourly `schedule` runs and no others -// (DEFAULT_RUN_EVENT in scripts/ci/select-shard-timings-run.mjs). A -// `merge_group` or `pull_request` build executes far more of the workspace on -// the same runners. On the morning the refreshed dataset went live this step -// redded 3 of 19 queue builds and the heads of three PRs (#22953, #22954, -// #22959, filed as #22966) at 1.56-1.62x, on packages none of them touched, -// while every hourly run read green. So both gates take the run's event -// (`--event "$GITHUB_EVENT_NAME"` in ci.yml) and: -// -// schedule exit-graded, exactly as before: a red exits 1 -// every other REPORT ONLY (merge_group, pull_request, push, -// event workflow_dispatch): the same table prints, each red becomes a -// `::warning::` annotation, and the step exits 0 -// -// A real slowdown merged to `main` reds the next hourly run. A missing or empty -// `--event` is refused, never defaulted: a step that lost its event would -// otherwise grade everywhere or nowhere, and say nothing either way. -export const TIMING_GRADED_EVENT = 'schedule'; - -/** 'grade' on TIMING_GRADED_EVENT, 'report' on any other event; a missing event is refused. */ +// ── TIMING IS A READING, NEVER A RED: BOTH TIMING GATES REPORT ON EVERY EVENT ── +// +// Ruling (#22949, letter E, item 1; the maintainer's word 「22949 同意」): +// https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6125775640 +// It retires the `schedule`-only red that letter C2 +// (https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511) +// left in place: the density problem C2 took off the queue lives on `schedule` too. +// +// The diagnosis, in two sentences. A wall-clock reading on a shared runner +// measures the run's load as much as the code: the same package reads 82 s on a +// quiet hour (most packages replayed) and 209 s on a busy one (the whole +// workspace executing), 2.56x, while the ceiling formula (window max x 1.25) allows 2.0x of weight. +// The baseline window was mostly quiet-density and every event, `schedule` +// included, meets both densities, so busy hours red the same near-ceiling +// packages whatever their code does: density, not code. +// +// So both timing gates -- this script's `--check-drift` and +// scripts/check-test-suite-ceilings.mjs, which share the one rule below -- take +// the run's event (`--event "$GITHUB_EVENT_NAME"` in ci.yml) and, on EVERY +// event: print the same table, raise each reading past its bound as a +// `::warning::` annotation, and exit 0. +// +// "The red" in the comments above names the DRIFT bound; since this ruling it +// is the upper warning tier and fails nothing. Regression detection for the +// wall moves to the dataset refresh's diff (the refresh lane, E2, #23059) and +// to the per-file budget; neither is this file's. +// +// A REFUSAL is not a timing reading and stays red on every event: a missing or +// empty `--event` (a step that lost its event would otherwise be mis-wired and +// say nothing), a malformed ceiling table or raise record, a dataset or run +// summary that does not parse. None of those varies with runner load. +// +// There is no graded mode left to select, so there is no event constant: a +// `TIMING_GRADED_EVENT` that no event matches would keep an exit-1 branch +// nothing can reach, and a dead branch reads as a switch somebody may flip. +// Grading a timing again is a new ruling and a new edit, not a flag. export function timingGateMode(event) { if (typeof event !== 'string' || event.trim() === '') { throw new Error( - `the run's event is ${JSON.stringify(event ?? null)}. The timing gates exit-grade on ` + - `\`${TIMING_GRADED_EVENT}\` alone and report on every other event, so they need it: pass ` + + `the run's event is ${JSON.stringify(event ?? null)}. The timing gates report on every event, but ` + + 'each step is wired to say which one it read, so a missing event is a mis-wired step: pass ' + '`--event "$GITHUB_EVENT_NAME"`. A missing event is refused rather than guessed.' ); } - return event === TIMING_GRADED_EVENT ? 'grade' : 'report'; + return 'report'; } -/** The one line both timing gates print to say which of the two modes this run is in. */ +/** The one line both timing gates print to say what mode this run is in. */ export function timingModeNote(event) { - return timingGateMode(event) === 'grade' - ? `exit-graded: a \`${event}\` run is the profile the baseline was measured on` - : `report only: a \`${event}\` run is not the profile the baseline was measured on ` + - `(\`${TIMING_GRADED_EVENT}\`), so a red reading is a warning here and the step exits 0`; + timingGateMode(event); + return ( + `report only on every event, this \`${event}\` run included: a wall-clock reading on a shared runner ` + + "measures the run's load as much as the code, so a reading past its bound is a warning and the step exits 0" + ); } // ── SHARDING ONE PACKAGE BELOW PACKAGE GRANULARITY (#16173) ──────────────── @@ -1132,11 +1142,12 @@ export function escapeWorkflowCommandMessage(text) { } // -// `event` is the run's (ci.yml passes `$GITHUB_EVENT_NAME`): off -// TIMING_GRADED_EVENT a DRIFT prints the same text, becomes a `::warning::` -// annotation, and exits 0 (C2, see timingGateMode). +// `event` is the run's (ci.yml passes `$GITHUB_EVENT_NAME`) and is named in the +// mode line. On every event a DRIFT prints its text, becomes one `::warning::` +// annotation, and exits 0 (see "TIMING IS A READING"); a missing event is +// refused before any verdict is rendered. export function renderDriftVerdict(report, label, event) { - const mode = timingGateMode(event); + timingGateMode(event); const skipped = report.unpredicted.length === 0 ? '' @@ -1159,7 +1170,7 @@ export function renderDriftVerdict(report, label, event) { const head = `${report.measuredTotal.toFixed(1)}s measured vs ${report.predictedTotal.toFixed(1)}s predicted ` + `across ${report.rows.length} package(s) = ${report.ratio.toFixed(2)}x ` + - `(warning past ${WARN_MEASURED_OVER_PREDICTED}x, red past ${MAX_MEASURED_OVER_PREDICTED}x)`; + `(warning past ${WARN_MEASURED_OVER_PREDICTED}x, DRIFT past ${MAX_MEASURED_OVER_PREDICTED}x)`; const ratioOf = (r) => (r.predicted > 0 ? `${(r.measured / r.predicted).toFixed(2)}x, ` : ''); const worst = report.rows .slice(0, 5) @@ -1183,23 +1194,21 @@ export function renderDriftVerdict(report, label, event) { " this script's own balance pins if a single suite has outgrown the acceptance bound:\n" + ' that is those pins working, and the remedy they name is splitting that suite below\n' + ` package granularity, never a different shard count.${note}`; - if (mode === 'report') { - const top = report.rows[0]; - return { - verdict: 'DRIFT', - exitCode: 0, - out: [ - `::warning title=Test Core shard timing drift::${escapeWorkflowCommandMessage( - `${label}: ${head}, past the red bound. Heaviest overshoot: ${top.name}, predicted ` + - `${top.predicted.toFixed(1)}s, measured ${top.measured.toFixed(1)}s. Report only on a \`${event}\` run: ` + - `this step exit-grades on \`${TIMING_GRADED_EVENT}\` runs alone, the profile ` + - 'scripts/test-shard-timings.json was measured on, so a drift that is real reds the next hourly run on main.' - )}`, - ], - err: [text], - }; - } - return { verdict: 'DRIFT', exitCode: 1, out: [], err: [text] }; + const top = report.rows[0]; + return { + verdict: 'DRIFT', + exitCode: 0, + out: [ + `::warning title=Test Core shard timing drift::${escapeWorkflowCommandMessage( + `${label}: ${head}, past the drift bound. Heaviest overshoot: ${top.name}, predicted ` + + `${top.predicted.toFixed(1)}s, measured ${top.measured.toFixed(1)}s. Report only, on every event ` + + `(this is a \`${event}\` run): a shard's wall measures the run's load as much as the dataset, so ` + + 'this is a reading, not a failure. Refresh scripts/test-shard-timings.json ' + + '(see scripts/measure-test-shard-timings.mjs).' + )}`, + ], + err: [text], + }; } if (report.warned) { @@ -1211,8 +1220,8 @@ export function renderDriftVerdict(report, label, event) { `::warning title=Test Core shard timing drift::${escapeWorkflowCommandMessage( `${label}: ${head}. Heaviest overshoot: ${top.name}, predicted ${top.predicted.toFixed(1)}s, ` + `measured ${top.measured.toFixed(1)}s. scripts/test-shard-timings.json is drifting from the ` + - 'runtime; refresh it before it reaches the red (see scripts/measure-test-shard-timings.mjs). ' + - 'Not a failure: this step stays green until the red bound.' + 'runtime; refresh it before it reaches the drift bound (see scripts/measure-test-shard-timings.mjs). ' + + 'Not a failure: this step stays green on every event.' )}`, ], err: [`shard-timing-drift: WARN -- ${label}, ${head}.${skipped}\n Heaviest overshoots:\n${worst}${note}`], @@ -1315,7 +1324,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'file-level slice items (#16173)': 20, 'file-level slices reach vitest through OS_TEST_SHARD (#19278)': 11, 'density cap and the per-run slice decision (#22075)': 14, - 'the drift step exit-grades on `schedule` alone, and reports everywhere else (C2)': 8, + 'the drift step reports on every event, `schedule` included, and refuses a missing event': 8, }); // DELETING an entry silences that battery's floor exactly as effectively as @@ -1964,8 +1973,11 @@ function selfTest() { // it is read as rendered text, never printed here (a self-test line opening // with a workflow command would mint a real annotation on every lint run). const label = 'Test Core (9/9)'; + // Every event reads the same; `schedule` is the one that used to exit-grade, + // so it is the strictest case to render with. + const EVENT = 'schedule'; check(() => { - const v = renderDriftVerdict(tierOf(131), label, TIMING_GRADED_EVENT); + const v = renderDriftVerdict(tierOf(131), label, EVENT); const commands = v.out.filter((line) => line.startsWith('::')); if (v.verdict !== 'WARN' || v.exitCode !== 0 || commands.length !== 1) { throw new Error( @@ -1977,19 +1989,29 @@ function selfTest() { } }); check(() => { - const v = renderDriftVerdict(tierOf(151), label, TIMING_GRADED_EVENT); - if (v.verdict !== 'DRIFT' || v.exitCode !== 1 || v.out.some((line) => line.startsWith('::'))) { - throw new Error(`drift warning: a red rendered ${v.verdict} exit ${v.exitCode}, ${v.out.length} stdout line(s)`); + // The upper tier is a warning too, and one the reader can tell from the lower + // one: its annotation names the drift bound, the lower one's does not. + const v = renderDriftVerdict(tierOf(151), label, EVENT); + const w = renderDriftVerdict(tierOf(131), label, EVENT); + const commands = v.out.filter((line) => line.startsWith('::')); + if (v.verdict !== 'DRIFT' || v.exitCode !== 0 || commands.length !== 1 || commands[0].includes('\n')) { + throw new Error(`drift warning: a DRIFT rendered ${v.verdict} exit ${v.exitCode}, ${commands.length} workflow command(s)`); + } + if (!commands[0].startsWith('::warning ') || !commands[0].includes(label) || !commands[0].includes('past the drift bound')) { + throw new Error(`drift warning: the DRIFT annotation is not one ::warning line naming the shard and the bound: ${commands[0]}`); + } + if (w.out[0].includes('past the drift bound')) { + throw new Error(`drift warning: the WARN annotation claims the drift bound: ${w.out[0]}`); } }); check(() => { for (const [seconds, expected] of [[118, 'OK'], [WARN_MEASURED_OVER_PREDICTED * 100, 'OK']]) { - const v = renderDriftVerdict(tierOf(seconds), label, TIMING_GRADED_EVENT); + const v = renderDriftVerdict(tierOf(seconds), label, EVENT); if (v.verdict !== expected || v.exitCode !== 0 || v.out.length !== 0) { throw new Error(`drift warning: ${seconds / 100}x rendered ${v.verdict} with ${v.out.length} stdout line(s)`); } } - const nm = renderDriftVerdict(driftReport(replayed.samples, driftTimings), label, TIMING_GRADED_EVENT); + const nm = renderDriftVerdict(driftReport(replayed.samples, driftTimings), label, EVENT); if (nm.verdict !== 'NOT MEASURED' || nm.exitCode !== 0 || nm.out.length !== 0) { throw new Error(`drift warning: a replay-only shard rendered ${nm.verdict}`); } @@ -2003,83 +2025,98 @@ function selfTest() { } }); - // -- EXIT-GRADED ON `schedule` ALONE (C2) -------------------------------- + // -- A READING ON EVERY EVENT, `schedule` INCLUDED (ruling E) ---------------- // - // The baseline is measured on hourly `schedule` runs, so only those exit 1 - // on a DRIFT; every other event prints the same text, annotates a warning - // and exits 0. Read as rendered text, like the tier above: printing a + // No event exit-grades: every verdict prints the same table, a DRIFT is one + // `::warning::` annotation, and the step exits 0. Only a refusal (a missing + // event) is red. Read as rendered text, like the tier above: printing a // `::warning::` from here would mint a real annotation on every lint run. - battery('the drift step exit-grades on `schedule` alone, and reports everywhere else (C2)'); - const offSchedule = ['merge_group', 'pull_request', 'push', 'workflow_dispatch']; + battery('the drift step reports on every event, `schedule` included, and refuses a missing event'); + const events = ['schedule', 'merge_group', 'pull_request', 'push', 'workflow_dispatch']; check(() => { - if (TIMING_GRADED_EVENT !== 'schedule' || timingGateMode('schedule') !== 'grade') { - throw new Error(`C2: the graded event is ${TIMING_GRADED_EVENT}, and schedule reads ${timingGateMode('schedule')}`); - } - }); - check(() => { - const graded = offSchedule.filter((e) => timingGateMode(e) !== 'report'); - if (graded.length) throw new Error(`C2: ${graded.join(', ')} exit-grade, but only schedule may`); + // Every event reads 'report', `schedule` among them, and so does one this + // list has never heard of: the mode is not a function of the event. + const other = [...events, 'repository_dispatch'].filter((e) => timingGateMode(e) !== 'report'); + if (other.length) throw new Error(`ruling E: ${other.join(', ')} read a mode other than report`); }); - // A step that lost its event is refused, never defaulted to either mode. + // A step that lost its event is refused, never defaulted to a mode. check(() => { const accepted = [undefined, null, '', ' '].filter((e) => !threw(() => timingGateMode(e))); - if (accepted.length) throw new Error(`C2: a missing event was read as a mode: ${JSON.stringify(accepted)}`); - if (!threw(() => renderDriftVerdict(tierOf(151), label))) throw new Error('C2: a render with no event was graded'); + if (accepted.length) throw new Error(`ruling E: a missing event was read as a mode: ${JSON.stringify(accepted)}`); + if (!threw(() => renderDriftVerdict(tierOf(151), label))) throw new Error('ruling E: a render with no event was accepted'); }); check(() => { + // `schedule` is the case that used to exit 1: it is a warning now. const v = renderDriftVerdict(tierOf(151), label, 'schedule'); - if (v.verdict !== 'DRIFT' || v.exitCode !== 1 || v.out.length !== 0 || !v.err[0].includes('exit-graded')) { - throw new Error(`C2: a schedule DRIFT rendered ${v.verdict} exit ${v.exitCode}, ${v.out.length} stdout line(s)`); + if (v.verdict !== 'DRIFT' || v.exitCode !== 0 || v.out.some((line) => line.startsWith('::error'))) { + throw new Error(`ruling E: a schedule DRIFT rendered ${v.verdict} exit ${v.exitCode}, ${JSON.stringify(v.out)}`); + } + if (v.err[0].includes('exit-graded') || !v.err[0].includes('report only on every event')) { + throw new Error(`ruling E: a schedule DRIFT does not say it is report only:\n${v.err[0]}`); } }); check(() => { - for (const event of offSchedule) { + for (const event of events) { const v = renderDriftVerdict(tierOf(151), label, event); const commands = v.out.filter((line) => line.startsWith('::')); if (v.verdict !== 'DRIFT' || v.exitCode !== 0 || commands.length !== 1) { - throw new Error(`C2: a ${event} DRIFT rendered ${v.verdict} exit ${v.exitCode} with ${commands.length} command(s)`); + throw new Error(`ruling E: a ${event} DRIFT rendered ${v.verdict} exit ${v.exitCode} with ${commands.length} command(s)`); } if (!commands[0].startsWith('::warning ') || !commands[0].includes(label) || !commands[0].includes(event) || commands[0].includes('\n')) { - throw new Error(`C2: a ${event} DRIFT is not one ::warning line naming the shard and the event: ${commands[0]}`); + throw new Error(`ruling E: a ${event} DRIFT is not one ::warning line naming the shard and the event: ${commands[0]}`); } } }); - // The SAME table on every event: only the mode line differs. + // The SAME table on every event: only the event the mode line names differs. check(() => { const body = (v) => v.err[0].split('\n').slice(0, -1).join('\n'); - const graded = renderDriftVerdict(tierOf(151), label, 'schedule'); - const reported = renderDriftVerdict(tierOf(151), label, 'merge_group'); - if (body(graded) !== body(reported) || !body(graded).includes('Heaviest overshoots:')) { - throw new Error(`C2: the report-only text is not the graded text:\n${graded.err[0]}\n---\n${reported.err[0]}`); - } - if (!reported.err[0].endsWith('so a red reading is a warning here and the step exits 0.')) { - throw new Error(`C2: the report-only text does not say so:\n${reported.err[0]}`); + const first = renderDriftVerdict(tierOf(151), label, events[0]); + for (const event of events.slice(1)) { + const other = renderDriftVerdict(tierOf(151), label, event); + if (body(first) !== body(other) || !body(first).includes('Heaviest overshoots:')) { + throw new Error(`ruling E: the ${event} text is not the ${events[0]} text:\n${first.err[0]}\n---\n${other.err[0]}`); + } + if (!other.err[0].endsWith('so a reading past its bound is a warning and the step exits 0.')) { + throw new Error(`ruling E: the ${event} text does not say so:\n${other.err[0]}`); + } } }); check(() => { - // The tiers under the red are unchanged off schedule: WARN annotates, OK and NOT MEASURED say nothing on stdout. + // The tiers under the DRIFT keep their effect on any event: WARN annotates, + // OK and NOT MEASURED say nothing on stdout, and none of them is red. const warn = renderDriftVerdict(tierOf(131), label, 'pull_request'); const ok = renderDriftVerdict(tierOf(118), label, 'push'); const nm = renderDriftVerdict(driftReport(replayed.samples, driftTimings), label, 'merge_group'); if (warn.verdict !== 'WARN' || warn.exitCode !== 0 || warn.out.length !== 1 || !warn.out[0].startsWith('::warning ')) { - throw new Error(`C2: a pull_request WARN rendered ${warn.verdict} exit ${warn.exitCode}`); + throw new Error(`ruling E: a pull_request WARN rendered ${warn.verdict} exit ${warn.exitCode}`); } - if (ok.verdict !== 'OK' || ok.exitCode !== 0 || ok.out.length !== 0 || !ok.err[0].includes('report only')) { - throw new Error(`C2: a push OK rendered ${ok.verdict} exit ${ok.exitCode}:\n${ok.err[0]}`); + if (ok.verdict !== 'OK' || ok.exitCode !== 0 || ok.out.length !== 0 || !ok.err[0].includes('report only on every event')) { + throw new Error(`ruling E: a push OK rendered ${ok.verdict} exit ${ok.exitCode}:\n${ok.err[0]}`); } if (nm.verdict !== 'NOT MEASURED' || nm.exitCode !== 0 || nm.out.length !== 0) { - throw new Error(`C2: a merge_group replay-only shard rendered ${nm.verdict}`); + throw new Error(`ruling E: a merge_group replay-only shard rendered ${nm.verdict}`); } }); // The CLI: `--event` is read, and its absence is refused before any summary is. check(() => { const parsed = parseDriftArgs(['--check-drift', 'a.json', '--label', 'L', '--event', 'merge_group']); if (parsed.event !== 'merge_group' || parsed.inputs.join() !== 'a.json' || parsed.label !== 'L') { - throw new Error(`C2: --check-drift parsed as ${JSON.stringify(parsed)}`); + throw new Error(`ruling E: --check-drift parsed as ${JSON.stringify(parsed)}`); + } + if (!threw(() => parseDriftArgs(['--check-drift', 'a.json']))) throw new Error('ruling E: --check-drift with no --event was accepted'); + if (!threw(() => parseDriftArgs(['--check-drift', 'a.json', '--event', '']))) throw new Error('ruling E: an empty --event was accepted'); + }); + check(() => { + // The sweep that would catch a grade creeping back in: no tier, on no event, exits non-zero. + const graded = []; + for (const seconds of [90, 118, 131, 151, 300, 1000]) { + for (const event of events) { + const v = renderDriftVerdict(tierOf(seconds), label, event); + if (v.exitCode !== 0 || v.out.some((line) => line.startsWith('::error'))) graded.push(`${seconds / 100}x on ${event}: ${v.verdict} exit ${v.exitCode}`); + } } - if (!threw(() => parseDriftArgs(['--check-drift', 'a.json']))) throw new Error('C2: --check-drift with no --event was accepted'); - if (!threw(() => parseDriftArgs(['--check-drift', 'a.json', '--event', '']))) throw new Error('C2: an empty --event was accepted'); + if (graded.length) throw new Error(`ruling E: a timing reading graded: ${graded.join('; ')}`); }); // -- FILE-LEVEL SLICE ITEMS (#16173) ------------------------------------ @@ -2614,16 +2651,17 @@ function selfTest() { // `--check-drift`: the shard just measured itself, so read that back. // // Every verdict this prints is one of exactly four -- OK, WARN, DRIFT and NOT -// MEASURED (renderDriftVerdict) -- and only DRIFT exits non-zero. WARN is the -// annotation tier under the red (WARN_MEASURED_OVER_PREDICTED), and NOT -// MEASURED is a first-class verdict rather than a quiet pass. A shard whose -// test tasks were all cache replays has said nothing about the dataset, and -// reporting that as OK is the #4690 shape -- a check that read nothing -// reporting as a check that found nothing wrong. -// -// `--event` is required (C2): the run's event decides whether a DRIFT exits 1 -// (`schedule`) or is reported as a warning (every other event), and a missing -// one is refused by timingGateMode rather than defaulted either way. +// MEASURED (renderDriftVerdict) -- and none of them exits non-zero: only a +// refusal does (parseDriftArgs, an unreadable dataset or run summary). WARN is +// the annotation tier under DRIFT (WARN_MEASURED_OVER_PREDICTED), DRIFT is the +// upper one, and NOT MEASURED is a first-class verdict rather than a quiet +// pass. A shard whose test tasks were all cache replays has said nothing about +// the dataset, and reporting that as OK is the #4690 shape -- a check that read +// nothing reporting as a check that found nothing wrong. +// +// `--event` is required: the run's event is named in the mode line every verdict +// prints, and a missing one is refused by timingGateMode rather than +// defaulted (no event changes the verdict: "TIMING IS A READING"). export function parseDriftArgs(argv) { const inputs = []; let label = 'this shard'; From 19690c98e29f04208547d4d9e8c9ee553365c688 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 14:31:31 +0000 Subject: [PATCH 2/2] ci(timing gates): the two timing steps' comments say why no event grades Only comments change: the drift step, the ceilings step and the aggregator note that called the ceilings step a gate. The step names, `--event "$GITHUB_EVENT_NAME"`, `if:` and `continue-on-error` are byte-identical. Claude-Session: https://claude.ai/code/session_01LqNgdPZB2Jnr5HpGbaqLeB Co-authored-by: Claude --- .github/workflows/ci.yml | 98 ++++++++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 38 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c8d0168a7ee..70300f9f967 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1081,22 +1081,32 @@ jobs: # OK at or under 1.3x (WARN_MEASURED_OVER_PREDICTED) # WARN past 1.3x, at or under 1.5x: a `::warning::` annotation # naming the shard and its heaviest overshoot; green - # DRIFT past 1.5x (MAX_MEASURED_OVER_PREDICTED): red + # DRIFT past 1.5x (MAX_MEASURED_OVER_PREDICTED): a second + # `::warning::` annotation, the upper tier; green # NOT MEASURED no executed test task carried a dataset entry; green # - # ONE red rule and a warning under it, on the same ratio: triage ruled - # the in-tree 1.5x the red and the card's 1.3x a warning only, and took no - # second red on wall time against `timeout-minutes`. Both constants, their - # measured basis and a self-test battery each live in the script. - # - # THE RED IS A `schedule` RED ALONE (C2, both timing steps, ruling - # https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511): - # the dataset's weights are measured on the hourly `schedule` runs, so - # only those exit 1 on DRIFT. On `merge_group`, `pull_request`, `push` - # and `workflow_dispatch` the step prints the same verdict, raises the - # DRIFT as a `::warning::` annotation and exits 0. The event reaches the - # script as `--event "$GITHUB_EVENT_NAME"`, which it requires; the rule - # and its self-test live there (TIMING_GRADED_EVENT, timingGateMode). + # Two thresholds on ONE ratio, and no second rule on wall time against + # `timeout-minutes`: triage ruled the in-tree 1.5x the upper tier and the + # card's 1.3x the lower. Both constants, their measured basis and a + # self-test battery each live in the script. + # + # NO EVENT GRADES (#22949 ruling E, item 1, both timing steps: + # https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6125775640). + # A wall-clock reading on a shared runner measures the run's load as much + # as the code: the same package reads 82 s on a quiet hour and 209 s on a + # busy one, 2.56x, past this step's 1.5x bound and the ceilings' 2.0x of + # weight. The dataset's weights come from a window that was mostly + # quiet-density and every event, `schedule` included, meets both + # densities, so busy hours put the same shards past the bound whatever + # their code does: density, not code. + # So on `schedule`, `merge_group`, `pull_request`, `push` and + # `workflow_dispatch` alike the step prints the same verdict, raises DRIFT + # as a `::warning::` annotation and exits 0. The event reaches the script + # as `--event "$GITHUB_EVENT_NAME"`, which it requires: a missing one is a + # refusal, and so is a dataset or run summary that does not parse, and a + # refusal is still a red step on every event. The rule and its self-test + # live in the script (timingGateMode); regression detection moves to the + # dataset refresh's diff (E2, #23059). # # EXECUTED WINDOWS ONLY. Measurements come from samplesFromSummary() in # measure-test-shard-timings.mjs, which drops cache HITs and failed @@ -1105,11 +1115,11 @@ jobs: # predicted and vouch for a rotted dataset, so a shard whose packages all # replayed is NOT MEASURED, never "fast". # - # NO `if:` and NO `continue-on-error` (the red is the point, and the - # `schedule`-only red is the script's `--event`, so neither is needed), - # and ABOVE the attestation pair for the #6082 reason documented on the - # upload above: a failed unguarded step skips that pair, so a drift red - # also withholds this shard's attestation, the fail-closed direction. A shard + # NO `if:` and NO `continue-on-error` (the only red left here is a + # refusal, and a refusal must stay red, so neither is wanted), and ABOVE + # the attestation pair for the #6082 reason documented on the upload + # above: a failed unguarded step skips that pair, so a refusal also + # withholds this shard's attestation, the fail-closed direction. A shard # with no packages runs no turbo and writes no summary; that is NOT # MEASURED here rather than a usage error from the script. # @@ -1459,8 +1469,10 @@ jobs: # diagnostics fault here (an artifact-service 403 is the measured one, # #14469) must not evict a green run. The suite-duration ceiling check # at the end of this job reads the same captures and is the one step - # here that DOES red the job, on purpose: it is a gate, not a report, and - # a capture that never arrived reads NOT MEASURED there, never red. + # here that can still red the job, on purpose and only on a refusal (a + # malformed table or raise record, no `--event`): a package over its + # ceiling is a `::warning::` on every event, and a capture that never + # arrived reads NOT MEASURED there, never red. # # ⛔ Steps, deliberately, NOT a new `needs:` entry: this job's # `OS_AGGREGATOR_MEMBERS` / `OS_AGGREGATOR_NON_MEMBERS` block is asserted @@ -1516,11 +1528,14 @@ jobs: if-no-files-found: ignore retention-days: 14 - # ── The per-package suite-duration ratchet (#16468) ────────────────── + # ── The per-package suite-duration ceilings (#16468) ───────────────── # - # A GATE, unlike the two reporting steps above: NO `continue-on-error`, - # so on a `schedule` run a package whose suite ran past its ceiling reds - # `Test Core`, naming the package and its slowest test files. The + # A READING, never a red (ruling E, below): a package whose suite ran + # past its ceiling is one `::warning::` annotation naming it and its + # slowest test files, and the step exits 0 on every event. Unlike the two + # reporting steps above it carries NO `continue-on-error`, because a + # REFUSAL (a malformed table or raise record, no `--event`) is not a + # timing reading and reds `Test Core` on every event. The # ceiling is the package's slowest executed run in the refresh window # x 1.25, generated into scripts/test-shard-timings.json (`ceilings`) by # the refresh lane and held by every later refresh; it rises only by a @@ -1538,19 +1553,26 @@ jobs: # samplesFromSummary, and every vitest file line); a capture that did not # arrive leaves its packages ungraded and says so, never red. # - # EXIT-GRADED ON `schedule` ALONE (C2, both timing steps, ruling - # https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511): - # the ceilings come from the hourly `schedule` runs, so only those red - # this job on a package over its ceiling. On `merge_group`, - # `pull_request`, `push` and `workflow_dispatch` the step prints the same - # table, raises each package over its ceiling as a `::warning::` - # annotation and exits 0; a real slowdown merged to `main` reds the next - # hourly run. A package whose ceiling is under the absolute floor (20s, - # CEILING_FLOOR_SECONDS) prints "below floor" and is never red. The - # event reaches the script as `--event "$GITHUB_EVENT_NAME"`, which it - # requires; a refusal (a malformed table or raise record) is red on every - # event. The rule, the floor's reading and their self-tests live in the - # script. + # NO EVENT GRADES (#22949 ruling E, item 1, both timing steps: + # https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6125775640). + # A per-package suite wall on a shared runner measures the run's load as + # much as the package: the same package reads 82 s on a quiet hour (most + # packages replayed) and 209 s on a busy one (the whole workspace + # executing), 2.56x, while the ceiling formula (window max x 1.25) allows + # 2.0x of weight. The window the ceilings come from was mostly + # quiet-density and every event, `schedule` included, meets both + # densities, so busy hours put the same near-ceiling packages over + # whatever their code does: density, not code. So on `schedule`, + # `merge_group`, `pull_request`, `push` and `workflow_dispatch` alike the + # step prints the same table, raises each package over its ceiling as a + # `::warning::` annotation and exits 0. A package whose ceiling is under + # the absolute floor (20s, CEILING_FLOOR_SECONDS) prints "below floor" and + # is never over. The event reaches the script as + # `--event "$GITHUB_EVENT_NAME"`, which it requires; a refusal (a + # malformed table or raise record, no event) is red on every event. The + # rule, the floor's reading and their self-tests live in the script; + # regression detection moves to the dataset refresh's diff (E2, #23059) + # and the per-file budget. # # NOT MEASURED, exit 0, while the dataset carries no `ceilings`: they are # generated only where the run-summary artifacts are downloadable (the