Skip to content

Commit 6d9e5a1

Browse files
ci(test-core): build declarations on one CLI slice only — pin the CLI's declaration readers into slice 1/n (#27018)
Part of #25870 Ruling-ref: 6439314592 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么:** CLI 测试里只有 5 个文件要读构建出的 `.d.ts`。现在 vitest 把它们固定排进第 1 片(sequencer 在 `packages/cli/vitest.config.ts`)。Test Core 只在带 cli 1/4 的那个分片构建声明,metadata 也被排到同一分片;其余分片都设 `OS_SKIP_DTS=1`。 - **为什么改:** 以前这 5 个文件按 sha1 散落在 4 个切片里,导致 5 个分片各自多花约 4.5 分钟构建声明。改后只剩 1 个分片构建声明。按模型推算,最慢分片从约 22 分钟降到约 19 分钟,CI 实际预计 20–21 分钟。 - **风险与回滚:** 另外 515 个 cli 文件里有 9 个换了切片,对负载敏感的邻居可能跟着变(#25968 那一类)。以后有人新增读声明的测试却没登记,它会在不构建声明的分片上以 TS7016 报红,失败是响亮的。回滚办法是 revert 本 PR,只涉及 3 个文件,不动数据也不动 schema。 - **先看:** `.github/workflows/ci.yml` 里 "Choose this shard's declaration build" 一步的 `case` 判断;`packages/cli/vitest.config.ts` 的 `DeclarationReaderSequencer.shard()`;`scripts/partition-test-shards.mjs` 的 `partition()` / `refineToCap()` 对 `rideWith` 的处理。 - **两分钟验证:** 运行 `node scripts/partition-test-shards.mjs --self-test`,应看到 `self-test OK` 和 `whole legs 143/1597/1597/925/924/923/1599/1597s`,其中第 1 分片只放 cli 1/4 和 metadata。 - **需要你做的:** 以 `GOVERNED_APPROVERS` 身份审阅并批准这个 Tier H PR(`.github/workflows/ci.yml`)。 ## What changes Ruling A (6439314592), as specified in 6436991767, built on seat 1's commit `7e64f7c1d3` (cherry-picked onto current `main`, every hunk re-read and re-verified here) plus one fix: - **`packages/cli/vitest.config.ts`.** `DECLARATION_READER_FILES` lists the 5 files that compile against built declarations. `DeclarationReaderSequencer` overrides only `shard()`. It puts the readers first and the remaining files in vitest's own sha1 order (key `/` + root-relative path, the same as `BaseSequencer`), then cuts that sequence with vitest's own equal-count ranges. The readers land in slice 1/n, and every other file moves at most 5 places along vitest's order. The config refuses to load when a listed reader is missing from the queue population. It also refuses when the readers do not fit in slice 1/n. With no shard set, or in a nightly-tier run (no readers in the population), the order is vitest's own. - **`.github/workflows/ci.yml`**, the Test Core "Choose this shard's declaration build" step and its comment only. `DTS_READER_SLICE='1'`: a shard keeps declarations when it carries a whole reader package or slice `1/*` of one. Every other shard sets `OS_SKIP_DTS=1`. The comment now lists `create-in-repo-scaffold-typechecks`, which the earlier list missed. - **`scripts/partition-test-shards.mjs`.** - The reader slice is charged the declaration closure as packing weight: 270 s × 2.9 = 783 s. This is never test weight, so the slice-count derivation, the drift step and `serialFloorOf()` are unchanged. - Every whole package in `DECLARATION_READER_PACKAGES` (`@objectstack/metadata`) rides into the reader slice's bin, so one shard builds declarations, not two. - A run with no reader slice (cli whole, cli absent, or a nightly document) is charged nothing and splits exactly as before. - The self-test reads the slice index back from all three places it is spelled (ci.yml, the cli config, this map). Battery 3e: 16 checks. - **Fix on top of the inherited commit:** `refineToCap()` could move the riding whole reader off the reader slice's shard. That would put declarations back on a second shard. Riders are now fixed in place like slices. The self-test gains the fixture where only that rule keeps the rider in place. ⛔ Unchanged: no test is skipped or quarantined, no timeout is raised, no test leaves Test Core, the shard count stays 8, and no other `ci.yml` job is touched. ## Tests All readings are local, on a shared 4-core box, so wall-clock figures are shared-box seconds. <details><summary>Reader set re-derived on current main (merge base 912076f)</summary> - `OS_SKIP_DTS=1 turbo run build --filter=@objectstack/cli --filter=@objectstack/metadata`: 61/61 tasks, and 0 `.d.ts` files under `packages/spec/dist` and `packages/cli/dist`. - The whole cli suite on that closure (`vitest run --maxWorkers=3`, under the verify lock) gave `VERDICT command-exit 1`, with `Test Files 5 failed | 514 passed | 1 skipped (520)` and `Tests 25 failed | 6432 passed | 9 skipped (6466)`. The red files are exactly the 5 listed in `DECLARATION_READER_FILES`, and every one fails for missing declarations: - `create-in-repo-scaffold-typechecks`: TS7016 - `published-subpath-console.pin`: TS7016, ENOENT on `dist/console.d.ts` - `published-subpath-hook-body.pin`: TS7016, ENOENT on `dist/hook-body.d.ts` - `scaffold-emission-typechecks`: TS7016 / TS2307 - `scaffold-test-script-passes`: TS7016 - No new reader was found. - Control leg, the same tree rebuilt with declarations (243 `.d.ts`), running `OS_TEST_SHARD=1/4`: `VERDICT command-exit 0`, `Test Files 129 passed | 1 skipped (130)`, `Tests 1750 passed | 2 skipped`. The 5 readers ran there green, 56/56 tests. </details> <details><summary>Slice map, from vitest's own context on the real config</summary> `createVitest` + `globTestSpecifications()` + `config.sequence.sequencer` against `BaseSequencer`, for `OS_TEST_SHARD=k/4`: | | files per slice | readers per slice | |---|---|---| | before (`BaseSequencer`) | 130/130/130/130 | 2/1/1/1 | | after (`DeclarationReaderSequencer`) | 130/130/130/130 | 5/0/0/0 | - 520 files in the union, 0 duplicated, 9 files changed slice. - The 1/4 run above executed exactly the after-map's slice 1 (130/130 files match). - Summed per-file seconds per slice (from the runs above): before 301/602/833/401, after 313/595/827/401. - The readers sum to 47 s in the DTS run, and they land on the lightest slice. </details> <details><summary>Placement and wall model, full `turbo ls` list (129 packages, dogfood excluded)</summary> | shard | before (main) | after | |---|---|---| | 1 | 1502 s whole | cli 1/4 + metadata (143 s whole), **declarations** | | 2 | 1502 s whole | 1598 s whole | | 3 | 818 s whole + cli 1/4, **declarations** | 1598 s whole | | 4 | 828 s whole + cli 2/4, **declarations** | 925 s whole + cli 2/4 | | 5 | 826 s whole + cli 3/4, **declarations** | 926 s whole + cli 3/4 | | 6 | 827 s whole + cli 4/4, **declarations** | 924 s whole + cli 4/4 | | 7 | 1502 s whole incl. metadata, **declarations** | 1591 s whole | | 8 | 1501 s whole | 1600 s whole | - Shards building declarations: 5 before, 1 after. - Wall model: step = whole/2.9 + slice, plus the closure (270 s with declarations, about 45–90 s without), plus about 90 s of setup. - Before, the slowest shard is about 22.0 min. CI read 23.9–25.4 min, so the model reads about 2 min low. - After, the slowest shard is about 18.8 min (shards 4–6), and shard 1 is about 18.1 min. - Expected in CI: about 20–21 min. - Every whole leg stays within the 1868 s cap. objectql and plugin-security stay apart. </details> <details><summary>Partitioner self-test and ablations</summary> - `node scripts/partition-test-shards.mjs --self-test`: `self-test OK (75 measured packages -> 78 shard items, 8 shards, max/mean 1.00x ..., whole legs 143/1597/1597/925/924/923/1599/1597s within 1868s, file-level slices: @objectstack/cli x4)`. - Ablation of the rider fix, through `scripts/ablation-replace.mjs`: - The anchor went from 1 hit to 0 and the blob from 9e282f4 to 042fd9beb508. - The self-test then exited 1 with `the cap repair moved the whole reader off the reader slice's shard`. - Restore: the blob equals HEAD and `git diff HEAD` is empty. - Inherited assertions, re-run here at `c5a59dfe5f`. Each went through `ablation-replace.mjs` with the anchor going 1 to 0, and each was restored to blob == HEAD with an empty `git diff HEAD`: - closure wall 270 to 0: exit 1, `cli 1/4's shard carries 826s of whole packages against [826, 827, 826]s ... not 300s less`; - rider placed in the next bin: exit 1, `@objectstack/metadata is on shard 2, not with @objectstack/cli 1/4 on shard 1`; - ci.yml `DTS_READER_SLICE='2'`: exit 1, `DECLARATION_READER_SLICES says slice 1, ci.yml DTS_READER_SLICE says 2`; - sequencer removed from the cli config: exit 1, `its vitest config does not install DeclarationReaderSequencer`; - reader dropped from `DECLARATION_READER_FILES` (slice map re-read through vitest's context): `scaffold-test-script-passes` moves to slice 2/4, a declaration-less slice, where the run above shows it red with TS7016. My first attempt at this one was refused by the tool, because the replacement text was not unique. It was restored, and the second attempt used a unique marker. </details> ## Acceptance notes - **Acceptance is NOT MEASURED here:** the card asks for the slowest Test Core shard ≤ ~20 min on 3 consecutive spec-touching `pull_request` runs. That can only be read on this PR in CI. After landing, the ruling repeats the 10-full-queue-build reading of 6430866969 before the card closes, so this PR says `Part of`, not `Fixes`. - Real slices are not equal-weight. The summed per-file seconds above make cli 3/4 the heaviest slice, which is the same slice as the slowest shard on main (5/8). This PR removes that shard's closure. It does not rebalance the slices (option C, not ruled). - The partitioner's per-shard "Ns predicted" bin totals now include the reader slice's 783 s packing charge (closure, never test weight). `driftReport()` reads per-package timings, not bin totals, so the drift verdict is unaffected. - Changeset: `skip-changeset`. `@objectstack/cli` ships `files: ["dist", "README.md", "CHANGELOG.md"]`. `DeclarationReaderSequencer` / `DECLARATION_READER_FILES` have 0 hits under those paths, and the positive control (`hook-body` under `dist/`) hits. `scripts/` and `.github/` are not published. - **Contract review (seat, before this PR opened):** card comment 6458259422, `CONTRACT_REVIEW_TIER`, PASS on `c5a59dfe5f`, no must-fix. Its two should-fix notes: - **S2, the one hole this design cannot see:** a NEW declaration-reading cli test that nobody adds to `DECLARATION_READER_FILES` passes as long as vitest's sha1 cut happens to put it in slice 1 (about one chance in four). A later population change that moves the cut then reds an unrelated PR. That red means "add the file to `DECLARATION_READER_FILES`", not a flake. A listed reader that is renamed or deleted already refuses the config at load. - **S1:** the shard log's `Ns predicted (all bins: ...)` line and `balanceOf()` now include the reader slice's 783 s packing charge without saying so. `driftReport()` is untouched, so no verdict changes. The wording fix rides a follow-up. ## Verification Final head `c5a59dfe5f`. It is a clean merge of `origin/main` that includes #26854 and the #25452 cache steps, in regions disjoint from this diff. `packages/cli` is unchanged across both merges. The reader and slice readings above were taken at `9d1cfdbf63`, which is merge base `912076f885` plus this diff. <details><summary>Derived gates at c5a59df</summary> - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 86 families. All 86 exited 0, each exit captured before any pipe. - `--ran` reported: `86 derived, 86 run, 0 NOT-MEASURED, 0 UNRUN` (a derived zero: all 86 recorded an exit code). - On the earlier head `849642eb8c`, `check:dts-closure` and `check:dual-build-cjs-loads` first exited 1 and 3. That run overlapped my own rebuild of the closure, which had `tsup` cleaning dists. Re-run on the settled tree, both exited 0: `233/233 declared declaration file(s) present` and `112 published require entry point(s) across 69 package(s) load`. Both are 0 again on the final head. - `pnpm --filter @objectstack/cli typecheck`, which compiles `vitest.config.ts` through `tsconfig.test.json`, gave `VERDICT command-exit 0` and `check:test-typecheck: OK`. A first run went red in `../objectql/src/picklist-resolution.ts` because `metadata-core`'s dist was stale after the merge. It went green after rebuilding the closure. - The ci.yml step's shell was exercised on 6 shard-file fixtures: - `cli 1/4` + metadata, `cli` whole, and `metadata` alone keep declarations; - `cli 2/4` + metadata-core, client-react + `cli 4/4`, and `cli 11/12` set `OS_SKIP_DTS=1`. - Not measured: - actionlint and shellcheck: no binary in this container; - CI check-runs: no PR yet, because the contract review comes first; - the card's acceptance (≤ ~20 min on 3 spec-touching runs): CI-only; - slices 2–4/4 were not re-run as slices (declared narrowing). The whole-suite no-declarations run covers every one of their files green, and the slice map shows they hold no reader. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QSeKYmTYfaPbLQD4BJfomW)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b3f7344 commit 6d9e5a1

3 files changed

Lines changed: 509 additions & 24 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -913,16 +913,29 @@ jobs:
913913
# hash in one namespace (turbo.json `globalEnv`).
914914
#
915915
# ⛔ EXCEPT a shard carrying a package in DTS_READERS, whose tests compile
916-
# against BUILT declarations and go red without them (TS7016). Measured
917-
# on 830a63a, the whole Test Core list under `OS_SKIP_DTS=1`: red only in
916+
# against BUILT declarations and go red without them (TS7016, or ENOENT
917+
# on a `dist/*.d.ts`). Measured on 830a63a, the whole Test Core list under
918+
# `OS_SKIP_DTS=1`, re-measured for the CLI on 43731031 (#25870): red only in
918919
# @objectstack/cli published-subpath-console/-hook-body.pin (its
919920
# own dist/*.d.ts), scaffold-emission-typechecks,
920-
# scaffold-test-script-passes (spec's .d.ts)
921+
# scaffold-test-script-passes,
922+
# create-in-repo-scaffold-typechecks (spec's .d.ts)
921923
# @objectstack/metadata typescript-serializer-annotation (spec's .d.ts)
922-
# A new test that reads a built `.d.ts` reds the same loud way on a
923-
# skipping shard: add its package here. Every other package's counts
924-
# matched the DTS-on run but one: spec's root-entry-type-nameability
925-
# pin skips (by its own design) and runs in lint.yml's built-dist step.
924+
# Every other package's counts matched the DTS-on run but one: spec's
925+
# root-entry-type-nameability pin skips (by its own design) and runs in
926+
# lint.yml's built-dist step.
927+
#
928+
# ⛔ A SLICED reader keeps declarations on ONE slice, DTS_READER_SLICE
929+
# (#25870). The CLI's vitest config pins its reader files into slice 1/n
930+
# (DECLARATION_READER_FILES, packages/cli/vitest.config.ts), so its other
931+
# slices read no `.d.ts` and skip them like any other shard; the
932+
# partitioner charges slice 1/n the closure and puts the whole readers
933+
# here on its shard (DECLARATION_READER_SLICES / DECLARATION_READER_PACKAGES,
934+
# scripts/partition-test-shards.mjs, whose self-test reads this step back).
935+
# A whole reader, or the CLI unsliced, keeps them as before. A new test
936+
# that reads a built `.d.ts` reds the same loud way on a skipping shard:
937+
# add its package here, or, for the CLI, its file to
938+
# DECLARATION_READER_FILES.
926939
#
927940
# Local cold closure, 65 packages, 4 cores, `--force`: 5m59s with
928941
# declarations, 1m24s without. The cost is replay: a skipping shard
@@ -931,10 +944,14 @@ jobs:
931944
- name: Choose this shard's declaration build
932945
run: |
933946
DTS_READERS='@objectstack/cli @objectstack/metadata'
947+
DTS_READER_SLICE='1'
934948
KEEP=""
935-
while read -r PKG _; do
949+
while read -r PKG SLICE; do
936950
for R in $DTS_READERS; do
937-
[ "$PKG" = "$R" ] && KEEP="$KEEP $PKG"
951+
[ "$PKG" = "$R" ] || continue
952+
case "$SLICE" in
953+
''|"$DTS_READER_SLICE"/*) KEEP="$KEEP $PKG${SLICE:+ $SLICE}" ;;
954+
esac
938955
done
939956
done < "$RUNNER_TEMP/shard-packages.txt"
940957
if [ -n "$KEEP" ]; then

‎packages/cli/vitest.config.ts‎

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,8 @@
643643
// `node_modules` exclusion: an exact-path list matches nothing it does not name.
644644
import { defineConfig } from 'vitest/config';
645645
import path from 'path';
646-
import { parseCLI, type TestUserConfig } from 'vitest/node';
646+
import { createHash } from 'node:crypto';
647+
import { BaseSequencer, parseCLI, type TestSpecification, type TestUserConfig } from 'vitest/node';
647648
import {
648649
runFilterPreflight,
649650
runProjectCliOverridePreflight,
@@ -740,6 +741,103 @@ runProjectCliOverridePreflight({
740741
// lives at its use site in the `test` block below, not in a helper binding
741742
// that could outlive the spread.
742743

744+
// #25870 — THE FILES THAT READ BUILT DECLARATIONS RUN IN SLICE 1/n, AND ONLY
745+
// THERE. Test Core builds a shard's dependency closure without `.d.ts`
746+
// (`OS_SKIP_DTS=1`, ci.yml "Choose this shard's declaration build") unless the
747+
// shard tests a package whose tests compile against BUILT declarations. In this
748+
// package exactly the files in `DECLARATION_READER_FILES` do: each runs `tsc`
749+
// over a packed tarball or a scaffolded project that resolves
750+
// `@objectstack/spec` (or this package) through `dist/*.d.ts`, and on a
751+
// declaration-less closure each reds with TS7016 or ENOENT. Measured, never
752+
// recalled: build the closure with `OS_SKIP_DTS=1`, run the whole suite, and
753+
// the red files are this list; with declarations they are green (the PR that
754+
// added this section carries both readings).
755+
//
756+
// vitest's own `shard()` scatters files by the sha1 of their path, so those
757+
// files sat in all four CLI slices and every slice shard paid the ~4.5 min
758+
// declaration closure for one or two files. `DeclarationReaderSequencer` puts
759+
// them first, then every other file in vitest's own sha1 order, and cuts that
760+
// one sequence into vitest's own equal-count ranges, so the readers take the
761+
// first places of slice 1/n and each other file moves at most a few places
762+
// along vitest's order. ci.yml then keeps declarations for slice 1/n only, and
763+
// `scripts/partition-test-shards.mjs` (`DECLARATION_READER_SLICES`) charges
764+
// that slice the closure so its shard carries less whole-package work. The
765+
// index is spelled in those three places; the partitioner's self-test reads the
766+
// other two back.
767+
//
768+
// ⛔ Checked at config load, both directions of drift:
769+
// - a listed file that is not in this run's queue population (renamed,
770+
// deleted, or moved to a nightly tier) refuses to load the config, naming
771+
// it -- a pin on a file that no longer runs would keep slice 1 holding the
772+
// closure for nothing;
773+
// - a NEW reader that is not listed lands wherever sha1 puts it. On slices
774+
// 2..n it reds its own PR, loudly and deterministically, with TS7016 or
775+
// ENOENT on a declaration-less closure: add it here. On slice 1 it passes
776+
// until a later file moves the cut past it, and then reds the same way.
777+
const DECLARATION_READER_SLICE = 1;
778+
export const DECLARATION_READER_FILES = Object.freeze([
779+
'test/create-in-repo-scaffold-typechecks.test.ts',
780+
'test/published-subpath-console.pin.test.ts',
781+
'test/published-subpath-hook-body.pin.test.ts',
782+
'test/scaffold-emission-typechecks.test.ts',
783+
'test/scaffold-test-script-passes.test.ts',
784+
]);
785+
786+
// The run's queue population holds every listed reader; a nightly-tier run
787+
// holds none of them, and the pin is inert there (its closure builds
788+
// declarations on every shard).
789+
if (readTierMode() === 'queue') {
790+
const population = new Set([...UNIT_FILES, ...INTEGRATION_FILES]);
791+
const missing = DECLARATION_READER_FILES.filter((file) => !population.has(file));
792+
if (missing.length > 0) {
793+
throw new Error(
794+
`packages/cli/vitest.config.ts: DECLARATION_READER_FILES names ${missing.join(', ')}, which this ` +
795+
'package\'s queue test population does not hold. Remove or rename the entry: a pinned reader that ' +
796+
'never runs keeps slice 1/n building declarations for nothing.'
797+
);
798+
}
799+
}
800+
801+
// vitest's equal-count range for slice `index` of `count` over `total` files
802+
// (`BaseSequencer.calculateShardRange`, which vitest declares private).
803+
function shardRange(total: number, index: number, count: number): [number, number] {
804+
const base = Math.floor(total / count);
805+
const remainder = total % count;
806+
if (remainder >= index) return [(base + 1) * (index - 1), (base + 1) * index];
807+
const start = remainder * (base + 1) + (index - remainder - 1) * base;
808+
return [start, start + base];
809+
}
810+
811+
const READER_SET = new Set<string>(DECLARATION_READER_FILES);
812+
813+
class DeclarationReaderSequencer extends BaseSequencer {
814+
override async shard(files: TestSpecification[]): Promise<TestSpecification[]> {
815+
const { root, shard } = this.ctx.config;
816+
if (!shard) return files;
817+
const { index, count } = shard;
818+
const relative = (spec: TestSpecification) =>
819+
path.relative(root, spec.moduleId).split(path.sep).join('/');
820+
// vitest's own key: the path below the root, with its leading separator.
821+
const key = (spec: TestSpecification) => createHash('sha1').update(`/${relative(spec)}`).digest('hex');
822+
const readers = files.filter((spec) => READER_SET.has(relative(spec)));
823+
const rest = files
824+
.filter((spec) => !READER_SET.has(relative(spec)))
825+
.map((spec) => ({ spec, hash: key(spec) }))
826+
.sort((a, b) => (a.hash < b.hash ? -1 : a.hash > b.hash ? 1 : 0))
827+
.map(({ spec }) => spec);
828+
const ordered = [...readers.sort((a, b) => (relative(a) < relative(b) ? -1 : 1)), ...rest];
829+
const [start, end] = shardRange(ordered.length, DECLARATION_READER_SLICE, count);
830+
if (readers.length > end - start) {
831+
throw new Error(
832+
`packages/cli/vitest.config.ts: ${readers.length} declaration readers do not fit in slice ` +
833+
`${DECLARATION_READER_SLICE}/${count}, which holds ${end - start} files. Pin fewer files, or slice fewer ways.`
834+
);
835+
}
836+
const [from, to] = shardRange(ordered.length, index, count);
837+
return ordered.slice(from, to);
838+
}
839+
}
840+
743841
// #23181 slice 14 — THE OCLIF MANIFEST SETUP. One `globalSetup` module writes
744842
// `oclif.manifest.json` from source when the `integration` project runs (the
745843
// tier every CLI-spawning file is derived into) and removes it at teardown, so
@@ -869,6 +967,10 @@ export default defineConfig({
869967
// The file-level slice, when Test Core runs one (#19278) — see the section
870968
// above `export default` for why it is spread and typed this way.
871969
...({ shard: process.env.OS_TEST_SHARD } satisfies Pick<TestUserConfig, 'shard'>),
970+
// The declaration readers into slice 1/n (#25870) -- see the section of
971+
// that number above `export default`. Read by the root context only, which
972+
// is the one that shards.
973+
sequence: { sequencer: DeclarationReaderSequencer },
872974
// A late console.* must not redden a green suite (#10374): vitest's worker
873975
// forwards console output over RPC and discards the promise, and a write
874976
// landing after teardown's rpcDone() snapshot is rejected into an unhandled

0 commit comments

Comments
 (0)