Repository navigation
perf(native-chat): stop re-rendering composer controls on every keystroke - #35913
Conversation
…roke (#35836) Each keystroke (and each IME compositionupdate) re-rendered the composer's action bar and TipTap editor, which re-applied editor options and re-decorated the view, even though neither had changed. The draft store also gave each write a fresh image-row array, which invalidated every image consumer. Memoize NativeChatComposerActions and NativeChatPromptEditor (initialValue only seeds the editor at mount), pass them stable handlers that call the latest committed closure, and keep the stored image row's identity when it's unchanged. Draft-store writes, onDraftChange and every consumer still see each edit synchronously, so composition semantics are unchanged. CDP-simulated pinyin IME, 4x CPU throttle, median per compositionupdate: 14.5ms -> 6.8ms (PTY composer), 14.3ms -> 7.2ms (structured composer).
 [Medium impact] The changes affect typing and draft updates in both native chat composer types. The PR appears safe to merge, with a non-blocking request to protect the typing improvement with a regression test. Findings
SummaryThis PR reduces work while typing in the native chat composer.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it('sends the composing text from a Send press that keeps focus', () => { | ||
| const onSend = vi.fn() | ||
| render(<LiveField onSend={onSend} />) | ||
| const field = textarea() | ||
| fireEvent.compositionStart(field) | ||
| changePrompt(field, 'nihao') | ||
|
|
||
| const sendButton = screen.getByRole('button', { name: 'Send' }) | ||
| expect(sendButton.hasAttribute('disabled')).toBe(false) | ||
| fireEvent.mouseDown(sendButton) | ||
| fireEvent.click(sendButton) | ||
| expect(onSend).toHaveBeenCalledExactlyOnceWith('nihao') |
There was a problem hiding this comment.
The new tests protect draft behavior, but none checks whether typing skips rendering NativeChatComposerActions and NativeChatPromptEditor. A fresh object or callback passed on every edit could silently undo this PR’s main benefit. Add a test that counts those renders across repeated non-empty edits while also checking that draft changes still publish immediately.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title "perf(native-chat): stop re-rendering composer controls on every keystroke" states the main change: memoizing the composer controls and prompt editor so they do not re-render on each keystro… Description check The description follows the repository template closely. It has an ELI5 section, a What Changed section with the mechanism, a Why section with measured before/after data and the alternatives considere… Linked Issues check The PR addresses the active linked issue [ #35836].NativeChatComposerActionsandNativeChatPromptEditorare memoized.useLatestHandlerkeeps TipTap callbacks stable while using current handlers.…Out of Scope Changes check The changed production files and tests remain within the native chat composer performance scope of [ #35836]. The newuseLatestHandlerhook supports stable TipTap callbacks. The draft-store identity …Full details: Title check
-
Explanation
The title "perf(native-chat): stop re-rendering composer controls on every keystroke" states the main change: memoizing the composer controls and prompt editor so they do not re-render on each keystroke. It names the affected area (native-chat) and the user-visible effect. It follows a conventional commit style and is specific enough for a teammate scanning history.
Full details: Description check
-
Explanation
The description follows the repository template closely. It has an ELI5 section, a What Changed section with the mechanism, a Why section with measured before/after data and the alternatives considered, a Linked Issue (Fixes
#35836), a Visual Proof section marked N/A with a reason, a Testing section with steps and automated test results, and checked boxes for the template items. Gaps: the Testing section does not name the platform actually tested (macOS, Linux, Windows, or SSH). The AI Disclosure section is blank, although the description says Claude Code generated it. The Review, Agent skill upstream boundary, and Notes sections are not addressed, and the Checklist is not ticked. Also, manual testing is unchecked, and the IME behavior was tested only in simulation, which the description states.Resolution
Optional improvements: name the platform(s) tested (for example macOS), complete the AI Disclosure section to match the "Generated with Claude Code" footer, and address or tick the Agent skill upstream boundary and Notes/Checklist items.
Full details: Linked Issues check
-
Explanation
The PR addresses the active linked issue [
#35836].NativeChatComposerActionsandNativeChatPromptEditorare memoized.useLatestHandlerkeeps TipTap callbacks stable while using current handlers. The draft store preserves image-array identity when images do not change. These changes reduce per-keystroke rendering without deferring draft writes oronDraftChange. The added composition tests cover immediate Send, blur, unmount, pasted-file, and window-hide behavior. The reported CDP IME measurements support the intended performance improvement. The issue's persistence and debounce items are suggestions, not mandatory acceptance criteria.
Full details: Out of Scope Changes check
-
Explanation
The changed production files and tests remain within the native chat composer performance scope of [
#35836]. The newuseLatestHandlerhook supports stable TipTap callbacks. The draft-store identity change supports the same render reduction. The test updates cover composition behavior and the changed asynchronous node-view timing. No unrelated product behavior or unrelated subsystem changes are shown.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 7 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
ELI5
Typing with a Chinese or Japanese input method in the native chat composer re-rendered the whole composer, including its buttons and editor wrapper, on every keystroke. Now each keystroke re-renders only what actually changed. Nothing about when the draft is saved or sent changes.
What Changed
NativeChatComposerActionsis memoized. It subscribes to UI-language changes, so translated labels still update.NativeChatPromptEditoris memoized and ignoresinitialValueafter mount, since that prop only seeds the editor.editorPropsobject, so TipTap ransetOptionsandview.updateStateon every keystroke.key={draftScopeKey}.src/renderer/src/lib/use-latest-handler.ts: stable handler identities that call the latest committed handler.onUpdate/onSelectionUpdateonce at creation, so the old code was also pinned to the first render's closures.useEffectEventcan't be passed as a prop, so this hook is needed.Every edit, including in-progress IME composition, still reaches the draft store and
onDraftChangeimmediately.Why
Measured in headless Chromium with CDP
Input.imeSetComposition, 4x CPU throttle, 66 updates per run, 3 runs. Values are the median percompositionupdate:React render time in the profile went from 768 ms to 206 ms. Ordinary typing gets the same speedup. Per #27143 this is a free win: no behavior change and nothing deferred.
Where this came from
The per-keystroke cost dates from #19616 (d7d21b2), which replaced the textarea with the TipTap editor. Since then every transaction re-rendered the composer pane, its controls and the editor options.
An earlier attempt that deferred draft publishing during composition was rejected in adversarial review. It caused five regressions: a stale Send payload, unmount overwriting a returned queued message, blur leaving edits buffered, a stale pasted-file snapshot, and quit losing the composition. This PR adds a regression test for each so that approach can't come back unnoticed.
Linked Issue
Fixes #35836
Visual Proof
N/A: no visual change. Skill and file chips now render one microtask after an edit instead of synchronously. That's still before paint, so it isn't visible.
Testing
Five new regression tests in
native-chat-composer-composition.test.tsx. They use the real draft hooks and fail against the deferral design. fix(native-chat): preserve IME composition across async updates #17169's original test is kept.native-chat-composer-workspace-file-drop.test.tsxnow awaits a microtask before reading chip DOM.Native-chat suite: 379 files / 3971 tests pass.
pnpm check:changed --maxWorkers=2: 22/22 checks pass.Adversarial reviews by Opus and Codex (gpt-6.1-sol, xhigh): both MERGE. Codex re-ran its earlier probes for all five regressions and for fix(native-chat): preserve IME composition across async updates #17169's guarantee, and all pass.
IME behaviour was simulated with CDP. A real OS IME was not tested.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
🤖 Generated with Claude Code