Skip to content

Commit d049d35

Browse files
committed
ci(shard-timings): decode a run summary written under the map this change replaced, so the refresh lane survives the retirement
The refresh lane's pull_request rehearsal on this branch (run 37074888579) refused all ten eligible hourly runs on main and regenerated nothing. Every retained run summary carries the CLI's slices as an OS_TEST_SHARD digest of 1/2 or 2/2, and with FILE_SHARDED_PACKAGES emptied the generator's digest matcher had no candidate for either, so it refused each summary whole. - partition-test-shards.mjs declares PREVIOUS_FILE_SHARDED_PACKAGES, the outgoing map ({ '@objectstack/cli': 2 }), with when it decodes nothing: run summaries are kept for one day. - sliceOfEnvironment matches a digest against both closed maps, exactly by hash. A decoded slice is still summed within its run (an incomplete set contributes nothing), and a digest neither map names is still refused. - measure self-test: the live-maps case covers both defaults, and a new case reads a pre-change run's two halves into its 733.33s whole while refusing a count neither map names. Battery floor 10 to 11. Claude-Session: https://claude.ai/code/session_01HRYqpqGcWpJuJkDmbRF75w Co-authored-by: Claude <noreply@anthropic.com>
1 parent c614a09 commit d049d35

2 files changed

Lines changed: 148 additions & 55 deletions

File tree

‎scripts/measure-test-shard-timings.mjs‎

Lines changed: 118 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,16 @@ import path from 'node:path';
8787
import { fileURLToPath } from 'node:url';
8888
import process from 'node:process';
8989

90-
// FILE_SHARDED_PACKAGES and SLICE_ENV are read only inside functions: this
91-
// import is circular (the partitioner imports samplesFromSummary from here), so
92-
// a top-level read would meet an uninitialised binding when the partitioner is
93-
// the entry point.
94-
import { countTestFiles, FILE_SHARDED_PACKAGES, SLICE_ENV } from './partition-test-shards.mjs';
90+
// FILE_SHARDED_PACKAGES, PREVIOUS_FILE_SHARDED_PACKAGES and SLICE_ENV are read
91+
// only inside functions: this import is circular (the partitioner imports
92+
// samplesFromSummary from here), so a top-level read would meet an
93+
// uninitialised binding when the partitioner is the entry point.
94+
import {
95+
countTestFiles,
96+
FILE_SHARDED_PACKAGES,
97+
PREVIOUS_FILE_SHARDED_PACKAGES,
98+
SLICE_ENV,
99+
} from './partition-test-shards.mjs';
95100
import { isEntrypoint } from './invoked-as.mjs';
96101

97102
const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
@@ -159,23 +164,34 @@ export function sliceOfCliArguments(args) {
159164
// `OS_TEST_SHARD=` with nothing after it when the value is empty.
160165
//
161166
// So the digest is matched against every slice the partitioner CAN emit for
162-
// that package, and nothing else: `k/n` for 1 <= k <= n, where n is
163-
// FILE_SHARDED_PACKAGES[package] from partition-test-shards.mjs. The bound is
164-
// that map -- n candidates for a package it slices, none for one it does not.
167+
// that package, or emitted under the map it replaced, and nothing else: `k/n`
168+
// for 1 <= k <= n, for n = FILE_SHARDED_PACKAGES[package] and n =
169+
// PREVIOUS_FILE_SHARDED_PACKAGES[package] from partition-test-shards.mjs. The
170+
// bound is those two closed maps -- n candidates per map that slices the
171+
// package, none for one that does not. The previous map is what lets a run
172+
// made before a map change still be read (its header says why it exists and
173+
// when it decodes nothing); a decoded slice is then summed within its run, so
174+
// an incomplete set still contributes nothing.
165175
//
166176
// ⛔ A digest that matches no candidate is REFUSED, never read as a whole
167177
// package. That is the #16173 direction exactly: one slice's window recorded as
168178
// the package's whole cost, a number that reads right and is n times too small.
169-
// It happens when FILE_SHARDED_PACKAGES changed since the run that wrote the
170-
// summary, or when the variable was set by hand to a slice the partitioner does
171-
// not emit; either way the remedy is a summary from a run under the current map.
179+
// It happens when the map changed twice within a summary's lifetime, or when
180+
// the variable was set by hand to a slice the partitioner does not emit; either
181+
// way the remedy is a summary from a run under the current map.
172182
//
173183
// An empty value is not a slice: the package's vitest config hands vitest an
174184
// empty `shard`, which it ignores, and the package runs whole. A task with no
175185
// `environmentVariables` record at all carries no evidence either way and is
176186
// read as carrying no env slice -- turbo 2.10.10 writes one on every task, and
177187
// the self-test fixtures predate it.
178-
export function sliceOfEnvironment(environmentVariables, name, label, sliced = FILE_SHARDED_PACKAGES) {
188+
export function sliceOfEnvironment(
189+
environmentVariables,
190+
name,
191+
label,
192+
sliced = FILE_SHARDED_PACKAGES,
193+
previous = PREVIOUS_FILE_SHARDED_PACKAGES
194+
) {
179195
const configured = environmentVariables?.configured;
180196
if (configured === undefined || configured === null) return null;
181197
if (!Array.isArray(configured)) {
@@ -186,28 +202,35 @@ export function sliceOfEnvironment(environmentVariables, name, label, sliced = F
186202
if (entry === undefined) return null;
187203
const digest = entry.slice(prefix.length);
188204
if (digest === '') return null;
189-
const count = Object.hasOwn(sliced, name) ? sliced[name] : 1;
205+
const counts = [];
206+
for (const map of [sliced, previous]) {
207+
const n = Object.hasOwn(map, name) ? map[name] : 1;
208+
if (n > 1 && !counts.includes(n)) counts.push(n);
209+
}
190210
const candidates = [];
191-
for (let index = 1; count > 1 && index <= count; index++) {
192-
const spec = `${index}/${count}`;
193-
if (createHash('sha256').update(spec).digest('hex') === digest) return { index, count };
194-
candidates.push(spec);
211+
for (const count of counts) {
212+
for (let index = 1; index <= count; index++) {
213+
const spec = `${index}/${count}`;
214+
if (createHash('sha256').update(spec).digest('hex') === digest) return { index, count };
215+
candidates.push(spec);
216+
}
195217
}
196218
throw new Error(
197219
`${label}: ${name} ran with ${SLICE_ENV} set (digest ${digest.slice(0, 16)}…), and that digest ` +
198-
`matches no slice the partitioner can emit for it (${candidates.length ? candidates.join(', ') : 'none: FILE_SHARDED_PACKAGES does not slice it'}). ` +
220+
'matches no slice the partitioner can emit for it, or emitted under the map it replaced ' +
221+
`(${candidates.length ? candidates.join(', ') : 'none: neither FILE_SHARDED_PACKAGES nor PREVIOUS_FILE_SHARDED_PACKAGES slices it'}). ` +
199222
'Refusing to read the window as a whole-package sample: it is one slice of the suite, and ' +
200-
'recording it as the whole cost is the #16173 defect. If FILE_SHARDED_PACKAGES changed since ' +
201-
'this run, refresh from a run made under the current map.'
223+
'recording it as the whole cost is the #16173 defect. If FILE_SHARDED_PACKAGES changed more ' +
224+
'than once since this run, refresh from a run made under the current map.'
202225
);
203226
}
204227

205228
// The slice one measured leg ran as, from either carrier. Both present and
206229
// disagreeing is not a reading to pick from: vitest would have run the CLI's,
207230
// but no workflow here sets both, so it is refused rather than resolved.
208-
function sliceOfLeg(leg, name, label, sliced) {
231+
function sliceOfLeg(leg, name, label, sliced, previous) {
209232
const fromArgs = sliceOfCliArguments(leg.cliArguments);
210-
const fromEnv = sliceOfEnvironment(leg.environmentVariables, name, label, sliced);
233+
const fromEnv = sliceOfEnvironment(leg.environmentVariables, name, label, sliced, previous);
211234
if (fromArgs && fromEnv && (fromArgs.index !== fromEnv.index || fromArgs.count !== fromEnv.count)) {
212235
throw new Error(
213236
`${label}: ${name} carries two different slices -- --shard=${fromArgs.index}/${fromArgs.count} ` +
@@ -249,7 +272,12 @@ const SAMPLED_TASKS = ['test', 'test:repo'];
249272
// Recording one leg's seconds alone (because the other was cached or failed)
250273
// would write a partial suite's cost as the package's whole cost, a reading
251274
// worse than today's undercount by #16466's own defect this card fixes.
252-
export function samplesFromSummary(parsed, label, sliced = FILE_SHARDED_PACKAGES) {
275+
export function samplesFromSummary(
276+
parsed,
277+
label,
278+
sliced = FILE_SHARDED_PACKAGES,
279+
previous = PREVIOUS_FILE_SHARDED_PACKAGES
280+
) {
253281
const tasks = parsed?.tasks;
254282
if (!Array.isArray(tasks)) {
255283
throw new Error(
@@ -312,7 +340,7 @@ export function samplesFromSummary(parsed, label, sliced = FILE_SHARDED_PACKAGES
312340
seconds += leg.seconds;
313341
// A passthrough slice (`cliArguments`, the nightly tiers) or an
314342
// `OS_TEST_SHARD` one (Test Core, #19278) -- see sliceOfEnvironment().
315-
slice ??= sliceOfLeg(leg, name, label, sliced);
343+
slice ??= sliceOfLeg(leg, name, label, sliced, previous);
316344
}
317345
samples.set(name, seconds);
318346
if (slice) slices.set(name, slice);
@@ -558,7 +586,7 @@ export function buildDataset({ perSummary, fileCounts, provenance, carryFrom = n
558586
// remedy is to find what stopped registering, never to lower the number.
559587
const SELF_TEST_BATTERIES = Object.freeze({
560588
'measure-test-shard-timings self-test': 56,
561-
'env-carried slices (#19278)': 10,
589+
'env-carried slices (#19278)': 11,
562590
});
563591

564592
// DELETING an entry silences that battery's floor exactly as effectively as
@@ -1155,7 +1183,7 @@ function selfTest() {
11551183
// carrier, an empty value, an unrelated variable -- and pin what the digest
11561184
// path must NOT read. Each case was ablated against the code it pins.
11571185
// `sliced` stands in for FILE_SHARDED_PACKAGES so the fixtures keep the short
1158-
// names above, and one case reads the REAL map.
1186+
// names above; one case reads the REAL maps, and one crosses a map change.
11591187
battery('env-carried slices (#19278)');
11601188
const digestOf = (value) => createHash('sha256').update(value).digest('hex');
11611189
const envTask = (pkg, start, end, value, extra = {}) => ({
@@ -1180,34 +1208,73 @@ function selfTest() {
11801208
if (r.samples.get('cli') !== 118.073) throw new Error('env slice: the slice window was not kept as the sample');
11811209
});
11821210
check(() => {
1183-
// The REAL map, a real package name, a digest of the form turbo writes:
1184-
// the default `sliced` IS the partitioner's live map. Written for either
1185-
// state the partitioner's slice-count pin can leave that map in -- a live
1186-
// entry must resolve its own last slice, and an EMPTY map must refuse a
1187-
// slice digest on the CLI naming the empty map -- so it reds when the
1188-
// default stops being the live map, whichever state that is.
1189-
const live = Object.entries(FILE_SHARDED_PACKAGES);
1190-
if (live.length > 0) {
1191-
const [name, n] = live[0];
1192-
const r = samplesFromSummary(summary([envTask(name, 0, 1000, `${n}/${n}`)]), 'f');
1193-
const s = r.slices.get(name);
1211+
// The REAL maps, a real package name, digests of the form turbo writes: the
1212+
// defaults ARE the partitioner's live FILE_SHARDED_PACKAGES and
1213+
// PREVIOUS_FILE_SHARDED_PACKAGES. Written for whatever state those maps are
1214+
// in -- every count either names for the CLI must decode its own last
1215+
// slice, and a count neither names must be refused listing exactly their
1216+
// candidates -- so it reds when a default stops being its live map.
1217+
const name = '@objectstack/cli';
1218+
const counts = [];
1219+
for (const map of [FILE_SHARDED_PACKAGES, PREVIOUS_FILE_SHARDED_PACKAGES]) {
1220+
const n = Object.hasOwn(map, name) ? map[name] : 1;
1221+
if (n > 1 && !counts.includes(n)) counts.push(n);
1222+
}
1223+
for (const n of counts) {
1224+
const s = samplesFromSummary(summary([envTask(name, 0, 1000, `${n}/${n}`)]), 'f').slices.get(name);
11941225
if (!s || s.index !== n || s.count !== n) {
1195-
throw new Error(`env slice: the live FILE_SHARDED_PACKAGES did not resolve ${name} ${n}/${n} (got ${JSON.stringify(s ?? null)})`);
1196-
}
1197-
} else {
1198-
let message = '';
1199-
try {
1200-
samplesFromSummary(summary([envTask('@objectstack/cli', 0, 1000, '1/2')]), 'f');
1201-
} catch (e) {
1202-
message = e.message;
1203-
}
1204-
if (!message.includes('FILE_SHARDED_PACKAGES does not slice it')) {
1205-
throw new Error(
1206-
'env slice: with the live FILE_SHARDED_PACKAGES empty, a CLI slice digest was not refused ' +
1207-
`naming the empty map (got ${JSON.stringify(message || 'no throw')})`
1208-
);
1226+
throw new Error(`env slice: the live maps did not resolve ${name} ${n}/${n} (got ${JSON.stringify(s ?? null)})`);
12091227
}
12101228
}
1229+
const stranger = Math.max(1, ...counts) + 1;
1230+
let message = '';
1231+
try {
1232+
samplesFromSummary(summary([envTask(name, 0, 1000, `1/${stranger}`)]), 'f');
1233+
} catch (e) {
1234+
message = e.message;
1235+
}
1236+
const listed =
1237+
counts.flatMap((n) => Array.from({ length: n }, (_, i) => `${i + 1}/${n}`)).join(', ') ||
1238+
'none: neither FILE_SHARDED_PACKAGES nor PREVIOUS_FILE_SHARDED_PACKAGES slices it';
1239+
if (!message.includes(`(${listed})`)) {
1240+
throw new Error(
1241+
`env slice: a ${name} digest of 1/${stranger}, a count neither live map names, was not refused ` +
1242+
`listing (${listed}) (got ${JSON.stringify(message || 'no throw')})`
1243+
);
1244+
}
1245+
});
1246+
check(() => {
1247+
// ACROSS A MAP CHANGE: a run made before it, under a map that cut `cli` in
1248+
// two, read by a generator whose current map slices nothing. Both halves
1249+
// decode against the previous map and are summed WITHIN the run to the
1250+
// whole cost -- the reading the refresh lane needs while every retained
1251+
// run predates the change -- and a count neither map names is still
1252+
// refused. Without the previous map both halves are refused and the run
1253+
// measures nothing.
1254+
const none = Object.freeze({});
1255+
const was2 = Object.freeze({ cli: 2 });
1256+
const legA = samplesFromSummary(summary([envTask('cli', 0, 300_000, '1/2')]), 'a', none, was2);
1257+
const legB = samplesFromSummary(summary([envTask('cli', 0, 433_330, '2/2')]), 'b', none, was2);
1258+
const set = buildDataset({
1259+
perSummary: [{ ...legA, run: 'r' }, { ...legB, run: 'r' }],
1260+
fileCounts: new Map([['cli', 300]]),
1261+
provenance: {},
1262+
});
1263+
if (set.packages.cli !== 733.33 || set.skippedIncompleteSlices.length !== 0) {
1264+
throw new Error(
1265+
`env slice: a pre-change run's two halves did not assemble to 733.33s (got ${set.packages.cli}; ` +
1266+
`incomplete ${JSON.stringify(set.skippedIncompleteSlices)})`
1267+
);
1268+
}
1269+
let message = '';
1270+
try {
1271+
samplesFromSummary(summary([envTask('cli', 0, 1000, '1/3')]), 'c', none, was2);
1272+
} catch (e) {
1273+
message = e.message;
1274+
}
1275+
if (!message.includes('(1/2, 2/2)')) {
1276+
throw new Error(`env slice: a digest neither map names was not refused (got ${JSON.stringify(message || 'no throw')})`);
1277+
}
12111278
});
12121279
check(() => {
12131280
// The passthrough carrier is untouched: the nightly tiers still use it.

‎scripts/partition-test-shards.mjs‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,12 +224,38 @@ export const MAX_MEASURED_OVER_PREDICTED = 1.5;
224224
// holding each package's WHOLE cost, and the division by n happens here. That
225225
// is what keeps a refresh comparable across a change to this map, and it is why
226226
// measure-test-shard-timings.mjs has to reassemble a package's slices before it
227-
// records one -- see `sliceOfCliArguments` there. A change to this map also
228-
// retires the summaries written under the old one: the generator refuses an
229-
// `OS_TEST_SHARD` slice the map can no longer emit rather than read it as a
230-
// whole package, so a refresh after a change measures runs made after it.
227+
// records one -- see `sliceOfCliArguments` there. A change to this map is also
228+
// a change to what that generator can DECODE, which is why the map it replaced
229+
// is kept below.
231230
export const FILE_SHARDED_PACKAGES = Object.freeze({});
232231

232+
// THE MAP AS IT STOOD BEFORE ITS LAST CHANGE, read only by the generator's
233+
// slice-digest matcher (measure-test-shard-timings.mjs `sliceOfEnvironment`).
234+
//
235+
// A run summary records a slice only as the sha256 of its `OS_TEST_SHARD`
236+
// value, and the matcher decodes that digest against the slices the
237+
// partitioner can emit, refusing the whole summary when nothing matches. So a
238+
// run made BEFORE a change to the map above -- slices the map no longer emits
239+
// -- is refused, and while every retained run predates the change, the
240+
// refresh lane can measure nothing at all. That is not hypothetical: on the
241+
// change that emptied the map, the lane's pull_request rehearsal (run
242+
// 37074888579) refused all ten eligible hourly runs on main and regenerated
243+
// nothing.
244+
//
245+
// With the outgoing map here, a pre-change slice decodes EXACTLY -- by hash
246+
// equality, against this closed map, never by guessing a count -- and is then
247+
// summed within its run like any slice, so a run that cannot assemble the
248+
// package still contributes nothing (`skippedIncompleteSlices`). A digest
249+
// matching neither map is refused as before.
250+
//
251+
// Set it to the OUTGOING map in the same PR that changes FILE_SHARDED_PACKAGES.
252+
// Its only readers are run summaries, which ci.yml keeps for one day
253+
// (`retention-days: 1` on `test-core-run-summary-*`), so a day after that PR
254+
// lands no retained summary predates the change and this map decodes nothing.
255+
export const PREVIOUS_FILE_SHARDED_PACKAGES = Object.freeze({
256+
'@objectstack/cli': 2,
257+
});
258+
233259
// The item grammar. A shard item is a package (`@objectstack/cli`) or a SLICE
234260
// of one (`@objectstack/cli 1/2`), and this pair of functions is the only place
235261
// that spelling is written or read -- ci.yml builds the turbo invocation from

0 commit comments

Comments
 (0)