Repository navigation
test(cli): cut per-spawn cost in five migrate integration suites (#23181 slice 12) - #26207
Merged
Merged
Conversation
Four suites spawned the source entry through the tsx binary; spawn it with this process's node, as the family's other spawners do (same entry, same src/, same pinned NODE_ENV). object-scope read its fixture database through a child node per read; read it in-process with better-sqlite3. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYjtBuXuGmMz385DUttmzN
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
Contributor
Author
席位核对 · ACCEPT ·
|
| item | reading |
|---|---|
| Shape | Draft, base main, Part of #23181 (slice 12), skip-changeset, mergeable clean. |
| Files | 5 files, +61/−37, all in packages/cli/src/commands/migrate/*.integration.test.ts, which is slice 12's surface. plan.boot-parity is this family's file, not src/utils/boot-preparation-parity.test.ts (#26147). |
| Fix | Four files spawn bin/run-dev.js with process.execPath instead of through the tsx binary. That is the same entry, the same src/ and the same pinned NODE_ENV, as the family's other spawners already do. It saves about 3.4 s per spawn. object-scope writes its off-shape value and reads its state in-process through better-sqlite3 (already the sibling's import) instead of 17 node children. Every expect is unchanged. No timeout is raised and nothing is deleted. |
| Evidence | Local medians (N=3): boot-parity 63 → 41 s, telemetry-sibling 64 → 43 s, declared-default 63 → 48 s, nullable 43 → 28 s, object-scope 113 → 105 s. User CPU falls in step, so the work is removed, not packed. Two shuffle seeds both pass 27/27. 58/58 derived gates exit 0 and --ran reconciles. On this head, 36 check runs succeeded and 16 skipped as expected, with 0 failed. |
| Seat ruling on the open question | The route for the remaining rows is A, then C. A, the source-entry oclif manifest, is already a seat ruling and is in development as slice 14 (a test-only globalSetup). It is not a separate lane decision. After slice 14 lands, the next refresh reads these rows again. Any row still over 10 s takes a declared budget under ruling 6181973363 B, from that CI reading. Not B (packing removes no CPU) and not D. |
| Landing | 「按照 pm-dispatch 规则复核过的都授权」 (Leehom, 2026-10-25T11:35Z): ready and auto-merge through the relay. |
Generated by Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by Leehom · project thread
Part of #23181 (slice 12, claim 6417640032). The card stays open: this PR cuts real work in five of the eight
packages/cli/src/commands/migrate/*.integration.test.tsrows, and closes none of the 10 s rows outright. The routes that could close them are in "Not taken" below.Clause-②: no
Metadata-protocol: none
skip-changeset: a test-only diff. The five files are*.integration.test.tsunderpackages/cli/src/, and@objectstack/clipublishesfiles: [dist, README.md, CHANGELOG.md]; the build program does not compile test files.The reading used
The newest refresh with a per-file list is PR #25960 (14 hourly full-battery runs,
measure-test-shard-timings --diff); #26089 landed from the same list. This family's rows in it, CI wall per file:data-commands.absent-database40200279306)object-scope40204927516)plan.boot-parity40204927516)plan.telemetry-sibling40200279306)unmapped-columns40204927516)plan.declared-default-datasourceplan.nullable-tenant-columns40204927516)resume.recorded-by40200279306)The family's ninth row,
plan.deferred-reads(18.22 s), is left for a later slice: the claim caps a slice at about 8 files.Mechanism: the order's hypothesis does not hold here
The order's usual shape, a per-case boot, is not present. Every file already builds its fixture once, in a
beforeAllit owns or in a case that owns its fixture alone. Each file's cost is the number of CLI spawns times the per-spawn floor, run one after another. On this box (4 cores, idle, inside the verify lock):node bin/run-dev.js --version: 4.93 to 4.99 s;migrate resume --jsonon an absent database: 5.25 to 6.06 s.tsx bin/run-dev.js --version: 8.28 to 8.44 s; the samemigrate resume: 9.35 to 10.09 s.node -echild that opens the database withdriver-sqland reads it: 0.45 to 0.54 s.Most of the 5 s floor is oclif importing every command from
src/, because nooclif.manifest.jsonships for the source entry. That is product code and outside this claim.What changed
node, not thetsxbinary (plan.boot-parity,plan.telemetry-sibling,plan.declared-default-datasource,plan.nullable-tenant-columns). The entry (bin/run-dev.js), itssrc/and theNODE_ENV=developmentit pins are unchanged. This is how the family's other spawners already spawn it (data-commands.absent-database,object-scope,unmapped-columns, andresume.recorded-byforserve). Thetsxwrapper added a second process and its own transform, about 3.4 s per spawn. Fixture configs still load through the CLI's ownbundle-require.vitest-tiers.tsstill files all four as integration (they value-importnode:child_processand namerun-dev).object-scopereads and writes its fixture database in-process throughbetter-sqlite3, asplan.boot-parityandplan.telemetry-siblingalready do. It used onenodechild per read: 16readStatecalls and 1 off-shape write. Each query is the same column list andORDER BY id, and the connection is opened read-write as knex opened it.Every assertion is unchanged. The diff touches no
expect, no case and no timeout.Measured
Same box, before (
a335f1de) and after (54d51cc3), N=3 per file, one file pervitest run --project integration --maxWorkers=1underos-verify-lock. These are process wall and user CPU, which include the hook (the JSON reporter's per-file time leaves outbeforeAll).plan.boot-parityplan.telemetry-siblingplan.declared-default-datasourceplan.nullable-tenant-columnsobject-scopeCPU falls with wall, so this is work removed, not work packed.
Projection, not proof. Scaling by each file's own CI-to-local ratio (CI reading ÷ local before-median):
plan.declared-default-datasource≈ 73 s, under its declared 80 s budget, so the next refresh should stop listing it. That is the one row this PR is expected to close. The others project toplan.boot-parity≈ 76 s,plan.telemetry-sibling≈ 75 s,plan.nullable-tenant-columns≈ 42 s andobject-scope≈ 168 s, all still over 10 s.Isolation. Two shuffled runs of the five files, all green:
--sequence.shuffle --sequence.seed=23181: 5/5 files, 27/27 tests.--sequence.seed=7with an execution-order reporter: file order nullable → boot-parity → object-scope → declared-default → telemetry, andobject-scope's 20 cases ran out of definition order; 27/27 passed.The hook files' cases only read what a hook recorded.
plan.boot-parityandplan.telemetry-siblingbuild a freshmkdtempfixture per case.Not taken: the routes that would close the remaining rows
None of these is inside the claim. They are open questions on the card for the seat.
bin/run.js). Its floor was measured at 2.46 to 2.66 s against 5.45 to 6.07 s (vitest.config.ts); slice 9 reads 2.7 to 3.0 against 10.0 to 10.3 s. It changes theNODE_ENVposture and makesdist/a prerequisite, so it needs a ruling. Slice 9 raises the same question.data-commands.absent-database(a local experiment, not committed): pool 1 → 263.1 s wall, 431.9 s CPU; pool 2 → 152.4 s, 422.5 s; pool 4 → 120.7 s, 433.4 s. Wall falls while CPU stays flat, so this packs the work rather than removing it.vitest.config.tsrejects the same trade formaxWorkersbecause it spends cores that belong to the shard.plan.declared-default-datasourceandaudit-mail-bodies.archivein this directory already use it. It conflicts with the card's "runs under 10 s".Gates
All at head
54d51cc3d.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 58 commands over the 5-path change set. Each one's exit code was captured by redirecting, then reading$?, with no pipe. All 58 exit 0.--ran: "Run reconciliation — 58 derived, 58 run, 0 NOT-MEASURED, 0 UNRUN", exit 0.check-plugin-teardown-shape --self-test(shallow clone; fixed by fetching its positive control621a4876), andcheck:dual-build-cjs-loads,check:i18n,check:i18n-coverage,check:i18n-walk-parity,check:type-check-debt(no builtdist/; fixed by a fullturbo run build, 121/121 tasks).eslint --no-inline-config packages/cli: exit 0.pnpm --filter @objectstack/cli typecheck(tsc --noEmit+check:test-typecheck):VERDICT command-exit 0.vitest run --project integration(the integration tier is this diff's own layer): 5/5 files and 27/27 tests in every run above (3 ordered runs and 2 shuffled).packages/cli/distafter the build contains no test file (dist/commands/migratehas 0*.test.*).Acceptance notes
unmapped-columnshas the same reader-child shape (2 children, about 1 s of its about 99 s). It is left as it is: its readers compare BLOB-carrying rows byte for byte, which is not worth the risk for one second.data-commands.absent-database,unmapped-columnsandresume.recorded-byalready spawn throughnode. Inside this claim their only lever is B, which is not taken.🤖 Generated with Claude Code
https://claude.ai/code/session_01XYjtBuXuGmMz385DUttmzN
Generated by Claude Code