Skip to content

Commit fb51eee

Browse files
committed
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 <dataset>` 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox
1 parent 4fe161b commit fb51eee

3 files changed

Lines changed: 402 additions & 47 deletions

File tree

‎.github/workflows/shard-timings-refresh.yml‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,19 @@
7373
# accumulated runs gets the median of three observations, which is the property
7474
# the dataset's own merge rule always claimed and could not previously deliver.
7575
#
76+
# ⚠ AND ACCUMULATION ALONE STILL FALLS SHORT, so the pass MERGES rather than
77+
# replaces. Measured on the same live runs: the union of all seven retained runs
78+
# reaches 57 of 71 packages and converges there, because 14 packages are cache
79+
# HITs in every one of them. Those are carried at their previous weights through
80+
# `--merge-into`, and the carry is sound for one specific reason — a cache HIT is
81+
# not missing data, it is POSITIVE EVIDENCE that the package's inputs are
82+
# unchanged since the run whose output was replayed, so its last measured weight
83+
# still describes it. Every carried package is named in the dataset's
84+
# `carriedOver` list, and a package absent for any OTHER reason is not carried at
85+
# all: it drops out and the coverage check names it as a refusal. The file's
86+
# invariant is preserved exactly — every number in it is a real measurement of
87+
# code as it stands, never an estimate.
88+
#
7689
# THE PINS, AND THE ONE THING A MACHINE MUST NOT DECIDE
7790
# ----------------------------------------------------
7891
# partition-test-shards.mjs `--self-test` grades the dataset against the
@@ -316,7 +329,14 @@ jobs:
316329
ARGS+=( "${RUN_FILES[@]}" )
317330
done
318331
332+
# `--merge-into` the committed dataset, because no retained run set
333+
# measures the whole workspace (see the header). A package this pass
334+
# did not measure keeps its previous weight ONLY when a turbo cache
335+
# HIT witnesses that its inputs are unchanged; anything else is left
336+
# out for the coverage check below to name. The workflow still writes
337+
# no byte the generator did not emit — the merge happens inside it.
319338
if ! node scripts/measure-test-shard-timings.mjs "${ARGS[@]}" \
339+
--merge-into scripts/test-shard-timings.json \
320340
--out "$WORK/refreshed.json"; then
321341
echo "::warning::The generator refused the set including run $RUN_ID; dropping that run and continuing."
322342
unset 'ACCEPTED[-1]'
@@ -337,7 +357,7 @@ jobs:
337357
done
338358
339359
if [ -z "$CHOSEN" ]; then
340-
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."
360+
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."
341361
exit 1
342362
fi
343363
# Candidates arrive newest-first, so ACCEPTED[0] is the most recent run
@@ -403,6 +423,24 @@ jobs:
403423
run: |
404424
set -euo pipefail
405425
426+
# Read out of the generated dataset itself rather than recomputed, so the
427+
# sentence in the PR cannot drift from the file it describes.
428+
CARRY_LINE=$(node -e '
429+
const d = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/refresh/refreshed.json", "utf8"));
430+
const carried = d.carriedOver ?? [];
431+
const total = Object.keys(d.packages).length;
432+
const fresh = total - carried.length;
433+
if (carried.length === 0) {
434+
console.log(`All ${total} package weights were measured in these runs; nothing was carried.`);
435+
} else {
436+
console.log(
437+
`${total} package weights: ${fresh} measured in these runs, and ${carried.length} carried ` +
438+
`forward at their previous values because a turbo cache HIT witnessed that their inputs are ` +
439+
`unchanged (so the old number still describes them). Carried: ${carried.join(", ")}.`
440+
);
441+
}
442+
')
443+
406444
SHARDS=$(node -e '
407445
const rs = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8"));
408446
const r = rs.find((x) => String(x.run_id) === process.env.RUN_ID);
@@ -434,6 +472,12 @@ jobs:
434472
echo " run-summary artifacts still retained; runs that were cancelled, failed or had lost"
435473
echo " their artifacts were rejected by name in the log before any of these were used."
436474
echo
475+
echo "$CARRY_LINE"
476+
echo
477+
echo "⚠️ This PR references #16173 and #16222 but does NOT carry a closing keyword for them,"
478+
echo "because a weekly lane cannot know which cards a given run ought to retire. If this is"
479+
echo "the first refresh to land, retire those two by hand as part of merging it."
480+
echo
437481
echo "## Measured per-shard suite time on the newest run in the set"
438482
echo
439483
echo "\`\`\`"

‎scripts/ci/select-shard-timings-run.mjs‎

Lines changed: 122 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,11 @@
4949
// node scripts/ci/select-shard-timings-run.mjs --self-test
5050
//
5151
// `--candidates` needs GITHUB_TOKEN and GITHUB_REPOSITORY in the environment and
52-
// prints a JSON array, newest first. `--check-coverage` exits non-zero when the
53-
// refreshed dataset lost a package the committed one measured and the workspace
54-
// still contains.
52+
// prints a JSON array, newest first. `--check-coverage` judges MEASURED UNION
53+
// CARRIED against the workspace and exits non-zero when a package that HAD a
54+
// measured weight has neither -- it names workspace packages that were never
55+
// measured too, but those are a report rather than a refusal, because the
56+
// partitioner already estimated them and this refresh did not change that.
5557

5658
import { readFileSync } from 'node:fs';
5759
import process from 'node:process';
@@ -162,23 +164,53 @@ export function runIsEligible({ jobs, artifacts }, shardCount = SHARD_COUNT) {
162164
// direction this refuses in. Comparing against the live workspace is what keeps
163165
// a package legitimately deleted from the monorepo from blocking every future
164166
// refresh -- it is gone from `workspace`, so it is not required.
167+
// Coverage is judged on MEASURED UNION CARRIED, which after a `--merge-into`
168+
// pass is simply the refreshed dataset's own key set: the generator has already
169+
// folded in every package a cache HIT witnessed as unchanged, and refused to
170+
// fold in anything else. Two outcomes are reported separately because they are
171+
// different facts and only one of them is a regression:
172+
//
173+
// `lost` — the package HAD a measured weight, this refresh neither measured
174+
// it nor found a cache HIT to witness it, so it would drop to a
175+
// test-file-count ESTIMATE. That is the silent degradation this
176+
// whole lane exists to prevent, so it is a REFUSAL, by name.
177+
//
178+
// `neverMeasured` — the package is in the workspace and has no measured
179+
// weight before OR after: a new package, or one the dataset has
180+
// never covered. The partitioner already estimates it from its
181+
// test-file count and this refresh changed nothing about it, so it
182+
// cannot be a regression — but it is NAMED rather than passed over
183+
// in silence, because "estimated" must never be something a reader
184+
// has to infer from an absence.
165185
export function coverageReport({ committed, refreshed, workspace, exclude = [] }) {
166186
const excluded = new Set(exclude);
167187
const inWorkspace = new Set(workspace);
168-
const required = Object.keys(committed?.packages ?? {}).filter(
188+
const priorPackages = committed?.packages ?? {};
189+
const covered = new Set(Object.keys(refreshed?.packages ?? {}));
190+
const carried = new Set(refreshed?.carriedOver ?? []);
191+
192+
const required = Object.keys(priorPackages).filter(
169193
(name) => inWorkspace.has(name) && !excluded.has(name)
170194
);
171-
const measured = new Set(Object.keys(refreshed?.packages ?? {}));
172-
const lost = required.filter((name) => !measured.has(name)).sort((a, b) => a.localeCompare(b, 'en'));
173-
const gained = [...measured]
174-
.filter((name) => !Object.hasOwn(committed?.packages ?? {}, name))
195+
const lost = required.filter((name) => !covered.has(name)).sort((a, b) => a.localeCompare(b, 'en'));
196+
197+
const neverMeasured = [...inWorkspace]
198+
.filter((name) => !excluded.has(name) && !covered.has(name) && !Object.hasOwn(priorPackages, name))
175199
.sort((a, b) => a.localeCompare(b, 'en'));
200+
201+
const gained = [...covered]
202+
.filter((name) => !Object.hasOwn(priorPackages, name))
203+
.sort((a, b) => a.localeCompare(b, 'en'));
204+
176205
return {
177206
ok: lost.length === 0,
178207
lost,
208+
neverMeasured,
179209
gained,
210+
carriedCount: carried.size,
211+
freshCount: covered.size - carried.size,
180212
requiredCount: required.length,
181-
measuredCount: measured.size,
213+
measuredCount: covered.size,
182214
};
183215
}
184216

@@ -265,7 +297,7 @@ export async function listCandidates({
265297
// stopped running, and the remedy is to find what stopped registering, never to
266298
// lower the number.
267299
const SELF_TEST_BATTERIES = Object.freeze({
268-
'select-shard-timings-run self-test': 30,
300+
'select-shard-timings-run self-test': 35,
269301
});
270302
const SELF_TEST_BATTERY_FLOOR = 1;
271303
const UNATTRIBUTED_BATTERY = '(no battery open)';
@@ -466,6 +498,66 @@ function selfTest() {
466498
if (!r.ok || r.gained.join(',') !== 'd') throw new Error(`coverage: a newly measured package was not reported (${r.gained.join(',')})`);
467499
});
468500

501+
// -- Coverage under the MERGE (#16464). After a `--merge-into` pass the
502+
// refreshed dataset already holds the carried weights, so coverage is
503+
// judged on measured UNION carried; what the cases below separate is the
504+
// two ways a package can be missing, because only one of them is a
505+
// regression.
506+
check(() => {
507+
// A carried package COUNTS as covered — it has a real weight, witnessed
508+
// unchanged by a cache hit — so a refresh that measured only `a` and
509+
// carried `b` and `c` is complete, not short.
510+
const r = coverageReport({
511+
committed,
512+
refreshed: { packages: { a: 11, b: 20, c: 30 }, carriedOver: ['b', 'c'] },
513+
workspace: ws,
514+
});
515+
if (!r.ok) throw new Error(`coverage: carried packages were not counted as covered (${r.lost.join(', ')})`);
516+
if (r.carriedCount !== 2 || r.freshCount !== 1) {
517+
throw new Error(`coverage: the carried/fresh split is wrong (carried ${r.carriedCount}, fresh ${r.freshCount})`);
518+
}
519+
});
520+
check(() => {
521+
// The regression that still refuses: `c` had a weight and is in NEITHER set.
522+
const r = coverageReport({
523+
committed,
524+
refreshed: { packages: { a: 11, b: 20 }, carriedOver: ['b'] },
525+
workspace: ws,
526+
});
527+
if (r.ok) throw new Error('coverage: a package that lost its measured weight was accepted');
528+
if (r.lost.join(',') !== 'c') throw new Error(`coverage: the lost package was not named (${r.lost.join(',')})`);
529+
});
530+
check(() => {
531+
// A workspace package that NEVER had a weight is named but is not a
532+
// refusal: the partitioner already estimated it and this refresh changed
533+
// nothing about it.
534+
const r = coverageReport({
535+
committed,
536+
refreshed: { packages: { a: 11, b: 20, c: 30 }, carriedOver: [] },
537+
workspace: [...ws, 'brand-new'],
538+
});
539+
if (!r.ok) throw new Error(`coverage: a never-measured package was treated as a regression (${r.lost.join(',')})`);
540+
if (r.neverMeasured.join(',') !== 'brand-new') {
541+
throw new Error(`coverage: the never-measured package was not named (${r.neverMeasured.join(',')})`);
542+
}
543+
});
544+
check(() => {
545+
// …and it is not confused with a carried one.
546+
const r = coverageReport({
547+
committed,
548+
refreshed: { packages: { a: 11, b: 20, c: 30 }, carriedOver: ['c'] },
549+
workspace: [...ws, 'brand-new'],
550+
});
551+
if (r.neverMeasured.includes('c') || r.carriedCount !== 1) {
552+
throw new Error(`coverage: carried and never-measured were conflated (never ${r.neverMeasured.join(',')}, carried ${r.carriedCount})`);
553+
}
554+
});
555+
check(() => {
556+
// A dataset with no carriedOver key at all (a plain replace) still reads.
557+
const r = coverageReport({ committed, refreshed: { packages: { a: 1, b: 2, c: 3 } }, workspace: ws });
558+
if (!r.ok || r.carriedCount !== 0) throw new Error('coverage: a dataset without carriedOver was misread');
559+
});
560+
469561
// -- The workspace reader refuses a shape it cannot trust, rather than
470562
// returning an empty list that would make every package look deleted.
471563
check(() => {
@@ -546,20 +638,32 @@ async function main() {
546638
const refreshed = readJson(value('--refreshed'));
547639
const workspace = workspaceNames(readJson(value('--workspace')));
548640
const report = coverageReport({ committed, refreshed, workspace, exclude });
641+
// Named whichever way the verdict goes: a package the partitioner estimates
642+
// must never be something a reader infers from an absence.
643+
if (report.neverMeasured.length > 0) {
644+
console.error(
645+
`select-shard-timings-run: ${report.neverMeasured.length} workspace package(s) have no measured ` +
646+
`weight before or after this refresh and are ESTIMATED by the partitioner from their ` +
647+
`test-file count: ${report.neverMeasured.join(', ')}. Not a regression — this refresh did not ` +
648+
'change their standing — but they are named rather than passed over, because an estimate that ' +
649+
'reads as a measurement is this dataset\'s signature hazard.'
650+
);
651+
}
549652
if (!report.ok) {
550653
console.error(
551-
`select-shard-timings-run: COVERAGE SHORTFALL -- ${report.lost.length} package(s) the committed ` +
552-
'dataset measured, and the workspace still contains, are NOT measured by the runs accumulated ' +
553-
`so far: ${report.lost.join(', ')}. Every one of them would silently fall back to the ` +
554-
'test-file-count ESTIMATE, so what has been read so far is a warm cache rather than the ' +
555-
'workspace. Not a verdict on any one run: no single run measures everything (turbo caches per ' +
556-
'shard, and the generator refuses hits), so the caller adds the next older run and asks again.'
654+
`select-shard-timings-run: COVERAGE SHORTFALL -- ${report.lost.length} package(s) HAD a measured ` +
655+
'weight and this refresh neither re-measured them nor found a turbo cache HIT to witness that ' +
656+
`they are unchanged: ${report.lost.join(', ')}. Each would drop to the test-file-count ` +
657+
'ESTIMATE, which is the silent degradation this lane exists to prevent, so this is a refusal. ' +
658+
'Not a verdict on any one run: no single run measures everything, so the caller adds the next ' +
659+
'older run and asks again.'
557660
);
558661
process.exit(1);
559662
}
560663
console.error(
561-
`select-shard-timings-run: coverage OK -- ${report.measuredCount} package(s) measured, ` +
562-
`${report.requiredCount} required, ${report.gained.length} newly measured` +
664+
`select-shard-timings-run: coverage OK -- ${report.measuredCount} package(s) covered ` +
665+
`(${report.freshCount} measured in these runs, ${report.carriedCount} carried on a cache-hit ` +
666+
`witness), ${report.requiredCount} required, ${report.gained.length} newly measured` +
563667
`${report.gained.length > 0 ? ` (${report.gained.join(', ')})` : ''}.`
564668
);
565669
return;

0 commit comments

Comments
 (0)