From f128ec62d03187a4ee19bab3102d0a04cfcc61b4 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 13 Sep 2026 00:13:39 -0400 Subject: [PATCH 1/3] docs: record why maxWorkers stays unset, with the measurement (#2336) No Vitest project sets `maxWorkers`, so every one inherits Vitest's `availableParallelism() - 1` (7 on the team's M3). #2336 asked whether that number should be stated or capped. Measured at the leased baseline (#2339), interleaved default / --maxWorkers=4 / default, three rounds each: a cap of 4 costs +14% wall on the web unit suite and +39% on coverage:web, saves 18-22% CPU, and prevents zero failures (0 of 18 runs either way). Against a sibling session's concurrent unit run it is a small politeness trade with no throughput gain; the only failures seen were one test's 5s inner waitFor at 14 workers, which is a site to fix rather than a reason to cap. So the default is kept, as a chosen value: the reasoning and the numbers sit beside TIMEOUTS in vitest.shared.mts, and AGENTS.md carries the rule so a reviewer can cite it against a future per-project cap. CI is unaffected. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PsmUAZ6aLUAFqenjJKSSRp Signed-off-by: cliffhall --- AGENTS.md | 9 +++++++++ vitest.shared.mts | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 082d2813..deb67323 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -457,6 +457,15 @@ free to run a full `local:gate`). Raising a budget nobody chose hides no race. project, reading the value Vitest resolved for the test — so a per-test option, a `describe` option, a project setting and a `--retry` flag all land on the same check. +- **`maxWorkers` stays unset — on measurement, not by omission (#2336).** Every + Vitest project inherits `availableParallelism() - 1`. Capping it to 4 was + measured at the leased baseline as +14% wall on the web unit suite and +39% + on `coverage:web` for zero fewer failures (0 of 18 runs either way), so the + cap is a permanent solo-run cost. The only thing it moved was one test's 5s + inner `waitFor` when two sessions ran suites at once — a site to fix, not a + reason to cap. The numbers, the sibling-session trade and what would reopen + the question are on the comment in `vitest.shared.mts`; do not add a + per-project cap without re-running that measurement. - **The Playwright locator budgets the web smokes use are named constants in `scripts/lib/browser-timeouts.mjs`** — raise one there, not at a call site. The *remaining* smoke-helper budgets and CI's missing `timeout-minutes` are diff --git a/vitest.shared.mts b/vitest.shared.mts index 5566a762..b9c49c9f 100644 --- a/vitest.shared.mts +++ b/vitest.shared.mts @@ -78,6 +78,55 @@ export const INTEGRATION_TIMEOUTS = Object.freeze({ testTimeout: 30_000, }); +/** + * `maxWorkers` is deliberately unset, here and in every project (#2336). + * + * Each project therefore inherits Vitest's non-watch default, + * `availableParallelism() - 1` — 7 on the eight-logical-core M3 this team + * works on. Keeping it is a choice, not an omission, and this is the + * measurement it rests on. On `v2/main` @ `f16a51d4` — after the gate lease + * (#2339) — with the arms interleaved (default / `--maxWorkers=4` / default), + * three rounds, one worktree, nothing else of ours running: + * + * web `unit`, 345 files default 54.9s wall 378s CPU + * 4 62.4s wall 294s CPU +14% wall, -22% CPU + * web `test:coverage`, 422 default 88.0s wall 461s CPU + * 4 122.5s wall 376s CPU +39% wall, -18% CPU + * failures across all 18 runs 0 + * + * So a cap is a permanent solo-run cost — ~35s on every `coverage:web`, a + * tenth of the whole gate — bought against a flake that did not occur once + * at the baseline every other #2338 number is tuned to. The CPU a cap gives + * back matters only when something else wants the cycles, and since #2339 + * that is no longer another gate: gates serialize under + * `scripts/gate-lease.mjs`. What remains is non-gate work in a sibling + * session. Measured against a concurrent bare `vitest run --project=unit` + * in the same worktree (5 / 3 / 3 pairings): with the sibling at the + * default, capping our run cost us +8% and gave the sibling 14% back; with + * both capped, both sides ran at ~100s — the same total throughput as both + * at the default (99s + 92s), because eight cores are saturated either way. + * The one thing a cap moved was failures: two of the ten 7-vs-7 runs lost + * `AppRenderer.test.tsx`'s theme-flip case to its 5s inner `waitFor` at 14 + * workers; none of the twelve runs with a cap on either side did. That is a + * single test's inner budget starving under two sessions' worth of workers + * — #2338's aspect 3, fixable at that site — not a case for taxing every + * solo run. + * + * Why not cap only locally, via an env var this file reads and `local:gate` + * sets? CI never runs `local:gate`, so CI would pay nothing — but every + * budget above is wall-clock, so a purely scheduling difference between the + * local gate and CI can flip an outcome, and the gate's whole promise is + * that passing it here means CI passes. One configuration in both places is + * the property worth keeping. CI's runners resolve the same expression + * against their own core count, so leaving it unset costs CI nothing. + * + * Reopen this if the lease goes away, or if `Test timed out` recurs on a run + * whose only contention is a sibling session — the 4-vs-4 figures above are + * the starting point. Whatever is committed then is a *chosen* number: macOS + * exposes no CPU affinity API, so no cap pins a worker to a performance core + * and none is "one per P-core". + */ + export function vitestSharedPaths(clientDir: string) { const dirname = path.resolve(clientDir); const repoRoot = path.resolve(dirname, "../.."); From 1c243377bedae1e573c883d17589d012ad7983e7 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 13 Sep 2026 00:25:07 -0400 Subject: [PATCH 2/3] docs: state the Storybook browser-pool exception (#2336) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review round 1: "every project inherits availableParallelism() - 1" overstated the scope. The five forks-pool projects do; web's storybook project runs the browser pool, whose default is min(12, availableParallelism() - 1) — the same 7 on this machine, but a different expression, capped on larger machines and 1 unless headless — and none of the arms measured it. A maxWorkers set on that project would still apply (getThreadsCount reads it first), so it is not exempt from the decision, only unmeasured; both files now say so. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PsmUAZ6aLUAFqenjJKSSRp Signed-off-by: cliffhall --- AGENTS.md | 8 ++++++-- vitest.shared.mts | 17 ++++++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index deb67323..2780e797 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -457,8 +457,12 @@ free to run a full `local:gate`). Raising a budget nobody chose hides no race. project, reading the value Vitest resolved for the test — so a per-test option, a `describe` option, a project setting and a `--retry` flag all land on the same check. -- **`maxWorkers` stays unset — on measurement, not by omission (#2336).** Every - Vitest project inherits `availableParallelism() - 1`. Capping it to 4 was +- **`maxWorkers` stays unset — on measurement, not by omission (#2336).** The + five forks-pool projects inherit `availableParallelism() - 1`; web's + `storybook` project runs the browser pool, whose own default is + `min(12, availableParallelism() - 1)` and which was not measured (a cap set + there would apply, so it is not exempt — it just needs its own numbers). + Capping the forks pool to 4 was measured at the leased baseline as +14% wall on the web unit suite and +39% on `coverage:web` for zero fewer failures (0 of 18 runs either way), so the cap is a permanent solo-run cost. The only thing it moved was one test's 5s diff --git a/vitest.shared.mts b/vitest.shared.mts index b9c49c9f..fb7754ec 100644 --- a/vitest.shared.mts +++ b/vitest.shared.mts @@ -81,9 +81,20 @@ export const INTEGRATION_TIMEOUTS = Object.freeze({ /** * `maxWorkers` is deliberately unset, here and in every project (#2336). * - * Each project therefore inherits Vitest's non-watch default, - * `availableParallelism() - 1` — 7 on the eight-logical-core M3 this team - * works on. Keeping it is a choice, not an omission, and this is the + * The five forks-pool projects — web `unit` and `integration`, `cli`, `tui`, + * `launcher` — therefore inherit Vitest's non-watch default, + * `availableParallelism() - 1`: 7 on the eight-logical-core M3 this team + * works on. Web's `storybook` project is the exception: it runs the browser + * pool, whose default is a different expression, + * `min(12, availableParallelism() - 1)` — also 7 here, but capped on larger + * machines because the main thread chokes past ~12 browser workers + * (vitest#7871) — and 1 unless the run is headless with file parallelism. A + * `maxWorkers` set on that project *would* apply to it (`getThreadsCount` + * reads it before falling back), so it is not exempt from this decision; + * but nothing below measured it. The numbers are the forks pool's, and a cap + * for Storybook would need its own measurement. + * + * Keeping the default is a choice, not an omission, and this is the * measurement it rests on. On `v2/main` @ `f16a51d4` — after the gate lease * (#2339) — with the arms interleaved (default / `--maxWorkers=4` / default), * three rounds, one worktree, nothing else of ours running: From 99d6b193d9bdc04cc23242ff855651f5702e5205 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 13 Sep 2026 00:40:12 -0400 Subject: [PATCH 3/3] docs: state both worker defaults with their lower clamp (#2336) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review round 2: both expressions omitted Vitest's `max(…, 1)`, so as written they described zero workers on a one-core runner. Now `max(availableParallelism() - 1, 1)` for the forks pool and `max(min(12, availableParallelism() - 1), 1)` for the browser pool, and the browser pool's single-worker condition names all three of its terms (headless, file parallelism, provider support). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PsmUAZ6aLUAFqenjJKSSRp Signed-off-by: cliffhall --- AGENTS.md | 4 ++-- vitest.shared.mts | 13 +++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2780e797..bf5a4415 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -458,9 +458,9 @@ free to run a full `local:gate`). Raising a budget nobody chose hides no race. option, a `describe` option, a project setting and a `--retry` flag all land on the same check. - **`maxWorkers` stays unset — on measurement, not by omission (#2336).** The - five forks-pool projects inherit `availableParallelism() - 1`; web's + five forks-pool projects inherit `max(availableParallelism() - 1, 1)`; web's `storybook` project runs the browser pool, whose own default is - `min(12, availableParallelism() - 1)` and which was not measured (a cap set + `max(min(12, availableParallelism() - 1), 1)` and which was not measured (a cap set there would apply, so it is not exempt — it just needs its own numbers). Capping the forks pool to 4 was measured at the leased baseline as +14% wall on the web unit suite and +39% diff --git a/vitest.shared.mts b/vitest.shared.mts index fb7754ec..7eba9e4b 100644 --- a/vitest.shared.mts +++ b/vitest.shared.mts @@ -83,12 +83,13 @@ export const INTEGRATION_TIMEOUTS = Object.freeze({ * * The five forks-pool projects — web `unit` and `integration`, `cli`, `tui`, * `launcher` — therefore inherit Vitest's non-watch default, - * `availableParallelism() - 1`: 7 on the eight-logical-core M3 this team - * works on. Web's `storybook` project is the exception: it runs the browser - * pool, whose default is a different expression, - * `min(12, availableParallelism() - 1)` — also 7 here, but capped on larger - * machines because the main thread chokes past ~12 browser workers - * (vitest#7871) — and 1 unless the run is headless with file parallelism. A + * `max(availableParallelism() - 1, 1)`: 7 on the eight-logical-core M3 this + * team works on. Web's `storybook` project is the exception: it runs the + * browser pool, whose default is a different expression, + * `max(min(12, availableParallelism() - 1), 1)` — also 7 here, but capped on + * larger machines because the main thread chokes past ~12 browser workers + * (vitest#7871) — and a single worker unless the run is headless, file + * parallelism is on, and the provider supports it. A * `maxWorkers` set on that project *would* apply to it (`getThreadsCount` * reads it before falling back), so it is not exempt from this decision; * but nothing below measured it. The numbers are the forks pool's, and a cap