From 7081f7b83f29e3927cb05058e15c0b127eec8825 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 15:56:37 +0000 Subject: [PATCH 1/2] =?UTF-8?q?ci(test-core):=20build=20declarations=20on?= =?UTF-8?q?=20one=20CLI=20slice=20only=20=E2=80=94=20pin=20the=20CLI's=20d?= =?UTF-8?q?eclaration=20readers=20into=20slice=201/n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The five CLI test files that compile against built .d.ts sat in all four CLI slices by vitest's sha1 split, so five Test Core shards built the declaration closure (~4.5 min) for one or two files each. A sequencer in packages/cli/vitest.config.ts now puts them first in vitest's own order, so they land in slice 1/n; ci.yml keeps declarations for a whole reader or slice 1/* only; the partitioner charges that slice the closure (270 s x 2.9 of packing weight) and seats @objectstack/metadata beside it, so one shard builds declarations instead of five. Claude-Session: https://claude.ai/code/session_01JQwxbaFpnMwgpCgneKe2qS Co-authored-by: Claude --- .github/workflows/ci.yml | 35 ++- packages/cli/vitest.config.ts | 104 ++++++++- scripts/partition-test-shards.mjs | 363 +++++++++++++++++++++++++++++- 3 files changed, 480 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31febc892d1..6fd4b07d168 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -913,16 +913,29 @@ jobs: # hash in one namespace (turbo.json `globalEnv`). # # ⛔ EXCEPT a shard carrying a package in DTS_READERS, whose tests compile - # against BUILT declarations and go red without them (TS7016). Measured - # on 830a63a, the whole Test Core list under `OS_SKIP_DTS=1`: red only in + # against BUILT declarations and go red without them (TS7016, or ENOENT + # on a `dist/*.d.ts`). Measured on 830a63a, the whole Test Core list under + # `OS_SKIP_DTS=1`, re-measured for the CLI on 43731031 (#25870): red only in # @objectstack/cli published-subpath-console/-hook-body.pin (its # own dist/*.d.ts), scaffold-emission-typechecks, - # scaffold-test-script-passes (spec's .d.ts) + # scaffold-test-script-passes, + # create-in-repo-scaffold-typechecks (spec's .d.ts) # @objectstack/metadata typescript-serializer-annotation (spec's .d.ts) - # A new test that reads a built `.d.ts` reds the same loud way on a - # skipping shard: add its package here. Every other package's counts - # matched the DTS-on run but one: spec's root-entry-type-nameability - # pin skips (by its own design) and runs in lint.yml's built-dist step. + # Every other package's counts matched the DTS-on run but one: spec's + # root-entry-type-nameability pin skips (by its own design) and runs in + # lint.yml's built-dist step. + # + # ⛔ A SLICED reader keeps declarations on ONE slice, DTS_READER_SLICE + # (#25870). The CLI's vitest config pins its reader files into slice 1/n + # (DECLARATION_READER_FILES, packages/cli/vitest.config.ts), so its other + # slices read no `.d.ts` and skip them like any other shard; the + # partitioner charges slice 1/n the closure and puts the whole readers + # here on its shard (DECLARATION_READER_SLICES / DECLARATION_READER_PACKAGES, + # scripts/partition-test-shards.mjs, whose self-test reads this step back). + # A whole reader, or the CLI unsliced, keeps them as before. A new test + # that reads a built `.d.ts` reds the same loud way on a skipping shard: + # add its package here, or, for the CLI, its file to + # DECLARATION_READER_FILES. # # Local cold closure, 65 packages, 4 cores, `--force`: 5m59s with # declarations, 1m24s without. The cost is replay: a skipping shard @@ -931,10 +944,14 @@ jobs: - name: Choose this shard's declaration build run: | DTS_READERS='@objectstack/cli @objectstack/metadata' + DTS_READER_SLICE='1' KEEP="" - while read -r PKG _; do + while read -r PKG SLICE; do for R in $DTS_READERS; do - [ "$PKG" = "$R" ] && KEEP="$KEEP $PKG" + [ "$PKG" = "$R" ] || continue + case "$SLICE" in + ''|"$DTS_READER_SLICE"/*) KEEP="$KEEP $PKG${SLICE:+ $SLICE}" ;; + esac done done < "$RUNNER_TEMP/shard-packages.txt" if [ -n "$KEEP" ]; then diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index 272e1b83941..1dc74c5b2ac 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -643,7 +643,8 @@ // `node_modules` exclusion: an exact-path list matches nothing it does not name. import { defineConfig } from 'vitest/config'; import path from 'path'; -import { parseCLI, type TestUserConfig } from 'vitest/node'; +import { createHash } from 'node:crypto'; +import { BaseSequencer, parseCLI, type TestSpecification, type TestUserConfig } from 'vitest/node'; import { runFilterPreflight, runProjectCliOverridePreflight, @@ -740,6 +741,103 @@ runProjectCliOverridePreflight({ // lives at its use site in the `test` block below, not in a helper binding // that could outlive the spread. +// #25870 — THE FILES THAT READ BUILT DECLARATIONS RUN IN SLICE 1/n, AND ONLY +// THERE. Test Core builds a shard's dependency closure without `.d.ts` +// (`OS_SKIP_DTS=1`, ci.yml "Choose this shard's declaration build") unless the +// shard tests a package whose tests compile against BUILT declarations. In this +// package exactly the files in `DECLARATION_READER_FILES` do: each runs `tsc` +// over a packed tarball or a scaffolded project that resolves +// `@objectstack/spec` (or this package) through `dist/*.d.ts`, and on a +// declaration-less closure each reds with TS7016 or ENOENT. Measured, never +// recalled: build the closure with `OS_SKIP_DTS=1`, run the whole suite, and +// the red files are this list; with declarations they are green (the PR that +// added this section carries both readings). +// +// vitest's own `shard()` scatters files by the sha1 of their path, so those +// files sat in all four CLI slices and every slice shard paid the ~4.5 min +// declaration closure for one or two files. `DeclarationReaderSequencer` puts +// them first, then every other file in vitest's own sha1 order, and cuts that +// one sequence into vitest's own equal-count ranges, so the readers take the +// first places of slice 1/n and each other file moves at most a few places +// along vitest's order. ci.yml then keeps declarations for slice 1/n only, and +// `scripts/partition-test-shards.mjs` (`DECLARATION_READER_SLICES`) charges +// that slice the closure so its shard carries less whole-package work. The +// index is spelled in those three places; the partitioner's self-test reads the +// other two back. +// +// ⛔ Checked at config load, both directions of drift: +// - a listed file that is not in this run's queue population (renamed, +// deleted, or moved to a nightly tier) refuses to load the config, naming +// it -- a pin on a file that no longer runs would keep slice 1 holding the +// closure for nothing; +// - a NEW reader that is not listed lands wherever sha1 puts it. On slices +// 2..n it reds its own PR, loudly and deterministically, with TS7016 or +// ENOENT on a declaration-less closure: add it here. On slice 1 it passes +// until a later file moves the cut past it, and then reds the same way. +const DECLARATION_READER_SLICE = 1; +export const DECLARATION_READER_FILES = Object.freeze([ + 'test/create-in-repo-scaffold-typechecks.test.ts', + 'test/published-subpath-console.pin.test.ts', + 'test/published-subpath-hook-body.pin.test.ts', + 'test/scaffold-emission-typechecks.test.ts', + 'test/scaffold-test-script-passes.test.ts', +]); + +// The run's queue population holds every listed reader; a nightly-tier run +// holds none of them, and the pin is inert there (its closure builds +// declarations on every shard). +if (readTierMode() === 'queue') { + const population = new Set([...UNIT_FILES, ...INTEGRATION_FILES]); + const missing = DECLARATION_READER_FILES.filter((file) => !population.has(file)); + if (missing.length > 0) { + throw new Error( + `packages/cli/vitest.config.ts: DECLARATION_READER_FILES names ${missing.join(', ')}, which this ` + + 'package\'s queue test population does not hold. Remove or rename the entry: a pinned reader that ' + + 'never runs keeps slice 1/n building declarations for nothing.' + ); + } +} + +// vitest's equal-count range for slice `index` of `count` over `total` files +// (`BaseSequencer.calculateShardRange`, which vitest declares private). +function shardRange(total: number, index: number, count: number): [number, number] { + const base = Math.floor(total / count); + const remainder = total % count; + if (remainder >= index) return [(base + 1) * (index - 1), (base + 1) * index]; + const start = remainder * (base + 1) + (index - remainder - 1) * base; + return [start, start + base]; +} + +const READER_SET = new Set(DECLARATION_READER_FILES); + +class DeclarationReaderSequencer extends BaseSequencer { + override async shard(files: TestSpecification[]): Promise { + const { root, shard } = this.ctx.config; + if (!shard) return files; + const { index, count } = shard; + const relative = (spec: TestSpecification) => + path.relative(root, spec.moduleId).split(path.sep).join('/'); + // vitest's own key: the path below the root, with its leading separator. + const key = (spec: TestSpecification) => createHash('sha1').update(`/${relative(spec)}`).digest('hex'); + const readers = files.filter((spec) => READER_SET.has(relative(spec))); + const rest = files + .filter((spec) => !READER_SET.has(relative(spec))) + .map((spec) => ({ spec, hash: key(spec) })) + .sort((a, b) => (a.hash < b.hash ? -1 : a.hash > b.hash ? 1 : 0)) + .map(({ spec }) => spec); + const ordered = [...readers.sort((a, b) => (relative(a) < relative(b) ? -1 : 1)), ...rest]; + const [start, end] = shardRange(ordered.length, DECLARATION_READER_SLICE, count); + if (readers.length > end - start) { + throw new Error( + `packages/cli/vitest.config.ts: ${readers.length} declaration readers do not fit in slice ` + + `${DECLARATION_READER_SLICE}/${count}, which holds ${end - start} files. Pin fewer files, or slice fewer ways.` + ); + } + const [from, to] = shardRange(ordered.length, index, count); + return ordered.slice(from, to); + } +} + // #23181 slice 14 — THE OCLIF MANIFEST SETUP. One `globalSetup` module writes // `oclif.manifest.json` from source when the `integration` project runs (the // tier every CLI-spawning file is derived into) and removes it at teardown, so @@ -869,6 +967,10 @@ export default defineConfig({ // The file-level slice, when Test Core runs one (#19278) — see the section // above `export default` for why it is spread and typed this way. ...({ shard: process.env.OS_TEST_SHARD } satisfies Pick), + // The declaration readers into slice 1/n (#25870) -- see the section of + // that number above `export default`. Read by the root context only, which + // is the one that shards. + sequence: { sequencer: DeclarationReaderSequencer }, // A late console.* must not redden a green suite (#10374): vitest's worker // forwards console output over RPC and discards the promise, and a write // landing after teardown's rpcDone() snapshot is rejected into an unhandled diff --git a/scripts/partition-test-shards.mjs b/scripts/partition-test-shards.mjs index acc132a98d5..23b2e691124 100644 --- a/scripts/partition-test-shards.mjs +++ b/scripts/partition-test-shards.mjs @@ -430,6 +430,139 @@ export const PREVIOUS_FILE_SHARDED_PACKAGES = Object.freeze({ '@objectstack/cli': 3, }); +// ── THE SLICE THAT READS BUILT DECLARATIONS CARRIES THEIR CLOSURE (#25870) ── +// +// A Test Core shard builds its dependency closure without `.d.ts` +// (`OS_SKIP_DTS=1`) unless it tests a package whose tests compile against +// built declarations (ci.yml "Choose this shard's declaration build", +// DTS_READERS). The CLI is one, and only a handful of its files read them; +// its vitest config pins those files into one slice (DECLARATION_READER_FILES +// and DeclarationReaderSequencer in packages/cli/vitest.config.ts), so only the +// shard carrying that slice builds declarations, and every other CLI slice's +// shard skips them. This map names that slice per package. +// +// That shard then pays a closure the other slice shards do not: measured on +// spec-touching pull_request run 40423632426, "Build this shard's dependency +// closure" took 4.2-5.8 min on the shards that kept declarations against +// 0.5-0.9 min on the ones that skipped them, so ~4.5 min +// (DECLARATION_CLOSURE_WALL_SECONDS). It is serial wall, before the tests, and +// the bins here are sums of test seconds that a shard's whole leg runs four at +// a time -- a shard's step reads close to wholeSum / 2.9 + slice (the +// SHARD_COUNT docblock's calibration). So the slice is charged the closure +// in that unit, 270s x 2.9: the whole-package seconds that take as long on its +// shard as the closure does. Charged anything less, LPT gives that shard back +// most of the whole leg the closure displaces, and it stays the slowest shard +// by the difference; charged this, its shard and the other slice shards come +// out equal in the wall model. +// +// ⛔ Packing weight only: the charge is no test load, so wholeLegProblems() +// (pin 3d) grades the whole legs it moves exactly as before -- the whole +// packages the reader slice's shard gives up land on the other shards, and +// WHOLE_LEG_CAP_SECONDS and LOAD_SENSITIVE_APART still bound where they land. +// The self-test reads ci.yml's DTS_READERS / DTS_READER_SLICE and the CLI +// config's DECLARATION_READER_SLICE back against this map, so the three +// spellings of the index cannot drift apart. +// +// A WHOLE package that reads declarations keeps them on its shard too +// (`@objectstack/metadata` today), and a second shard building them would hand +// back the minutes the slice pin saved. So in a run that carries the reader +// slice, every whole package in DECLARATION_READER_PACKAGES rides with it, onto +// the one shard that builds declarations anyway (`rideWith`, partition()). A +// run with no reader slice -- the CLI whole, or not in the run -- charges and +// moves nothing, and splits as it always did. +export const DECLARATION_READER_SLICES = Object.freeze({ + '@objectstack/cli': 1, +}); +export const DECLARATION_READER_PACKAGES = Object.freeze(['@objectstack/cli', '@objectstack/metadata']); +export const DECLARATION_READERS = Object.freeze({ + slices: DECLARATION_READER_SLICES, + packages: DECLARATION_READER_PACKAGES, +}); +export const NO_DECLARATION_READERS = Object.freeze({ slices: Object.freeze({}), packages: Object.freeze([]) }); +export const DECLARATION_CLOSURE_WALL_SECONDS = 270; +export const WHOLE_LEG_WALL_DIVISOR = 2.9; +export const DECLARATION_CLOSURE_CHARGE_SECONDS = DECLARATION_CLOSURE_WALL_SECONDS * WHOLE_LEG_WALL_DIVISOR; + +// What an item weighs in a bin: its test seconds plus any closure it is charged. +export function packedWeight(item) { + return item.weight + (item.closure ?? 0); +} + +// The three spellings of the reader slice, judged against each other. Pure: +// `ciYml` is the workflow's text, `configSources` maps each package +// DECLARATION_READER_SLICES names to its vitest config's text (or null). Read +// back, in order: ci.yml's DTS_READERS names exactly DECLARATION_READER_PACKAGES; +// its DTS_READER_SLICE is the index every DECLARATION_READER_SLICES entry names; +// each such package's vitest config pins the same index through the sequencer, +// in code position; and every package that is both sliced (`sliced`) and a +// reader has a reader slice -- without one its slices past the first skip the +// declarations its readers need, wherever sha1 puts them. +export function judgeDeclarationReaders({ + ciYml, + configSources, + slices = DECLARATION_READER_SLICES, + packages = DECLARATION_READER_PACKAGES, + sliced = FILE_SHARDED_PACKAGES, +}) { + const problems = []; + const readersAt = /^\s*DTS_READERS='([^']*)'\s*$/m.exec(ciYml); + const sliceAt = /^\s*DTS_READER_SLICE='([1-9]\d*)'\s*$/m.exec(ciYml); + if (!readersAt) problems.push("ci.yml: no `DTS_READERS='...'` line in \"Choose this shard's declaration build\""); + if (!sliceAt) problems.push("ci.yml: no `DTS_READER_SLICE='N'` line in \"Choose this shard's declaration build\""); + if (readersAt) { + const named = readersAt[1].split(/\s+/).filter(Boolean).sort(); + const here = [...packages].sort(); + if (named.join(' ') !== here.join(' ')) { + problems.push(`ci.yml DTS_READERS is [${named.join(', ')}] but DECLARATION_READER_PACKAGES is [${here.join(', ')}]`); + } + } + for (const [name, index] of Object.entries(slices)) { + if (!packages.includes(name)) problems.push(`${name}: has a reader slice but is not in DECLARATION_READER_PACKAGES`); + if (sliceAt && Number(sliceAt[1]) !== index) { + problems.push(`${name}: DECLARATION_READER_SLICES says slice ${index}, ci.yml DTS_READER_SLICE says ${sliceAt[1]}`); + } + const source = configSources.get(name) ?? null; + if (source === null) { + problems.push(`${name}: no vitest config to read its DECLARATION_READER_SLICE from`); + continue; + } + const code = maskCommentsAndLiterals(source); + const pinned = /\bconst\s+DECLARATION_READER_SLICE\s*=\s*([1-9]\d*)\s*;/.exec(code); + if (!pinned || Number(pinned[1]) !== index) { + problems.push( + `${name}: its vitest config pins its readers to slice ${pinned ? pinned[1] : '(none)'}, ` + + `DECLARATION_READER_SLICES says ${index}` + ); + } + if (!/\bsequencer\s*:\s*DeclarationReaderSequencer\b/.test(code)) { + problems.push(`${name}: its vitest config does not install DeclarationReaderSequencer as \`sequence.sequencer\``); + } + } + for (const name of packages) { + if ((sliced[name] ?? 1) > 1 && !Object.hasOwn(slices, name)) { + problems.push( + `${name}: sliced ${sliced[name]} ways and a declaration reader, but it has no reader slice, so ` + + 'its slices past the first skip the declarations its tests read' + ); + } + } + return problems; +} + +// judgeDeclarationReaders() on the live tree's vitest configs, against the +// workflow text the caller read (the self-test hands in the ci.yml it already +// reads for pin 2, so this module names no workflow path of its own). +export function declarationReaderProblems(ciYml, root = REPO_ROOT) { + const dirOf = new Map(workspacePackages(root).map(({ dir, manifest }) => [manifest?.name, dir])); + const configSources = new Map(); + for (const name of Object.keys(DECLARATION_READER_SLICES)) { + const dir = dirOf.get(name); + const file = dir === undefined ? undefined : VITEST_CONFIG_NAMES.find((f) => existsSync(path.join(root, dir, f))); + configSources.set(name, file ? readFileSync(path.join(root, dir, file), 'utf8') : null); + } + return judgeDeclarationReaders({ ciYml, configSources }); +} + // The item grammar. A shard item is a package (`@objectstack/cli`) or a SLICE // of one (`@objectstack/cli 1/2`), and this pair of functions is the only place // that spelling is written or read -- ci.yml builds the turbo invocation from @@ -593,7 +726,17 @@ export function sliceWiringProblems(root = REPO_ROOT, sliced = FILE_SHARDED_PACK // package's suite runs as (testTaskCount). A slice leg runs `test` alone, so a // slice is one task. A caller that knows no manifest -- the fixtures -- passes // none and is weighed as a single-task suite. -export function expandSlices(items) { +// +// `closure` rides along on the one slice DECLARATION_READER_SLICES names: the +// declaration closure that slice's shard builds and no other slice's does. It +// is packing weight only (packedWeight), never test weight, so `weight` -- +// what serialFloorOf(), the slice-count derivation and the drift step read -- +// is the same with or without it. `rideWith` names that slice on each whole +// package of DECLARATION_READER_PACKAGES, when the expansion carries the slice. +// `readers` is the live pair everywhere but the fixtures and a run whose +// document states its own slicing (planRun()). +export function expandSlices(items, readers = DECLARATION_READERS) { + const readerSlices = readers.slices; const out = []; for (const it of items) { const count = it.sliceCount ?? sliceCountFor(it.name); @@ -601,6 +744,7 @@ export function expandSlices(items) { out.push({ name: it.name, weight: it.weight, pkg: it.name, slice: null, tasks: it.tasks ?? 1 }); continue; } + const readerIndex = Object.hasOwn(readerSlices, it.name) ? readerSlices[it.name] : null; for (let index = 1; index <= count; index++) { const slice = { index, count }; out.push({ @@ -609,9 +753,16 @@ export function expandSlices(items) { pkg: it.name, slice, tasks: 1, + ...(index === readerIndex ? { closure: DECLARATION_CLOSURE_CHARGE_SECONDS } : {}), }); } } + const anchor = out.find((i) => i.closure !== undefined); + if (anchor) { + for (const i of out) { + if (!i.slice && readers.packages.includes(i.name)) i.rideWith = anchor.name; + } + } return out; } @@ -905,19 +1056,29 @@ function readManifest(dir) { // LPT greedy: heaviest package into the currently lightest bin. Deterministic: // input order never matters because both the package sort and the bin choice -// break ties explicitly (by name / by lowest bin index). +// break ties explicitly (by name / by lowest bin index). An item weighs its +// packedWeight(): its test seconds, plus the declaration closure on the one +// slice charged it (DECLARATION_READER_SLICES). An item that `rideWith`s that +// slice goes into the slice's bin, straight after it (DECLARATION_READER_PACKAGES); +// a rider whose slice is not in `items` is placed like any other item. export function partition(items, shardCount) { - const sorted = [...items].sort( - (a, b) => b.weight - a.weight || a.name.localeCompare(b.name, 'en') - ); + const present = new Set(items.map((i) => i.name)); + const rides = (it) => it.rideWith !== undefined && present.has(it.rideWith); + const byWeight = (a, b) => packedWeight(b) - packedWeight(a) || a.name.localeCompare(b.name, 'en'); + const sorted = items.filter((i) => !rides(i)).sort(byWeight); + const riders = items.filter(rides).sort(byWeight); const bins = Array.from({ length: shardCount }, () => ({ total: 0, names: [] })); + const put = (it, at) => { + bins[at].names.push(it.name); + bins[at].total += packedWeight(it); + }; for (const it of sorted) { let best = 0; for (let i = 1; i < bins.length; i++) { if (bins[i].total < bins[best].total) best = i; } - bins[best].names.push(it.name); - bins[best].total += it.weight; + put(it, best); + for (const r of riders) if (r.rideWith === it.name) put(r, best); } return bins; } @@ -1127,10 +1288,17 @@ export function placeItems(items, shardCount, cap = Infinity) { // decisions already made. Returns the items, the split main() runs (placed the // way the decision was judged) and one decision per configured package present // in the run, which main() prints. -export function planShards(weighed, shardCount, { bound = MAX_SHARD_OVER_MEAN, cap = Infinity } = {}) { +export function planShards( + weighed, + shardCount, + { bound = MAX_SHARD_OVER_MEAN, cap = Infinity, readers = DECLARATION_READERS } = {} +) { const chosen = new Map(weighed.map((p) => [p.name, 1])); const expandWith = (name, n) => - expandSlices(weighed.map((p) => ({ ...p, sliceCount: p.name === name ? n : chosen.get(p.name) }))); + expandSlices( + weighed.map((p) => ({ ...p, sliceCount: p.name === name ? n : chosen.get(p.name) })), + readers + ); const decisions = []; const configured = weighed .filter((p) => (p.sliceCount ?? 1) > 1) @@ -1158,7 +1326,7 @@ export function planShards(weighed, shardCount, { bound = MAX_SHARD_OVER_MEAN, c chosen.set(pkg.name, count); decisions.push({ name: pkg.name, configured: pkg.sliceCount, count, needed, spreads, withinCap, own, target, densest, cap }); } - const items = expandSlices(weighed.map((p) => ({ ...p, sliceCount: chosen.get(p.name) }))); + const items = expandSlices(weighed.map((p) => ({ ...p, sliceCount: chosen.get(p.name) })), readers); return { items, bins: placeItems(items, shardCount, cap), decisions }; } @@ -1528,7 +1696,11 @@ export function planRun(parsed, listPath, shardCount, { excluded = new Set(), ti // run's list (densityCap): the density proven green is a fact about the // dataset, and the run's own list is what is graded against it. const cap = densityCapOf(dataset, excluded, shardCount); - const { items: weighted, bins: placed, decisions } = planShards(weighed, shardCount, { cap }); + // A document that states its own slicing is no Test Core run, and its shards + // skip no declarations (test-nightly-tiers.yml sets no OS_SKIP_DTS), so no + // slice of it is charged the declaration closure and nothing rides with one. + const readers = acrossShards === null ? DECLARATION_READERS : NO_DECLARATION_READERS; + const { items: weighted, bins: placed, decisions } = planShards(weighed, shardCount, { cap, readers }); const bins = assertSlicesSpread(placed); return { bins, weighted, decisions, estimated, packages, cap, acrossShards, uiTree }; } @@ -1573,6 +1745,10 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'the balancing pins (#10472)': 25, // 1 grades the committed dataset's split; 4 are fixtures, both directions. 'the whole-package leg cap and the load-sensitive pair (#25870)': 5, + // 4 grade the committed dataset and the live tree, 1 drives planRun() on + // both document shapes, 10 are fixtures (7 of the three spellings, 3 of the + // placement). + 'the declaration-reading slice carries the closure (#25870)': 15, 'predicted-vs-measured drift (#16173)': 9, 'drift warning tier under the red (#16465)': 12, 'file-level slice items (#16173)': 20, @@ -1589,7 +1765,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ // 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 = 16; +const SELF_TEST_BATTERY_FLOOR = 17; // 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 @@ -2115,6 +2291,169 @@ function selfTest() { } }); + // 3e. THE DECLARATION-READING SLICE CARRIES THE CLOSURE (#25870) -- see + // DECLARATION_READER_SLICES. The committed dataset as CI splits it first: + // one item is charged, it is the reader slice, the whole readers ride with + // it, and its shard carries less whole-package work than every other + // slice shard; then the three spellings of the index read back from the + // tree; then fixtures for each half, both directions. + battery('the declaration-reading slice carries the closure (#25870)'); + const readerLabel = (name) => + formatShardItem(name, { index: DECLARATION_READER_SLICES[name], count: FILE_SHARDED_PACKAGES[name] }); + const binOf = (bins, label) => bins.findIndex((b) => b.names.includes(label)); + check(() => { + const charged = datasetItems.filter((i) => i.closure !== undefined); + const expected = Object.keys(DECLARATION_READER_SLICES).map(readerLabel); + if (charged.map((i) => i.name).join('|') !== expected.join('|') || + charged.some((i) => i.closure !== DECLARATION_CLOSURE_CHARGE_SECONDS)) { + throw new Error( + `declaration closure: the committed dataset charged [${charged.map((i) => `${i.name} ${i.closure}s`).join(', ')}], ` + + `expected [${expected.join(', ')}] at ${DECLARATION_CLOSURE_CHARGE_SECONDS}s` + ); + } + }); + check(() => { + for (const name of Object.keys(DECLARATION_READER_SLICES)) { + const at = binOf(real, readerLabel(name)); + for (const rider of DECLARATION_READER_PACKAGES.filter((p) => p !== name && datasetItems.some((i) => i.name === p))) { + if (binOf(real, rider) !== at) { + throw new Error(`declaration closure: ${rider} is on shard ${binOf(real, rider) + 1}, not with ${readerLabel(name)} on shard ${at + 1}`); + } + } + } + }); + // A literal, not the charge: a charge refreshed to nothing must red here. + // 300s is well under the ~780s today's charge makes room for. + const ROOM_SECONDS = 300; + check(() => { + for (const name of Object.keys(DECLARATION_READER_SLICES)) { + const at = binOf(real, readerLabel(name)); + const others = real + .map((b, i) => ({ i, sliced: b.names.some((n) => parseShardItem(n).name === name) })) + .filter((b) => b.sliced && b.i !== at) + .map((b) => wholeLeg.sums[b.i]); + if (others.length === 0 || !(wholeLeg.sums[at] + ROOM_SECONDS < Math.min(...others))) { + throw new Error( + `declaration closure: ${readerLabel(name)}'s shard carries ${wholeLeg.sums[at].toFixed(0)}s of whole packages ` + + `against [${others.map((s) => s.toFixed(0)).join(', ')}]s on the other slice shards, not ${ROOM_SECONDS}s less -- ` + + 'the closure it builds alone is not making room for itself' + ); + } + } + }); + check(() => { + const problems = declarationReaderProblems(ciYml); + if (problems.length > 0) throw new Error(`declaration readers, live tree:\n${problems.map((p) => ` - ${p}`).join('\n')}`); + }); + { + const ciOk = " DTS_READERS='a m'\n DTS_READER_SLICE='1'\n"; + const cfgOk = 'const DECLARATION_READER_SLICE = 1;\nexport default { test: { sequence: { sequencer: DeclarationReaderSequencer } } };'; + const judge = (over) => + judgeDeclarationReaders({ + ciYml: ciOk, + configSources: new Map([['a', cfgOk]]), + slices: { a: 1 }, + packages: ['a', 'm'], + sliced: { a: 4 }, + ...over, + }); + const refused = (label, over, needle) => + check(() => { + const r = judge(over); + if (!r.some((m) => m.includes(needle))) { + throw new Error(`declaration readers: ${label} was accepted (${r.join(' | ') || 'no problem named'})`); + } + }); + check(() => { + const r = judge({}); + if (r.length > 0) throw new Error(`declaration readers: a consistent fixture was refused: ${r.join(' | ')}`); + }); + refused('a ci.yml slice index that differs', { ciYml: ciOk.replace("'1'", "'2'") }, 'DTS_READER_SLICE says 2'); + refused('a ci.yml reader list missing a package', { ciYml: ciOk.replace("'a m'", "'a'") }, 'DTS_READERS is [a]'); + refused('a config pinning another slice', { configSources: new Map([['a', cfgOk.replace('= 1;', '= 2;')]]) }, 'slice 2'); + refused( + 'a config pin that only a comment spells', + { configSources: new Map([['a', `// const DECLARATION_READER_SLICE = 1;\n${cfgOk.split('\n')[1]}`]]) }, + 'slice (none)' + ); + refused('a config without the sequencer', { configSources: new Map([['a', cfgOk.split('\n')[0]]]) }, 'does not install'); + refused('a sliced reader with no reader slice', { slices: {}, configSources: new Map() }, 'a: sliced 4 ways'); + } + { + // Two shards. `a` cut in two, its first half the reader slice, `m` a whole + // reader. Charged, a 1/2's shard gives whole-package work to a 2/2's and + // `m` rides with it; with no readers declared the halves split the wholes + // evenly. + const readers = { slices: { a: 1 }, packages: ['a', 'm'] }; + const pkgs = [ + { name: 'a', weight: 2000, sliceCount: 2 }, + mk('m', 50), + mk('w1', 600), + mk('w2', 600), + mk('w3', 400), + mk('w4', 400), + mk('w5', 200), + mk('w6', 200), + ]; + const wholeOn = (bins, items, label) => { + const w = new Map(items.map((i) => [i.name, i.weight])); + return bins[binOf(bins, label)].names.filter((n) => !parseShardItem(n).slice).reduce((s, n) => s + w.get(n), 0); + }; + const charged = expandSlices(pkgs, readers); + const chargedBins = partition(charged, 2); + const plain = expandSlices(pkgs, NO_DECLARATION_READERS); + const plainBins = partition(plain, 2); + check(() => { + if (binOf(chargedBins, 'm') !== binOf(chargedBins, 'a 1/2')) { + throw new Error(`declaration closure: the whole reader did not ride with the reader slice: ${JSON.stringify(chargedBins)}`); + } + }); + check(() => { + const [one, two] = ['a 1/2', 'a 2/2'].map((l) => wholeOn(chargedBins, charged, l)); + const [p1, p2] = ['a 1/2', 'a 2/2'].map((l) => wholeOn(plainBins, plain, l)); + if (!(two - one >= 500) || !(Math.abs(p1 - p2) <= 100)) { + throw new Error( + `declaration closure: whole work beside a 1/2 / a 2/2 is ${one}/${two}s charged and ${p1}/${p2}s plain -- ` + + 'the charge is not moving whole packages off the reader slice\'s shard' + ); + } + }); + check(() => { + // No reader slice in the run (the package whole): nothing is charged, + // nothing rides, and the split is the plain one, byte for byte. + const whole = pkgs.map((p) => ({ ...p, sliceCount: 1 })); + const items = expandSlices(whole, readers); + if (items.some((i) => i.closure !== undefined || i.rideWith !== undefined) || + JSON.stringify(partition(items, 2)) !== JSON.stringify(partition(expandSlices(whole, NO_DECLARATION_READERS), 2))) { + throw new Error('declaration closure: a run with no reader slice was charged or re-placed'); + } + }); + } + check(() => { + // Through planRun(), the function main() runs: a Test Core document charges + // the CLI's reader slice and seats metadata beside it; a document that + // states its own slicing (the nightly's) charges nothing. + const items = [ + { name: '@objectstack/cli', path: 'packages/cli' }, + { name: '@objectstack/metadata', path: 'packages/metadata' }, + ]; + const core = planRun({ packages: { count: 2, items } }, 'test core fixture', SHARD_COUNT, { timings }); + const nightly = planRun( + { packages: { count: 2, items }, [SLICE_ACROSS_SHARDS_KEY]: ['@objectstack/cli'] }, + 'nightly fixture', + 2, + { timings } + ); + const label = readerLabel('@objectstack/cli'); + if (binOf(core.bins, '@objectstack/metadata') !== binOf(core.bins, label) || + !core.weighted.some((i) => i.name === label && i.closure === DECLARATION_CLOSURE_CHARGE_SECONDS)) { + throw new Error(`declaration closure: a Test Core document split as [${core.bins.map((b) => b.names.join(',')).join(' | ')}]`); + } + if (nightly.weighted.some((i) => i.closure !== undefined || i.rideWith !== undefined)) { + throw new Error('declaration closure: a document that states its own slicing was charged the declaration closure'); + } + }); + // -- PREDICTED VS MEASURED (#16173) ------------------------------------- // // The balancing pins above all read the dataset as GIVEN. None of them can From 9d1cfdbf6328d4f53623579c8419294df0efe5db Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 23:36:38 +0000 Subject: [PATCH 2/2] fix(partition-test-shards): keep the declaration reader that rides with the reader slice out of the cap repair refineToCap() moved any whole package off the densest bin, the rider included, which would put declarations on a second shard. The rider now stays with its slice like the slice itself; the self-test gains the fixture where only that rule keeps it there. Claude-Session: https://claude.ai/code/session_01QSeKYmTYfaPbLQD4BJfomW Co-authored-by: Claude --- scripts/partition-test-shards.mjs | 37 ++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/scripts/partition-test-shards.mjs b/scripts/partition-test-shards.mjs index 23b2e691124..9e282f42e9d 100644 --- a/scripts/partition-test-shards.mjs +++ b/scripts/partition-test-shards.mjs @@ -1214,10 +1214,18 @@ export function densityCapOf(timings, excluded, shardCount) { // Deterministic (candidates in bin and item order, the first strictly best // kept), and it terminates: every accepted move lowers the sum of squared bin // totals. A split already within the cap comes back unchanged. +// +// ⛔ A whole package that rides with the declaration-reading slice (`rideWith`, +// DECLARATION_READER_PACKAGES) stays put like the slice itself: moved off, it +// would take declarations to a second shard, which then builds the closure the +// slice pin exists to build once. export function refineToCap(bins, items, cap) { const weightOf = new Map(items.map((i) => [i.name, i.weight])); - const slices = new Set(items.filter((i) => i.slice).map((i) => i.name)); - const movable = (name) => !slices.has(name); + const present = new Set(items.map((i) => i.name)); + const fixed = new Set( + items.filter((i) => i.slice || (i.rideWith !== undefined && present.has(i.rideWith))).map((i) => i.name) + ); + const movable = (name) => !fixed.has(name); const out = bins.map((b) => ({ total: b.total, names: [...b.names] })); const EPSILON = 1e-9; for (let step = 0; step < 10_000; step++) { @@ -1746,9 +1754,9 @@ const SELF_TEST_BATTERIES = Object.freeze({ // 1 grades the committed dataset's split; 4 are fixtures, both directions. 'the whole-package leg cap and the load-sensitive pair (#25870)': 5, // 4 grade the committed dataset and the live tree, 1 drives planRun() on - // both document shapes, 10 are fixtures (7 of the three spellings, 3 of the - // placement). - 'the declaration-reading slice carries the closure (#25870)': 15, + // both document shapes, 11 are fixtures (7 of the three spellings, 3 of the + // placement, 1 of the cap repair). + 'the declaration-reading slice carries the closure (#25870)': 16, 'predicted-vs-measured drift (#16173)': 9, 'drift warning tier under the red (#16465)': 12, 'file-level slice items (#16173)': 20, @@ -2428,6 +2436,25 @@ function selfTest() { throw new Error('declaration closure: a run with no reader slice was charged or re-placed'); } }); + check(() => { + // The cap repair moves whole packages off the densest bin; the whole + // reader riding with the reader slice is not one it may move. Here moving + // `m` would strictly lower the 800s bin (to 600s / 500s), so only the + // rider rule keeps it beside `a 1/2`. + const items = [ + { name: 'a 1/2', weight: 500, pkg: 'a', slice: { index: 1, count: 2 }, tasks: 1 }, + { name: 'a 2/2', weight: 300, pkg: 'a', slice: { index: 2, count: 2 }, tasks: 1 }, + { name: 'm', weight: 300, pkg: 'm', slice: null, tasks: 1, rideWith: 'a 1/2' }, + ]; + const bins = [ + { total: 800, names: ['a 1/2', 'm'] }, + { total: 300, names: ['a 2/2'] }, + ]; + const out = refineToCap(bins, items, 600); + if (binOf(out, 'm') !== binOf(out, 'a 1/2')) { + throw new Error(`declaration closure: the cap repair moved the whole reader off the reader slice's shard: ${JSON.stringify(out)}`); + } + }); } check(() => { // Through planRun(), the function main() runs: a Test Core document charges