Repository navigation
perf(native-chat): read the composer's text and caret straight from the editor - #28064
Conversation
…he editor Every composer read (text, caret, selection) rebuilt a per-character text-to-position map of the whole draft: 8 times per typed character and 3 per caret key. Reads now ask the editor directly (ProseMirror textBetween) and convert a text offset to an editor position with one node-level walk only when a caller sets the selection or edits. STA-7911
|
Long native-chat drafts rebuilt a character-to-editor-position table repeatedly while typing or moving the caret. This PR removes that table and reads the editor directly, reducing measured renderer work while preserving the existing draft/caret contract; it replaces #27784 without changing that PR. The full review loop is complete at Verification:
All six owned remote Electron rigs have actual graceful-exit receipts. The standalone QA driver exited, its owned shell terminals closed, the QA checkout was removed from Orca and the M4Air slot released. Local benchmark captures remain bounded; process absence without an exit receipt was treated as unverifiable, not proof of exit. The PR is ready for human review and remains unmerged. Screenshots below show the final branch fixture; assertions provide the selection and instance-isolation evidence. File attachment alongside text:
Image attachment preview alongside text:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 7 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: reading composer text and caret directly from the editor to improve native-chat performance. Description check The description is complete and directly matches the template. It explains the user impact, implementation, rationale, alternatives, linked issue reference, visual proof, testing, review status, limit… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ 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
When you type or move the cursor in a long chat draft, the composer has to know two things: the text of your draft, and where your cursor sits in it. Until now it got those by redrawing a full "character → spot in the editor" chart of the whole draft every time anything asked, several times per keypress. This PR stops drawing that chart at all and asks the editor directly instead, the way rich-text composers usually do. In the final large-draft comparison, typing used less than half the renderer work, and caret keys also used less.
What Changed
What you saw (STA-7911): in a long native chat draft (we test with about 55,000 characters and a few skill and file pills), every keypress did noticeably more work than it needed to. In the final same-host comparison, main spent 11.86 ms of renderer task time per typed character and 5.59–6.43 ms per caret key (medians). These timings are from the measured Mac only; Windows and Linux latency was not measured.
Why it happened: the composer gives the rest of the chat code a textarea-shaped view of the editor (its text, the caret as a character offset, "put the caret at offset N"). Every one of those reads rebuilt a list with one entry per character of the draft, plus the whole text one character at a time. That happened 8 times per typed character and 3 times per cursor key.
What is different now: typing and moving the cursor in a long draft require less renderer work (table below). Supported composer behavior stays the same: text, pills, cursor positions, line breaks and emoji, including modifier-selected paragraph replacement.
Mechanism:
native-chat-prompt-document.ts:promptTextMap(the per-character list) is gone. The text is ProseMirror's owndoc.textBetween(…, '\n', promptLeafText), the same read the clipboard copy already used (a hard break reads as a newline, a pill as its token). The caret offset is the length of the text before the caret. Going the other way (offset → editor position, used only when code sets the selection, inserts a skill pill, or replaces the draft) walks the draft's blocks and pills once, not its characters. An offset inside a pill's token snaps to the pill's start, or to its end when a replacement must swallow the whole pill, exactly as before.NativeChatPromptEditor.tsx: the textarea-shaped view answers from the editor through those functions. The "caret is on the first/last line" check for prompt recall uses the text before and after the caret instead of the per-character list.NativeChatComposer.tsx: each change or selection event reads the text and caret once instead of twice.Why
This follows the common pattern: the composer reads the editor directly instead of rebuilding a text copy. Direct reads still walk document nodes and construct strings; they avoid the per-character position array. The direct reads reuse editor nodes and avoid constructing an entry for every draft character. The final production-bundle comparison below measures the remaining work as well as the removed map builds.
Alternatives considered:
/,$and@pickers. Moving about ten modules and their tests to editor positions would broaden this focused change across persistence, send, recall and insertion; it is not required to remove the per-character map. The view stays; only how it answers changes.Paragraph selection parity: Cmd-click on macOS or Ctrl-click elsewhere can select a whole paragraph. Review found that a direct prefix read initially missed the newline before that selection, so redirected typing could join two lines. The final implementation uses the editor’s resolved position to include that separator, preserving the old behavior without rebuilding a character map. The regression tests select paragraphs through the actual mouse handler and exercise the production insertion hook, including an empty preceding line and emoji.
Intended edge difference: a pill with an empty token reads as zero characters, correcting the old off-by-one caret count. Normal pill creation always provides a token; behavior for empty-token restored documents is not claimed identical.
Linked Issue
Linear: STA-7911. Supersedes #27784 (same problem; this removes the per-character list instead of caching it).
Visual Proof
Final reviewed head
e217dbbwas rebuilt and measured against the original baseb40554b2dc4and that same base with #27784's cached-map patch. Nine hidden production-build runs, interleaved main/cache/direct three times, used the original workload on the same Mac laptop: 55,061 UTF-16 characters, 715 paragraphs, five skill pills and three file pills. A stand-in Codex executable opens the native chat; no prompt is sent. DevTools sends 100 presses per individual key at 60 ms intervals.The table records renderer main-thread busy time from each keydown's task to the next, including other renderer work in that window. It is not a time-to-paint measurement. p50/p95 are medians of the three per-run percentiles; parentheses show the runs' p50 min–max. Build/source/loaded-chunk hashes, raw or bounded extracted trace/coverage captures and PID cleanup records are retained with the measurement artifacts.
Typing p50 was 5.16 ms versus main's 11.86 ms (56.5% lower) and the cached comparison's 5.75 ms. Some cursor medians and p95 values are higher than the cached comparison, but every direct/cached per-run p50 and p95 range overlaps. These samples support comparable cached-build costs. The shared M5 Pro host had substantial ordinary background activity; small timing differences should be read within that spread. Windows/Linux performance was not measured.
Map builds are exact separate V8 call counts: 8 / 1 / 0 per typed character and 3 / 0 / 0 per caret key. Direct editor text reads remain: seven per typed character and two or three per caret key. These read document nodes and strings; draft JSON serialization and existing picker scans also remain. The complete key path is not traversal-free.
Hidden rendered Electron QA on the M4Air completed at the final head: 30/31 initial assertions and 13/13 continuation assertions passed. Real modifier-click paragraph replacement, pills, hard breaks, emoji, wrapped recall, redirected typing, equal-content paste, trusted file/image drops, undo and two concurrently mounted composers were exercised. Dictation-style contenteditable insertion passed; live speech capture was not exercised. The initial synthetic image-paste miss did not populate the native clipboard used by the save bridge; trusted drops passed. Screenshots are attached to the final review status comment. This is fixture-based end-to-end build evidence, not packaged-app or live-agent evidence.
Testing
Final fix: 281 tests in 28 files passed, covering document/editor parity, pills, breaks, emoji, recall, redirected typing, composition, draft restore, attachments, dictation target/segments, undo and separate editors.
The new modifier-click paragraph-selection tests failed before the fix and passed after it, including an empty preceding line and UTF-16 text.
Fresh independent review: 40 additional tests and 585-document base comparisons passed, including 7,701 document positions, paragraph selection endpoints and 34,664 replacement comparisons. Identical-text replacement still publishes a changed selection when the editor emits no update callback.
tc:webpassed at the final head; the original full native TypeScript baseline passed. Pre-pushorca-ci-checks --no-typecheckran all 23 applicable steps: 21 passed; the two known local failures were unchanged script imports and local Bun 1.3.14 unable to execute the configuredbun checkstep. No unrelated source/toolchain patch was made. Final-head hosted CI passed all 13 applicable checks, including static analysis/typecheck, five test shards, relay integration and macOS/Windows packaging; 17 jobs were skipped and provide no runtime evidence.Final nine-run production benchmark completed with exact build identity and preserved operation counts. Renderer/platform, native microphone and full live-agent/SSH evidence limits are stated separately from unit results.
Automated tests added/updated
Hidden end-to-end keyboard benchmark completed on the same laptop
Remote rendered flow QA complete
Review
Two independent review rounds, an architecture challenge, dedicated performance pass and final readiness review completed. One reachable paragraph-selection regression was fixed in
e217dbb; the fresh follow-up found no remaining source correction. The direct-read design and existing composer contract were retained. Final-head CI and remote rendered QA completed; remaining evidence limits are listed below. No source finding remains open or deferred.Agent skill upstream boundary
Notes
Renderer-only: no IPC, wire contract, paths, shortcuts or execution-host routing changed. No application text/index cache or new stored state was added. Draft-document serialization and existing picker scans remain, and are included in the key-window measurements. Actual Windows/Linux/SSH runtime, microphone capture, paint latency and heap behavior were not measured by this review.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)