Repository navigation
🤖 perf: stop New Workspace creation controls re-rendering per keystroke - #5938
Conversation
Compiles the real useDraftWorkspaceSettings source with the shared reactCompilerConfig and checks that selectedRuntime and setSelectedRuntime keep their identity across unrelated re-renders (#5937). Fails on main. Also covers a live Settings default runtime change replacing an explicit pick. _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
Build the pure default runtime after the hook's last hook call. Built before the later useState/useRef/useEffect calls, React Compiler left it uncached, so with no runtime pick selectedRuntime and setSelectedRuntime were new on every render and CreationControls re-rendered per typed character (#5937). _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The compiler test loaded @babel/plugin-transform-modules-commonjs, which package.json does not declare. Use @babel/preset-env (declared, same module output as babel.config.js) instead. _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
@codex review Round 2: the only change since your last review is a test-only commit. The compiler test now uses the declared |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Summary
Since #5916, the New Workspace composer re-renders its creation controls on every keystroke. The nightly Perf Profiles test "perf: type in the New Workspace composer" allows at most 1 render and saw 165. This PR moves the pure default-runtime derivation in
useDraftWorkspaceSettingsbelow the hook's last hook call. React Compiler then caches it, and the controls stop re-rendering while you type.Fixes #5937
Cause
src/browser/hooks/useDraftWorkspaceSettings.ts:404-418on main (9554bb4):useState(() => defaultRuntime)withselectedRuntime = runtimePick ?? defaultRuntime(line 418), so the selection follows defaults that load after mount.defaultRuntime = buildRuntimeForMode(...)(line 404) runs before the hook's lateruseState,useRefanduseEffectcalls. In that layout React Compiler does not cache it: the compiled output callsbuildRuntimeForModeon every render.selectedRuntimeis a new object on every render.setSelectedRuntimeuses it, so the setter is new on every render too.CreationControlsthroughuseCreationWorkspaceandChatInput/index.tsx(selectedRuntime,onSelectedRuntimeChange), so each typed character re-rendered the controls.A temporary prop-diff log on a dev server (removed) showed only
selectedRuntimeandonSelectedRuntimeChangechanging per keystroke, plus two fields thatCreationControlsrebuilds or does not read.Fix
Build
defaultRuntimeandselectedRuntimeafter the last hook call (now line 451). Hook order is unchanged, and both values are read only after they are built. The PR adds no manualuseMemo,useCallbackorReact.memo. #5916's behavior stays: preferences and workspace AI picks live on the server, defaults that arrive after mount still apply, and a live Settings change still replaces a pick. The comment at the moved code names the test that guards the identity: this placement is not a permanent memoization guarantee.Product LoC: 31 (+17 / -14), in one file.
Tests
Commit 1 adds
useDraftWorkspaceSettings.compiler.test.tsx(a later test-only commit switches it to the declared@babel/preset-env, and the three results below were rerun with that version). Bun and Jest load the hook without React Compiler, so a plain render test cannot see this bug. The new test compiles the real hook source with the sharedsrc/vite/reactCompilerConfig, evaluates it with arequirebound to the hook's path (same React, stores and contexts), renders it through React, and re-renders with only an unrelated prop changed. It asserts thatselectedRuntimeandsetSelectedRuntimekeep their identity. It does not inspect compiled code or cache slots, and it changes no global Jest, Babel or Bun config.check-test-routingroutes it to the Bun unit lane.9554bb4e4f(merge base)toBeonselectedRuntime: "Received: serializes to the same string")selectedRuntimehunk reverted (scratch)Commit 1 also adds "a live Settings default runtime change replaces an explicit pick". It passes on main, and it fails when I disable the effect that applies the Settings change. The existing tests for late-arriving defaults ("applies remembered runtime options that load after mount") and explicit picks still pass (12/12).
Validation
make static-check,make check-startup-importsand the sibling tests pass.make check-react-compiler: "23/24 hot components compile (1 known skipped)".useCreationWorkspace.test.tsxhas 3 order-dependent failures in a full-file run locally. They fail the same way on origin/main and pass when run alone. This PR does not touch that file.env -i,XUM_DISABLE_TELEMETRY=1, tempXUM_ROOT): thechat-input.creation-controlsmarker recorded 13 renders for 11 typed characters on main and 0 with the fix (auto-naming off; with auto-naming on, 2 renders came from the generated name). The runtime select (Worktree → Local), the branch select (main → feature-x) and the auto-naming toggle work, and typing after those picks recorded 0 renders. Screenshots at 1200 px and 390 px are below.Perf Profiles dispatch 37921214088 (
perf_profiles=small) on the final head250f27ace3: success, 9 passed, 0 failed. "perf: type in the New Workspace composer" passed. The earlier dispatch 37914713834 on885281161ealso passed. The head changed after it, so it was rerun.58b5ea91d0(before #5916)9554bb4e4f(main)885281161e(this PR, earlier head)250f27ace3(this PR, final head)RecalcStyleCount is from each run's
perf-summary.json, one sample per run. This PR does not target it, and two samples are not enough to call a trend.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$7.86