test(cli): attribute the suite's import term per file, and price the trade - #13872
Merged
Conversation
Comment-only: records one full-suite measurement and its per-file attribution in the config header that asks the next re-measurer to print their commit.
Contributor
📓 Docs Drift Check
What this run could not see
Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-steve
marked this pull request as ready for review
August 31, 2026 16:48
os-steve
pushed a commit
that referenced
this pull request
Aug 31, 2026
…cked workspace packages The `server.deps.external` COSTS paragraph told the next editor to re-check whether a package they externalise is mocked here, then named its census in the present tense: three targets, one of them a workspace package. There are eight targets over eleven sites, and three of them are workspace packages (`@objectstack/cloud-connection`, `@objectstack/platform-objects/plugin`, `@objectstack/lint`) — so the re-check's own starting point said that externalising a workspace package was free of mock conflicts. Comment-only. Stamped with the commit it was taken on, matching the convention this file adopted after #12499 and re-used by #13872's import-term section. Also records the case trap that makes a naive `vi\.(do)?mock\(` grep drop every `vi.doMock` site — a third of the census, silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk
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.
Part of #13504 — this PR lands the measurement half. The routing decision the card
asks for is a Definition-of-done rule, which lives on a governed surface, so it is
escalated rather than taken here; the card stays open for it.
What this changes
packages/cli/vitest.config.tsonly, and comment-only: 117 added lines, 0 deleted,0 of them outside a
//comment. No test's configuration moves, no test is skipped,renamed, quarantined or re-routed, and
pnpm testfor this package still runs the wholesuite. The file is where it lands because the file asks for it: its cost section says
"Whoever re-measures next: print your commit here" and "this file is where the next
person looking for a lever will arrive."
Positive control that the comment-only claim is real, rather than inferred from the diff:
pnpm --filter @objectstack/cli exec vitest list --filesOnlyexits 0 and enumerates thesame 220 files after the edit.
The measurement
One run,
f532630d02, through the shared verify lock, dependency closure built withturbo run build(56 tasks, exit 0):The card's baseline has drifted down (1436.54s / import 401.08s / tests 2419.71s, exit 1);
the population has not (220 files then and now, 2525 → 2529 tests). Both are shared-box
seconds — the lock excludes other locked runs, never a sibling's unlocked gate script.
The headline: the
importterm is a floor, not a hotspotThe instrument is vitest's own arithmetic rather than a proxy — the printed
importtermIS
sum(file.collectDuration)and the printedteststerm ISsum(file.result.duration)(vitest 4.1.10, the summary block in
dist/chunks/index.*.js), so per-filecollectDurationis a decomposition of the printed number. This run's per-file sumsreproduce both printed terms exactly.
The most expensive single file is 10.66s — 3.5% of the term. Half the term takes forty
files.
What that settles, on the numbers
in-process hold 3.4% of the test term. Convert all of them and the wall moves about
half a minute. Not wrong; not the lever.
with them and the wall then falls only as far as the cores allow. On this box the cores
are the binding constraint (4, at
--maxWorkers=2, beside other agents' unlocked work).(below).
One hypothesis is also killed, so nobody spends another 17-minute hold on it. The import
cost is neither transform nor bundle size; it is the per-file boundary. The control was
already inside the run, because this config externalises exactly one package and inlines
every other:
packages/types/dist/index.mjspackages/spec/dist/index.mjsThe externalised 33 KB module costs more per file than the inlined 2 MB one. This is not
a paired A/B — two different bundles, not one bundle measured both ways — so it bounds the
idea rather than settling it, and that is stated in the comment too. It is enough to
refuse "externalise more workspace
dist/" as a speed fix for this term.Where the time actually is, and why it is not waste
73.4% of the test term is 35 files that spawn the real CLI, carrying 306 of 2529 cases.
The cost is a product rather than a mystery: roughly 13 spawns in a heavy file times the
per-spawn floor this config already records (5.45-6.07s for
bin/run-dev.js, 2.46-2.66sfor
bin/run.js) is the 84-96s those files take. There is no idle time to reclaim — theharness waits on a pattern, never on a fixed sleep (
test/helpers/serve-process.ts).The one lever the attribution does surface is per-case work, not a sweep: all ten of the
heaviest spawner files spawn the source entry (
bin/run-dev.js, through tsx), whosefloor is 2.2x the built entry's, and #11707 moved three files the other way and measured
2.06x. The two entries are not interchangeable —
bin/run-dev.jspinsNODE_ENV=developmentand readssrc/, and adist/merely behind its source turns agreen run into a verdict about build state — so each file needs its own argument. The
comment names the lever and stops there.
The trade, priced
Estimated wall uses this run's own measured effective parallelism (2043.29s of per-file
work over a 1041s wall = 1.96):
*.e2e.test.ts*.e2e.test.ts13% of the cases hold 75% of the wall. This PR does not take that trade and adds no lane
script: which lane a card's Definition of done owes is an AGENTS.md rule, and no agent
seat lands one. What the section removes is the option of not knowing the price.
Corroboration from the container, at no extra cost
The shared lock's own ledger (170 records spanning 12h29m at the time of reading) says the
same thing from the other side: the largest hold in the whole ledger is this suite —
pnpm --filter @objectstack/cli test, 833s, after waiting 418s — against a p50 hold of79s and a p90 of 354s. Seven cli runs appear in that window: one ran the whole suite,
six narrowed to a hand-typed file list. The narrowing side of the card's dichotomy is
already what happens six times out of seven, ad hoc, with nothing declared.
And this PR's own measurement reproduced the card against itself: at 15:21:51Z, 708s into
the hold, a sibling seat's ticket timed out at the full 540s budget — recorded in the
ledger as
outcome=queue-timeout waited=540 held=0 rc=99. One neighbour lost a run toexit 99 so that this number could exist.
Verification
origin/mainand after the last edit, withnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands.(proven above by
vitest list), and a second 17-minute hold to re-confirm a comment isthe exact cost this card is about. Declared here rather than silently skipped.
pnpm --filter '<pkg>^...' buildfails on an UNMODIFIED origin/main — two independent observations, two different packages, same construct #13513, which is a different cause on the same lock. The ledger read forthis PR happens to corroborate it (two
pnpm --filter '@objectstack/cli^...' buildrowsat rc=1 followed by a green
turbo run buildon the same tree) — recorded, not foldedin, and [finding]
pnpm --filter '<pkg>^...' buildfails on an UNMODIFIED origin/main — two independent observations, two different packages, same construct #13513 remains open.Gate reconciliation
Derived at
1d6e3a0d97(after theorigin/mainmerge and after the last edit), harvestedwith
--commands, each exit code captured before any pipe:Both non-zero results are the gates' own NOT MEASURED code, not findings, quoted from
their output:
node scripts/check-test-completeness.mjs(exit 3) — "There is no local log to handit, so the local reading for this gate is NOT MEASURED. It is not a red, and there is
nothing here to fix." It grades a saved
turbo run testlog, which a local family rundoes not have.
pnpm check:dual-build-cjs-loads(exit 3) — "PREREQUISITE NOT MET — this gate readsbuilt output, and some package has no dist/ … This is NOT a pass: nothing was
measured." Twelve unbuilt packages, none of them this diff's; satisfying it means a
whole-repo build, which is another long hold of the lock this card is about. CI builds
first and will run it.
Named verdict lines from the two gates that actually read the edited file:
packages/clikeeps its existingKNOWN_UNALIASED_TEST_IMPORTSentry unchanged — nothinghere adds to or shrinks that ledger.
Changeset
skip-changeset. This diff publishes nothing:packages/clishipsfiles: ["dist", "README.md", "CHANGELOG.md"], and the only edited file is a vitest config, which is notin it. The counter-argument was weighed rather than assumed — every versioned package in
this repo sits in one
fixedgroup in.changeset/config.json, so a changeset for acomment-only test-config edit would bump all 71 of them.
Generated by Claude Code