fix(frontend): stop rebuilding i18n and leaking mounted trees in the test suite (#14860, #14842, #14854, #14613)

[mrveiss]

Thinking Path

Four issues describe one cluster: frontend specs crossing the 10s per-test
testTimeout on a different test each run, reddening PRs that touch none of
the code involved. #14860 names the cost — mount helpers rebuilding a ~400KB
i18n instance on every mount — and #14842 asks for the environment overhead
behind it. Both #14842 and #14854 explicitly rule out raising the global
timeout, and they are right to: a bigger ceiling keeps the cost and only raises
the load needed to trip it, while making a genuinely hung test take longer to
say so.

So the work was to remove cost, not to buy time. Reading the files turned up two
independent sources of it, plus one measurement that is not what it looks like:

1. i18n rebuilt per mount (#14860). 32 test files still constructed a fresh
createI18n — carrying the full en, and often also ar, bundle at ~400KB
each — inside the mount helper. A file that mounts 30 times re-ingested the
whole message tree 30 times for a plugin that is read-only in the test.

#14860 warns not to sweep this mechanically, and that warning holds: the config
is not uniform. Files were read individually and fall into three shapes —
static-and-never-mutated (hoist), locale-as-a-real-parameter (memoize per
locale, because a hoist would silently return English where Arabic was asked
for), and locale-deliberately-mutated (must keep building its own). All three
appear in this diff and are handled differently.

2. Nothing was ever unmounted. src/test/vitest-setup.ts resets
document.body.innerHTML before each test. That detaches a mounted tree's DOM
but never unmounts the app: component instances, watchers and effects stay live
for the rest of the file. A file that mounts 30 times ran its last test with 29
live trees still reacting to every shared ref it touched, so the per-test cost
grew as the file progressed. This is exactly the "passes alone, fails in a
larger run" shape in #14613, and OrgChart.canvasFilters.test.ts had already
diagnosed and fixed it for itself — its comment measures the accumulation at
~3.4s on a quiet runner against the 10s ceiling on a loaded one, and notes that
"no global enableAutoUnmount is configured". It was the only file that had the
teardown; every heavy file had the problem. That teardown is now global and the
private copy is removed.

The interaction matters: sharing one i18n instance per file (fix 1) makes fix 2
more necessary, not less, because a locale change on a shared instance now
reaches every tree still alive from an earlier test.

3. The "environment is 3x tests" figure in #14842 is a sum, not a ratio of
like things. Vitest resolves pool: "forks" with isolate: true (defaults;
neither is overridden here), and an isolated task never shares a runner —
vitest/dist/chunks/cli-api…js throws "Isolated tasks should not share runners"
on that path. So each of the 253 files pays its own worker spawn and its own
jsdom construction, and the reported environment total is the sum across
workers running in parallel, which necessarily exceeds wall clock and is not
comparable to the tests figure. #14842's acceptance criterion allows a written
reason instead of a changed number, and this is it. Two consequences follow, and
both are load-bearing for the other three issues:

• cross-file leakage — a leaked mock or shared module state from another file —
  cannot be the cause of test(frontend): two flaky tests are producing red checks unrelated to the change under test #14613's second flake, because files never share a
  process. The interference is intra-file accumulation (fix 2) plus contention
  from the parallel run.
• the per-test budget each spec actually gets is whatever the contended runner
  leaves it, which is why the loser is effectively random.

Where a timeout was raised, and why. Once only, per file, and stated:
ChatInterface.test.ts. All 23 of its tests render the full chat tree — sidebar,
session list, message list, composer, settings — with a Pinia store and a router.
#14613 asks whether that is a slow mount or an unresolved promise; it is a slow
mount, and the file itself proves it: every wait goes through waitFor(), which
carries its own 1s budget and fails with "unable to find element", so a 10s
failure has always meant the render was still running. There is nothing to
resolve and nothing to stub away that would not delete the coverage. It gets 20s
for that file; the global stays at 10s. The budget is per file rather than
pinned to one test because #14842 recorded a different test failing each run —
naming one would be pretending we know which.

No other timeout was raised. The 30s budget on the 200-node canvas test and the
locale warm-up in rtl.spec.ts are already in Dev_new_gui (landed with
#14826); this PR adds their measured margin rather than re-doing them.

What Changed

Cost removed — i18n (#14860), 32 files:

• New autobot-frontend/src/test/utils/i18n-cache.ts exporting
  memoizeByLocale(). Deliberately generic over the built value rather than
  typed to vue-i18n, so each call site keeps its own createI18n inference and
  its own message bundles. Documents the one case it must not be used for.
• Hoisted to a module-scope const (static config, instance never mutated) —
  20 files: WorkflowCanvas.{geometry,grid,groupKind,marquee,minimap,multiSelect,panClick,processNodeA11y,saveDialogFocus,undoRedo,zoomFit},
  OrgPeopleList.inactive, CostDashboard.{contract,rollup},
  OrgChart.{deepLink,processMutation,processNodes,toolMutation},
  RolesView.{stepCost,test}. This matches the pattern already landed in ~50
  other files rather than introducing a second one.
• Memoized per locale (the helper takes locale as a real parameter, so a
  blind hoist would be wrong) — 11 files:
  WorkflowCanvas.{contextMenu,keyboardA11y,orgMode,touch,rules,search,nodeAriaLabel,processNodeDetach,toolNodeDetach},
  OrgPeopleList.contactEdit, RolesView.operations. RolesView.operations is
  the file test(frontend): mount helpers rebuild a ~400KB i18n instance on every mount #14860 warned must not be hoisted as-is; it is one of these.
• Left per-call, deliberately — OrgChart.teamCanvas.test.ts mutates
  i18n.global.locale.value in two RTL cases. Its factory stays and those two
  cases keep building their own instance; only the shared default for the other
  seven mounts changed.
• src/test/utils/test-utils.ts: renderComponent() built a fresh instance on
  every render. Now one per module.

Cost removed — mounted trees (#14842, #14613):

• src/test/vitest-setup.ts calls enableAutoUnmount(afterEach), so every
  mounted tree is destroyed after the test that made it.
• OrgChart.canvasFilters.test.ts: its private mountedWrappers list and
  afterEach teardown are removed in favour of the global one, and its comment
  now points at it.

Timeout raised, once, with a reason (#14613, #14842):

• ChatInterface.test.ts: vi.setConfig({ testTimeout: 20_000 }) at file scope,
  with the reasoning recorded next to it. vitest.config.ts is unchanged —
  testTimeout stays 10000.

No assertion, fixture, message bundle or node count was altered anywhere in this
diff.

Verification

Constraint on the session that wrote this, stated plainly: it could not
execute the suite or install anything, so every number below is from CI on this
branch. Where a claim could not be settled that way it says so rather than
asserting.

Suite outcome — four runs, all green

run	commit	Test Files	wall	transform	setup	import	tests	environment
base 32653009997	a0202ec	259 passed	268.17s	96.00	196.90	339.28	345.81	1251.74
PR 32657637862	6498a52	259 passed	444.58s	149.31	525.83	428.72	586.10	2080.47
PR 32659232366 #1	603a17b	259 passed	210.25s	72.11	237.27	197.30	268.16	962.44
PR 32659232366 #2	603a17b	259 passed	233.51s	80.53	269.17	219.91	273.21	1075.60

The last two are the same commit, re-run — #14842's first acceptance
criterion, both green. Zero failed files in any of the four.

environment is a sum across parallel workers — measured, not argued

Run 32659232366 #1 reports environment 962.44s in a run whose wall clock was
210.25s. A per-run cost cannot be 4.58x the run. It is the sum of each file's
own jsdom construction across workers executing at the same time, which is what
pool: "forks" + isolate: true produces — and it is why the figure looks like
a multiple of tests. #14842's second criterion allows a written reason instead
of a changed number; this is the measurement behind it.

What the change is worth — and the honest limit on saying so

The per-file durations move together with the runner, in both directions. Summing
every file's own reported duration:

comparison	files this PR touched	files it did not
PR run 1 (slow machine) vs base	1.67x	1.70x
PR run 2 (fast machine) vs base	0.78x	0.76x

Touched and untouched move by the same factor each time. So these runs cannot
separate the effect of this change from runner load, and no speedup is claimed
from them. What they do establish is the absence of a regression: if removing
33 per-mount i18n constructions and adding a global unmount had cost anything
per file, the touched column would have diverged from the untouched one. It does
not, in either direction.

The work removed is provable by reading instead: 32 files went from one
createI18n per mount to one per file (or one per locale), and every
mounted tree is now destroyed instead of accumulating.

Margin against the ceiling — the thing the issues actually asked for

Slowest individual test in the entire suite, excluding the one test with a
declared budget:

run	slowest test	vs the 10s ceiling
PR run 1 (the slowest machine of the four)	5500ms	45% margin
PR run 2	1684ms	83% margin
PR run 3	2508ms	75% margin

On the loaded run — the one that came in at 1.67x base — nothing unbudgeted got
closer than 4.5 seconds to the limit.

The two files named in #14854, before (from the issue) and after:

file / test	before	run 1 (loaded)	run 2	run 3
WorkflowCanvas.connections.test.ts (whole file, 9 tests)	11476ms passing	22411ms	9806ms	9167ms
— renders 200 nodes… (declared 30s budget)	15305ms, timed out at 10s	17154ms	7669ms	6728ms
— its other 8 tests	—	all under 2445ms	—	—
i18n/__tests__/rtl.spec.ts (whole file, 18 tests)	8440ms passing	4774ms	3188ms	3835ms
— sets dir=rtl for Hebrew (he)	10043ms, timed out at 10s	under 2445ms	—	—
ChatInterface.test.ts — renders the main chat interface	timed out at 10s (#14613, #14842)	2617ms	1455ms	1966ms

rtl.spec.ts now clears #14854's "comfortably under half the limit" target with
room to spare. The 200-node case does not and is not meant to: it holds the one
declared per-test budget, and it used 17.2s of its 30s even on the slowest run —
recorded here so the next person has the number rather than a guess. Both of
those fixes landed with #14826; what this PR adds for them is the measurement.

The auto-unmount change did not disturb anything

[Vue warn] count in the job log: 347 on base, and 347 on each of the three PR
runs — identical. No new warning, including from the 19 files that unmount
their own wrappers and from the 4 that use @testing-library/vue's own cleanup.

Verified by reading beforehand, and borne out: no test file mounts inside
beforeAll (only rtl.spec.ts uses beforeAll, and it mounts nothing); Vue's
app.unmount() is guarded by isMounted so a second call cannot throw; and no
test can depend on a previous test's DOM because the existing beforeEach
already wipes document.body. Hook order is right too — the setup file's
afterEach is registered outermost and Vitest's default sequence.hooks: "stack" runs afterEach in reverse, so a file's own teardown still goes first.

Found while verifying, not caused by this PR — filed as #14879

The job log carries 23 EnvironmentTeardownError lines from
ChatInterface.test.ts, one per test: an async ProviderFallbackChip import
that outlives the environment, so those tests render an ErrorBoundary fallback
instead of the component and pass anyway. 23 on base and 23 on PR run 1; 0 on
runs 2 and 3 — so which component tree those 23 tests exercise depends on
runner load, and the suite reports the same green either way. Pre-existing on
both sides; filed rather than folded into this PR.

Reproductions, for a reviewer who can run them

cd autobot-frontend
npx vitest run src/components/__tests__/ChatInterface.test.ts
npx vitest run src/composables/llc src/components/llc src/views/llc
npx vitest run src/components/workflow/__tests__/WorkflowCanvas.connections.test.ts src/i18n/__tests__/rtl.spec.ts

The second is #14613's ordering reproduction (367 tests). It is a contention
reproduction, not a deterministic one — it did not fail on every run when it was
reported either.

Risks

• enableAutoUnmount was the one change with suite-wide blast radius. Four green
  runs and an unchanged [Vue warn] count (347 on base, 347 on each PR run)
  settle it; if it had reddened anything the right response would have been to
  drop that commit, not to widen a timeout to hide it.
• memoizeByLocale would be wrong for a call site that mutates the instance it
  gets back. The module says so, and the one file in the suite that does mutate
  is explicitly excluded and keeps its factory.
• The before/after runs are not load-matched — the runner is a contended
  singleton and the four runs span 210s to 445s of wall clock for the same 259
  files. That is why the touched/untouched control above is the load-bearing
  comparison and no speedup is claimed from the raw totals.
• The 200-node canvas test used 17.2s of its declared 30s budget on the slowest
  run. It is within budget, but the headroom is 1.75x rather than the 3x the
  number suggests at rest.

Model Used

Claude Opus 5 (claude-opus-5[1m])

Issue Link

Closes #14860, #14842, #14854, #14613

Note: closing keywords do not fire on merges into Dev_new_gui — these are
closed by hand after merge.

Changelog fragment

• N/A — internal test-infrastructure change, no user-visible behaviour

Checklist

• Code follows AutoBot patterns from CLAUDE.md
• Tests added or updated (or N/A with reason) — this PR is a change to the
  test suite; no assertion was altered
• Documentation updated if behavior changed — the reasoning lives next to the
  code it explains
• Pre-commit hooks pass (git commit runs them automatically)
• PR targets Dev_new_gui (not main)
• No secrets or credentials in the diff

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!