Skip to content

Commit a039f1e

Browse files
committed
fix(devx): --check-drift predicts from the slice the SUMMARY records, not the config
A package's slice count for the drift comparison now comes from the run's own `cliArguments`, so a package the summaries show running WHOLE is charged the whole dataset entry. FILE_SHARDED_PACKAGES stays the default for callers with no run in hand. The two agree on a Test Core shard; only the observed one is right anywhere else -- a developer running the CLI suite locally runs it whole, and charging that a half-sized prediction reported a ~2x drift that was purely this function's arithmetic. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
1 parent 4e2869e commit a039f1e

1 file changed

Lines changed: 51 additions & 8 deletions

File tree

scripts/partition-test-shards.mjs

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -321,15 +321,22 @@ export function assertSlicesSpread(bins) {
321321
return bins;
322322
}
323323

324-
// What THIS shard was predicted to spend on a package it just ran. A shard runs
324+
// What a run was predicted to spend on a package it just ran. A shard runs
325325
// exactly one slice of a file-sharded package -- the slices are placed in
326326
// distinct bins, asserted in main() -- so the prediction to compare a measured
327327
// window against is the dataset's whole-package entry divided by the slice
328328
// count. Charging a slice the whole package's entry would read as a ~n x
329329
// under-run and, worse, dilute a real overshoot elsewhere on the same shard
330330
// into a ratio that stays under the bound.
331-
export function predictedSecondsFor(name, timings) {
332-
return timings.packages[name] / sliceCountFor(name);
331+
//
332+
// `sliceCount` is passed in by `--check-drift` from what the SUMMARY says the
333+
// run actually was, not from FILE_SHARDED_PACKAGES. The two agree on a Test Core
334+
// shard, and only the observed one is right anywhere else: a developer running
335+
// the suite locally runs the CLI whole, and charging that whole run a half-sized
336+
// prediction would report a 2x drift that is purely this function's arithmetic.
337+
// The config remains the default for callers with no run in hand.
338+
export function predictedSecondsFor(name, timings, sliceCount = sliceCountFor(name)) {
339+
return timings.packages[name] / sliceCount;
333340
}
334341

335342
// Where a `packages.items[].path` actually points.
@@ -514,7 +521,7 @@ export function balanceOf(bins, items = null) {
514521
// a second reader -- the generator's ~0s-for-a-replayed-suite hazard is the
515522
// same hazard here, pointing the other way (a cached shard would read as
516523
// enormously FASTER than predicted and quietly vouch for a rotted dataset).
517-
export function driftReport(measured, timings, factor = MAX_MEASURED_OVER_PREDICTED) {
524+
export function driftReport(measured, timings, factor = MAX_MEASURED_OVER_PREDICTED, observedSlices = null) {
518525
const rows = [];
519526
const unpredicted = [];
520527
let predictedTotal = 0;
@@ -526,7 +533,12 @@ export function driftReport(measured, timings, factor = MAX_MEASURED_OVER_PREDIC
526533
}
527534
// Through predictedSecondsFor, never the raw dataset entry: a shard that
528535
// ran one SLICE of a file-sharded package was predicted one slice's cost.
529-
const predicted = predictedSecondsFor(name, timings);
536+
// When the caller observed the run's own slice spec, that wins over the
537+
// configured one -- `observedSlices` present but silent about a package
538+
// means the summaries show it running WHOLE, which is a fact about the run.
539+
const predicted = observedSlices
540+
? predictedSecondsFor(name, timings, observedSlices.get(name)?.count ?? 1)
541+
: predictedSecondsFor(name, timings);
530542
predictedTotal += predicted;
531543
measuredTotal += seconds;
532544
rows.push({ name, predicted, measured: seconds, overshoot: seconds - predicted });
@@ -638,7 +650,7 @@ const SELF_TEST_BATTERIES = Object.freeze({
638650
// the measured count for the same reason.
639651
'the balancing pins (#10472)': 21,
640652
'predicted-vs-measured drift (#16173)': 9,
641-
'file-level slice items (#16173)': 18,
653+
'file-level slice items (#16173)': 20,
642654
});
643655

644656
// DELETING an entry silences that battery's floor exactly as effectively as
@@ -1300,6 +1312,30 @@ function selfTest() {
13001312
}
13011313
});
13021314

1315+
check(() => {
1316+
// The run wins over the config. An OBSERVED whole run of a configured-sliced
1317+
// package is predicted the WHOLE entry, so the same 1000s reading is a
1318+
// comfortable under-run rather than the 1.67x above. Without this, anyone
1319+
// running the suite locally (where the CLI runs whole) would get a drift red
1320+
// that is purely predictedSecondsFor's arithmetic.
1321+
const r = driftReport(measuredMap({ '@objectstack/cli': 1000 }), sliceTimings, undefined, new Map());
1322+
if (r.drifted) {
1323+
throw new Error(`drift: an observed WHOLE run was charged a slice-sized prediction (${r.ratio.toFixed(2)}x)`);
1324+
}
1325+
if (Math.abs(r.predictedTotal - 1200) > 1e-9) {
1326+
throw new Error(`drift: an observed whole run was predicted ${r.predictedTotal}s, not the whole 1200s`);
1327+
}
1328+
});
1329+
check(() => {
1330+
// ...and an observed slice count that differs from the configured one is
1331+
// honoured, because the summary is the record of what actually ran.
1332+
const observed = new Map([['@objectstack/cli', { index: 1, count: 3 }]]);
1333+
const r = driftReport(measuredMap({ '@objectstack/cli': 400 }), sliceTimings, undefined, observed);
1334+
if (Math.abs(r.predictedTotal - 400) > 1e-9) {
1335+
throw new Error(`drift: an observed 1/3 slice was predicted ${r.predictedTotal}s, not 400s`);
1336+
}
1337+
});
1338+
13031339
check(() => {
13041340
// The REAL weighing path, on the REAL package: main() must hand partition()
13051341
// slices, not one CLI-shaped lump. Pin 6 above proves weighItems reads
@@ -1408,14 +1444,21 @@ function checkDrift(argv) {
14081444
// asking whether a shard fits inside a wall, and the leg that answers that is
14091445
// the slow one.
14101446
const merged = new Map();
1447+
// What the summaries say each package was RUN as. A shard that carries a
1448+
// file-level slice writes two summaries -- one per turbo invocation -- and
1449+
// only the slice leg's tasks carry `--shard=k/n`, so this is per package and
1450+
// comes from the run rather than from FILE_SHARDED_PACKAGES. A package absent
1451+
// here ran whole; that is a reading, not a default.
1452+
const observedSlices = new Map();
14111453
for (const input of inputs) {
1412-
const { samples } = samplesFromSummary(JSON.parse(readFileSync(input, 'utf8')), input);
1454+
const { samples, slices } = samplesFromSummary(JSON.parse(readFileSync(input, 'utf8')), input);
14131455
for (const [name, seconds] of samples) {
14141456
merged.set(name, Math.max(merged.get(name) ?? 0, seconds));
14151457
}
1458+
for (const [name, slice] of slices ?? []) observedSlices.set(name, slice);
14161459
}
14171460
const timings = loadTimings();
1418-
const report = driftReport(merged, timings);
1461+
const report = driftReport(merged, timings, MAX_MEASURED_OVER_PREDICTED, observedSlices);
14191462
const skipped =
14201463
report.unpredicted.length === 0
14211464
? ''

0 commit comments

Comments
 (0)