From d577b1b3bc0a69bf1b4ff41b5a05b67a4574bc55 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 11:36:16 +0000 Subject: [PATCH 1/2] fix(devx): give the shard package list one path convention, and a stated resolution base (#10056) `turbo ls --output=json` emits every `packages.items[].path` repo-relative (measured on turbo 2.10.10: 0 of 77 items absolute). `--union-into` appended absolute ones, so a single document could carry both conventions, and the obvious way to read it -- `join(REPO_ROOT, it.path)`, correct for every entry turbo wrote -- produces a garbage path for exactly the appended entries, which are the cross-package scans that step exists to keep running. Two halves, because the document and its reader each owned a piece: - the union now appends repo-relative paths, so the document turbo wrote and the document we hand on carry one rule; - `partition-test-shards.mjs` resolves `it.path` against the repo root explicitly instead of inheriting `process.cwd()`. `path.resolve` leaves an already-absolute entry alone, so a document written by the old union step still resolves to the directory it always did. The cwd half is the one that fails silently: `countTestFiles()` returns 0 for a path it cannot read, and the LPT partitioner absorbs a zero weight without complaint -- not a red step, a shard matrix that quietly stops balancing. Measured before this was pinned, same document and tree, cwd `/`: `shard 1/1: 1/1 packages, weight 0`. Placement is unchanged: against the full `turbo ls` payload this tree produces, all three shards keep byte-identical package lists and the bins stay 783/783/782. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0185KTgmREZA4sY5pnRbubXj --- scripts/check-cross-package-test-inputs.mjs | 40 +++++++++++++-- scripts/partition-test-shards.mjs | 54 ++++++++++++++++++++- 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/scripts/check-cross-package-test-inputs.mjs b/scripts/check-cross-package-test-inputs.mjs index bdd2f265c5..dfdae22bfc 100644 --- a/scripts/check-cross-package-test-inputs.mjs +++ b/scripts/check-cross-package-test-inputs.mjs @@ -109,8 +109,9 @@ // node scripts/check-cross-package-test-inputs.mjs --list-escapes // node scripts/check-cross-package-test-inputs.mjs --self-test -import { readFileSync, readdirSync, statSync, existsSync, writeFileSync } from 'node:fs'; -import { join, resolve, relative, dirname, sep } from 'node:path'; +import { readFileSync, readdirSync, statSync, existsSync, writeFileSync, mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve, relative, dirname, sep, isAbsolute } from 'node:path'; import { fileURLToPath } from 'node:url'; import process from 'node:process'; @@ -1070,7 +1071,16 @@ function unionInto(listPath, changedPath) { if (!hit) continue; const dir = escapingDirs.get(name); if (!dir) continue; - items.push({ name, path: join(REPO_ROOT, dir) }); + // Repo-relative, because that is the convention `turbo ls` emits for every + // entry it wrote (measured on turbo 2.10.10: 0 of 77 items absolute). An + // absolute path here is not wrong for today's only consumer, but it makes a + // single document carry two conventions, and the obvious way to read such a + // document -- `join(REPO_ROOT, it.path)`, correct for every entry turbo + // wrote -- produces a garbage path for exactly these appended entries, which + // are the cross-package scans this function exists to keep running. One + // document, one convention; the consumer resolves it explicitly + // (partition-test-shards.mjs `packageDir()`). + items.push({ name, path: dir }); added.push(`${name} (declared glob matched ${hit})`); } // The push above changed the list's size, so the size the document DECLARES @@ -1416,6 +1426,30 @@ function selfTest() { ok('the write never invents items', written({ count: 0, items: [] }).items.length === 0); ok('the write leaves turbo\'s other fields alone', JSON.parse(serializePackageList({ packageManager: 'pnpm9', packages: { count: 0, items: [] } })).packageManager === 'pnpm9'); + // The path convention this function appends in. `turbo ls` writes every entry + // of this document repo-relative; an entry appended in the other convention + // is not wrong for today's consumer but it makes one array carry two rules, + // and the obvious way to read it -- `join(REPO_ROOT, it.path)` -- then breaks + // on exactly the appended entries. End-to-end through the real `unionInto()` + // and the real serializer, on the fixture that first measured the divergence: + // a diff touching `scripts/**` pulls @objectstack/spec in by its declaration. + const unionDir = mkdtempSync(join(tmpdir(), 'os-union-into-')); + const unionList = join(unionDir, 'turbo-ls.json'); + const unionChanged = join(unionDir, 'changed-files.txt'); + writeFileSync(unionList, JSON.stringify({ packageManager: 'pnpm9', packages: { count: 0, items: [] } })); + writeFileSync(unionChanged, 'scripts/sync-template-versions.mjs\n'); + unionInto(unionList, unionChanged); + const unioned = JSON.parse(readFileSync(unionList, 'utf8')).packages.items; + ok('the union appends the package its declaration matched', unioned.length > 0); + ok( + 'every appended path is repo-relative, the convention `turbo ls` emits', + unioned.length > 0 && unioned.every((i) => !isAbsolute(i.path)), + ); + ok( + 'and each one still names a real directory once resolved against the repo root', + unioned.length > 0 && unioned.every((i) => existsSync(resolve(REPO_ROOT, i.path))), + ); + const failed = cases.filter((c) => !c.cond); for (const c of cases) console.log(`${c.cond ? 'ok ' : 'FAIL'} ${c.label}`); if (failed.length) { diff --git a/scripts/partition-test-shards.mjs b/scripts/partition-test-shards.mjs index a34002d26a..e7177eadd6 100644 --- a/scripts/partition-test-shards.mjs +++ b/scripts/partition-test-shards.mjs @@ -37,8 +37,32 @@ import { readFileSync, readdirSync } from 'node:fs'; import path from 'node:path'; +import { fileURLToPath } from 'node:url'; import process from 'node:process'; +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); + +// Where a `packages.items[].path` actually points. +// +// The document has two writers -- `turbo ls`, which emits repo-relative paths, +// and `--union-into` in check-cross-package-test-inputs.mjs -- so the base this +// resolves against must be stated, not inherited. `process.cwd()` is the base +// you get by saying nothing, and it is the one base that can be wrong: CI runs +// this from the repo root, so a relative entry happens to land on the right +// directory, and the day something runs it from anywhere else countTestFiles() +// reads nothing, returns 0, and the partitioner absorbs the zero without +// complaint. Measured before this was pinned -- same document, same tree, cwd +// `/`: `shard 1/1: 1/1 packages, weight 0`. That failure mode is not a red +// step, it is a shard matrix that quietly stops balancing. +// +// `path.resolve` is also the reason this stays correct for both conventions: +// given an already-absolute entry it returns that entry unchanged, so an old +// document written by the previous absolute-path union step still resolves to +// the same directory it always did. +export function packageDir(itemPath) { + return path.resolve(REPO_ROOT, itemPath); +} + const TEST_FILE = /\.test\.[cm]?[jt]sx?$/; const SKIP_DIRS = new Set(['node_modules', 'dist', 'coverage', '.turbo', '.next']); @@ -173,6 +197,34 @@ function selfTest() { if (!threw(() => readPackageItems(doc({ count: 0, items: {} }), 'f'))) throw new Error('payload: a non-array items was accepted'); if (!threw(() => readPackageItems({}, 'f'))) throw new Error('payload: a document with no packages key was accepted'); + // Path resolution. `it.path` reaches this script in two conventions and the + // weight it produces must not depend on where the process happens to stand. + // The cwd leg is the one that matters: it is the exact measurement that made + // this a defect rather than a style question, and it fails SILENTLY (weight 0, + // package still assigned) rather than loudly, so nothing but an assertion can + // hold it. + if (packageDir('packages/spec') !== path.join(REPO_ROOT, 'packages', 'spec')) { + throw new Error('path: a repo-relative entry did not resolve against the repo root'); + } + const absolute = path.join(REPO_ROOT, 'packages', 'spec'); + if (packageDir(absolute) !== absolute) { + throw new Error('path: an already-absolute entry was not left alone'); + } + const hereWeight = countTestFiles(packageDir('packages/spec')); + if (hereWeight === 0) throw new Error('path: fixture package `packages/spec` has no test files to weigh'); + const cwdBefore = process.cwd(); + try { + process.chdir(path.parse(REPO_ROOT).root); + if (packageDir('packages/spec') !== absolute) { + throw new Error('path: resolution moved with the cwd'); + } + if (countTestFiles(packageDir('packages/spec')) !== hereWeight) { + throw new Error('path: weight changed with the cwd -- the silent weight-0 regression is back'); + } + } finally { + process.chdir(cwdBefore); + } + console.log('partition-test-shards: self-test OK'); } @@ -210,7 +262,7 @@ function main() { throw new Error(`${listPath}: package entry missing name/path: ${JSON.stringify(it)}`); } if (excluded.has(it.name)) continue; - weighted.push({ name: it.name, weight: countTestFiles(it.path) }); + weighted.push({ name: it.name, weight: countTestFiles(packageDir(it.path)) }); } const bins = partition(weighted, shardCount); const mine = bins[shardIndex - 1]; From d80ca888ec61897ab1dc66306258ea4e92620567 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 11:38:49 +0000 Subject: [PATCH 2/2] docs(devx): record what the test-file-count weight proxy actually costs, measured in queue builds Comment only -- no behaviour change, and deliberately no change to the weight function, the partitioner or the shard count. The header asserted the proxy as settled ("file count tracks duration far better than package count") and nothing recorded how well it holds, so the same measurement has now been re-derived twice from scratch. Measured 2026-08-20 from `merge_group` CI, per-package durations read out of the turbo task groups: - the proxy holds within roughly +-20% across the big packages, and is off by ~2.8x on one of them (@objectstack/cli, 135 files, 548.6s); - the binning is not at fault: on the full package list a queue build hands this script the bins come out 783/783/782, a one-file spread across 2348; - the hard limit: sharding is by package, so a shard cannot finish faster than its heaviest package, and @objectstack/spec (496s) and @objectstack/cli (548s) each already exceed #4859's ~7min threshold alone. In run 32352993803 Test Core shard 1 took 8m17.77s and spec's own suite was 8m16.4s of it -- the shard IS that package; - run-to-run variance is separate and equally large: the cache key is namespaced per shard and only main pushes write it, so legs of the same shard index ranged from 79/79 cached (866ms, FULL TURBO) to 0/85 cached. Part of #10149. The weight input is not changed here: the measurement says re-weighting alone cannot reach that card's acceptance threshold, so which way to go is a decision for the maintainer rather than something to pick silently. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0185KTgmREZA4sY5pnRbubXj --- scripts/partition-test-shards.mjs | 37 +++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/scripts/partition-test-shards.mjs b/scripts/partition-test-shards.mjs index e7177eadd6..b55b44c1e5 100644 --- a/scripts/partition-test-shards.mjs +++ b/scripts/partition-test-shards.mjs @@ -22,6 +22,43 @@ // all ties broken by name, so every shard computes the identical split from // the same input without coordinating. // +// HOW WELL THAT PROXY HOLDS, MEASURED (2026-08-20, queue builds, not a guess). +// Per-package durations read out of the turbo task groups in `merge_group` CI +// logs, against each package's weight in the same tree: +// +// @objectstack/cli 135 files 548.6s / 474.4s ~3.5-4.1 s/file +// @objectstack/spec 414 files 496.4s ~1.20 s/file +// @objectstack/service-automation 83 files 118.9s ~1.43 s/file +// @objectstack/driver-turso 39 files 53.5s ~1.37 s/file +// @objectstack/client 24 files 34.7s ~1.45 s/file +// @objectstack/example-showcase 21 files 21.6s ~1.03 s/file +// +// So the proxy tracks within roughly +-20% across the big packages and is off +// by ~2.8x on ONE of them (@objectstack/cli). The binning is not the problem: +// on the full package list a queue build hands this script, the three bins come +// out 783/783/782 -- a one-file spread across 2348, far inside LPT's <=4/3 +// bound. Duration spread between shards in the same builds was up to 3.4x, and +// that gap is the proxy's, not the algorithm's. +// +// ⛔ THE HARD LIMIT, AND THE REASON RE-WEIGHTING ALONE CANNOT MEET #4859. +// Sharding is BY PACKAGE, so a shard can never finish faster than its single +// heaviest package. Two packages are already over #4859's "Test Core 最慢分片 +// <= ~7min" threshold on their own: @objectstack/spec at 496s (8m16s) and +// @objectstack/cli at 548s (9m09s). Measured consequence -- in run +// 32352993803, Test Core shard 1 took 8m17.77s wall and @objectstack/spec's own +// suite accounted for 8m16.4s of it: the shard IS that one package. No weight +// function and no shard count changes that; only splitting those suites below +// package granularity, or moving the threshold, does. Anyone arriving here to +// swap the weight input should read that bound first (#10149). +// +// Run-to-run variance is a SEPARATE and equally large effect, and it is not +// placement: this job's Turbo cache key is namespaced per shard +// (`...-turbo---...`) and only main `push` runs write it, so +// each shard's cache ages independently. Measured legs of the same shard index +// ranged from 79/79 tasks cached (866ms, ">>> FULL TURBO") to 0/85 cached +// (10m08s). A single build's shard spread therefore says nothing about +// placement on its own -- compare legs at the same cache state or not at all. +// // Usage: // node scripts/partition-test-shards.mjs --shard N/M \ // [--exclude ]...