wip(devx): partition-test-shards gains --check-drift (predicted vs measured)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8

Author: claude

scripts/partition-test-shards.mjs

@@ -74,8 +74,19 @@
 // Usage:
 //   node scripts/partition-test-shards.mjs <turbo-ls.json> --shard N/M \
 //     [--exclude <pkg>]...
+//   node scripts/partition-test-shards.mjs --check-drift <run-summary.json>... \
+//     [--label <text>]
 //   node scripts/partition-test-shards.mjs --self-test
 //
+// `--check-drift` is the half that keeps the dataset honest AFTER it is
+// written (#16173). Every Test Core shard already passes `--summarize`, so the
+// run it just finished has written the measured truth to `.turbo/runs/`; this
+// mode reads that back, compares it to what this script PREDICTED for the same
+// packages, and reds past MAX_MEASURED_OVER_PREDICTED. Without it the dataset
+// rots silently in one direction and the only instrument that notices is a
+// shard killed by the job timeout -- which is a shard that produced NO reading
+// while the rollup read green.
+//
 // The weight dataset is scripts/test-shard-timings.json, regenerated by
 // scripts/measure-test-shard-timings.mjs. It is required, not optional: this
 // script refuses to shard rather than fall back to the old file-count proxy.
@@ -94,6 +105,7 @@ import { fileURLToPath } from 'node:url';
 import process from 'node:process';
 
 import { isEntrypoint } from './invoked-as.mjs';
+import { samplesFromSummary } from './measure-test-shard-timings.mjs';
 
 const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
 const TIMINGS_PATH = path.join(REPO_ROOT, 'scripts', 'test-shard-timings.json');
@@ -113,6 +125,34 @@ export const SHARD_COUNT = 6;
 // numerator up.
 export const MAX_SHARD_OVER_MEAN = 1.3;
 
+// The factor a shard's MEASURED test total may exceed its PREDICTED total by
+// before `--check-drift` reds. The durable half of #16173.
+//
+// Everything above balances a GENERATED dataset, and a generated file that
+// nothing re-measures rots in one direction only: suites get slower, the
+// numbers stay put, and the shard that drifted heavy reads as perfectly
+// balanced on paper right up until the job timeout kills it. That is not a
+// hypothesis. The reading this bound is written against, from run 34009395649
+// attempt 2 (job 101422473016 is the attempt-1 leg that was killed):
+//
+//   @objectstack/cli   predicted 458.15s   measured 1231.52s   = 2.69x
+//
+// -- one package, 68% of a 30-minute wall, on a shard the previous attempt had
+// already lost at 30m05s. Every step was green; the split's own max/mean read
+// 1.00x, because a perfectly balanced split of stale numbers is still perfectly
+// balanced. Nothing in this repo compared a prediction to an outcome, so the
+// only instrument that ever noticed was a killed job.
+//
+// 1.5 is the smallest round factor satisfying both ends:
+//
+//   - it must fire well below the 2.69x measured above, or the gate would have
+//     been green straight through the incident it exists to catch;
+//   - it must sit ABOVE MAX_SHARD_OVER_MEAN, because a dataset accurate to
+//     within the balance bound cannot be the thing that breaks balance. Gating
+//     tighter than the split's own tolerance reds on drift the partitioner is
+//     built to absorb, and a gate that reds on healthy input gets muted.
+export const MAX_MEASURED_OVER_PREDICTED = 1.5;
+
 // Where a `packages.items[].path` actually points.
 //
 // The document has two writers -- `turbo ls`, which emits repo-relative paths,
@@ -267,6 +307,64 @@ export function balanceOf(bins, items = null) {
   return { totals, sum, mean, max, min: Math.min(...totals), ratio: mean === 0 ? 1 : max / mean, floor };
 }
 
+// Compare what a shard was PREDICTED to cost against what it actually cost.
+//
+// The comparison is over the INTERSECTION of two sets, and both restrictions
+// are load-bearing:
+//
+//   - a package measured on this shard but absent from the dataset contributes
+//     to NEITHER total. Its weight came from the test-file-count estimate in
+//     weighPackage(), so calling the estimate wrong would red on a brand-new
+//     package rather than on a rotted dataset entry. It is named in
+//     `unpredicted` instead, because a shard full of estimates is its own
+//     (quieter) signal that a refresh is due.
+//   - a package in the dataset but not in this summary contributes to neither
+//     either. A shard runs a subset -- of the six bins, and on a pull_request
+//     of `turbo ls --affected` on top of that -- so charging a shard for
+//     packages it never ran would make the ratio a function of the diff.
+//
+// Cache hits and failed suites never reach here: the measurements come from
+// samplesFromSummary(), which drops both. That is deliberate reuse rather than
+// a second reader -- the generator's ~0s-for-a-replayed-suite hazard is the
+// same hazard here, pointing the other way (a cached shard would read as
+// enormously FASTER than predicted and quietly vouch for a rotted dataset).
+export function driftReport(measured, timings, factor = MAX_MEASURED_OVER_PREDICTED) {
+  const rows = [];
+  const unpredicted = [];
+  let predictedTotal = 0;
+  let measuredTotal = 0;
+  for (const [name, seconds] of measured) {
+    if (!Object.hasOwn(timings.packages, name)) {
+      unpredicted.push(name);
+      continue;
+    }
+    const predicted = timings.packages[name];
+    predictedTotal += predicted;
+    measuredTotal += seconds;
+    rows.push({ name, predicted, measured: seconds, overshoot: seconds - predicted });
+  }
+  unpredicted.sort((a, b) => a.localeCompare(b, 'en'));
+  // Sorted by ABSOLUTE overshoot, not by ratio: the reader of a red verdict
+  // wants the package that cost the shard its minutes, and a 0.1s package that
+  // came in at 5x its 0.02s entry is noise wearing the biggest ratio.
+  rows.sort((a, b) => b.overshoot - a.overshoot || a.name.localeCompare(b.name, 'en'));
+  // `predictedTotal > 0` is the guard against a verdict of Infinity, which is
+  // what a shard carrying only zero-weight entries would otherwise produce --
+  // a red naming no cause. Zero measured packages is the same state and reads
+  // the same way: NOT MEASURED is not a pass, and it is not a failure either.
+  const measurable = rows.length > 0 && predictedTotal > 0;
+  const ratio = measurable ? measuredTotal / predictedTotal : null;
+  return {
+    rows,
+    unpredicted,
+    predictedTotal,
+    measuredTotal,
+    ratio,
+    measurable,
+    drifted: measurable && ratio > factor,
+  };
+}
+
 // Reads the package list out of a `turbo ls --output=json` payload, asserting
 // two independent properties. They fail for different reasons and both are
 // loud, because the failure this whole file guards against is the quiet one --
@@ -347,11 +445,12 @@ const SELF_TEST_BATTERIES = Object.freeze({
   // remedy is the one the pin itself names -- pick a new inversion pair from
   // the dataset -- never lowering this number.
   'the balancing pins (#10472)': 16,
+  'predicted-vs-measured drift (#16173)': 9,
 });
 
 // DELETING an entry silences that battery's floor exactly as effectively as
 // zeroing it, so the roster's own size is pinned too.
-const SELF_TEST_BATTERY_FLOOR = 7;
+const SELF_TEST_BATTERY_FLOOR = 8;
 
 // The key an assertion is filed under when no battery is open. It is not a
 // declared battery, so it reds by the same set difference rather than silently
@@ -693,6 +792,97 @@ function selfTest() {
     });
   }
 
+  // -- PREDICTED VS MEASURED (#16173) -------------------------------------
+  //
+  // The balancing pins above all read the dataset as GIVEN. None of them can
+  // fail because a number in it is wrong: a perfectly balanced split of stale
+  // weights satisfies every one of them, and did, at 1.00x max/mean, on the
+  // very build whose shard 1 was killed by the 30-minute wall. These pin the
+  // one comparison that can tell a good dataset from a rotted one.
+  battery('predicted-vs-measured drift (#16173)');
+  const driftTimings = { packages: { slow: 100, fine: 50, zero: 0 }, rate: 2 };
+  const measuredMap = (o) => new Map(Object.entries(o));
+
+  // The card's own reading, to scale: 100s predicted, 269s measured = 2.69x.
+  const drifted = driftReport(measuredMap({ slow: 269 }), driftTimings);
+  check(() => {
+    if (!drifted.drifted) {
+      throw new Error(`drift: a ${drifted.ratio?.toFixed(2)}x gap was not reported as drift`);
+    }
+  });
+  check(() => {
+    if (Math.abs(drifted.ratio - 2.69) > 1e-9) throw new Error(`drift: ratio was ${drifted.ratio}`);
+  });
+
+  // A suite that ran a little long is NOT drift -- the bound sits above
+  // MAX_SHARD_OVER_MEAN precisely so ordinary runner variance stays green.
+  check(() => {
+    if (driftReport(measuredMap({ slow: 140 }), driftTimings).drifted) {
+      throw new Error('drift: a 1.4x reading red under a 1.5x bound');
+    }
+  });
+  // The boundary itself: `>` not `>=`, so exactly at the bound is still green.
+  check(() => {
+    if (driftReport(measuredMap({ slow: 150 }), driftTimings).drifted) {
+      throw new Error('drift: a reading exactly AT the bound was called a breach');
+    }
+  });
+
+  // A package the dataset has never seen is weighed by ESTIMATE in
+  // weighPackage(), so charging the estimate to the dataset would red on a new
+  // package instead of on a rotted entry. Excluded from both totals, named.
+  const withNew = driftReport(measuredMap({ slow: 100, 'brand-new': 900 }), driftTimings);
+  check(() => {
+    if (withNew.drifted) throw new Error('drift: an UNMEASURED package was charged to the dataset');
+  });
+  check(() => {
+    if (withNew.unpredicted.join() !== 'brand-new') {
+      throw new Error(`drift: unpredicted was ${withNew.unpredicted.join()}`);
+    }
+  });
+
+  // The mirror restriction: a dataset entry this shard never ran must not
+  // inflate the predicted side. `fine` and `zero` are in driftTimings and not
+  // in the summary; if they counted, 100/150 would read as a fast shard and
+  // vouch for the dataset.
+  check(() => {
+    if (driftReport(measuredMap({ slow: 269 }), driftTimings).predictedTotal !== 100) {
+      throw new Error('drift: a package this shard never ran inflated the predicted total');
+    }
+  });
+
+  // Infinity is a red naming no cause. A shard carrying only zero-weight
+  // entries -- and a shard carrying nothing at all -- is NOT MEASURED, which is
+  // neither a pass nor a failure.
+  check(() => {
+    const z = driftReport(measuredMap({ zero: 30 }), driftTimings);
+    if (z.measurable || z.drifted || z.ratio !== null) {
+      throw new Error(`drift: a zero predicted total produced ratio ${z.ratio}`);
+    }
+  });
+
+  // THE ONE THAT MATTERS FOR THE READING, and the reason this reuses the
+  // generator's extractor instead of parsing summaries a second time: a cache
+  // HIT replays a stored log in milliseconds. Read as a measurement it says the
+  // suite got ~1000x FASTER than predicted -- a shard that would vouch, loudly
+  // and in the wrong direction, for whatever the dataset happens to say. Both
+  // legs go through samplesFromSummary(), which drops replays and failures.
+  const replayed = samplesFromSummary(
+    { tasks: [
+      { taskId: 'slow#test', task: 'test', package: 'slow', cache: { status: 'HIT' },
+        execution: { startTime: 0, endTime: 40, exitCode: 0 } },
+      { taskId: 'fine#test', task: 'test', package: 'fine', cache: { status: 'MISS' },
+        execution: { startTime: 0, endTime: 200_000, exitCode: 1 } },
+    ] },
+    'drift pin'
+  );
+  check(() => {
+    const r = driftReport(replayed.samples, driftTimings);
+    if (r.measurable) {
+      throw new Error('drift: a summary of one replay and one failure was read as a measurement');
+    }
+  });
+
   // -- The floor: every declared battery RAN, and ran its cases (#13489) ----
   //
   // Evaluated after every battery has had its chance and BEFORE the verdict, so
@@ -749,6 +939,94 @@ function selfTest() {
   return SELF_TEST_VERDICT;
 }
 
+// `--check-drift`: the shard just measured itself, so read that back.
+//
+// Every verdict this prints is one of exactly three, and NOT MEASURED is a
+// first-class one rather than a quiet pass. A shard whose test tasks were all
+// cache replays has said nothing about the dataset, and reporting that as OK is
+// the #4690 shape -- a check that read nothing reporting as a check that found
+// nothing wrong.
+function checkDrift(argv) {
+  const inputs = [];
+  let label = 'this shard';
+  for (let i = 0; i < argv.length; i++) {
+    const arg = argv[i];
+    if (arg === '--check-drift') continue;
+    else if (arg === '--label') label = argv[++i];
+    else if (arg.startsWith('--')) throw new Error(`unrecognized argument: ${arg}`);
+    else inputs.push(arg);
+  }
+  if (inputs.length === 0) {
+    console.error(
+      'usage: partition-test-shards.mjs --check-drift <run-summary.json>... [--label <text>]'
+    );
+    process.exit(1);
+  }
+  // A package normally appears in exactly one summary -- it runs on exactly one
+  // shard -- so this merge is for the case where it does not (a directory
+  // holding several runs, or every shard's summary handed over at once). The
+  // LONGEST window wins, deliberately unlike the generator's median: the
+  // generator is choosing a weight to balance FUTURE splits with, where one
+  // unlucky leg must not ratchet the dataset upward forever, while this is
+  // asking whether a shard fits inside a wall, and the leg that answers that is
+  // the slow one.
+  const merged = new Map();
+  for (const input of inputs) {
+    const { samples } = samplesFromSummary(JSON.parse(readFileSync(input, 'utf8')), input);
+    for (const [name, seconds] of samples) {
+      merged.set(name, Math.max(merged.get(name) ?? 0, seconds));
+    }
+  }
+  const timings = loadTimings();
+  const report = driftReport(merged, timings);
+  const skipped =
+    report.unpredicted.length === 0
+      ? ''
+      : ` ${report.unpredicted.length} package(s) carry no dataset entry and were excluded ` +
+        `(estimated, not predicted): ${report.unpredicted.join(', ')}.`;
+
+  if (!report.measurable) {
+    console.error(
+      `shard-timing-drift: NOT MEASURED -- ${label} finished no test task that was both a cache ` +
+        'MISS and carried a dataset entry, so this run says nothing about whether ' +
+        `scripts/test-shard-timings.json is still true.${skipped}`
+    );
+    return;
+  }
+
+  const head =
+    `${report.measuredTotal.toFixed(1)}s measured vs ${report.predictedTotal.toFixed(1)}s predicted ` +
+    `across ${report.rows.length} package(s) = ${report.ratio.toFixed(2)}x ` +
+    `(bound ${MAX_MEASURED_OVER_PREDICTED}x)`;
+  const worst = report.rows
+    .slice(0, 5)
+    .map(
+      (r) =>
+        `    ${r.name}: predicted ${r.predicted.toFixed(1)}s, measured ${r.measured.toFixed(1)}s ` +
+        `(${r.predicted > 0 ? `${(r.measured / r.predicted).toFixed(2)}x, ` : ''}` +
+        `${r.overshoot >= 0 ? '+' : ''}${r.overshoot.toFixed(1)}s)`
+    )
+    .join('\n');
+
+  if (!report.drifted) {
+    console.error(`shard-timing-drift: OK -- ${label}, ${head}.${skipped}`);
+    return;
+  }
+  console.error(
+    `shard-timing-drift: DRIFT -- ${label}, ${head}.${skipped}\n` +
+      '  Heaviest overshoots:\n' +
+      `${worst}\n` +
+      '  scripts/test-shard-timings.json no longer describes this workspace, so the shard split\n' +
+      '  is balancing a quantity that is not the runtime. Refresh it -- see\n' +
+      '  scripts/measure-test-shard-timings.mjs for the two refresh paths -- and ⛔ do NOT\n' +
+      '  hand-edit the dataset or raise this bound to absorb the gap. Expect the refresh to red\n' +
+      "  this script's own balance pins if a single suite has outgrown the acceptance bound:\n" +
+      '  that is those pins working, and the remedy they name is splitting that suite below\n' +
+      '  package granularity, never a different shard count.'
+  );
+  process.exit(1);
+}
+
 function main() {
   const argv = process.argv.slice(2);
   if (argv.includes('--self-test')) {
@@ -762,6 +1040,10 @@ function main() {
     }
     return;
   }
+  if (argv.includes('--check-drift')) {
+    checkDrift(argv);
+    return;
+  }
 
   let listPath = null;
   let shardSpec = null;