From d73649c2b58ee99c595de63ae2c13dfef8e05690 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 03:35:54 +0000 Subject: [PATCH 1/8] fix(scripts): assemble a file-sharded package's slices per run, then median across runs `buildDataset`'s slice ledger was keyed by (package, slice count, slice index) with no notion of which run a summary came from, so every run collapsed into one entry and both merge rules broke at once, silently: * last-wins across runs. Three runs measuring 400/600/1000s recorded 1000 -- whichever run was read last -- while every unsliced package in the same refresh correctly took its median. The one package the slicing machinery exists for was the single least robust reading in the file. * a cross-run splice. Run B's 1/2 and 2/2 plus run C's 1/2 summed to 800s, a duration no run observed, with `skippedIncompleteSlices` EMPTY because from that ledger's point of view the set was complete -- it just was not from one run. The ledger is now keyed by run first. Slices are summed WITHIN a run, the per-run sums are medianed ACROSS runs exactly as an unsliced package's observations are, and a run that cannot assemble every slice contributes no sample and is named with its run in `skippedIncompleteSlices`. The grouping key comes from the caller, because a run summary carries no run id to infer one from: `--run ` opens a group and the summaries after it belong to it. Summaries before any `--run` share one implicit group, which is the ordinary single-run refresh; feeding several runs that way is REFUSED by name with `--run` given as the remedy, never resolved by taking the last value. `provenance.mergeRule` said "median across summaries", which was only ever true of unsliced packages; it now states the two composed rules, and `provenance.runs` records the declared run ids. Self-test: 7 new cases, floor 34 -> 41 -- the three-run sliced median (600 where the runs give 400/600/1000), the unsliced control in the same dataset (300, which proves the median rule was alive and the sliced path alone bypassed it), the cross-run splice refused and named, and the duplicate refusal with its remedy. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox --- scripts/measure-test-shard-timings.mjs | 294 ++++++++++++++++++++++--- 1 file changed, 258 insertions(+), 36 deletions(-) diff --git a/scripts/measure-test-shard-timings.mjs b/scripts/measure-test-shard-timings.mjs index 2867931ed7..8ccad1c87b 100644 --- a/scripts/measure-test-shard-timings.mjs +++ b/scripts/measure-test-shard-timings.mjs @@ -31,6 +31,12 @@ // green queue build and: // node scripts/measure-test-shard-timings.mjs /*.json \ // --out scripts/test-shard-timings.json +// Feeding MORE THAN ONE run means saying which is which -- `--run ` +// before each run's six summaries. Slices are summed within a run and the +// per-run sums medianed across runs, so a file-sharded package gets the same +// median treatment as every other package (#16473); undeclared multi-run +// input is REFUSED rather than resolved by guessing. +// `.github/workflows/shard-timings-refresh.yml` runs this on a weekly timer. // // Locally, on a 4-vCPU box (the hosted runner's shape): // pnpm exec turbo run build @@ -50,6 +56,8 @@ // // Usage: // node scripts/measure-test-shard-timings.mjs ... [--out ] +// node scripts/measure-test-shard-timings.mjs --run ... \ +// --run ... [--out ] // node scripts/measure-test-shard-timings.mjs --self-test import { existsSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; @@ -193,10 +201,42 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { // summaries, and the whole-package cost this dataset records is their SUM -- // not their median, which is the rule for repeat measurements of one package // and would write 1/n of the truth here. So slices are held back and summed - // per (package, slice count) set; the sum then enters the median pool as ONE - // sample, which keeps the two rules composable when several runs are fed in. + // per (run, package, slice count) set; the sum then enters the median pool as + // ONE sample per run, which is exactly what an unsliced package contributes + // per run, so the two rules compose instead of competing. + // + // ⛔ THE SUM IS ONLY MEANINGFUL WITHIN ONE RUN (#16473). The order is + // load-bearing and there is only one correct one: SUM the slices a single run + // produced, then MEDIAN those per-run sums across runs. Keyed by (package, + // slice count, index) ALONE -- which is what this ledger used to be -- every + // run collapses into one entry and BOTH halves break at once, silently: + // + // * the innermost value is overwritten by each successive summary carrying + // that index, so the package takes whichever run was read LAST no matter + // how many are fed. Measured on three runs giving 400/600/1000s: 1000 + // recorded, median 600. Every OTHER package in the same refresh gets its + // median, so the one package the slicing machinery exists for -- the + // heaviest suite in the workspace -- is the single least robust reading + // in the file, with no line of output saying so. + // * a set completed from slices of DIFFERENT runs is summed as though it + // were one measurement. Run B's 1/2=300 and 2/2=300 with run C's 1/2=500 + // recorded 800s -- a duration no run observed -- and + // `skippedIncompleteSlices` stayed EMPTY, because from that ledger's point + // of view the set IS complete. It just is not from one run. + // + // Both are this file's own signature hazard (a wrong number that reads exactly + // like a right one) surviving on the axis of WHICH RUN, on the package whose + // mis-weighting killed a shard twelve times in a day. + // + // The grouping key comes from the CALLER, because a run summary carries no run + // identifier to infer one from: the six artifacts of one CI run are named and + // fetched together, so the caller is the only actor that knows which is which. + // An entry with no `run` joins one implicit group -- correct for the ordinary + // single-run refresh, which is what the scheduled workflow feeds -- and the + // duplicate refusal below is what keeps that default honest when more than one + // run is fed without declaring itself. const sliceLedger = new Map(); - for (const { samples, skippedCached, slices } of perSummary) { + for (const { samples, skippedCached, slices, run = null } of perSummary) { for (const n of skippedCached) cachedNames.add(n); for (const [name, seconds] of samples) { const slice = slices?.get(name) ?? null; @@ -204,30 +244,59 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { push(name, seconds); continue; } - if (!sliceLedger.has(name)) sliceLedger.set(name, new Map()); - const byCount = sliceLedger.get(name); + if (!sliceLedger.has(run)) sliceLedger.set(run, new Map()); + const byName = sliceLedger.get(run); + if (!byName.has(name)) byName.set(name, new Map()); + const byCount = byName.get(name); if (!byCount.has(slice.count)) byCount.set(slice.count, new Map()); - byCount.get(slice.count).set(slice.index, seconds); + const seen = byCount.get(slice.count); + // ⛔ REFUSE, never take the last. One run runs each slice exactly once, so + // a second window for the same (package, slice) means these summaries come + // from different runs sharing one group -- which is the ambiguity that + // produced both numbers above. Taking either value, or their sum, records + // a weight assembled from runs that never happened together. + if (seen.has(slice.index)) { + throw new Error( + `${name} slice ${slice.index}/${slice.count} was measured twice in ` + + `${run === null ? 'this summary set' : `run ${run}`} (${seen.get(slice.index)}s and ` + + `${seconds}s). A run runs each slice exactly once, so these summaries come from ` + + 'DIFFERENT runs. Group them with `--run ` before each run\'s summaries: slices ' + + 'are summed WITHIN a run and the per-run sums are medianed ACROSS runs. Refusing to ' + + 'pick one, which would record the last run read rather than a median (#16473).' + ); + } + seen.set(slice.index, seconds); } } - // ⛔ An INCOMPLETE slice set is not summed. Summing 1 of 2 slices would record - // half a suite as the whole of it -- a wrong number that reads exactly like a - // right one, which is the hazard this file's cache rule already refuses in the - // other direction. The package instead drops out of `packages` entirely and is - // ESTIMATED from its test-file count like any unmeasured package, and it is - // named in the dataset so a refresh built on a partial artifact set is visible - // in the file rather than inferred from the split going strange later. + // ⛔ An INCOMPLETE slice set is not summed, and completeness is judged WITHIN + // ONE RUN. Summing 1 of 2 slices would record half a suite as the whole of it + // -- a wrong number that reads exactly like a right one, which is the hazard + // this file's cache rule already refuses in the other direction. Borrowing the + // missing slice from ANOTHER run to complete the set is the same wrong number + // wearing a complete set's clothes (#16473), so a run that cannot assemble the + // package on its own contributes nothing rather than something spliced. The + // package drops out of that run's sample and, if no run could assemble it, out + // of `packages` entirely -- ESTIMATED from its test-file count like any + // unmeasured package -- and every partial set is named, with its run, so a + // refresh built on a partial artifact set is visible in the file rather than + // inferred from the split going strange three weeks later. const incompleteSlices = []; - for (const [name, byCount] of sliceLedger) { - for (const [count, seen] of byCount) { - if (seen.size === count) { - push(name, [...seen.values()].reduce((a, b) => a + b, 0)); - continue; + for (const [run, byName] of sliceLedger) { + for (const [name, byCount] of byName) { + for (const [count, seen] of byCount) { + if (seen.size === count) { + push(name, [...seen.values()].reduce((a, b) => a + b, 0)); + continue; + } + const missing = []; + for (let i = 1; i <= count; i++) if (!seen.has(i)) missing.push(`${i}/${count}`); + incompleteSlices.push( + run === null + ? `${name} (missing ${missing.join(', ')})` + : `${name} in run ${run} (missing ${missing.join(', ')})` + ); } - const missing = []; - for (let i = 1; i <= count; i++) if (!seen.has(i)) missing.push(`${i}/${count}`); - incompleteSlices.push(`${name} (missing ${missing.join(', ')})`); } } incompleteSlices.sort((a, b) => a.localeCompare(b, 'en')); @@ -287,7 +356,7 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { // must not red. A battery BELOW its floor means cases stopped running; the // remedy is to find what stopped registering, never to lower the number. const SELF_TEST_BATTERIES = Object.freeze({ - 'measure-test-shard-timings self-test': 34, + 'measure-test-shard-timings self-test': 41, }); // DELETING an entry silences that battery's floor exactly as effectively as @@ -539,8 +608,9 @@ function selfTest() { } }); - // The two merge rules compose: sum WITHIN a run, median ACROSS runs. - const twoRuns = buildDataset({ + // One run's two slices sum to the whole package. This is the SUM half only -- + // it says nothing about the median half, which is what the block below pins. + const oneRunTwoSlices = buildDataset({ perSummary: [ samplesFromSummary(summary([slicedTask('cli', 0, 500_000, 1, 2)]), 'r1a'), samplesFromSummary(summary([slicedTask('cli', 0, 500_000, 2, 2)]), 'r1b'), @@ -550,8 +620,134 @@ function selfTest() { provenance: {}, }); check(() => { - if (twoRuns.packages.cli !== 1000) { - throw new Error(`slice: a 2-slice set summed to ${twoRuns.packages.cli}, expected 1000`); + if (oneRunTwoSlices.packages.cli !== 1000) { + throw new Error(`slice: a 2-slice set summed to ${oneRunTwoSlices.packages.cli}, expected 1000`); + } + }); + + // The two merge rules COMPOSE, and in one order only (#16473): sum the slices + // WITHIN a run, then median those per-run sums ACROSS runs. Every case below + // failed before the ledger was keyed by run, and each fails in a different + // direction, so none of them can be satisfied by accident: + // + // * the sliced median: last-wins answered 1000 where the median is 600 + // * the control: proves the median rule was alive the whole time, so the + // sliced path alone was bypassing it -- without this leg a broken median + // would look like a broken slice rule + // * the splice: 800s assembled from two different runs, with the file's own + // `skippedIncompleteSlices` guard silent because the set looked complete + // * the refusal: the guard that makes the UNDECLARED default safe, so the + // ordinary single-run call needs no ceremony and a multi-run one cannot + // quietly do the wrong thing + const fromRun = (run, tasks, label) => ({ ...samplesFromSummary(summary(tasks), label), run }); + + // Three runs of a 2-way sliced package: 200+200, 300+300, 500+500 -> the runs + // measured 400, 600 and 1000s, so the package's weight is the median 600. The + // unsliced control rides in the same dataset, fed 100/300/500 across the same + // three runs, and must answer its own median 300. + const threeRuns = buildDataset({ + perSummary: [ + fromRun('A', [slicedTask('cli', 0, 200_000, 1, 2)], 'a1'), + fromRun('A', [slicedTask('cli', 0, 200_000, 2, 2), testTask('ctl', 0, 100_000)], 'a2'), + fromRun('B', [slicedTask('cli', 0, 300_000, 1, 2)], 'b1'), + fromRun('B', [slicedTask('cli', 0, 300_000, 2, 2), testTask('ctl', 0, 300_000)], 'b2'), + fromRun('C', [slicedTask('cli', 0, 500_000, 1, 2)], 'c1'), + fromRun('C', [slicedTask('cli', 0, 500_000, 2, 2), testTask('ctl', 0, 500_000)], 'c2'), + ], + fileCounts: new Map([['cli', 300], ['ctl', 100]]), + provenance: {}, + }); + check(() => { + if (threeRuns.packages.cli !== 600) { + throw new Error( + `slice: three runs measuring 400/600/1000s recorded ${threeRuns.packages.cli}, expected the ` + + 'median 600 (1000 is the last run read -- the #16473 last-wins ledger)' + ); + } + }); + check(() => { + if (threeRuns.packages.ctl !== 300) { + throw new Error( + `slice: the unsliced control recorded ${threeRuns.packages.ctl}, expected its median 300 -- ` + + 'the median rule itself is broken, not just the sliced path' + ); + } + }); + check(() => { + if (threeRuns.skippedIncompleteSlices.length !== 0) { + throw new Error( + `slice: every run assembled a complete set, but ${threeRuns.skippedIncompleteSlices.join('; ')} ` + + 'was reported incomplete' + ); + } + }); + + // The cross-run splice. Run B is complete (300+300); run C fed only 1/2. The + // ledger must NOT reach into run B for run C's missing slice: the answer is + // run B's 600 alone, and run C is named as the partial set it is. Keyed + // without a run this recorded 800s -- a duration no run observed -- and named + // nothing. + const spliced = buildDataset({ + perSummary: [ + fromRun('B', [slicedTask('cli', 0, 300_000, 1, 2)], 'b1'), + fromRun('B', [slicedTask('cli', 0, 300_000, 2, 2)], 'b2'), + fromRun('C', [slicedTask('cli', 0, 500_000, 1, 2), testTask('ctl', 0, 10_000)], 'c1'), + ], + fileCounts: new Map([['cli', 300], ['ctl', 100]]), + provenance: {}, + }); + check(() => { + if (spliced.packages.cli !== 600) { + throw new Error( + `slice: a set completed ACROSS runs recorded ${spliced.packages.cli}, expected run B's own 600 ` + + '(800 is B 1/2 + B 2/2 + C 1/2 spliced -- a weight no run measured)' + ); + } + }); + check(() => { + if (!spliced.skippedIncompleteSlices.some((s) => s.includes('cli') && s.includes('run C') && s.includes('2/2'))) { + throw new Error( + 'slice: run C could not assemble the package and was not named in skippedIncompleteSlices ' + + `(${spliced.skippedIncompleteSlices.join('; ') || 'empty'})` + ); + } + }); + + // The refusal that makes the undeclared default honest. Two runs' slices fed + // as one group is not resolvable -- the summaries carry no run id of their own + // -- so it is a named error, never the last value read. + check(() => { + if (!threw(() => + buildDataset({ + perSummary: [ + samplesFromSummary(summary([slicedTask('cli', 0, 300_000, 1, 2)]), 'x1'), + samplesFromSummary(summary([slicedTask('cli', 0, 500_000, 1, 2)]), 'x2'), + ], + fileCounts: new Map([['cli', 300]]), + provenance: {}, + }) + )) { + throw new Error('slice: two runs fed as one group were resolved by last-wins instead of refused'); + } + }); + check(() => { + let message = ''; + try { + buildDataset({ + perSummary: [ + samplesFromSummary(summary([slicedTask('cli', 0, 300_000, 2, 2)]), 'x1'), + samplesFromSummary(summary([slicedTask('cli', 0, 500_000, 2, 2)]), 'x2'), + ], + fileCounts: new Map([['cli', 300]]), + provenance: {}, + }); + } catch (error) { + message = String(error?.message ?? ''); + } + // The remedy has to be IN the refusal: a caller who hits this is holding + // several runs' artifacts and needs to be told the flag, not just told no. + if (!message.includes('--run') || !message.includes('cli slice 2/2')) { + throw new Error(`slice: the duplicate-slice refusal does not name the slice and the remedy (${message})`); } }); @@ -690,20 +886,35 @@ function main() { return; } let out = DEFAULT_OUT; + // Each input carries the run it belongs to (#16473). `--run ` opens a + // group and every summary AFTER it belongs to that run, so one CI run's six + // artifacts are named together the way they are fetched together. Summaries + // before any `--run` share one implicit group, which is the ordinary + // single-run refresh; feeding two runs that way is not silently averaged or + // last-won, it is refused by buildDataset with `--run` named as the remedy. const inputs = []; + let currentRun = null; for (let i = 0; i < argv.length; i++) { if (argv[i] === '--out') out = path.resolve(argv[++i]); - else if (argv[i].startsWith('--')) throw new Error(`unrecognized argument: ${argv[i]}`); - else inputs.push(argv[i]); + else if (argv[i] === '--run') { + const value = argv[++i]; + if (typeof value !== 'string' || value.length === 0 || value.startsWith('--')) { + throw new Error('--run needs a run identifier (the id of the CI run whose summaries follow it)'); + } + currentRun = value; + } else if (argv[i].startsWith('--')) throw new Error(`unrecognized argument: ${argv[i]}`); + else inputs.push({ file: argv[i], run: currentRun }); } if (inputs.length === 0) { - console.error('usage: measure-test-shard-timings.mjs ... [--out ]'); + console.error( + 'usage: measure-test-shard-timings.mjs [--run ] ... [--out ]' + ); process.exit(1); } const perSummary = []; - for (const input of inputs) { - perSummary.push(samplesFromSummary(JSON.parse(readFileSync(input, 'utf8')), input)); + for (const { file, run } of inputs) { + perSummary.push({ ...samplesFromSummary(JSON.parse(readFileSync(file, 'utf8')), file), run }); } const fileCounts = new Map(); @@ -715,17 +926,28 @@ function main() { } } + const declaredRuns = [...new Set(inputs.map((i) => i.run).filter((r) => r !== null))]; const dataset = buildDataset({ perSummary, fileCounts, provenance: { measuredAt: new Date().toISOString().slice(0, 10), - summaries: inputs.map((i) => path.basename(i)), - mergeRule: 'median across summaries', + summaries: inputs.map((i) => path.basename(i.file)), + ...(declaredRuns.length > 0 ? { runs: declaredRuns } : {}), + // Stated as the two composed rules it actually is (#16473). "median across + // summaries" was true only of unsliced packages: a file-sharded package's + // summaries are PARTS of one measurement, not repeats of it, so they are + // summed within their run first and only the per-run sums are medianed. + mergeRule: + 'median across runs; a file-sharded package is summed from its slices WITHIN one run ' + + 'first, and a run that cannot assemble every slice contributes no sample for it', refresh: - 'node scripts/measure-test-shard-timings.mjs ... --out scripts/test-shard-timings.json ' + - '(summaries: the `test-core-run-summary--of-6` artifacts of any green merge_group run, or a local ' + - '`pnpm exec turbo run test --concurrency=4 --summarize`)', + 'node scripts/measure-test-shard-timings.mjs [--run ] ... --out ' + + 'scripts/test-shard-timings.json (summaries: the `test-core-run-summary--of-6` artifacts of ' + + 'any green run, or a local `pnpm exec turbo run test --concurrency=4 --summarize`. Feeding more ' + + 'than one run REQUIRES a `--run ` before each run\'s summaries, so a sliced package is ' + + 'assembled per run and then medianed like every other package. `.github/workflows/' + + 'shard-timings-refresh.yml` does this weekly.)', }, }); writeFileSync(out, `${JSON.stringify(dataset, null, 2)}\n`); From 2135d2f73286d09a9f09ed0ce86f5405fc3b5958 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 03:44:27 +0000 Subject: [PATCH 2/8] ci: regenerate the shard-timings dataset on a weekly timer and open the PR scripts/test-shard-timings.json is the balancing input for the Test Core shard split, and it was the only part of that loop with no clock on it: generated, but only when someone remembered. It went 13 days without a refresh while ~700 test files were added, and the shard it mis-weighted was killed by the job wall twelve times in one day and ejected from the merge queue twice. The workflow runs weekly (Monday 05:30Z) and on dispatch. It selects a green run, downloads its six run-summary artifacts, re-runs the generator, and opens a PR only when the file changed by byte comparison. It never hand-edits the dataset and never touches a bound, a timeout or the shard matrix. It runs on a GitHub-hosted runner because that is where the inputs are reachable: the artifact host is denied to every agent container by egress policy, so the documented refresh path cannot be walked from a dev seat at all. Running here removes the channel from the loop rather than working around the denial. Choosing the run is the part that needed a tested unit, so it is one -- scripts/ci/select-shard-timings-run.mjs, with a 30-case self-test. "The newest run" is wrong three ways: cancel-in-progress censors most push runs on main, a run whose six shards all concluded success can still have replayed most of the workspace from the turbo cache, and artifacts expire after a day. The replay case is judged on what was MEASURED -- a package the committed dataset measured and this refresh does not has silently fallen back to the file-count estimate -- so a partial replay is rejected by name rather than by a duration threshold. The partitioner's balancing pins are RUN but never acted on. They red by design on a refresh that outgrows the acceptance bound, and the remedy they name (raise the slice count, never the bound) is a decision; the verdict is quoted into the PR body verbatim and the PR is opened either way, because withholding it would lose the measurement. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox --- .github/workflows/shard-timings-refresh.yml | 413 +++++++++++++ scripts/ci/select-shard-timings-run.mjs | 605 ++++++++++++++++++++ 2 files changed, 1018 insertions(+) create mode 100644 .github/workflows/shard-timings-refresh.yml create mode 100644 scripts/ci/select-shard-timings-run.mjs diff --git a/.github/workflows/shard-timings-refresh.yml b/.github/workflows/shard-timings-refresh.yml new file mode 100644 index 0000000000..3eb1d5d732 --- /dev/null +++ b/.github/workflows/shard-timings-refresh.yml @@ -0,0 +1,413 @@ +# Regenerate scripts/test-shard-timings.json on a timer, and open the PR. +# +# ══════════════════════════════════════════════════════════════════════════════ +# WHY THIS EXISTS: THE BALANCING INPUT WAS THE ONLY PART OF THE LOOP WITH NO +# CLOCK ON IT. (#16464) +# ══════════════════════════════════════════════════════════════════════════════ +# +# scripts/test-shard-timings.json is the per-package duration dataset the Test +# Core split is binned from. It is GENERATED — scripts/measure-test-shard- +# timings.mjs turns a green run's six turbo summaries into it — but until now it +# was generated only when a person remembered to. It went 13 days without a +# refresh while ~700 test files were added, and the shard it mis-weighted was +# killed by the job wall twelve times in one day and ejected from the merge queue +# twice (#16173). +# +# The rot is one-directional and silent, which is what makes a timer the fix +# rather than more discipline: suites only get slower, the table stays put, and +# the shard that drifted heavy reads as balanced on paper right up to the moment +# it is killed — and a killed shard produces NO measurement while the rollup +# reads green. +# +# WHAT THIS WORKFLOW IS NOT ALLOWED TO DO, AND WHY EACH ONE IS LOAD-BEARING +# ------------------------------------------------------------------------ +# ⛔ It never hand-edits the dataset. Every byte it commits came out of the +# generator. A hand-touched number wearing a generated file's `provenance` +# is worse than a stale one: the staleness is at least visible in +# `measuredAt`. +# ⛔ It never touches `timeout-minutes`, the shard matrix, MAX_SHARD_OVER_MEAN, +# MAX_MEASURED_OVER_PREDICTED or FILE_SHARDED_PACKAGES. The partitioner's +# own header calls raising a bound "the one move that cannot be right", and +# its pin 3c is built to red on exactly that. If an honest refresh breaches +# a bound, that breach is the REPORT, not a problem to be tuned away — it is +# quoted verbatim into the PR body and a human decides. See "THE PINS" below. +# ⛔ It never opens a PR when the regenerated file is byte-identical. A weekly +# no-op PR trains everyone to ignore this PR. +# +# HOW IT GETS ITS INPUTS, AND WHY IT RUNS HERE RATHER THAN IN AN AGENT WORKTREE +# ---------------------------------------------------------------------------- +# The documented refresh path downloads the six `test-core-run-summary-N-of-6` +# artifacts of a green run. `GET /actions/artifacts/{id}/zip` redirects to +# `productionresultssa*.blob.core.windows.net`, and every agent container's +# egress policy answers 403 to CONNECT for that host — reproduced four ways, on +# two artifacts across two runs, on the pre-signed URL as well as the API one, on +# two separate days and containers (#16222, and again on #16173). So the +# documented path cannot be walked from a dev seat at all. +# +# A GitHub-hosted runner reaches that host natively — it is the same host +# `actions/download-artifact` uses. Running the refresh here is therefore not a +# workaround for the denial, it is the removal of the channel from the loop: the +# regeneration happens where the data already is, on a timer, and no seat needs +# reachability it does not have. +# +# CHOOSING THE RUN IS THE HARD PART — see scripts/ci/select-shard-timings-run.mjs +# ------------------------------------------------------------------------------ +# "The newest run" is wrong three different ways here (cancelled runs, cache +# replays, expired artifacts), and a refresh built on the wrong run is worse than +# no refresh because it stamps a fresh `measuredAt` on numbers nobody measured. +# That script carries the argument and the self-test; this file only drives it. +# +# THE PINS, AND THE ONE THING A MACHINE MUST NOT DECIDE +# ---------------------------------------------------- +# partition-test-shards.mjs `--self-test` grades the dataset against the +# acceptance bound. On a refresh that makes a package heavier than any split can +# bin, it reds BY DESIGN and names the remedy (raise the slice count — never the +# bound), and its pin 3c additionally demands a decision the day the CLI comes +# back under the bound on its own. Both are judgement, so this workflow REPORTS +# the verdict and never acts on it: the self-test is run on the refreshed file +# and its output — pass or fail, verbatim — goes into the PR body. The PR is +# opened either way, because a refusal to open it would lose the measurement, +# which is the exact silent-rot failure this workflow exists to end. + +name: Shard Timings Refresh + +on: + schedule: + # Monday 05:30 UTC. After the nightly lanes, and early enough in the week + # that the PR is in front of someone before the week's merge volume builds. + - cron: '30 5 * * 1' + workflow_dispatch: + +# Read-only at the top level; the one job widens to exactly what it writes. +permissions: + contents: read + +# One refresh at a time. A second run while the first is mid-push would race on +# the same bot branch; the newer inputs are the better ones, so the in-flight run +# yields. +concurrency: + group: shard-timings-refresh + cancel-in-progress: true + +jobs: + refresh: + name: Regenerate the shard-timings dataset + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + # Declaring any `permissions:` block drops every scope not listed, so all + # three are spelled even though only two are writes. + contents: write # push the bot branch + pull-requests: write # open the PR and add its label + actions: read # list runs, read jobs, download run-summary artifacts + steps: + - name: Checkout repository + uses: actions/checkout@v7 + with: + # A PAT when one exists, the Actions token otherwise — the same choice + # cut-rc.yml makes, for the same reason, and the difference is stated + # in the PR body rather than left to be discovered: a PR opened with + # the Actions `GITHUB_TOKEN` starts NO workflow runs (GitHub's + # recursion guard), so with the fallback credential the refresh PR + # arrives with no CI of its own until someone pushes to it or reopens + # it. With the PAT its checks run immediately. + token: ${{ secrets.RELEASE_PUSH_TOKEN || github.token }} + + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: '22' + + - name: Setup pnpm + uses: ./.github/actions/setup-pnpm + + - name: Get pnpm store directory + shell: bash + run: echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV + + # Restore-only: scheduled runs read main's store cache; the per-push + # workflows own saving it. Same shape as coverage-nightly.yml. + - name: Restore pnpm cache + uses: actions/cache/restore@v6 + with: + path: ${{ env.STORE_PATH }} + key: ${{ runner.os }}-pnpm-store-v3-${{ hashFiles('**/pnpm-lock.yaml') }} + restore-keys: | + ${{ runner.os }}-pnpm-store-v3- + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + # Verify the instruments BEFORE trusting their output. Both scripts carry a + # battery floor, so this also catches the case where their assertions + # stopped running — which would otherwise let a wrong dataset through a + # green-looking pipeline. + - name: Self-test the generator and the run selector + run: | + node scripts/measure-test-shard-timings.mjs --self-test + node scripts/ci/select-shard-timings-run.mjs --self-test + + # The package list the coverage check judges against. `turbo ls` is + # experimental, so the reader asserts its payload loudly rather than + # defaulting around it (an empty list would make every package look deleted + # and every coverage check pass). + - name: List the workspace + run: pnpm exec turbo ls --output=json > "$RUNNER_TEMP/turbo-ls.json" + + - name: Choose a green, uncensored, un-replayed run + id: select + env: + GITHUB_TOKEN: ${{ github.token }} + run: | + node scripts/ci/select-shard-timings-run.mjs --candidates --limit 15 \ + > "$RUNNER_TEMP/candidates.json" + echo "Eligible runs, newest first:" + node -e ' + const runs = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8")); + for (const r of runs) console.log(` ${r.run_id} ${r.created_at} ${r.head_sha.slice(0, 10)}`); + ' + + # Each eligible run is tried in turn: download, generate, and then judge + # whether it MEASURED the workspace. A run that passed selection can still + # be a cache replay, and the only honest test for that is the resulting + # dataset's package coverage — so the loop is the point, not a retry. + - name: Regenerate the dataset from the first run that actually measured + id: generate + env: + GITHUB_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + WORK="$RUNNER_TEMP/refresh" + mkdir -p "$WORK" + CHOSEN='' + + RUN_COUNT=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8")).length)') + for i in $(seq 0 $((RUN_COUNT - 1))); do + RUN_ID=$(node -e 'const r=JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP+"/candidates.json","utf8"))[Number(process.argv[1])];console.log(r.run_id)' "$i") + echo "::group::Candidate run $RUN_ID" + SUMDIR="$WORK/$RUN_ID" + rm -rf "$SUMDIR"; mkdir -p "$SUMDIR" + + OK=1 + for N in 1 2 3 4 5 6; do + ART=$(node -e 'const r=JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP+"/candidates.json","utf8"))[Number(process.argv[1])];console.log(r.artifact_ids[process.argv[2]])' "$i" "$N") + # curl drops the Authorization header across the redirect to the + # blob host, which is correct: that URL is pre-signed and the + # header would be rejected. + if ! curl -sSL --fail -H "Authorization: Bearer $GITHUB_TOKEN" \ + -H "Accept: application/vnd.github+json" \ + "https://api.github.com/repos/$GITHUB_REPOSITORY/actions/artifacts/$ART/zip" \ + -o "$SUMDIR/$N.zip"; then + echo "::warning::Could not download artifact $ART (shard $N) of run $RUN_ID; skipping this run." + OK=0; break + fi + unzip -qo "$SUMDIR/$N.zip" -d "$SUMDIR/$N" + done + if [ "$OK" -ne 1 ]; then echo "::endgroup::"; continue; fi + + # `find` rather than a fixed-depth glob: `upload-artifact` with + # `path: .turbo/runs/` puts the summaries at the artifact root, but a + # shape assumption here would silently collect NOTHING and hand the + # generator an empty argument list, which reads like a refusal for + # the wrong reason. + find "$SUMDIR" -name '*.json' -type f > "$SUMDIR/summaries.txt" + SUMMARY_COUNT=$(wc -l < "$SUMDIR/summaries.txt") + echo "Collected $SUMMARY_COUNT run summary file(s) from run $RUN_ID." + if [ "$SUMMARY_COUNT" -eq 0 ]; then + echo "::warning::Run $RUN_ID's artifacts contained no run-summary JSON; skipping this run." + echo "::endgroup::"; continue + fi + + # One run, so no `--run` grouping is needed: every summary here came + # from run $RUN_ID. Feeding SEVERAL runs would require `--run ` + # before each run's six files — the generator refuses the ambiguity + # rather than taking the last value (#16473). + # An array, not `xargs`: a split invocation would run the generator + # twice and the second would overwrite the first's output with a + # partial dataset. + mapfile -t SUMMARY_FILES < "$SUMDIR/summaries.txt" + if ! node scripts/measure-test-shard-timings.mjs "${SUMMARY_FILES[@]}" \ + --out "$WORK/refreshed.json"; then + echo "::warning::The generator refused run $RUN_ID's summaries; skipping this run." + echo "::endgroup::"; continue + fi + + if node scripts/ci/select-shard-timings-run.mjs --check-coverage \ + --committed scripts/test-shard-timings.json \ + --refreshed "$WORK/refreshed.json" \ + --workspace "$RUNNER_TEMP/turbo-ls.json" \ + --exclude @objectstack/dogfood; then + CHOSEN="$RUN_ID" + echo "::endgroup::" + break + fi + echo "::endgroup::" + done + + if [ -z "$CHOSEN" ]; then + echo "::error::No eligible run produced a complete measurement of the workspace. Every candidate was a cache replay, refused by the generator, or lost its artifacts. NOTHING was regenerated and no PR was opened — this is a refusal, not a quiet success." + exit 1 + fi + echo "run_id=$CHOSEN" >> "$GITHUB_OUTPUT" + echo "head_sha=$(node -e 'const rs=JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP+"/candidates.json","utf8"));console.log(rs.find(r=>String(r.run_id)===process.argv[1]).head_sha)' "$CHOSEN")" >> "$GITHUB_OUTPUT" + + # Byte comparison, and it decides everything downstream. `cmp -s` rather + # than a diff of parsed JSON: the committed artefact is the file, so the + # file is what has to differ for a PR to be worth anyone's attention. + - name: Compare against the committed dataset + id: compare + run: | + if cmp -s scripts/test-shard-timings.json "$RUNNER_TEMP/refresh/refreshed.json"; then + echo "changed=false" >> "$GITHUB_OUTPUT" + echo "The regenerated dataset is BYTE-IDENTICAL to the committed one; no PR." | tee -a "$GITHUB_STEP_SUMMARY" + else + echo "changed=true" >> "$GITHUB_OUTPUT" + fi + + # The before/after halves of ruling 3, taken from the partitioner's OWN + # verdict line rather than recomputed here — a second implementation of the + # binning would be a second answer to grade against. + # + # ⛔ `continue-on-error` on the AFTER leg only, and it is not leniency: a + # red here is the partitioner refusing an honest measurement, which is + # information the PR must carry, not a reason to withhold the PR. The + # verdict text goes into the body either way and the PR's own lint job + # grades it again. + - name: Predicted bins, before and after + id: bins + if: steps.compare.outputs.changed == 'true' + run: | + node scripts/partition-test-shards.mjs --self-test > "$RUNNER_TEMP/bins-before.txt" 2>&1 || true + cp "$RUNNER_TEMP/refresh/refreshed.json" scripts/test-shard-timings.json + set +e + node scripts/partition-test-shards.mjs --self-test > "$RUNNER_TEMP/bins-after.txt" 2>&1 + echo "partitioner_exit=$?" >> "$GITHUB_OUTPUT" + set -e + echo "BEFORE: $(cat "$RUNNER_TEMP/bins-before.txt")" + echo "AFTER: $(cat "$RUNNER_TEMP/bins-after.txt")" + + - name: Push the refresh branch and open the pull request + if: steps.compare.outputs.changed == 'true' + env: + GH_TOKEN: ${{ secrets.RELEASE_PUSH_TOKEN || github.token }} + RUN_ID: ${{ steps.generate.outputs.run_id }} + HEAD_SHA: ${{ steps.generate.outputs.head_sha }} + PARTITIONER_EXIT: ${{ steps.bins.outputs.partitioner_exit }} + USED_PAT: ${{ secrets.RELEASE_PUSH_TOKEN != '' }} + run: | + set -euo pipefail + BRANCH="claude/shard-timings-refresh-$RUN_ID" + git config user.name 'github-actions[bot]' + git config user.email '41898282+github-actions[bot]@users.noreply.github.com' + git switch -c "$BRANCH" + git add scripts/test-shard-timings.json + # Separate -m flags rather than one embedded newline: a YAML-indented + # heredoc would carry its own leading whitespace into the message body. + git commit \ + -m "chore(ci): refresh the Test Core shard-timings dataset" \ + -m "Regenerated by .github/workflows/shard-timings-refresh.yml from the six test-core-run-summary artifacts of run $RUN_ID ($HEAD_SHA). Generated, never hand-edited." + git push origin "$BRANCH" + + SHARDS=$(node -e ' + const rs = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8")); + const r = rs.find((x) => String(x.run_id) === process.env.RUN_ID); + const s = r.shard_seconds ?? {}; + console.log([1,2,3,4,5,6].map((n) => (s[n] == null ? "?" : `${n}: ${s[n]}s`)).join(" | ")); + ') + + { + echo "Refreshes \`scripts/test-shard-timings.json\`, the balancing input for the Test Core" + echo "shard split. Opened automatically by \`.github/workflows/shard-timings-refresh.yml\`." + echo "Every byte came out of \`scripts/measure-test-shard-timings.mjs\`; nothing here was" + echo "hand-edited, and no bound, timeout or matrix entry was touched." + echo + echo "## Source" + echo + echo "- Run: https://github.com/$GITHUB_REPOSITORY/actions/runs/$RUN_ID" + echo "- Commit measured: \`$HEAD_SHA\`" + echo "- Selected because its six \`Test Core (N/6)\` jobs all concluded \`success\`, its six" + echo " run-summary artifacts were still retained, and the dataset it produced measures every" + echo " package the committed one measured that is still in the workspace (the check that" + echo " rejects a cache replay)." + echo + echo "## Measured per-shard suite time on that run" + echo + echo "\`\`\`" + echo "$SHARDS" + echo "\`\`\`" + echo + echo "## Predicted bins, before and after" + echo + echo "\`\`\`" + echo "BEFORE $(cat "$RUNNER_TEMP/bins-before.txt")" + echo "AFTER $(cat "$RUNNER_TEMP/bins-after.txt")" + echo "\`\`\`" + echo + if [ "${PARTITIONER_EXIT:-0}" != "0" ]; then + echo "## The partitioner's own pins RED on this refresh — read this before merging" + echo + echo "This is the designed behaviour, not a defect in the refresh: the acceptance bound is" + echo "a ratio, and a package that has grown past what any six-way split can bin makes the" + echo "pins fail with the arithmetic in the message. The remedy the partitioner names is to" + echo "raise the file-level slice count for that package — ⛔ never to raise the bound, and" + echo "⛔ never to hand-edit this dataset. This workflow deliberately does neither: it" + echo "reports and stops, because both are decisions." + echo + fi + if [ "$USED_PAT" != "true" ]; then + echo "## No checks will start on this PR by themselves" + echo + echo "It was opened with the Actions \`GITHUB_TOKEN\`, and GitHub's recursion guard means a" + echo "PR opened that way triggers no workflow runs. Push any commit to the branch, or close" + echo "and reopen the PR, to start CI." + echo + fi + # ⛔ No closing keyword is emitted here, in any form. GitHub's + # parser matches `fixes`/`closes`/`resolves` plus a number and + # ignores every negation around them, so even a sentence saying a + # card is NOT closed would close it. This PR is the workflow's + # OUTPUT; the cards about the workflow are referenced only. + echo "Refs #16464, #16173, #16222." + } > "$RUNNER_TEMP/pr-body.md" + + PR_URL=$(gh pr create --base main --head "$BRANCH" \ + --title "chore(ci): refresh the Test Core shard-timings dataset" \ + --body-file "$RUNNER_TEMP/pr-body.md") + echo "Opened $PR_URL" | tee -a "$GITHUB_STEP_SUMMARY" + + # ADDITIVE label write only. A whole-set PUT replaces the PR's labels + # and destroys any that land in between — measured on this repo, one + # second wide (see pr-automation.yml's header). POST names only what it + # adds, so no interleaving can lose another writer's label. + PR_NUMBER=$(gh pr view "$PR_URL" --json number --jq .number) + gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" \ + -f "labels[]=skip-changeset" > /dev/null + + # Read back, because an additive write is necessary and not sufficient: + # a concurrent whole-set PUT from another workflow can still strip the + # label after a successful POST. `skip-changeset` is this PR's exemption + # from the changeset gate — it publishes nothing — so losing it turns + # the gate red on a PR that legitimately has no changeset. + if gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" --jq '.[].name' \ + | grep -qxF 'skip-changeset'; then + echo "skip-changeset confirmed on PR #$PR_NUMBER." + else + echo "::warning::skip-changeset did not survive the write on PR #$PR_NUMBER (a concurrent whole-set label PUT strips it). Re-applying once." + gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" \ + -f "labels[]=skip-changeset" > /dev/null + gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" --jq '.[].name' \ + | grep -qxF 'skip-changeset' \ + || echo "::error::skip-changeset is still absent from PR #$PR_NUMBER; the changeset gate will demand a changeset this PR legitimately has none of. Apply the label by hand." + fi + + - name: Say what happened when nothing changed + if: steps.compare.outputs.changed == 'false' + run: | + { + echo "### Shard timings: byte-identical, no PR" + echo + echo "Run \`${{ steps.generate.outputs.run_id }}\` (\`${{ steps.generate.outputs.head_sha }}\`)" + echo "regenerated \`scripts/test-shard-timings.json\` byte-for-byte identically to the" + echo "committed file, so there is nothing to open a pull request about. The dataset is" + echo "current, and this is the loop working rather than the loop skipping." + } >> "$GITHUB_STEP_SUMMARY" diff --git a/scripts/ci/select-shard-timings-run.mjs b/scripts/ci/select-shard-timings-run.mjs new file mode 100644 index 0000000000..9b61bc20d2 --- /dev/null +++ b/scripts/ci/select-shard-timings-run.mjs @@ -0,0 +1,605 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// select-shard-timings-run -- choose the CI run whose artifacts +// `.github/workflows/shard-timings-refresh.yml` regenerates +// scripts/test-shard-timings.json from, and judge afterwards whether that run +// actually measured the workspace (#16464). +// +// WHY SELECTION IS NOT "THE NEWEST RUN". +// +// The dataset is the balancing input for the Test Core shard split, and a +// refresh built on the wrong run is not a missing refresh -- it is a WRONG one, +// which is worse, because the file then carries a fresh `measuredAt` over +// numbers nobody measured. Three ways the newest run is the wrong one, all +// measured on this repo rather than imagined: +// +// 1. CANCELLED RUNS. `cancel-in-progress` kills a push run on main the moment +// the next merge lands, and merges arrive faster than Test Core finishes: +// 36 of the last 60 push runs were censored that way. Such a run leaves +// SOME artifacts, so "the artifacts exist" is not the test -- every one of +// the six Test Core jobs must have concluded `success`. +// +// 2. CACHE REPLAYS. A run whose six jobs all concluded `success` can still +// have replayed most of the workspace from the turbo cache: shards were +// measured at 7s and 98s against a 672s prediction (0.01x, 0.15x). The +// generator REFUSES cached tasks, so those packages simply are not in the +// resulting dataset -- and an absent package does not red anything, it +// falls back to the test-file-count ESTIMATE. A partial replay therefore +// trades measured weights for guesses, silently, on whichever slice of the +// workspace happened to be warm. `coverageReport` below is the half that +// catches this, and it catches it by looking at what was MEASURED rather +// than at a duration threshold nobody can defend. +// +// 3. EXPIRED ARTIFACTS. Retention is 1 day, so the eligible window is +// genuinely short and an empty candidate list is a normal Monday if main +// was quiet -- it is reported as a loud, named refusal rather than as a +// no-op, because "no PR opened" and "no run was good enough" must not read +// the same way in the log. +// +// ⛔ It never picks a run it cannot fully justify. Every rejection is named with +// its reason, so the workflow log says WHY the newest four runs were passed over +// rather than leaving a bare run id to be taken on faith. +// +// Usage: +// node scripts/ci/select-shard-timings-run.mjs --candidates [--limit ] +// node scripts/ci/select-shard-timings-run.mjs --check-coverage \ +// --committed --refreshed --workspace \ +// [--exclude ]... +// node scripts/ci/select-shard-timings-run.mjs --self-test +// +// `--candidates` needs GITHUB_TOKEN and GITHUB_REPOSITORY in the environment and +// prints a JSON array, newest first. `--check-coverage` exits non-zero when the +// refreshed dataset lost a package the committed one measured and the workspace +// still contains. + +import { readFileSync } from 'node:fs'; +import process from 'node:process'; + +import { isEntrypoint } from '../invoked-as.mjs'; + +// The Test Core shard count. Spelled here as the number of jobs and artifacts a +// complete run must present; partition-test-shards.mjs owns the split itself and +// its own self-test reads ci.yml back, so this is a reader's expectation rather +// than a second declaration of the split. +export const SHARD_COUNT = 6; + +const API = 'https://api.github.com'; + +// `Test Core (3/6)` -> 3. Anchored on both ends: a job merely CONTAINING that +// text (a future "Test Core (3/6) rerun") is not this job, and reading it as one +// would let a run qualify on a job that never ran the suite. +export function testCoreShard(jobName, shardCount = SHARD_COUNT) { + const m = /^Test Core \((\d+)\/(\d+)\)$/.exec(String(jobName ?? '').trim()); + if (!m) return null; + if (Number(m[2]) !== shardCount) return null; + const index = Number(m[1]); + return index >= 1 && index <= shardCount ? index : null; +} + +// `test-core-run-summary-3-of-6` -> 3, by the same anchoring argument. +export function summaryArtifactShard(artifactName, shardCount = SHARD_COUNT) { + const m = /^test-core-run-summary-(\d+)-of-(\d+)$/.exec(String(artifactName ?? '').trim()); + if (!m) return null; + if (Number(m[2]) !== shardCount) return null; + const index = Number(m[1]); + return index >= 1 && index <= shardCount ? index : null; +} + +// How long a shard's suite actually ran, from the job's own step window. This +// is the tighter reading than job wall-clock -- setup, install and cache restore +// are not the suite -- and it is what the refresh PR quotes as "per-shard +// durations" so a reviewer can compare measured against predicted without +// opening the run. +export const TEST_STEP_NAME = "Run this shard's tests"; +export function testStepSeconds(job, stepName = TEST_STEP_NAME) { + const step = (job?.steps ?? []).find((s) => s?.name === stepName); + if (!step?.started_at || !step?.completed_at) return null; + const seconds = (Date.parse(step.completed_at) - Date.parse(step.started_at)) / 1000; + return Number.isFinite(seconds) && seconds >= 0 ? seconds : null; +} + +// Is this run a complete, uncensored measurement? Both halves are required and +// they fail for different reasons, so both are named separately. +// +// ⛔ `conclusion === 'success'` is the test, NOT `status === 'completed'`: a +// cancelled job is completed too, and a cancelled Test Core shard is exactly the +// censored run this refuses. A job that is still running is likewise not a +// success -- it may yet fail -- so anything other than the literal string is a +// rejection. +export function runIsEligible({ jobs, artifacts }, shardCount = SHARD_COUNT) { + const reasons = []; + + const byShard = new Map(); + for (const job of jobs ?? []) { + const shard = testCoreShard(job?.name, shardCount); + if (shard === null) continue; + // A re-run leaves several job records for one shard; the run qualifies if + // ANY attempt of that shard concluded success, which is the same rule the + // artifacts follow (the successful attempt is the one that uploaded). + if (job?.conclusion === 'success') byShard.set(shard, job); + } + const missingJobs = []; + for (let i = 1; i <= shardCount; i++) if (!byShard.has(i)) missingJobs.push(`${i}/${shardCount}`); + if (missingJobs.length > 0) { + reasons.push(`Test Core ${missingJobs.join(', ')} did not conclude success (cancelled, failed or never ran)`); + } + + const artifactByShard = new Map(); + for (const artifact of artifacts ?? []) { + const shard = summaryArtifactShard(artifact?.name, shardCount); + if (shard === null) continue; + // An expired artifact is a 410 at download time, so it is not an input. + if (artifact?.expired === true) continue; + if (!artifactByShard.has(shard)) artifactByShard.set(shard, artifact.id); + } + const missingArtifacts = []; + for (let i = 1; i <= shardCount; i++) if (!artifactByShard.has(i)) missingArtifacts.push(`${i}-of-${shardCount}`); + if (missingArtifacts.length > 0) { + reasons.push(`run summary artifact(s) ${missingArtifacts.join(', ')} are missing or expired`); + } + + const shardSeconds = {}; + for (const [shard, job] of [...byShard.entries()].sort((a, b) => a[0] - b[0])) { + shardSeconds[shard] = testStepSeconds(job); + } + + return { + eligible: reasons.length === 0, + reasons, + artifactIds: Object.fromEntries([...artifactByShard.entries()].sort((a, b) => a[0] - b[0])), + shardSeconds, + }; +} + +// The half that catches a cache replay, an incomplete slice set, or any other +// reason a run "succeeded" without measuring much (#16464 hazard 2 above). +// +// The judgement is deliberately about MEASUREMENT and not about duration: a +// package the committed dataset measured, that the workspace still contains, and +// that the refreshed dataset does NOT measure, has silently been demoted to the +// test-file-count estimate. That is the direction nobody notices, so it is the +// direction this refuses in. Comparing against the live workspace is what keeps +// a package legitimately deleted from the monorepo from blocking every future +// refresh -- it is gone from `workspace`, so it is not required. +export function coverageReport({ committed, refreshed, workspace, exclude = [] }) { + const excluded = new Set(exclude); + const inWorkspace = new Set(workspace); + const required = Object.keys(committed?.packages ?? {}).filter( + (name) => inWorkspace.has(name) && !excluded.has(name) + ); + const measured = new Set(Object.keys(refreshed?.packages ?? {})); + const lost = required.filter((name) => !measured.has(name)).sort((a, b) => a.localeCompare(b, 'en')); + const gained = [...measured] + .filter((name) => !Object.hasOwn(committed?.packages ?? {}, name)) + .sort((a, b) => a.localeCompare(b, 'en')); + return { + ok: lost.length === 0, + lost, + gained, + requiredCount: required.length, + measuredCount: measured.size, + }; +} + +// `turbo ls --output=json` -> package names. The payload shape is asserted +// loudly for the same reason partition-test-shards.mjs asserts it: `turbo ls` is +// marked experimental, and a silently-empty package list here would make every +// package look deleted and every coverage check pass. +export function workspaceNames(parsed) { + const items = parsed?.packages?.items; + if (!Array.isArray(items)) { + throw new Error( + 'workspace: expected `turbo ls --output=json` output with a {packages:{items:[...]}} array' + ); + } + const names = items.map((item) => item?.name).filter((n) => typeof n === 'string' && n.length > 0); + if (names.length === 0) throw new Error('workspace: `turbo ls` listed no packages'); + return names; +} + +// --------------------------------------------------------------------------- +// The network half. Thin on purpose: every judgement above is a pure function +// with a self-test, and what is left here is paging and shape-reading. +// --------------------------------------------------------------------------- + +async function api(pathname, { token, fetchImpl = fetch }) { + const response = await fetchImpl(`${API}${pathname}`, { + headers: { + accept: 'application/vnd.github+json', + authorization: `Bearer ${token}`, + 'x-github-api-version': '2022-11-28', + }, + }); + if (!response.ok) { + throw new Error(`GET ${pathname} -> HTTP ${response.status} ${response.statusText}`); + } + return response.json(); +} + +// Newest first, bounded. `limit` is how many runs are EXAMINED, not how many +// come back: the answer is often the fourth or fifth run, and stopping at the +// first eligible one would hide the fact that the newest four were replays. +export async function listCandidates({ + repo, + token, + workflow = 'ci.yml', + shardCount = SHARD_COUNT, + limit = 12, + fetchImpl = fetch, +} = {}) { + const runs = await api( + `/repos/${repo}/actions/workflows/${workflow}/runs` + + `?event=push&branch=main&status=completed&per_page=${limit}`, + { token, fetchImpl } + ); + const examined = []; + for (const run of runs?.workflow_runs ?? []) { + const [jobsPayload, artifactsPayload] = await Promise.all([ + api(`/repos/${repo}/actions/runs/${run.id}/jobs?per_page=100`, { token, fetchImpl }), + api(`/repos/${repo}/actions/runs/${run.id}/artifacts?per_page=100`, { token, fetchImpl }), + ]); + const verdict = runIsEligible( + { jobs: jobsPayload?.jobs, artifacts: artifactsPayload?.artifacts }, + shardCount + ); + examined.push({ + run_id: run.id, + head_sha: run.head_sha, + created_at: run.created_at, + html_url: run.html_url, + eligible: verdict.eligible, + reasons: verdict.reasons, + artifact_ids: verdict.artifactIds, + shard_seconds: verdict.shardSeconds, + }); + } + return examined; +} + +// --------------------------------------------------------------------------- +// -- The self-test's own battery roster and floor --------------------------- +// +// Same shape as the two scripts this one serves: what is pinned is the +// registered NAMES, and the count is a FLOOR -- a battery below it means cases +// stopped running, and the remedy is to find what stopped registering, never to +// lower the number. +const SELF_TEST_BATTERIES = Object.freeze({ + 'select-shard-timings-run self-test': 30, +}); +const SELF_TEST_BATTERY_FLOOR = 1; +const UNATTRIBUTED_BATTERY = '(no battery open)'; + +// Returned by `selfTest()` only after its verdict is printed, so a `return` that +// leaves the function early cannot report as a pass. +const SELF_TEST_VERDICT = 'select-shard-timings-run self-test reached its verdict'; + +function selfTest() { + const batterySeen = new Map(); + let openBattery = null; + const battery = (name) => { + openBattery = name; + }; + const check = (fn) => { + const b = openBattery ?? UNATTRIBUTED_BATTERY; + batterySeen.set(b, (batterySeen.get(b) ?? 0) + 1); + fn(); + }; + const threw = (fn) => { + try { + fn(); + return false; + } catch { + return true; + } + }; + + battery('select-shard-timings-run self-test'); + + // -- The two name matchers. Both are anchored, and the pins that matter are + // the NEAR MISSES: a matcher that also accepts a rerun job or a + // differently-sharded artifact would qualify a run on evidence from a run + // that is not this one. + check(() => { + if (testCoreShard('Test Core (3/6)') !== 3) throw new Error('job matcher: the plain shape did not parse'); + }); + check(() => { + if (testCoreShard('Test Core (1/6)') !== 1 || testCoreShard('Test Core (6/6)') !== 6) { + throw new Error('job matcher: an endpoint shard did not parse'); + } + }); + check(() => { + if (testCoreShard('Test Core (0/6)') !== null || testCoreShard('Test Core (7/6)') !== null) { + throw new Error('job matcher: an out-of-range shard index was accepted'); + } + }); + check(() => { + if (testCoreShard('Test Core (3/6) rerun') !== null) { + throw new Error('job matcher: a job merely CONTAINING the shape was accepted'); + } + }); + check(() => { + if (testCoreShard('Dogfood (3/6)') !== null) throw new Error('job matcher: a different job was accepted'); + }); + check(() => { + if (testCoreShard('Test Core (3/8)') !== null) { + throw new Error('job matcher: a job from a different shard count was accepted'); + } + }); + check(() => { + if (testCoreShard(undefined) !== null || testCoreShard('') !== null) { + throw new Error('job matcher: a missing name was not rejected'); + } + }); + check(() => { + if (summaryArtifactShard('test-core-run-summary-4-of-6') !== 4) { + throw new Error('artifact matcher: the plain shape did not parse'); + } + }); + check(() => { + if (summaryArtifactShard('test-core-run-summary-4-of-8') !== null) { + throw new Error('artifact matcher: a different shard count was accepted'); + } + }); + check(() => { + if (summaryArtifactShard('test-core-run-summary-4-of-6-retry') !== null) { + throw new Error('artifact matcher: a longer name was accepted'); + } + }); + + // -- Eligibility. The fixtures are built from the two rejection shapes this + // workflow actually meets: a cancelled shard, and an expired artifact. + const jobs = (conclusions) => + conclusions.map((conclusion, i) => ({ name: `Test Core (${i + 1}/6)`, conclusion, status: 'completed' })); + const artifacts = (present, expired = []) => + present.map((n) => ({ id: 1000 + n, name: `test-core-run-summary-${n}-of-6`, expired: expired.includes(n) })); + const allSix = ['success', 'success', 'success', 'success', 'success', 'success']; + + check(() => { + const v = runIsEligible({ jobs: jobs(allSix), artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (!v.eligible) throw new Error(`eligibility: a complete run was rejected (${v.reasons.join('; ')})`); + }); + check(() => { + const v = runIsEligible({ jobs: jobs(allSix), artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (v.artifactIds['5'] !== 1005) throw new Error(`eligibility: artifact ids were not returned per shard (${JSON.stringify(v.artifactIds)})`); + }); + check(() => { + const cancelled = [...allSix]; + cancelled[2] = 'cancelled'; + const v = runIsEligible({ jobs: jobs(cancelled), artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (v.eligible) throw new Error('eligibility: a run with a CANCELLED shard was accepted'); + if (!v.reasons.some((r) => r.includes('3/6'))) throw new Error(`eligibility: the cancelled shard was not named (${v.reasons.join('; ')})`); + }); + check(() => { + // A cancelled shard that still uploaded its artifact is the exact censored + // shape: "the artifacts exist" must not be enough on its own. + const cancelled = [...allSix]; + cancelled[0] = 'cancelled'; + const v = runIsEligible({ jobs: jobs(cancelled), artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (v.eligible) throw new Error('eligibility: a censored run passed because its artifacts were complete'); + }); + check(() => { + const stillRunning = [...allSix]; + stillRunning[4] = null; + const v = runIsEligible({ jobs: jobs(stillRunning), artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (v.eligible) throw new Error('eligibility: a shard with no conclusion was read as a success'); + }); + check(() => { + const v = runIsEligible({ jobs: jobs(allSix), artifacts: artifacts([1, 2, 3, 4, 5]) }); + if (v.eligible) throw new Error('eligibility: a run missing an artifact was accepted'); + if (!v.reasons.some((r) => r.includes('6-of-6'))) throw new Error(`eligibility: the missing artifact was not named (${v.reasons.join('; ')})`); + }); + check(() => { + const v = runIsEligible({ jobs: jobs(allSix), artifacts: artifacts([1, 2, 3, 4, 5, 6], [2]) }); + if (v.eligible) throw new Error('eligibility: an EXPIRED artifact was counted as retained'); + }); + check(() => { + // A re-run leaves a failed attempt beside the successful one. The shard + // qualifies on the attempt that succeeded, which is also the one that + // uploaded the artifact. + const withRetry = [...jobs(allSix), { name: 'Test Core (2/6)', conclusion: 'failure', status: 'completed' }]; + const v = runIsEligible({ jobs: withRetry, artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (!v.eligible) throw new Error(`eligibility: a re-run's earlier failed attempt disqualified the shard (${v.reasons.join('; ')})`); + }); + + // -- The per-shard duration the PR body quotes. Read from the suite step's + // own window, and ABSENT rather than zero when the step never ran -- a + // missing duration reported as 0s would read as an instant suite, which is + // the cache-replay shape this file exists to catch. + check(() => { + const job = { + steps: [ + { name: 'Install dependencies', started_at: '2026-09-07T00:00:00Z', completed_at: '2026-09-07T00:00:30Z' }, + { name: TEST_STEP_NAME, started_at: '2026-09-07T00:01:00Z', completed_at: '2026-09-07T00:10:20Z' }, + ], + }; + if (testStepSeconds(job) !== 560) throw new Error(`step window: got ${testStepSeconds(job)}, expected 560`); + }); + check(() => { + if (testStepSeconds({ steps: [{ name: 'Something else', started_at: 'x', completed_at: 'y' }] }) !== null) { + throw new Error('step window: a different step was measured as the suite'); + } + }); + check(() => { + if (testStepSeconds({ steps: [{ name: TEST_STEP_NAME, started_at: '2026-09-07T00:00:00Z' }] }) !== null) { + throw new Error('step window: an unfinished step was reported as a duration'); + } + }); + check(() => { + if (testStepSeconds({}) !== null || testStepSeconds(undefined) !== null) { + throw new Error('step window: a job with no steps was not rejected'); + } + }); + + // -- Coverage, the cache-replay catcher. The control leg first: an identical + // package set must pass, so the rejection below is about the LOSS and not + // about the comparison being broken. + const committed = { packages: { a: 10, b: 20, c: 30 } }; + const ws = ['a', 'b', 'c']; + check(() => { + const r = coverageReport({ committed, refreshed: { packages: { a: 11, b: 21, c: 31 } }, workspace: ws }); + if (!r.ok) throw new Error(`coverage: a complete refresh was rejected (${r.lost.join(', ')})`); + }); + check(() => { + const r = coverageReport({ committed, refreshed: { packages: { a: 11 } }, workspace: ws }); + if (r.ok) throw new Error('coverage: a refresh that lost two thirds of the workspace was accepted'); + if (r.lost.join(',') !== 'b,c') throw new Error(`coverage: the lost packages were not named (${r.lost.join(',')})`); + }); + check(() => { + // A package deleted from the monorepo is NOT a loss -- otherwise one + // deletion blocks every refresh from then on. + const r = coverageReport({ committed, refreshed: { packages: { a: 11, b: 21 } }, workspace: ['a', 'b'] }); + if (!r.ok) throw new Error(`coverage: a package removed from the workspace was counted as lost (${r.lost.join(',')})`); + }); + check(() => { + // ci.yml excludes dogfood from Test Core, so it is absent by construction. + const r = coverageReport({ + committed: { packages: { a: 10, '@objectstack/dogfood': 99 } }, + refreshed: { packages: { a: 11 } }, + workspace: ['a', 'ptsc/dogfood'.replace('ptsc/', '@objectstack/')], + exclude: ['@objectstack/dogfood'], + }); + if (!r.ok) throw new Error(`coverage: an excluded package was required (${r.lost.join(',')})`); + }); + check(() => { + const r = coverageReport({ committed, refreshed: { packages: { a: 1, b: 2, c: 3, d: 4 } }, workspace: [...ws, 'd'] }); + if (!r.ok || r.gained.join(',') !== 'd') throw new Error(`coverage: a newly measured package was not reported (${r.gained.join(',')})`); + }); + + // -- The workspace reader refuses a shape it cannot trust, rather than + // returning an empty list that would make every package look deleted. + check(() => { + if (workspaceNames({ packages: { items: [{ name: 'a' }, { name: 'b' }] } }).join(',') !== 'a,b') { + throw new Error('workspace: a valid turbo ls payload did not parse'); + } + }); + check(() => { + if (!threw(() => workspaceNames({ packages: {} }))) throw new Error('workspace: a payload with no items array was accepted'); + }); + check(() => { + if (!threw(() => workspaceNames({ packages: { items: [] } }))) throw new Error('workspace: an EMPTY package list was accepted'); + }); + + // -- The floor: every declared battery ran, and ran its cases. Evaluated + // before the verdict, so the success line can only be printed by a run in + // which the set of batteries that registered EQUALS the set declared. + const floorFailures = []; + const declared = Object.keys(SELF_TEST_BATTERIES); + if (declared.length < SELF_TEST_BATTERY_FLOOR) { + floorFailures.push( + `SELF_TEST_BATTERIES declares ${declared.length} batteries, below the pinned ${SELF_TEST_BATTERY_FLOOR}.` + ); + } + for (const [name, count] of batterySeen) { + if (declared.includes(name)) continue; + floorFailures.push(`battery "${name}" registered ${count} case(s) but is not declared.`); + } + for (const name of declared) { + const count = batterySeen.get(name) ?? 0; + if (count >= SELF_TEST_BATTERIES[name]) continue; + floorFailures.push( + count === 0 + ? `battery "${name}" DID NOT RUN -- 0 cases registered, ${SELF_TEST_BATTERIES[name]} pinned.` + : `battery "${name}" registered ${count} case(s), below its pinned floor of ${SELF_TEST_BATTERIES[name]}.` + ); + } + if (floorFailures.length > 0) { + throw new Error( + `select-shard-timings-run self-test floor (${floorFailures.length} breach(es)):\n` + + floorFailures.map((f) => ` - ${f}`).join('\n') + + '\n A battery at or below its floor means cases STOPPED RUNNING -- the battery is the bug, ' + + 'not the number.' + ); + } + + console.log('select-shard-timings-run: self-test OK'); + return SELF_TEST_VERDICT; +} + +function readJson(file) { + return JSON.parse(readFileSync(file, 'utf8')); +} + +async function main() { + const argv = process.argv.slice(2); + + if (argv.includes('--self-test')) { + if (selfTest() !== SELF_TEST_VERDICT) { + console.error( + '\nx select-shard-timings-run self-test: selfTest() returned without reaching its verdict,\n' + + 'so no success line was printed. Exiting 0 here would report a self-test that never\n' + + 'finished as a self-test that passed.\n' + ); + process.exit(1); + } + return; + } + + if (argv.includes('--check-coverage')) { + const value = (flag) => { + const i = argv.indexOf(flag); + return i === -1 ? null : argv[i + 1]; + }; + const exclude = []; + for (let i = 0; i < argv.length; i++) if (argv[i] === '--exclude') exclude.push(argv[i + 1]); + const committed = readJson(value('--committed')); + const refreshed = readJson(value('--refreshed')); + const workspace = workspaceNames(readJson(value('--workspace'))); + const report = coverageReport({ committed, refreshed, workspace, exclude }); + if (!report.ok) { + console.error( + `select-shard-timings-run: COVERAGE SHORTFALL -- ${report.lost.length} package(s) the committed ` + + 'dataset measured, and the workspace still contains, are NOT measured by this refresh: ' + + `${report.lost.join(', ')}. Every one of them would silently fall back to the test-file-count ` + + 'ESTIMATE, so this run measured a warm cache rather than the workspace. Rejecting it and trying ' + + 'an older run.' + ); + process.exit(1); + } + console.error( + `select-shard-timings-run: coverage OK -- ${report.measuredCount} package(s) measured, ` + + `${report.requiredCount} required, ${report.gained.length} newly measured` + + `${report.gained.length > 0 ? ` (${report.gained.join(', ')})` : ''}.` + ); + return; + } + + if (argv.includes('--candidates')) { + const token = process.env.GITHUB_TOKEN; + const repo = process.env.GITHUB_REPOSITORY; + if (!token || !repo) throw new Error('--candidates needs GITHUB_TOKEN and GITHUB_REPOSITORY in the environment'); + const limitAt = argv.indexOf('--limit'); + const limit = limitAt === -1 ? 12 : Number(argv[limitAt + 1]); + const examined = await listCandidates({ repo, token, limit }); + for (const run of examined) { + console.error( + run.eligible + ? ` ELIGIBLE ${run.run_id} ${run.created_at} ${String(run.head_sha).slice(0, 10)}` + : ` rejected ${run.run_id} ${run.created_at} ${run.reasons.join('; ')}` + ); + } + const eligible = examined.filter((r) => r.eligible); + if (eligible.length === 0) { + console.error( + `select-shard-timings-run: NO ELIGIBLE RUN among the ${examined.length} most recent completed push ` + + 'runs on main. Every one was censored, failed, or has lost its run-summary artifacts to the ' + + '1-day retention window. This is a refusal, not a no-op: nothing was regenerated.' + ); + process.exit(1); + } + console.log(JSON.stringify(eligible, null, 2)); + return; + } + + console.error( + 'usage: select-shard-timings-run.mjs --candidates [--limit ]\n' + + ' select-shard-timings-run.mjs --check-coverage --committed --refreshed --workspace [--exclude ]...\n' + + ' select-shard-timings-run.mjs --self-test' + ); + process.exit(1); +} + +// Exports bindings, so an import for those exports alone must run nothing. +if (isEntrypoint(import.meta.url)) { + await main(); +} From 8b5d047184ec9e8c77d0d01cf3df9bc890366027 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 03:57:34 +0000 Subject: [PATCH 3/8] ci: route the shard-timings instrument self-tests through a collector A bare sequence under `bash -e` aborts at the first non-zero exit, so the second self-test would be neither green nor red -- and this step exists to say WHICH instrument is broken before the dataset is trusted. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox --- .github/workflows/shard-timings-refresh.yml | 28 +++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/.github/workflows/shard-timings-refresh.yml b/.github/workflows/shard-timings-refresh.yml index 3eb1d5d732..8ec0fe5458 100644 --- a/.github/workflows/shard-timings-refresh.yml +++ b/.github/workflows/shard-timings-refresh.yml @@ -144,8 +144,32 @@ jobs: # green-looking pipeline. - name: Self-test the generator and the run selector run: | - node scripts/measure-test-shard-timings.mjs --self-test - node scripts/ci/select-shard-timings-run.mjs --self-test + # A COLLECTOR, not a bare sequence. Under `bash -e` the first non-zero + # exit aborts the step, so a plain `a && b` list leaves the second + # self-test neither green nor red -- and this step exists precisely to + # say which instrument is broken before the dataset is trusted. + # ⛔ Never let the collector swallow the exit code: a green step over a + # red self-test looks identical to success. + failed="" + run_self_test() { + echo "-- $*" + if "$@"; then + echo "PASS $*" + else + echo "FAIL $*" + failed="${failed} $*"$'\n' + fi + return 0 + } + run_self_test node scripts/measure-test-shard-timings.mjs --self-test + run_self_test node scripts/ci/select-shard-timings-run.mjs --self-test + if [ -n "$failed" ]; then + echo "" + echo "Shard-timings instrument self-tests — the following FAILED:" + printf "%s" "$failed" + exit 1 + fi + echo "Shard-timings instrument self-tests — both ran and passed" # The package list the coverage check judges against. `turbo ls` is # experimental, so the reader asserts its payload loudly rather than From 6b030881d159ae7e74d6c72496124703bbe3e8a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 04:00:39 +0000 Subject: [PATCH 4/8] fix(scripts): give check-self-test-wired's path extractor a right boundary `js` is a prefix of `json`, and the invocation pattern had a left boundary but none on its extension alternation. A workflow line naming a DATA file under scripts/ therefore matched as far as the `.js` and minted exactly the phantom key the left boundary was added to abolish: `scripts/test-shard-timings.json` was keyed as `scripts/test-shard-timings.js`, a path with no file behind it, silent in both directions -- the real file is audited by nothing, and the phantom key reconciles against no carrier. It stayed invisible only because no workflow had yet named a `.json` under scripts/. The first one to do it reddens the gate's `live corpus` battery, which is the battery that exists to catch precisely this. `(?![\w-])` is the spelling already used on `--self-test` two groups along, so both boundaries on this pattern now read the same way: a match ends where the token ends. Four cases pin it, control first (a real `.js` script is still seen), floor 4 -> 8. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox --- scripts/check-self-test-wired.mjs | 44 +++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/scripts/check-self-test-wired.mjs b/scripts/check-self-test-wired.mjs index cfca4ddcb0..db95b1025a 100644 --- a/scripts/check-self-test-wired.mjs +++ b/scripts/check-self-test-wired.mjs @@ -193,9 +193,25 @@ const ROOT_DIR_WATCH_HINTS = ['scripts/**/*.mjs', 'scripts/**/*.mts', 'scripts/* * Measured after this change over the live corpus: 186 named paths, ZERO of * which lack a file on disk (before: exactly one, the phantom above). The * `live corpus` battery holds both halves of that reading. + * + * ## The RIGHT boundary, and the phantom it mints without one (#16464) + * + * The extension alternation needs the same treatment the left edge got, for the + * same reason and with the same failure shape. `js` is a PREFIX of `json`, so + * without a right boundary a workflow line naming `scripts/test-shard- + * timings.json` -- a data file, not a script -- matched as far as + * `scripts/test-shard-timings.js` and minted exactly the phantom key the left + * boundary was added to abolish: a path with no file behind it, silent in both + * directions. + * + * It stayed invisible only because no workflow had yet named a `.json` under + * `scripts/`; the first one to do it (the shard-timings refresh) reddened the + * `live corpus` battery. `(?![\w-])` is the spelling already used on + * `--self-test` two groups along, so both boundaries on this pattern now read + * the same way: a match must end where the token ends. */ const INVOCATION_RE = - /(? Date: Mon, 7 Sep 2026 04:11:59 +0000 Subject: [PATCH 5/8] ci: give the shard-timings refresh a PR-time trigger, and split the write from the body A gate family reachable ONLY on a schedule appears on no card's gate list, so it is graded by nobody until the next sweep -- dispatch-gates.mjs reds on exactly that, and this lane was the first family to arrive scheduled-only. The remedy is the posture half-state-patrol.yml and required-set-patrol.yml already keep: a `pull_request` trigger scoped to this lane's own files. On such a run everything executes for real -- selection, download, regeneration, the coverage check, the byte comparison, the partitioner's verdict -- and only the WRITE is skipped, so a PR editing this workflow can never push a bot branch or open a second PR. Composing the PR body is therefore its own step, consumed either by the write or by the dry-run step that renders it to the run's step summary. A reviewer of a change to this lane sees the real output rather than the diff of the code that produces it. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox --- .github/workflows/shard-timings-refresh.yml | 79 +++++++++++++++++---- 1 file changed, 66 insertions(+), 13 deletions(-) diff --git a/.github/workflows/shard-timings-refresh.yml b/.github/workflows/shard-timings-refresh.yml index 8ec0fe5458..bf9fd216aa 100644 --- a/.github/workflows/shard-timings-refresh.yml +++ b/.github/workflows/shard-timings-refresh.yml @@ -77,6 +77,24 @@ on: # that the PR is in front of someone before the week's merge volume builds. - cron: '30 5 * * 1' workflow_dispatch: + # Changes to this lane get exercised before they merge — the same posture + # half-state-patrol.yml and required-set-patrol.yml keep, and the reason a + # scheduled-only lane is not an option: `dispatch-gates.mjs` reds when a gate + # family is reachable ONLY on a schedule, because a family no PR-time event + # reaches appears on no card's gate list and is therefore graded by nobody + # until the next sweep. + # + # On a `pull_request` run everything executes — selection, download, + # regeneration, the coverage check, the byte comparison and the partitioner's + # verdict — so the transport and the flags are proven on a real runner rather + # than argued about. The WRITE is what is skipped: no branch is pushed, no PR + # is opened, no label is written, and the body that would have been posted is + # rendered to the run's step summary instead. + pull_request: + paths: + - '.github/workflows/shard-timings-refresh.yml' + - 'scripts/ci/select-shard-timings-run.mjs' + - 'scripts/measure-test-shard-timings.mjs' # Read-only at the top level; the one job widens to exactly what it writes. permissions: @@ -310,27 +328,19 @@ jobs: echo "BEFORE: $(cat "$RUNNER_TEMP/bins-before.txt")" echo "AFTER: $(cat "$RUNNER_TEMP/bins-after.txt")" - - name: Push the refresh branch and open the pull request + # Composition is separated from the WRITE on purpose: a `pull_request` run + # of this lane must exercise the body-building — the shard durations, the + # bins, the conditional blocks — without pushing anything. Both the write + # step and the dry-run step below consume this file. + - name: Compose the pull request body if: steps.compare.outputs.changed == 'true' env: - GH_TOKEN: ${{ secrets.RELEASE_PUSH_TOKEN || github.token }} RUN_ID: ${{ steps.generate.outputs.run_id }} HEAD_SHA: ${{ steps.generate.outputs.head_sha }} PARTITIONER_EXIT: ${{ steps.bins.outputs.partitioner_exit }} USED_PAT: ${{ secrets.RELEASE_PUSH_TOKEN != '' }} run: | set -euo pipefail - BRANCH="claude/shard-timings-refresh-$RUN_ID" - git config user.name 'github-actions[bot]' - git config user.email '41898282+github-actions[bot]@users.noreply.github.com' - git switch -c "$BRANCH" - git add scripts/test-shard-timings.json - # Separate -m flags rather than one embedded newline: a YAML-indented - # heredoc would carry its own leading whitespace into the message body. - git commit \ - -m "chore(ci): refresh the Test Core shard-timings dataset" \ - -m "Regenerated by .github/workflows/shard-timings-refresh.yml from the six test-core-run-summary artifacts of run $RUN_ID ($HEAD_SHA). Generated, never hand-edited." - git push origin "$BRANCH" SHARDS=$(node -e ' const rs = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8")); @@ -393,6 +403,29 @@ jobs: # OUTPUT; the cards about the workflow are referenced only. echo "Refs #16464, #16173, #16222." } > "$RUNNER_TEMP/pr-body.md" + echo "Composed a $(wc -l < "$RUNNER_TEMP/pr-body.md")-line pull request body." + + # The WRITE. Skipped on a `pull_request` run of this lane: a PR that only + # edits this workflow must never push a bot branch or open a second PR. + - name: Push the refresh branch and open the pull request + if: steps.compare.outputs.changed == 'true' && github.event_name != 'pull_request' + env: + GH_TOKEN: ${{ secrets.RELEASE_PUSH_TOKEN || github.token }} + RUN_ID: ${{ steps.generate.outputs.run_id }} + HEAD_SHA: ${{ steps.generate.outputs.head_sha }} + run: | + set -euo pipefail + BRANCH="claude/shard-timings-refresh-$RUN_ID" + git config user.name 'github-actions[bot]' + git config user.email '41898282+github-actions[bot]@users.noreply.github.com' + git switch -c "$BRANCH" + git add scripts/test-shard-timings.json + # Separate -m flags rather than one embedded newline: a YAML-indented + # heredoc would carry its own leading whitespace into the message body. + git commit \ + -m "chore(ci): refresh the Test Core shard-timings dataset" \ + -m "Regenerated by .github/workflows/shard-timings-refresh.yml from the six test-core-run-summary artifacts of run $RUN_ID ($HEAD_SHA). Generated, never hand-edited." + git push origin "$BRANCH" PR_URL=$(gh pr create --base main --head "$BRANCH" \ --title "chore(ci): refresh the Test Core shard-timings dataset" \ @@ -424,6 +457,26 @@ jobs: || echo "::error::skip-changeset is still absent from PR #$PR_NUMBER; the changeset gate will demand a changeset this PR legitimately has none of. Apply the label by hand." fi + # The dry-run half of the `pull_request` posture. Everything above ran for + # real; this renders the body that WOULD have been posted, so a reviewer of + # a change to this lane sees the actual output rather than the diff of the + # code that produces it. + - name: Dry run — the pull request this would have opened + if: steps.compare.outputs.changed == 'true' && github.event_name == 'pull_request' + run: | + { + echo "### Shard timings: dry run (no branch pushed, no PR opened, no label written)" + echo + echo "This is a \`pull_request\` run of the refresh lane itself. The run selection, the" + echo "artifact download, the regeneration, the coverage check and the partitioner's verdict" + echo "all executed for real; only the write was skipped. The body below is what a scheduled" + echo "run would have posted." + echo + echo "---" + echo + cat "$RUNNER_TEMP/pr-body.md" + } >> "$GITHUB_STEP_SUMMARY" + - name: Say what happened when nothing changed if: steps.compare.outputs.changed == 'false' run: | From da33db7b5db406d1e81e3c014dae54eab23fc207 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 04:23:12 +0000 Subject: [PATCH 6/8] ci: scope the refresh lane's concurrency group by ref One global group would have two PRs touching this lane cancel each other's dry run and report the cancellation as though the lane were busy. Two SCHEDULED refreshes still cannot overlap, which is the race the group exists for. Also corrects a comment that described a `continue-on-error` the step does not use: the AFTER leg's exit is captured with `set +e` and read before any pipe. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox --- .github/workflows/shard-timings-refresh.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/workflows/shard-timings-refresh.yml b/.github/workflows/shard-timings-refresh.yml index bf9fd216aa..2a48ac7b3e 100644 --- a/.github/workflows/shard-timings-refresh.yml +++ b/.github/workflows/shard-timings-refresh.yml @@ -104,7 +104,12 @@ permissions: # the same bot branch; the newer inputs are the better ones, so the in-flight run # yields. concurrency: - group: shard-timings-refresh + # Scoped BY REF, not one global group. Two scheduled refreshes cannot overlap + # (they would race on the same bot branch, and the newer inputs are the better + # ones, so the in-flight run yields) — but two PRs touching this lane are + # unrelated runs, and a single global group would have each cancel the other's + # dry run and report a cancellation as though the lane were busy. + group: shard-timings-refresh-${{ github.ref }} cancel-in-progress: true jobs: @@ -310,11 +315,13 @@ jobs: # verdict line rather than recomputed here — a second implementation of the # binning would be a second answer to grade against. # - # ⛔ `continue-on-error` on the AFTER leg only, and it is not leniency: a - # red here is the partitioner refusing an honest measurement, which is - # information the PR must carry, not a reason to withhold the PR. The - # verdict text goes into the body either way and the PR's own lint job - # grades it again. + # ⛔ The AFTER leg's non-zero exit is CAPTURED, not propagated, and that is + # not leniency: a red there is the partitioner refusing an honest + # measurement, which is information the PR must carry rather than a reason + # to withhold the PR. `set +e` around that one command, with `$?` read + # immediately and before any pipe, is what keeps the verdict readable + # without letting the step's own status swallow it; the text goes into the + # body either way and the PR's own lint job grades it again. - name: Predicted bins, before and after id: bins if: steps.compare.outputs.changed == 'true' From 4fe161b41aac24e1811d743dc1d9f2ef2d45e5e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 04:48:02 +0000 Subject: [PATCH 7/8] ci: accumulate runs until the workspace is covered, instead of hunting for one that covers it The lane's first live run (34083991141) measured the fact the documented refresh procedure does not state: NO single green run measures the whole workspace. Of the seven retained green push runs on main, the best measured 52 of the 71 packages the committed dataset holds; the rest measured 2, 3, 13, 18, 22 and 49. All seven were partial cache replays, so every candidate was rejected and the job exited 1 having regenerated nothing. That is the cache design rather than luck: turbo's key is namespaced per shard and only main pushes write it, so a package whose inputs have not changed is a HIT -- and the generator refuses hits rather than recording a replayed ~0.1s window as a suite's cost. "Download six artifacts from any green run" therefore measures a SLICE of the workspace. Runs are now accumulated until coverage is satisfied, each fenced by its own `--run ` group. That grouping is exactly what the #16473 fix on this branch added: slices are summed WITHIN a run and the per-run sums are medianed ACROSS runs, so a package measured by three of the accumulated runs gets the median of three observations -- the property the dataset's merge rule always claimed and could not previously deliver. Feeding several runs without that grouping is refused by name, so this path could not have been taken before the rider. The argument list is rebuilt from the accepted set each round rather than appended to, so a run the generator refuses is dropped cleanly instead of poisoning every later attempt. The PR body and commit message now name every contributing run; the newest is the one the refresh is dated from. When even the whole accumulation falls short the job still refuses loudly, and its message names the remedy that would be a DECISION rather than a tuning knob: keeping the last measured weight for a package this refresh did not measure. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox --- .github/workflows/shard-timings-refresh.yml | 117 +++++++++++++++----- scripts/ci/select-shard-timings-run.mjs | 9 +- 2 files changed, 95 insertions(+), 31 deletions(-) diff --git a/.github/workflows/shard-timings-refresh.yml b/.github/workflows/shard-timings-refresh.yml index 2a48ac7b3e..d48747113a 100644 --- a/.github/workflows/shard-timings-refresh.yml +++ b/.github/workflows/shard-timings-refresh.yml @@ -50,13 +50,29 @@ # regeneration happens where the data already is, on a timer, and no seat needs # reachability it does not have. # -# CHOOSING THE RUN IS THE HARD PART — see scripts/ci/select-shard-timings-run.mjs +# CHOOSING THE RUNS IS THE HARD PART — see scripts/ci/select-shard-timings-run.mjs # ------------------------------------------------------------------------------ # "The newest run" is wrong three different ways here (cancelled runs, cache # replays, expired artifacts), and a refresh built on the wrong run is worse than # no refresh because it stamps a fresh `measuredAt` on numbers nobody measured. # That script carries the argument and the self-test; this file only drives it. # +# ⚠ AND IT IS "RUNS", PLURAL, WHICH THE DOCUMENTED PROCEDURE DOES NOT SAY. +# Measured on this lane's first live run (34083991141): of the seven retained +# green push runs on main, the BEST measured 52 of the 71 packages the committed +# dataset holds, and the others measured 2, 3, 13, 18, 22 and 49. All seven were +# partial cache replays. That follows from the cache design rather than from luck +# — turbo's key is namespaced per shard and only main pushes write it, so a +# package whose inputs have not changed is a HIT, and the generator refuses hits +# rather than recording a replayed ~0.1s window as a suite's cost. "Download six +# artifacts from any green run" therefore measures a SLICE of the workspace. +# +# So the regeneration step accumulates runs, each under its own `--run ` +# group — the grouping #16473 added, which sums a sliced package's slices within +# a run and medians the per-run sums across runs. A package seen in three of the +# accumulated runs gets the median of three observations, which is the property +# the dataset's own merge rule always claimed and could not previously deliver. +# # THE PINS, AND THE ONE THING A MACHINE MUST NOT DECIDE # ---------------------------------------------------- # partition-test-shards.mjs `--self-test` grades the dataset against the @@ -214,11 +230,30 @@ jobs: for (const r of runs) console.log(` ${r.run_id} ${r.created_at} ${r.head_sha.slice(0, 10)}`); ' - # Each eligible run is tried in turn: download, generate, and then judge - # whether it MEASURED the workspace. A run that passed selection can still - # be a cache replay, and the only honest test for that is the resulting - # dataset's package coverage — so the loop is the point, not a retry. - - name: Regenerate the dataset from the first run that actually measured + # ACCUMULATE runs until the workspace is covered — do not look for one run + # that covers it, because there is no such run. + # + # MEASURED on the first live run of this lane (run 34083991141), and it is + # the fact that shapes this step: of the seven retained green push runs on + # main, the BEST measured 52 of the 71 packages the committed dataset + # holds, and the rest measured 2, 3, 13, 18, 22 and 49. Every one of them + # was a partial cache replay. That is not bad luck, it is the cache design: + # turbo's key is namespaced per shard and only main pushes write it, so a + # package whose inputs have not changed is a HIT — and the generator + # refuses hits rather than recording a replayed ~0.1s window as a suite's + # cost. "Download six artifacts from any green run" therefore measures a + # SLICE of the workspace, never all of it. + # + # So runs are accumulated. Each contributes its six summaries under its own + # `--run ` group, which is exactly the grouping #16473 added: slices are + # summed within a run and the per-run sums are medianed across runs, so a + # package measured by three of these runs gets a median of three + # observations rather than whichever run happened to be read last. Feeding + # several runs WITHOUT that grouping is refused by the generator by name. + # + # The loop stops at the first accumulation that covers the workspace, so a + # quiet week costs one run's download and a busy one costs a few. + - name: Regenerate the dataset, accumulating runs until the workspace is covered id: generate env: GITHUB_TOKEN: ${{ github.token }} @@ -227,6 +262,7 @@ jobs: WORK="$RUNNER_TEMP/refresh" mkdir -p "$WORK" CHOSEN='' + ACCEPTED=() RUN_COUNT=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8")).length)') for i in $(seq 0 $((RUN_COUNT - 1))); do @@ -265,20 +301,29 @@ jobs: echo "::endgroup::"; continue fi - # One run, so no `--run` grouping is needed: every summary here came - # from run $RUN_ID. Feeding SEVERAL runs would require `--run ` - # before each run's six files — the generator refuses the ambiguity - # rather than taking the last value (#16473). - # An array, not `xargs`: a split invocation would run the generator - # twice and the second would overwrite the first's output with a - # partial dataset. - mapfile -t SUMMARY_FILES < "$SUMDIR/summaries.txt" - if ! node scripts/measure-test-shard-timings.mjs "${SUMMARY_FILES[@]}" \ + ACCEPTED+=("$RUN_ID") + + # The argument list is REBUILT from the accepted set every round + # rather than appended to, so a run the generator refuses can be + # dropped cleanly instead of poisoning every later attempt. Each run + # is fenced by its own `--run `; an array, not `xargs`, because a + # split invocation would run the generator twice and the second would + # overwrite the first's output with a partial dataset. + ARGS=() + for R in "${ACCEPTED[@]}"; do + ARGS+=( --run "$R" ) + mapfile -t RUN_FILES < "$WORK/$R/summaries.txt" + ARGS+=( "${RUN_FILES[@]}" ) + done + + if ! node scripts/measure-test-shard-timings.mjs "${ARGS[@]}" \ --out "$WORK/refreshed.json"; then - echo "::warning::The generator refused run $RUN_ID's summaries; skipping this run." + echo "::warning::The generator refused the set including run $RUN_ID; dropping that run and continuing." + unset 'ACCEPTED[-1]' echo "::endgroup::"; continue fi + echo "Accumulated ${#ACCEPTED[@]} run(s): ${ACCEPTED[*]}" if node scripts/ci/select-shard-timings-run.mjs --check-coverage \ --committed scripts/test-shard-timings.json \ --refreshed "$WORK/refreshed.json" \ @@ -292,11 +337,18 @@ jobs: done if [ -z "$CHOSEN" ]; then - echo "::error::No eligible run produced a complete measurement of the workspace. Every candidate was a cache replay, refused by the generator, or lost its artifacts. NOTHING was regenerated and no PR was opened — this is a refusal, not a quiet success." + echo "::error::The ${#ACCEPTED[@]} eligible run(s) on main, accumulated together, still do not measure every package the committed dataset holds — the shortfall above names what is missing. A package can stay unmeasured across every retained run: turbo's cache key is namespaced per shard and only main pushes write it, so a package whose inputs have not changed is a HIT in all of them, and the generator refuses hits rather than recording a replay as a duration. NOTHING was regenerated and no PR was opened — this is a refusal, not a quiet success. If this is the steady state rather than a quiet week, the dataset needs a merge rule (keep the last measured weight for a package this refresh did not measure) rather than a wider run window; that is a decision, not a tuning knob, and it is tracked on the card." exit 1 fi - echo "run_id=$CHOSEN" >> "$GITHUB_OUTPUT" - echo "head_sha=$(node -e 'const rs=JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP+"/candidates.json","utf8"));console.log(rs.find(r=>String(r.run_id)===process.argv[1]).head_sha)' "$CHOSEN")" >> "$GITHUB_OUTPUT" + # Candidates arrive newest-first, so ACCEPTED[0] is the most recent run + # in the set and its commit is the one the refresh is dated from. The + # full list travels alongside it: every run in it contributed + # measurements, and the PR body names them all rather than implying one. + NEWEST="${ACCEPTED[0]}" + echo "run_id=$NEWEST" >> "$GITHUB_OUTPUT" + echo "runs=${ACCEPTED[*]}" >> "$GITHUB_OUTPUT" + echo "run_count=${#ACCEPTED[@]}" >> "$GITHUB_OUTPUT" + echo "head_sha=$(node -e 'const rs=JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP+"/candidates.json","utf8"));console.log(rs.find(r=>String(r.run_id)===process.argv[1]).head_sha)' "$NEWEST")" >> "$GITHUB_OUTPUT" # Byte comparison, and it decides everything downstream. `cmp -s` rather # than a diff of parsed JSON: the committed artefact is the file, so the @@ -343,6 +395,8 @@ jobs: if: steps.compare.outputs.changed == 'true' env: RUN_ID: ${{ steps.generate.outputs.run_id }} + RUNS: ${{ steps.generate.outputs.runs }} + RUN_COUNT: ${{ steps.generate.outputs.run_count }} HEAD_SHA: ${{ steps.generate.outputs.head_sha }} PARTITIONER_EXIT: ${{ steps.bins.outputs.partitioner_exit }} USED_PAT: ${{ secrets.RELEASE_PUSH_TOKEN != '' }} @@ -364,14 +418,23 @@ jobs: echo echo "## Source" echo - echo "- Run: https://github.com/$GITHUB_REPOSITORY/actions/runs/$RUN_ID" - echo "- Commit measured: \`$HEAD_SHA\`" - echo "- Selected because its six \`Test Core (N/6)\` jobs all concluded \`success\`, its six" - echo " run-summary artifacts were still retained, and the dataset it produced measures every" - echo " package the committed one measured that is still in the workspace (the check that" - echo " rejects a cache replay)." + echo "Measured across $RUN_COUNT accumulated run(s). No single green run measures the whole" + echo "workspace — turbo's cache is namespaced per shard and only main pushes write it, so a" + echo "package whose inputs have not changed is a HIT and the generator refuses hits rather" + echo "than recording a replay as a duration. Runs are therefore accumulated, each fenced by" + echo "its own \`--run\` group, until every package the committed dataset holds is measured" + echo "again; a package seen in several of them gets the median of those observations." + echo + for R in $RUNS; do + echo "- https://github.com/$GITHUB_REPOSITORY/actions/runs/$R" + done + echo + echo "- Newest run in the set: \`$RUN_ID\`, commit \`$HEAD_SHA\` — the date this refresh carries." + echo "- Every run above had all six \`Test Core (N/6)\` jobs conclude \`success\` with its six" + echo " run-summary artifacts still retained; runs that were cancelled, failed or had lost" + echo " their artifacts were rejected by name in the log before any of these were used." echo - echo "## Measured per-shard suite time on that run" + echo "## Measured per-shard suite time on the newest run in the set" echo echo "\`\`\`" echo "$SHARDS" @@ -431,7 +494,7 @@ jobs: # heredoc would carry its own leading whitespace into the message body. git commit \ -m "chore(ci): refresh the Test Core shard-timings dataset" \ - -m "Regenerated by .github/workflows/shard-timings-refresh.yml from the six test-core-run-summary artifacts of run $RUN_ID ($HEAD_SHA). Generated, never hand-edited." + -m "Regenerated by .github/workflows/shard-timings-refresh.yml from the test-core-run-summary artifacts of $RUN_COUNT accumulated run(s) ($RUNS), newest $RUN_ID at $HEAD_SHA. Generated, never hand-edited." git push origin "$BRANCH" PR_URL=$(gh pr create --base main --head "$BRANCH" \ diff --git a/scripts/ci/select-shard-timings-run.mjs b/scripts/ci/select-shard-timings-run.mjs index 9b61bc20d2..7ba856c0b2 100644 --- a/scripts/ci/select-shard-timings-run.mjs +++ b/scripts/ci/select-shard-timings-run.mjs @@ -549,10 +549,11 @@ async function main() { if (!report.ok) { console.error( `select-shard-timings-run: COVERAGE SHORTFALL -- ${report.lost.length} package(s) the committed ` + - 'dataset measured, and the workspace still contains, are NOT measured by this refresh: ' + - `${report.lost.join(', ')}. Every one of them would silently fall back to the test-file-count ` + - 'ESTIMATE, so this run measured a warm cache rather than the workspace. Rejecting it and trying ' + - 'an older run.' + 'dataset measured, and the workspace still contains, are NOT measured by the runs accumulated ' + + `so far: ${report.lost.join(', ')}. Every one of them would silently fall back to the ` + + 'test-file-count ESTIMATE, so what has been read so far is a warm cache rather than the ' + + 'workspace. Not a verdict on any one run: no single run measures everything (turbo caches per ' + + 'shard, and the generator refuses hits), so the caller adds the next older run and asks again.' ); process.exit(1); } From fb51eee39e95b98182ec67f6b29bd509f10fbfbb Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 08:18:13 +0000 Subject: [PATCH 8/8] feat(scripts): merge a refresh into the dataset, carrying cache-hit weights on their witness Ruled on the card after the lane's first two live runs measured that the acceptance rule as written cannot be met: no retained run set covers the workspace. The best single green run measured 52 of 71 packages, the accumulation of all seven converged at 57, and the last 14 are turbo cache HITs in every one of them -- the cache key is namespaced per shard and only main pushes write it, so a package whose inputs have not changed is a HIT, and this generator refuses hits rather than recording a replay as a duration. So "regenerate" now means MERGE, not replace, and the merge is sound for one specific reason: a cache HIT is not missing data, it is positive evidence that the package's inputs are unchanged since the run whose output was replayed, so its last measured weight still describes it. The file's invariant is preserved exactly -- every number in it remains a real measurement of code as it stands, never an estimate. * `--merge-into ` carries a package's previous weight ONLY when a cache HIT witnesses it. A package absent for any other reason -- never ran, suite failed, slices unassemblable -- has no evidence behind it and is left out for the caller to name. `skippedAsCached` is exactly the witnessed set, so the carry reads it rather than inventing a second classification. * Carried packages are named in a top-level `carriedOver` list beside `skippedAsCached`. That shape rather than a per-package `measuredAt` because the reader forces it: partition-test-shards.mjs reads `packages` as name -> NUMBER and never opens `provenance`, so per-package dates would mean changing that shape and every consumer of it. A single `provenance.measuredAt` paired with the list carries the same information and needs no partitioner change. * Carried weights vote on `secondsPerTestFileFallback` like measured ones, which keeps that rate derived from the numbers actually in the file. * An all-cached pass still REFUSES: carrying can never manufacture a refresh out of nothing. Coverage is judged on measured union carried, and the two ways a package can be missing are now reported separately: one that HAD a weight and has neither is a refusal by name, while a workspace package that never had one is named as partitioner-estimated but is not a regression, because this refresh did not change its standing. The header sentence "Download all six from any green queue build and re-run the generator" is corrected in the same change: it is optimistic in a way nobody had measured, and now states the measured fact. Self-test: 9 new generator cases (carried / measured-not-carried / witnessed versus absent / empty list on a plain replace / all-cached still refuses / monotone accumulation), floor 41 -> 50; 5 new selector cases for the union semantics, floor 30 -> 35. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox --- .github/workflows/shard-timings-refresh.yml | 46 +++- scripts/ci/select-shard-timings-run.mjs | 140 +++++++++-- scripts/measure-test-shard-timings.mjs | 263 +++++++++++++++++--- 3 files changed, 402 insertions(+), 47 deletions(-) diff --git a/.github/workflows/shard-timings-refresh.yml b/.github/workflows/shard-timings-refresh.yml index d48747113a..2b2274d322 100644 --- a/.github/workflows/shard-timings-refresh.yml +++ b/.github/workflows/shard-timings-refresh.yml @@ -73,6 +73,19 @@ # accumulated runs gets the median of three observations, which is the property # the dataset's own merge rule always claimed and could not previously deliver. # +# ⚠ AND ACCUMULATION ALONE STILL FALLS SHORT, so the pass MERGES rather than +# replaces. Measured on the same live runs: the union of all seven retained runs +# reaches 57 of 71 packages and converges there, because 14 packages are cache +# HITs in every one of them. Those are carried at their previous weights through +# `--merge-into`, and the carry is sound for one specific reason — a cache HIT is +# not missing data, it is POSITIVE EVIDENCE that the package's inputs are +# unchanged since the run whose output was replayed, so its last measured weight +# still describes it. Every carried package is named in the dataset's +# `carriedOver` list, and a package absent for any OTHER reason is not carried at +# all: it drops out and the coverage check names it as a refusal. The file's +# invariant is preserved exactly — every number in it is a real measurement of +# code as it stands, never an estimate. +# # THE PINS, AND THE ONE THING A MACHINE MUST NOT DECIDE # ---------------------------------------------------- # partition-test-shards.mjs `--self-test` grades the dataset against the @@ -316,7 +329,14 @@ jobs: ARGS+=( "${RUN_FILES[@]}" ) done + # `--merge-into` the committed dataset, because no retained run set + # measures the whole workspace (see the header). A package this pass + # did not measure keeps its previous weight ONLY when a turbo cache + # HIT witnesses that its inputs are unchanged; anything else is left + # out for the coverage check below to name. The workflow still writes + # no byte the generator did not emit — the merge happens inside it. if ! node scripts/measure-test-shard-timings.mjs "${ARGS[@]}" \ + --merge-into scripts/test-shard-timings.json \ --out "$WORK/refreshed.json"; then echo "::warning::The generator refused the set including run $RUN_ID; dropping that run and continuing." unset 'ACCEPTED[-1]' @@ -337,7 +357,7 @@ jobs: done if [ -z "$CHOSEN" ]; then - echo "::error::The ${#ACCEPTED[@]} eligible run(s) on main, accumulated together, still do not measure every package the committed dataset holds — the shortfall above names what is missing. A package can stay unmeasured across every retained run: turbo's cache key is namespaced per shard and only main pushes write it, so a package whose inputs have not changed is a HIT in all of them, and the generator refuses hits rather than recording a replay as a duration. NOTHING was regenerated and no PR was opened — this is a refusal, not a quiet success. If this is the steady state rather than a quiet week, the dataset needs a merge rule (keep the last measured weight for a package this refresh did not measure) rather than a wider run window; that is a decision, not a tuning knob, and it is tracked on the card." + echo "::error::The ${#ACCEPTED[@]} eligible run(s) on main, accumulated and merged with the committed dataset, still leave a package that HAD a measured weight with neither a fresh measurement nor a turbo cache HIT to witness that it is unchanged — the shortfall above names them. Each would drop to the test-file-count ESTIMATE, which is the silent degradation this lane exists to prevent. NOTHING was regenerated and no PR was opened: this is a refusal, not a quiet success. A package that merely went unmeasured is NOT this error — that case is carried on its cache-hit witness — so a shortfall here means a suite failed, a package was renamed or removed, or its slices could not be assembled in any run." exit 1 fi # Candidates arrive newest-first, so ACCEPTED[0] is the most recent run @@ -403,6 +423,24 @@ jobs: run: | set -euo pipefail + # Read out of the generated dataset itself rather than recomputed, so the + # sentence in the PR cannot drift from the file it describes. + CARRY_LINE=$(node -e ' + const d = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/refresh/refreshed.json", "utf8")); + const carried = d.carriedOver ?? []; + const total = Object.keys(d.packages).length; + const fresh = total - carried.length; + if (carried.length === 0) { + console.log(`All ${total} package weights were measured in these runs; nothing was carried.`); + } else { + console.log( + `${total} package weights: ${fresh} measured in these runs, and ${carried.length} carried ` + + `forward at their previous values because a turbo cache HIT witnessed that their inputs are ` + + `unchanged (so the old number still describes them). Carried: ${carried.join(", ")}.` + ); + } + ') + SHARDS=$(node -e ' const rs = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8")); const r = rs.find((x) => String(x.run_id) === process.env.RUN_ID); @@ -434,6 +472,12 @@ jobs: echo " run-summary artifacts still retained; runs that were cancelled, failed or had lost" echo " their artifacts were rejected by name in the log before any of these were used." echo + echo "$CARRY_LINE" + echo + echo "⚠️ This PR references #16173 and #16222 but does NOT carry a closing keyword for them," + echo "because a weekly lane cannot know which cards a given run ought to retire. If this is" + echo "the first refresh to land, retire those two by hand as part of merging it." + echo echo "## Measured per-shard suite time on the newest run in the set" echo echo "\`\`\`" diff --git a/scripts/ci/select-shard-timings-run.mjs b/scripts/ci/select-shard-timings-run.mjs index 7ba856c0b2..1c8acec426 100644 --- a/scripts/ci/select-shard-timings-run.mjs +++ b/scripts/ci/select-shard-timings-run.mjs @@ -49,9 +49,11 @@ // node scripts/ci/select-shard-timings-run.mjs --self-test // // `--candidates` needs GITHUB_TOKEN and GITHUB_REPOSITORY in the environment and -// prints a JSON array, newest first. `--check-coverage` exits non-zero when the -// refreshed dataset lost a package the committed one measured and the workspace -// still contains. +// prints a JSON array, newest first. `--check-coverage` judges MEASURED UNION +// CARRIED against the workspace and exits non-zero when a package that HAD a +// measured weight has neither -- it names workspace packages that were never +// measured too, but those are a report rather than a refusal, because the +// partitioner already estimated them and this refresh did not change that. import { readFileSync } from 'node:fs'; import process from 'node:process'; @@ -162,23 +164,53 @@ export function runIsEligible({ jobs, artifacts }, shardCount = SHARD_COUNT) { // direction this refuses in. Comparing against the live workspace is what keeps // a package legitimately deleted from the monorepo from blocking every future // refresh -- it is gone from `workspace`, so it is not required. +// Coverage is judged on MEASURED UNION CARRIED, which after a `--merge-into` +// pass is simply the refreshed dataset's own key set: the generator has already +// folded in every package a cache HIT witnessed as unchanged, and refused to +// fold in anything else. Two outcomes are reported separately because they are +// different facts and only one of them is a regression: +// +// `lost` — the package HAD a measured weight, this refresh neither measured +// it nor found a cache HIT to witness it, so it would drop to a +// test-file-count ESTIMATE. That is the silent degradation this +// whole lane exists to prevent, so it is a REFUSAL, by name. +// +// `neverMeasured` — the package is in the workspace and has no measured +// weight before OR after: a new package, or one the dataset has +// never covered. The partitioner already estimates it from its +// test-file count and this refresh changed nothing about it, so it +// cannot be a regression — but it is NAMED rather than passed over +// in silence, because "estimated" must never be something a reader +// has to infer from an absence. export function coverageReport({ committed, refreshed, workspace, exclude = [] }) { const excluded = new Set(exclude); const inWorkspace = new Set(workspace); - const required = Object.keys(committed?.packages ?? {}).filter( + const priorPackages = committed?.packages ?? {}; + const covered = new Set(Object.keys(refreshed?.packages ?? {})); + const carried = new Set(refreshed?.carriedOver ?? []); + + const required = Object.keys(priorPackages).filter( (name) => inWorkspace.has(name) && !excluded.has(name) ); - const measured = new Set(Object.keys(refreshed?.packages ?? {})); - const lost = required.filter((name) => !measured.has(name)).sort((a, b) => a.localeCompare(b, 'en')); - const gained = [...measured] - .filter((name) => !Object.hasOwn(committed?.packages ?? {}, name)) + const lost = required.filter((name) => !covered.has(name)).sort((a, b) => a.localeCompare(b, 'en')); + + const neverMeasured = [...inWorkspace] + .filter((name) => !excluded.has(name) && !covered.has(name) && !Object.hasOwn(priorPackages, name)) .sort((a, b) => a.localeCompare(b, 'en')); + + const gained = [...covered] + .filter((name) => !Object.hasOwn(priorPackages, name)) + .sort((a, b) => a.localeCompare(b, 'en')); + return { ok: lost.length === 0, lost, + neverMeasured, gained, + carriedCount: carried.size, + freshCount: covered.size - carried.size, requiredCount: required.length, - measuredCount: measured.size, + measuredCount: covered.size, }; } @@ -265,7 +297,7 @@ export async function listCandidates({ // stopped running, and the remedy is to find what stopped registering, never to // lower the number. const SELF_TEST_BATTERIES = Object.freeze({ - 'select-shard-timings-run self-test': 30, + 'select-shard-timings-run self-test': 35, }); const SELF_TEST_BATTERY_FLOOR = 1; const UNATTRIBUTED_BATTERY = '(no battery open)'; @@ -466,6 +498,66 @@ function selfTest() { if (!r.ok || r.gained.join(',') !== 'd') throw new Error(`coverage: a newly measured package was not reported (${r.gained.join(',')})`); }); + // -- Coverage under the MERGE (#16464). After a `--merge-into` pass the + // refreshed dataset already holds the carried weights, so coverage is + // judged on measured UNION carried; what the cases below separate is the + // two ways a package can be missing, because only one of them is a + // regression. + check(() => { + // A carried package COUNTS as covered — it has a real weight, witnessed + // unchanged by a cache hit — so a refresh that measured only `a` and + // carried `b` and `c` is complete, not short. + const r = coverageReport({ + committed, + refreshed: { packages: { a: 11, b: 20, c: 30 }, carriedOver: ['b', 'c'] }, + workspace: ws, + }); + if (!r.ok) throw new Error(`coverage: carried packages were not counted as covered (${r.lost.join(', ')})`); + if (r.carriedCount !== 2 || r.freshCount !== 1) { + throw new Error(`coverage: the carried/fresh split is wrong (carried ${r.carriedCount}, fresh ${r.freshCount})`); + } + }); + check(() => { + // The regression that still refuses: `c` had a weight and is in NEITHER set. + const r = coverageReport({ + committed, + refreshed: { packages: { a: 11, b: 20 }, carriedOver: ['b'] }, + workspace: ws, + }); + if (r.ok) throw new Error('coverage: a package that lost its measured weight was accepted'); + if (r.lost.join(',') !== 'c') throw new Error(`coverage: the lost package was not named (${r.lost.join(',')})`); + }); + check(() => { + // A workspace package that NEVER had a weight is named but is not a + // refusal: the partitioner already estimated it and this refresh changed + // nothing about it. + const r = coverageReport({ + committed, + refreshed: { packages: { a: 11, b: 20, c: 30 }, carriedOver: [] }, + workspace: [...ws, 'brand-new'], + }); + if (!r.ok) throw new Error(`coverage: a never-measured package was treated as a regression (${r.lost.join(',')})`); + if (r.neverMeasured.join(',') !== 'brand-new') { + throw new Error(`coverage: the never-measured package was not named (${r.neverMeasured.join(',')})`); + } + }); + check(() => { + // …and it is not confused with a carried one. + const r = coverageReport({ + committed, + refreshed: { packages: { a: 11, b: 20, c: 30 }, carriedOver: ['c'] }, + workspace: [...ws, 'brand-new'], + }); + if (r.neverMeasured.includes('c') || r.carriedCount !== 1) { + throw new Error(`coverage: carried and never-measured were conflated (never ${r.neverMeasured.join(',')}, carried ${r.carriedCount})`); + } + }); + check(() => { + // A dataset with no carriedOver key at all (a plain replace) still reads. + const r = coverageReport({ committed, refreshed: { packages: { a: 1, b: 2, c: 3 } }, workspace: ws }); + if (!r.ok || r.carriedCount !== 0) throw new Error('coverage: a dataset without carriedOver was misread'); + }); + // -- The workspace reader refuses a shape it cannot trust, rather than // returning an empty list that would make every package look deleted. check(() => { @@ -546,20 +638,32 @@ async function main() { const refreshed = readJson(value('--refreshed')); const workspace = workspaceNames(readJson(value('--workspace'))); const report = coverageReport({ committed, refreshed, workspace, exclude }); + // Named whichever way the verdict goes: a package the partitioner estimates + // must never be something a reader infers from an absence. + if (report.neverMeasured.length > 0) { + console.error( + `select-shard-timings-run: ${report.neverMeasured.length} workspace package(s) have no measured ` + + `weight before or after this refresh and are ESTIMATED by the partitioner from their ` + + `test-file count: ${report.neverMeasured.join(', ')}. Not a regression — this refresh did not ` + + 'change their standing — but they are named rather than passed over, because an estimate that ' + + 'reads as a measurement is this dataset\'s signature hazard.' + ); + } if (!report.ok) { console.error( - `select-shard-timings-run: COVERAGE SHORTFALL -- ${report.lost.length} package(s) the committed ` + - 'dataset measured, and the workspace still contains, are NOT measured by the runs accumulated ' + - `so far: ${report.lost.join(', ')}. Every one of them would silently fall back to the ` + - 'test-file-count ESTIMATE, so what has been read so far is a warm cache rather than the ' + - 'workspace. Not a verdict on any one run: no single run measures everything (turbo caches per ' + - 'shard, and the generator refuses hits), so the caller adds the next older run and asks again.' + `select-shard-timings-run: COVERAGE SHORTFALL -- ${report.lost.length} package(s) HAD a measured ` + + 'weight and this refresh neither re-measured them nor found a turbo cache HIT to witness that ' + + `they are unchanged: ${report.lost.join(', ')}. Each would drop to the test-file-count ` + + 'ESTIMATE, which is the silent degradation this lane exists to prevent, so this is a refusal. ' + + 'Not a verdict on any one run: no single run measures everything, so the caller adds the next ' + + 'older run and asks again.' ); process.exit(1); } console.error( - `select-shard-timings-run: coverage OK -- ${report.measuredCount} package(s) measured, ` + - `${report.requiredCount} required, ${report.gained.length} newly measured` + + `select-shard-timings-run: coverage OK -- ${report.measuredCount} package(s) covered ` + + `(${report.freshCount} measured in these runs, ${report.carriedCount} carried on a cache-hit ` + + `witness), ${report.requiredCount} required, ${report.gained.length} newly measured` + `${report.gained.length > 0 ? ` (${report.gained.join(', ')})` : ''}.` ); return; diff --git a/scripts/measure-test-shard-timings.mjs b/scripts/measure-test-shard-timings.mjs index 8ccad1c87b..61b1b71827 100644 --- a/scripts/measure-test-shard-timings.mjs +++ b/scripts/measure-test-shard-timings.mjs @@ -27,16 +27,37 @@ // // From CI, no special run needed. Every Test Core shard passes --summarize // and, on merge_group builds, uploads `.turbo/runs/` as the -// `test-core-run-summary--of-6` artifact. Download all six from any -// green queue build and: -// node scripts/measure-test-shard-timings.mjs /*.json \ +// `test-core-run-summary--of-6` artifact. +// +// ⚠ ONE RUN IS NOT ENOUGH, AND THIS SENTENCE USED TO SAY IT WAS. It read +// "download all six from any green queue build and re-run the generator", +// which is optimistic in a way nobody had measured until the refresh lane ran +// for real: a single green run yields between 2 and 52 of the ~71 measurable +// packages, depending on nothing but how warm that run's turbo cache was, and +// the union of every retained green run converges around 57. The rest are +// cache HITs, which this file refuses (see the CACHED TASKS rule below). So +// the honest procedure is: +// +// node scripts/measure-test-shard-timings.mjs \ +// --run /*.json \ +// --run /*.json \ +// --merge-into scripts/test-shard-timings.json \ // --out scripts/test-shard-timings.json -// Feeding MORE THAN ONE run means saying which is which -- `--run ` -// before each run's six summaries. Slices are summed within a run and the -// per-run sums medianed across runs, so a file-sharded package gets the same -// median treatment as every other package (#16473); undeclared multi-run -// input is REFUSED rather than resolved by guessing. -// `.github/workflows/shard-timings-refresh.yml` runs this on a weekly timer. +// +// `--run` fences each run: slices are summed within a run and the per-run sums +// medianed across runs, so a file-sharded package gets the same median +// treatment as every other package (#16473); undeclared multi-run input is +// REFUSED rather than resolved by guessing. +// +// `--merge-into` is what makes a partial measurement sound. A package this +// pass did not measure keeps its previous weight -- but ONLY when a cache HIT +// witnesses that its inputs are unchanged, which is the evidence that the old +// number still describes it. Those packages are named in `carriedOver`. +// Anything absent for any other reason is simply not in the output, so the +// caller can name it rather than estimate it. +// +// `.github/workflows/shard-timings-refresh.yml` does all of this on a weekly +// timer and opens the PR. // // Locally, on a 4-vCPU box (the hosted runner's shape): // pnpm exec turbo run build @@ -57,7 +78,7 @@ // Usage: // node scripts/measure-test-shard-timings.mjs ... [--out ] // node scripts/measure-test-shard-timings.mjs --run ... \ -// --run ... [--out ] +// --run ... [--merge-into ] [--out ] // node scripts/measure-test-shard-timings.mjs --self-test import { existsSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; @@ -189,7 +210,7 @@ export function fallbackRate(measured, fileCounts) { return Math.round(median(rates) * 1000) / 1000; } -export function buildDataset({ perSummary, fileCounts, provenance }) { +export function buildDataset({ perSummary, fileCounts, provenance, carryFrom = null }) { const bySample = new Map(); const cachedNames = new Set(); const push = (name, seconds) => { @@ -311,6 +332,44 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { const measured = new Map( [...bySample.entries()].map(([name, values]) => [name, Math.round(median(values) * 100) / 100]) ); + + // ⛔ A CARRIED WEIGHT NEEDS A CACHE HIT AS ITS WITNESS. NOTHING ELSE CARRIES. + // + // Measured on the first two live runs of the refresh lane: NO set of retained + // green runs measures the whole workspace. The best single run covered 52 of + // the 71 packages the dataset holds, the accumulation of all seven converged + // at 57, and the last 14 were turbo cache HITs in every one of them. That is + // the cache design rather than luck -- the key is namespaced per shard and + // only main pushes write it, so a package whose inputs have not changed is a + // HIT, and this file refuses hits rather than recording a replayed ~0.1s + // window as a suite's cost. + // + // So "regenerate" cannot mean "replace". It means MERGE, and the merge is + // sound for one specific reason: a cache HIT is not missing data, it is + // POSITIVE EVIDENCE that the package's inputs are unchanged since the run + // whose output was replayed. Its last measured weight therefore still + // describes it -- the number is not stale, and turbo is the witness. Carrying + // it forward preserves this file's whole invariant: every number in it is a + // real measurement of code as it stands, never an estimate. + // + // The witness requirement is what keeps that from becoming a licence to keep + // anything. A package absent from the summaries for ANY OTHER reason -- it + // never ran, its suite failed, its slices could not be assembled -- has no + // evidence behind it, so it is NOT carried. It drops out and the caller's + // coverage check names it. `skippedAsCached` is exactly the witnessed set, so + // the carry reads it rather than inventing a second classification. + const carriedOver = []; + if (carryFrom) { + for (const name of Object.keys(carryFrom).sort((a, b) => a.localeCompare(b, 'en'))) { + if (measured.has(name)) continue; + if (!cachedNames.has(name)) continue; + const seconds = carryFrom[name]; + if (typeof seconds !== 'number' || !(seconds >= 0)) continue; + measured.set(name, seconds); + carriedOver.push(name); + } + } + const packages = {}; for (const name of [...measured.keys()].sort((a, b) => a.localeCompare(b, 'en'))) { packages[name] = measured.get(name); @@ -319,12 +378,22 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { note: 'GENERATED by scripts/measure-test-shard-timings.mjs -- do not hand-edit. Per-package ' + '`turbo run test` durations in seconds, the balancing input for the Test Core shard ' + - 'split (scripts/partition-test-shards.mjs). See `provenance.refresh` to regenerate.', + 'split (scripts/partition-test-shards.mjs). See `provenance.refresh` to regenerate. ' + + 'Every weight is a real measurement: the ones in `carriedOver` were measured by an earlier ' + + 'refresh and re-confirmed unchanged by a turbo cache HIT in this one.', provenance, secondsPerTestFileFallback: fallbackRate(measured, fileCounts), packages, skippedAsCached: [...cachedNames].sort((a, b) => a.localeCompare(b, 'en')), skippedIncompleteSlices: incompleteSlices, + // Beside `skippedAsCached` rather than folded into a per-package + // `measuredAt`, and the choice is forced by the reader: partition-test- + // shards.mjs reads `packages` as name -> NUMBER (`timings.packages[name] / + // sliceCount`) and never opens `provenance` at all, so per-package dates + // would mean changing that shape and every consumer of it. A single + // `provenance.measuredAt` paired with this list carries the same + // information and needs no change in the partitioner. + carriedOver, }; } @@ -356,7 +425,7 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { // must not red. A battery BELOW its floor means cases stopped running; the // remedy is to find what stopped registering, never to lower the number. const SELF_TEST_BATTERIES = Object.freeze({ - 'measure-test-shard-timings self-test': 41, + 'measure-test-shard-timings self-test': 50, }); // DELETING an entry silences that battery's floor exactly as effectively as @@ -751,6 +820,110 @@ function selfTest() { } }); + // The MERGE, and the witness rule that bounds it (#16464). No retained run set + // measures the whole workspace, so a refresh that REPLACED the dataset would + // demote every cache-hit package to a test-file-count estimate. Carrying is + // sound only because a cache HIT is evidence the package is unchanged, so the + // cases below pin the carry AND its boundary: what has a witness carries, what + // does not is simply absent for the caller to name. + const priorDataset = { a: 10, cached: 500, vanished: 700 }; + const mergeSummary = summary([ + testTask('a', 0, 12_000), + { taskId: 'cached#test', task: 'test', package: 'cached', cache: { status: 'HIT' }, execution: { startTime: 0, endTime: 90, exitCode: 0 } }, + ]); + const mergedSet = buildDataset({ + perSummary: [samplesFromSummary(mergeSummary, 'm')], + fileCounts: new Map([['a', 6], ['cached', 250], ['vanished', 300]]), + provenance: {}, + carryFrom: priorDataset, + }); + + // 1. A CARRIED package: weight unchanged, and named. + check(() => { + if (mergedSet.packages.cached !== 500) { + throw new Error(`merge: a cache-hit package was not carried at its previous weight (got ${mergedSet.packages.cached}, expected 500)`); + } + }); + check(() => { + if (!mergedSet.carriedOver.includes('cached')) { + throw new Error(`merge: the carried package was not named in carriedOver (${mergedSet.carriedOver.join(', ') || 'empty'})`); + } + }); + // 2. A freshly MEASURED package takes the new number, and is NOT called carried. + check(() => { + if (mergedSet.packages.a !== 12) throw new Error(`merge: a measured package did not take its new weight (${mergedSet.packages.a})`); + }); + check(() => { + if (mergedSet.carriedOver.includes('a')) throw new Error('merge: a package measured in this pass was reported as carried'); + }); + // 3. HIT-WITNESSED CARRY versus ABSENT: `vanished` is in the prior dataset but + // appears in NO summary, so nothing witnesses that it is unchanged. It must + // NOT be carried — this is the case that separates a merge from "keep + // whatever was there", and without it the carry would launder a stale + // number for a package that may have been deleted, renamed, or gone red. + check(() => { + if (Object.hasOwn(mergedSet.packages, 'vanished')) { + throw new Error(`merge: a package with no cache-hit witness was carried anyway (${mergedSet.packages.vanished})`); + } + }); + check(() => { + if (mergedSet.carriedOver.includes('vanished')) throw new Error('merge: an unwitnessed package was named as carried'); + }); + // 4. No carryFrom at all is the plain replace, and reports an empty list + // rather than omitting the field — an absent key and "nothing was carried" + // must not read the same way to the caller's coverage check. + check(() => { + const plain = buildDataset({ + perSummary: [samplesFromSummary(mergeSummary, 'm')], + fileCounts: new Map([['a', 6]]), + provenance: {}, + }); + if (!Array.isArray(plain.carriedOver) || plain.carriedOver.length !== 0) { + throw new Error(`merge: a run with no --merge-into did not report an empty carriedOver (${JSON.stringify(plain.carriedOver)})`); + } + }); + // 5. The merge does NOT rescue a run that measured nothing: an all-cached pass + // still refuses, so "everything carried" can never masquerade as a refresh. + check(() => { + if (!threw(() => + buildDataset({ + perSummary: [samplesFromSummary(summary([testTask('a', 0, 40, 'HIT')]), 'f')], + fileCounts: new Map([['a', 6]]), + provenance: {}, + carryFrom: priorDataset, + }) + )) { + throw new Error('merge: a pass that measured NOTHING produced a dataset out of carried weights alone'); + } + }); + // 6. MONOTONE ACCUMULATION: feeding a second run measures more, and a package + // measured by the newer run stops being carried and takes its real number. + check(() => { + const oneRun = buildDataset({ + perSummary: [{ ...samplesFromSummary(mergeSummary, 'm'), run: 'r1' }], + fileCounts: new Map([['a', 6], ['cached', 250]]), + provenance: {}, + carryFrom: priorDataset, + }); + const twoRuns = buildDataset({ + perSummary: [ + { ...samplesFromSummary(mergeSummary, 'm'), run: 'r1' }, + { ...samplesFromSummary(summary([testTask('cached', 0, 480_000)]), 'n'), run: 'r2' }, + ], + fileCounts: new Map([['a', 6], ['cached', 250]]), + provenance: {}, + carryFrom: priorDataset, + }); + if (oneRun.carriedOver.length !== 1 || twoRuns.carriedOver.length !== 0) { + throw new Error( + `merge: accumulation is not monotone — one run carried ${oneRun.carriedOver.length}, two carried ${twoRuns.carriedOver.length} (expected 1 then 0)` + ); + } + if (twoRuns.packages.cached !== 480) { + throw new Error(`merge: the second run measured the package but it kept its carried weight (${twoRuns.packages.cached})`); + } + }); + // Workspace resolution, at the depth that actually caught a defect. A // one-level scan resolves `packages/*` and returns null for the ~60% of the // workspace that lives under `packages/drivers/*`, `packages/services/*` and @@ -886,6 +1059,7 @@ function main() { return; } let out = DEFAULT_OUT; + let mergeInto = null; // Each input carries the run it belongs to (#16473). `--run ` opens a // group and every summary AFTER it belongs to that run, so one CI run's six // artifacts are named together the way they are fetched together. Summaries @@ -896,7 +1070,13 @@ function main() { let currentRun = null; for (let i = 0; i < argv.length; i++) { if (argv[i] === '--out') out = path.resolve(argv[++i]); - else if (argv[i] === '--run') { + else if (argv[i] === '--merge-into') { + const value = argv[++i]; + if (typeof value !== 'string' || value.length === 0 || value.startsWith('--')) { + throw new Error('--merge-into needs the path of the dataset to carry unchanged weights from'); + } + mergeInto = path.resolve(value); + } else if (argv[i] === '--run') { const value = argv[++i]; if (typeof value !== 'string' || value.length === 0 || value.startsWith('--')) { throw new Error('--run needs a run identifier (the id of the CI run whose summaries follow it)'); @@ -907,7 +1087,8 @@ function main() { } if (inputs.length === 0) { console.error( - 'usage: measure-test-shard-timings.mjs [--run ] ... [--out ]' + 'usage: measure-test-shard-timings.mjs [--run ] ... ' + + '[--merge-into ] [--out ]' ); process.exit(1); } @@ -917,19 +1098,36 @@ function main() { perSummary.push({ ...samplesFromSummary(JSON.parse(readFileSync(file, 'utf8')), file), run }); } - const fileCounts = new Map(); - for (const { samples } of perSummary) { - for (const name of samples.keys()) { - if (fileCounts.has(name)) continue; - const dir = packageDirForName(name); - fileCounts.set(name, dir ? countTestFiles(dir) : 0); + // The prior dataset, when a merge was asked for. Read before the file counts, + // because a carried package needs a count too -- it votes on the fallback rate + // exactly like a freshly measured one, which is what keeps that rate derived + // from the numbers actually in the file rather than from a subset of them. + let carryFrom = null; + if (mergeInto !== null) { + const prior = JSON.parse(readFileSync(mergeInto, 'utf8')); + if (!prior || typeof prior.packages !== 'object' || prior.packages === null) { + throw new Error( + `--merge-into ${path.relative(REPO_ROOT, mergeInto)}: expected a dataset with a {packages:{...}} ` + + 'map to carry unchanged weights from. Refusing to merge into a shape this did not write.' + ); } + carryFrom = prior.packages; + } + + const fileCounts = new Map(); + const needCount = new Set(); + for (const { samples } of perSummary) for (const name of samples.keys()) needCount.add(name); + if (carryFrom) for (const name of Object.keys(carryFrom)) needCount.add(name); + for (const name of needCount) { + const dir = packageDirForName(name); + fileCounts.set(name, dir ? countTestFiles(dir) : 0); } const declaredRuns = [...new Set(inputs.map((i) => i.run).filter((r) => r !== null))]; const dataset = buildDataset({ perSummary, fileCounts, + carryFrom, provenance: { measuredAt: new Date().toISOString().slice(0, 10), summaries: inputs.map((i) => path.basename(i.file)), @@ -940,14 +1138,23 @@ function main() { // summed within their run first and only the per-run sums are medianed. mergeRule: 'median across runs; a file-sharded package is summed from its slices WITHIN one run ' + - 'first, and a run that cannot assemble every slice contributes no sample for it', + 'first, and a run that cannot assemble every slice contributes no sample for it; a package ' + + 'not measured in this pass keeps its previous weight ONLY when a turbo cache HIT witnesses ' + + 'that its inputs are unchanged, and every such package is named in `carriedOver`', + // `measuredAt` is the date of THIS pass, and it dates the measured + // weights. The carried ones were measured earlier and re-confirmed + // unchanged by a cache HIT today; `carriedOver` names them, which is why a + // single date is enough and per-package dates are not needed. See the note + // on `carriedOver` in buildDataset for why the reader forces that choice. refresh: - 'node scripts/measure-test-shard-timings.mjs [--run ] ... --out ' + - 'scripts/test-shard-timings.json (summaries: the `test-core-run-summary--of-6` artifacts of ' + - 'any green run, or a local `pnpm exec turbo run test --concurrency=4 --summarize`. Feeding more ' + - 'than one run REQUIRES a `--run ` before each run\'s summaries, so a sliced package is ' + - 'assembled per run and then medianed like every other package. `.github/workflows/' + - 'shard-timings-refresh.yml` does this weekly.)', + 'node scripts/measure-test-shard-timings.mjs [--run ] ... ' + + '--merge-into scripts/test-shard-timings.json --out scripts/test-shard-timings.json ' + + '(summaries: the `test-core-run-summary--of-6` artifacts of any green run, or a local ' + + '`pnpm exec turbo run test --concurrency=4 --summarize`. ⚠ ONE RUN COVERS ONLY 2-52 of ~71 ' + + 'packages depending on cache warmth, so feed SEVERAL runs -- a `--run ` before each ' + + 'run\'s summaries is REQUIRED, so a sliced package is assembled per run and then medianed ' + + 'like every other package -- and `--merge-into` to carry the cache-hit remainder. ' + + '`.github/workflows/shard-timings-refresh.yml` does all of this weekly.)', }, }); writeFileSync(out, `${JSON.stringify(dataset, null, 2)}\n`);