Repository navigation
perf(agent-pane): defer mock fixtures and preload startup module - #33740
Conversation
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Gallery before/after for
States
Pick other states with a Passes: CI passes on CI passes on Written by red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on 45a1cec to compare; do not merge until green; see lane-rules Shipped in:
|
teamleaderleo
left a comment
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (claude-opus-5-5): SHIP at a52ec71
Review (subagent claude-opus-5-5 medium): SHIP at a52ec71
- The real connect path is unchanged: the await only happens for the mock transport, so native and bridge handshakes start in the same turn.
- A failed mock import lands in connectHost's existing catch and retry. The new cancelled check stops a pane that was torn down from connecting.
- The debug fixture APIs were already async.
- The modulepreload uses the same URL and credentials mode as the module script, never runs it, and works in a production build.
- The tests check real behaviour, including that the mock and fixture code are missing from the eager chunk graph.
Follow-ups:
- [follow-up] The startup-bundle test only checks the mock transport and the shared fixture. Add a sentinel string for each of streamDebug, loadDebug, synthetic and the other lazy fixtures so none of them creeps back into the eager code.
- [follow-up] The modulepreload has no before/after number; the preload scanner may already find pane.js early. Measure it, or drop it if it shows no gain.
- [follow-up] A failed mock-chunk import shows as a generic handshake error and retries forever (dev/mock page only). Name the failure.
|
Review (codex, router-review): SHIP at a52ec71 Reviewer: codex gpt-6.1-sol xhigh, fast tier. Follow-ups: 1. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward. SHIP at a52ec71 No correctness bugs found. Source import-graph, TS/TSX parsing, and shell syntax checks passed; Bun tests and native measurements remain unverified.
Follow-ups:
|
|
Found 10 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
a52ec71 to
2398093
Compare
GitHub Actions
Inactive
GitHub Actions
Inactive
teamleaderleo
left a comment
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (claude-opus-5-5): SHIP at 2398093
Review (claude-opus-5-5 medium): SHIP at 2398093
This carries Opus review 5751700375 from a52ec71. 2398093 is a rebase onto 2a48ef7 (with #33741). git diff eb7026a1..a52ec71d and git diff 2a48ef76..23980936 have the same stable patch-id (21c51e8e…), so the reviewed change is unchanged.
Follow-ups: same as 5751700375 (sentinels for the other lazy fixtures; measure or drop the modulepreload; name the error when the mock-chunk import fails).
GitHub Actions
Inactive
teamleaderleo
left a comment
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (claude-opus-5-5): SHIP at 521e355
Review (claude-opus-5-5 medium): SHIP at 521e355
This carries review 5751865046 from 2398093. Reviewed scope: 2398093..521e355, 7 lines in 2 files.
- gallery/frame/agentPane.ts: awaits
import(".../acpmux/mock")before mounting main only whenstate.ready.transport === "mock". That warms the same module App.tsx's connectHost later imports, so the fixture snapshot can't race a cold chunk. Preview-transport variants are unchanged. - acpmux/dev.tsx: awaits the mock import at top level, inside the existing
?mock-without-host branch only. dev.tsx is the dev server's entry, and build-agent-pane-web.sh only bundles it for sheet order (--css-inputs-only, esm, so top-level await is fine). The production pane entry and its eager graph don't change, and the import stays dynamic, so the mock code stays in a lazy chunk.
Follow-ups: same as 5751700375.
GitHub Actions
Inactive
teamleaderleo
left a comment
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (claude-opus-5-5): SHIP at 7adb04b
Review (claude-opus-5-5 medium): SHIP at 7adb04b
This carries review 5752396282 from 521e355. Reviewed scope: 521e355..7adb04b, dev.tsx only. The same mock pre-import moves from inside the else if branch to just before await import("./main"). Its condition (!devHost && ?mock && no agentSession handler) matches the old branch exactly, and it still runs before main loads, so the behaviour is the same. The gallery preload and the production entry are unchanged.
Follow-ups: same as 5751700375.
GitHub Actions
Inactive
teamleaderleo
left a comment
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (claude-opus-5-5): SHIP at 9f1ce1b
Review (claude-opus-5-5 medium): SHIP at 9f1ce1b
This carries review 5752474669 from 7adb04b. Reviewed scope: 7adb04b..9f1ce1b (4 files, +23/−4).
- New mockTransport.ts: a module-level slot holding the mock module. It references
./mockonly as a type (typeof import("./mock")), so nothing from the mock enters the eager graph. The +41 bytes is the getter. - dev.tsx and gallery/frame/agentPane.ts: after awaiting the mock import on the existing mock conditions, they fill the slot. dev.tsx keeps main's import first in source order, which preserves the stylesheet order derived from it.
- App.tsx connectHost: in the mock branch it reads the prepared module synchronously, and falls back to the dynamic import. So the ready reply and the connect stay in one turn with no microtask yield, which is the actual cause of the mock layout regressions. Native and bridge paths never take the mock branch, so production is unchanged. A non-mock gallery variant after a mock one never reads the slot, because it's only read when
mockis set.
The fleet run 41643000975 shows the four mock regressions before and the 10 cases within limits after.
Follow-ups: same as 5751700375.
GitHub Actions
Inactive
teamleaderleo
left a comment
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (claude-opus-5-5): SHIP at e0ea71b
Review (claude-opus-5-5 medium): SHIP at e0ea71b
Visual verdict: unchanged by this diff (test classifier only).
Carried from my SHIP at 9f1ce1b. The delta 9f1ce1b..e0ea71b is one line in webviews/test/pane-english.ts: it adds debugFixtures to SKIP beside debug. That clears the one red this PR introduced in webview run 41642551521 (pane-english flagged the new lazy debugFixtures.ts re-exporting the not-shipped fixtures). On base, debug.ts imported those fixtures directly and was already skipped. No production change. The remaining session-info-overlay-toggle reds are labelled base by the verdict job.
Follow-ups: none.
The idle probe loads on demand like the other debug fixtures (#33740). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-cb01117327fc358e9ba1
Native chats eagerly loaded the mock daemon and synthetic debug transcripts. Load those modules only when a mock page or an existing async debug command needs them, reducing eager JavaScript by 76,592 bytes (1.59%). Preload the module before locale scripts and styles; evaluation still follows localization.
Leo: “things should be instant and not shift layouts AND not take many resources”. This reduces bytes on the open-to-page-painted path. It does not establish a startup timing improvement or meet the under-300 ms goal by itself.
Mock gallery/dev entries prepare the daemon before mounting, preserving the ready handshake's synchronous ordering. Native connection ordering stays unchanged. The localization scanner classifies the extracted debug-fixture barrel alongside the existing debug helpers. No timers, observers, loops, warm browser, or persistent idle activity are added. No new idle memory is reserved; runtime RSS was not measured for this bundle change.
Validation at final head
e0ea71b8eaa2f7faacdee2419bb9309cc06350f0:9f1ce1b1passes 20 focused tests, full web formatting/lint/types/protocol checks, and ten light/dark gallery cases. The final delta is only the test scanner's exemption.Performance against base
2a48ef76ce442a85d2c83af440b49d13726a87d3:Both native arms used
scripts/measure/chat-open.sh --conditions=idle --cases=big,bloated --runs=3 --turns=10000; six opens completed per arm without timeout or flash. The after arm is intermediate head23980936ffe3ecb3897670a2584d58462823812c, not the final head. Most timing difference occurred before navigation. These observations support no causal speedup or regression claim; the demonstrated improvement is fewer eager bytes.Final red attribution:
PullRequestTiers.test_every_routed_input_triggers_the_workflowfails on the same fourskills/cmux-customizationpaths in the completed exact-base job at4575ff48a5502d89384180a933114a5a3eea4e90. Test, workflow, router and input blobs match the PR merge.4575ff48and merge0a34ea94trees. All 177 emitted CSS files are byte-identical (2,441,026 bytes). Five light/dark passes per arm reproduce both exact failures on base:new-tab#suggestions-chat-url, shift 0.0473 against 0.03673 while typing; andcomposer-context#started-browse-error, 5,000 ms alert timeout. Both also pass in other runs. These scenes execute unchanged preview/component branches, not the changed mock/debug branches.Nonblocking follow-ups: #33792 (broader lazy-module sentinels), #33793 (isolate preload benefit), #33794 (mock-load diagnostic).
Agent session: agent session