Repository navigation
🤖 refactor: move ChatInput control flow React Compiler can't lower into plain helpers - #4425
Merged
Merged
Conversation
ThomasK33
added this pull request to stack #4430
September 24, 2026 11:31
ThomasK33
changed the title
ThomasK33
marked this pull request as ready for review
September 24, 2026 11:55
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. |
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Sep 24, 2026
## Summary `ChatInputInner` now compiles under React Compiler, and the guard's `KNOWN_SKIPPED` baseline drops it. The audit shows no `CompileError` for it. ## Background After coder#4425, three kinds of blockers were left: a `react-hooks/exhaustive-deps` suppression, ref writes during render, and manual `useCallback`/`useMemo` calls whose dependencies the compiler could not preserve (the compiler skips the whole component when it can't keep an existing manual memo). ## Implementation - **Edit-mode effect (lint suppression removed).** It used to depend only on `editingMessage?.id` and hid the draft callbacks from its deps. It now lists all deps and uses an applied-edit-id ref, so it still applies the edit draft once per edit target and never clobbers in-progress edit text. The ref resets when editing ends, so re-editing the same message re-applies it, as before. - **Render-time ref writes → layout effects.** `editingMessageIdRef` and `handleSendRef` are now synced in `useLayoutEffect`. Layout effects run in the same task as the commit, so no async callback can observe the old value after the commit, which is the guarantee the existing comment asked for. - **Manual memos removed** where the compiler rejected them: `handleToastDismiss`, `idleCompactionProps`, `setPreferredModel`, `cycleToNextModel`. The compiler memoizes these now. This follows AGENTS.md ("don't hand-memoize") and adds no new manual memoization. - The global keydown listener reads `cycleToNextModel` through a latest-value ref instead of re-subscribing when it changes (same pattern as `composerApiRef` in this file). ## Risks Medium-low. Edit-mode entry and the global model-cycle shortcut are the behavior-sensitive paths; both keep their semantics. The memoization is now compiler-managed. ## Validation - Guard: `10/18 hot components compile (8 known skipped)`. - `tests/ui/chat`, `tests/ui/compaction`, ChatInput unit tests, `useAIViewKeybinds` tests pass. - Perf (Electron perf e2e, local, serial, 3 runs each): see Measurements. ## Measurements Electron perf e2e (`tests/e2e/scenarios/perf.*.spec.ts`, `XUM_PROFILE_REACT=1`), local, serial, 3 runs each, medians. Baseline is `main` at 349657d; "after" is the top of this stack (coder#4429), because the layers were measured together. React time is the summed `actualDuration` of the profiled subtrees. | Scenario | Profiled subtree | main | stack top | | --- | --- | --- | --- | | Open chat with a 1000-file review | `chat-pane.transcript` | 388 ms | 292 ms | | Open chat with a 1000-file review | `chat-pane.input` | 76 ms | 61 ms | | Open chat with a 1000-file review | all profiled | 530 ms | 424 ms | | Open workspace, large history | `chat-pane.transcript` | 331 ms | 295 ms | | Open workspace, large history | `chat-pane.input` | 52 ms | 46 ms | | Open workspace, medium history | `chat-pane.transcript` | 206 ms | 191 ms | Commit counts are unchanged; the gain is less work per commit. Typing in a large chat and the review hunk specs show no change beyond noise. ChatInput alone, measured before coder#4418 landed: New Workspace typing went from 165 `CreationControls` renders per run to 2. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$4.05`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=4.05 -->
This was referenced Sep 24, 2026
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Sep 25, 2026
## Summary `ImmersiveReviewView` now compiles under React Compiler and leaves the guard's `KNOWN_SKIPPED` baseline (17/18 hot components compile). Iterating hunks in immersive review spends about 18% less script time. ## Background Follow-up to the React Compiler coverage stack (coder#4422, coder#4425–coder#4429) and coder#4439. The compiler skipped the whole component because of `??=`, a `try/catch/finally` in the copy-file handler, three render-time ref writes, and one ref read during render. ## Implementation - `oldLineMap ??= …` becomes `oldLineMap = oldLineMap ?? …`. - The copy-file handler's `try/catch/finally` uses `runWithCatchFinally` with the same body, error handler, and cleanup. Early `return`s keep their meaning because the `try` was the handler's last statement. - `activeFilePathRef`, `activeFileContentVersionRef`, and `handleCopyFileRef` are synced in `useLayoutEffect` instead of during render. Layout effects run in the commit's task, before passive effects, which is the ordering the existing comment requires for `isStale()`. - `selectedLineSummary` (render path) read `hunkJumpLineRangeRef` through `getCurrentLineSelection`. A state mirror, `hunkJumpLineRange`, now feeds render. Every ref write also sets the state, next to the cursor/selection updates it batches with, and the hot-path callbacks keep reading the ref, so their identities stay stable. One visible difference: after a comment submit or composer cancel that clears the hunk-jump range while no line selection is active, the "Lines …" label now updates right away. Before, it kept showing the cleared range until the next render. ## Validation - Guard: `17/18 hot components compile (1 known skipped)`; `make static-check` green. - `src/browser/features/RightSidebar/CodeReview` unit tests (79) and `tests/ui/review` (14) pass. ## Measurements Electron perf e2e, local, serial, 5 runs each, `main` at 334a5a6 vs this branch. ScriptDuration per run: | Scenario | main (sorted runs) | this PR (sorted runs) | Median | | --- | --- | --- | --- | | Immersive hunk iteration, 1500 lines / 150 hunks | 343, 351, 355, 356, 440 ms | 270, 273, 290, 294, 299 ms | 355 → 290 ms | | Immersive mark-read, 1000 files | 14, 16, 18, 19, 19 ms | 14, 14, 20, 25, 26 ms | 18 → 20 ms (noise) | Hunk-iteration wall time went from 448 to 376 ms (median). The regular review reopen scenario, which doesn't render this component, moved about 10% between runs, which is this host's noise level. The hunk-iteration ranges don't overlap. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$4.05`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=4.05 -->
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
1 participant
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.
Summary
First of two PRs that let React Compiler compile
ChatInput. This one moves control flow the compiler can't lower out of the component body, without changing behavior.ChatInputstill stays in the guard'sKNOWN_SKIPPEDbaseline until the next PR (#4426) removes its last blockers.Background
React Compiler 1.0 skips a whole component when its body, including nested callbacks, contains
try/finally,trywithoutcatch,for await,??=, or a conditional/logical/optional-chaining expression insidetry/catch.ChatInputhad all of these, so it was never compiled and got no memoization. #4422 added the coverage guard; this stack works through its baseline.Implementation
src/browser/utils/compilerSafeControlFlow.tswith three plain helpers:runWithCatch,runWithFinally,runWithCatchFinally. The compiler compiles only components and hooks, so thetrylives in these helpers.tryblocks inhandleSendbecome named async closures at the same indentation (const runWorkspaceSend = async () => { … }; await runWithFinally(runWorkspaceSend, …)), so the diff stays reviewable. Earlyreturns keep their meaning because each convertedtrywas the last statement of its scope.try/finallynow returns a result object instead of assigning outerlets (destructuring assignment into captured variables is also unsupported).for awaitloop moves to the module-levelforEachProviderConfigChange.sendResolutionAbortRef.current ??= new AbortController()becomes an explicit read and write.Risks
Low. Every converted block keeps its body, catch, and cleanup code; the only timing difference is one extra microtask before a
finallycleanup runs, from the helper'sawait. Areas touched: message send (workspace and creation), edit-send, the preference-persistence wait before send, workflow run card projection, and voice-input provider availability.Validation
make static-check; guard:9/18 hot components compile (9 known skipped)(unchanged at this layer).tests/ui/chatandtests/ui/compaction(19 suites, 70 tests) andsrc/browser/features/ChatInputunit tests pass on the stack.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$4.05