Skip to content

fix(native-chat): preserve IME composition across async updates - #17169

Merged
nwparker merged 13 commits into
stablyai:mainfrom
nwparker:nwparker/fix-16911-native-chat-ime-streaming
Aug 30, 2026
Merged

nwparker merged 13 commits into
stablyai:mainfrom
nwparker:nwparker/fix-16911-native-chat-ime-streaming

Conversation

@nwparker

@nwparker nwparker commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

While someone is composing Hangul or CJK text, the browser temporarily owns characters that React has not received yet. Orca now leaves that provisional text alone—even while a response streams or an attachment finishes uploading—and applies the settled draft and attachment exactly once afterward.

Root cause

The composer previously had two competing authorities during IME composition:

  • the browser-owned provisional textarea DOM, and
  • asynchronous React draft updates from transcript renders or attachment resolution.

When an attachment resolved during composition, React could render the application draft while the browser still held a newer preedit. Settlement then re-adopted stale DOM and erased the attachment. Whole-draft reconciliation could not determine which side was semantically newer.

The ownership boundary is now explicit: the browser exclusively owns provisional composition DOM. React owns only settled drafts, while asynchronously resolved attachment paths wait at their semantic sink.

What changed

  • Use one shared useImeEnterGestureOwnership() instance for the field, keyboard handler, and attachment path.
  • Synchronize programmatic drafts into the textarea only while composition is idle.
  • Queue already-resolved, host-readable attachment paths while the IME owns the DOM; preserve completion order and duplicates.
  • On the first settlement event, adopt the browser DOM before clearing and flushing the queue.
  • Handle both batched compositionend → blur and blur → compositionend orders exactly once.
  • Never focus from the deferred flush; blur fallback therefore cannot steal focus back.
  • Discard queued work when the composer becomes disabled and reset the entire owner when draft ownership moves to another pane.
  • Keep the existing IME Enter-gesture guard so a commit Enter cannot submit while the next deliberate Enter still works.

Linked issue

Fixes #16911

Shared acceptance checklist: #17172.

Visible Electron proof

Exact app identity, worktree, and branch were checked before exercising the real Codex Chat composer in a fresh Electron profile.

  • During active preedit, the transcript advanced from 6,393 to 6,461 characters while 出力中の変換 remained unchanged.
  • Settlement produced 出力中の変換確定 exactly once.
  • An async image paste remained at zero attachments during composition and became exactly one after settlement; the committed Japanese draft remained intact.
  • After blur fallback, document.activeElement remained BODY, proving the deferred attachment flush did not steal focus.
  • Both settlement event orders were exercised visibly.

Native Chat IME attachment settlement proof

Electron/DOM proof validates Orca's ownership contract. It does not claim observation of an operating-system candidate window.

Exact-head verification

Published head: 5eeda87c29cc4231cd3512f7c887ad81290906c4

  • pnpm install --frozen-lockfile: passed locally.
  • pnpm tc: passed.
  • pnpm tc:web: passed.
  • Native Chat suite: 88 files / 892 tests passed.
  • Final focused race suite: 4 files / 44 tests passed.
  • pnpm run check:code-quality:changed: zero findings.
  • Type-aware changed-code quality: zero findings.
  • React Doctor: zero findings.
  • Formatting and whitespace checks: passed.
  • Fresh GitHub CI run 33293924982 completed cleanly: 18 successful, 8 skipped, 0 failed or pending (including verify). Prior failures were the happy-dom OffscreenCanvas environment regression, not Native Chat assertions.

Skipped checks are the non-applicable Git compatibility, xterm patch sync, shell contracts, native-cache preparation, browser-provider, cross-version wire, managed-hooks Node 18, and e2e jobs. macOS and Windows package jobs passed.

Native-host evidence

Host Verdict Exact-head evidence Residual gap
m4air live Darwin arm64, macOS 26.6.2; frozen install, 4 files / 44 tests, and pnpm tc:web passed; worktree clean. Native CJK is unverifiable because the selected input source is ABC.
awin live Windows exact isolated worktree; 4 files / 44 tests passed and renderer TypeScript passed using the installed compiler. Full postinstall is blocked by the existing Visual Studio node-pty toolchain, so native candidate-window behavior is unverifiable.
SSH openclaw live Linux x86_64, Ubuntu GLIBC 2.39; frozen install, 4 files / 44 tests, and pnpm tc:web passed. Graphical/native behavior is unverifiable because the SSH host is headless.

No host is classified as exited merely because graphical native evidence is unavailable.

Readiness review

Exact-head readiness review found zero proven P0/P1/P2 issues; counsel was not required because there were no candidate findings.

  • SSH / remote: attachment paths are queued only after local selection, pasted-image persistence, or SSH upload has produced paths readable by the execution host. No RPC, stream frame, reconnect, or execution-host ownership contract changed.
  • Crash / cleanup: the pending queue is cleared before application, discarded on disable, and reset on pane-owner remount. There is no retry loop, timer, promise ownership change, unsafe cast, or session-lifetime accumulation.
  • Security / supply chain: no dependency, lockfile, install hook, shell, network sink, or credential surface changed.
  • Performance: no polling, broad scan, new render subscription, layout read, or persistent listener was added. Work is proportional to attachments resolved during one composition.
  • Correctness / compatibility: tests cover streaming rerenders, deletion, idle external synchronization, two concurrent SSH uploads, reverse completion order, duplicates, DOM-before-flush, both event orders, disabled discard, Enter ownership, blur fallback, and pane remount.
  • Mobile / wire / persistence: no mobile-facing source, persisted schema, IPC/RPC payload, protocol capability, PTY, xterm, provider, or Git behavior changed.
  • Cross-platform / workspace: there are no new path, shell, shortcut, or native-module assumptions; local, folder/structured, SSH, macOS, Windows, and Linux boundaries were reviewed.
  • Organization: the behavior stays in the existing composer field and attachment hooks that already own these semantics.

Code elegance

  • This fixes the ownership race at the semantic attachment sink rather than patching xterm or masking symptoms in a parent UI.
  • One stable IME owner replaces competing full-draft reconciliation; no second composition state machine remains.
  • The final attachment-race correction changes production code by 85 additions and 84 deletions: net +1 line.
  • The queue stores semantic paths, not whole-draft snapshots; it preserves order and duplicates and clears before re-entry.
  • DOM adoption precedes path application, and deferred application deliberately never focuses.
  • No unrelated dictation or xterm churn remains.

Independent root-cause/elegance audits found zero P0/P1/P2 findings and no materially simpler correct implementation.

AI disclosure

AI-assisted implementation, test construction, and review were used. Claims above are tied to deterministic tests, rendered inspection, exact-head host runs, or explicitly named residual gaps.

Agent skill upstream boundary

  • Not applicable.

Checklist

  • Root cause fixed at the browser/React ownership boundary
  • Existing IME owner and attachment pipeline reused; no parallel mechanism
  • Streaming and asynchronous attachment races covered
  • Local, SSH, folder/structured-workspace, mobile/backcompat, and platform boundaries reviewed
  • Exact-head local, Electron, native-host, package, quality, typecheck, and CI evidence recorded
  • Native graphical gaps stated without overclaiming
  • Readiness and code-elegance reviews have zero findings

@greptile-apps

greptile-apps Bot commented Aug 29, 2026 •

Copy link
Copy Markdown

Greptile Summary

The PR gives the browser exclusive ownership of provisional IME text while React retains ownership of settled drafts. It also coordinates deferred attachment insertion with the same pane-scoped composition owner.

  • Synchronizes programmatic drafts only while composition is idle.
  • Adopts settled browser text before flushing queued attachment paths.
  • Remounts composer ownership when paneKey changes.
  • Adds coverage for streaming rerenders, settlement ordering, attachment completion, disabled state, and pane transitions.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/renderer/src/components/native-chat/NativeChatComposer.tsx Shares one IME owner across keyboard, field, and attachment paths and keys pane-owned composer state by paneKey.
src/renderer/src/components/native-chat/NativeChatComposerField.tsx Converts the textarea to browser-owned provisional state and settles composition once through composition-end or blur.
src/renderer/src/components/native-chat/use-native-chat-composer-attachments.ts Queues resolved paths during composition, bounds the queue, discards it when disabled, and flushes without restoring focus.
src/renderer/src/components/native-chat/use-native-chat-external-attachments.ts Prevents new or asynchronously completed external attachments from being delivered while the composer is disabled.
src/renderer/src/components/native-chat/native-chat-composer-composition.test.tsx Covers stale rerenders, idle synchronization, deletion, Enter ownership, blur fallback, and both settlement event orders.

Sequence Diagram

sequenceDiagram
    participant U as User/IME
    participant F as Composer field
    participant O as Shared IME owner
    participant A as Attachment hook
    participant R as React draft
    U->>F: compositionstart
    F->>O: setComposing(true)
    U->>F: Update provisional DOM text
    A->>O: isComposing()
    O-->>A: true
    A->>A: Queue resolved paths
    U->>F: compositionend or blur
    F->>O: Release composition ownership
    F->>R: Adopt settled DOM value
    F->>A: Flush queued paths
    A->>R: Apply attachment references once
Loading

Reviews (8): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

@coderabbitai

coderabbitai Bot commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c05b0f87-6879-4310-ab9d-6fbd74175c7a

📥 Commits

Reviewing files that changed from the base of the PR and between d540dec and b723f90.

📒 Files selected for processing (3)
  • src/renderer/src/components/native-chat/use-native-chat-composer-attachments.test.tsx
  • src/renderer/src/components/native-chat/use-native-chat-composer-attachments.ts
  • src/renderer/src/i18n/locales/en.json

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Native chat now shares IME ownership across the uncontrolled textarea, keyboard handling, and attachment handling. Draft synchronization pauses during composition and reports the browser value when composition settles or focus is lost. Pending attachment paths queue during composition and flush after draft adoption. Disabled transitions clear attachment queues and block new insertion. The composer remounts when pane ownership changes. Tests cover composition, settlement ordering, Enter handling, attachment ordering, disabled states, pane remounting, and autogrow integration.

Merge Risk: ⚪ Minimal · up to b723f

The change keeps provisional IME text intact while streaming and attachments resolve, then applies the settled draft and attachments exactly once; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 9 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #16911 by preserving provisional Hangul/CJK composition during streaming renders, synchronizing only settled drafts, and retaining the existing IME Enter guard.
Out of Scope Changes check ✅ Passed The changes remain within the Native Chat composer and its IME, draft, and attachment handling. The added tests and localization support the reported behavior.
Title check ✅ Passed The title clearly and concisely identifies the main change: preserving IME composition during asynchronous Native Chat updates.
Description check ✅ Passed The description is complete and directly addresses the change, root cause, linked issue, visual proof, testing, AI disclosure, platform considerations, and checklist items. It provides detailed valida…
Full details: Docstring Coverage

Explanation

Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 9 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description is complete and directly addresses the change, root cause, linked issue, visual proof, testing, AI disclosure, platform considerations, and checklist items. It provides detailed validation evidence and states the remaining native CJK verification gaps.

  • Fix all pre-merge checks with AI

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker nwparker changed the title fix(native-chat): preserve IME composition during streaming fix(native-chat): preserve IME composition across async updates Aug 29, 2026
@nwparker
nwparker requested a review from brennanb2025 as a code owner August 30, 2026 04:01
@nwparker

Copy link
Copy Markdown
Contributor Author

Fresh CI on head b723f90afa30 has 10 passing, 8 skipped, and 8 failures. The seven unit-shard failures and aggregate verify all stop in xterm WidthCache because merged #17330 upgraded happy-dom to 20.11.8, whose adapter-less OffscreenCanvas returns no 2D context; this reproduces in #17330 itself and is independent of this Native Chat diff. Focused exact-head host suites remain green (m4air/awin/openclaw evidence in the PR body). Shared test-boundary repair is #17336; no xterm production workaround is being added here.

@nwparker

Copy link
Copy Markdown
Contributor Author

Reviewed — merging, with two follow-ups filed

The fix was verified in a real engine, not just happy-dom, which matters because happy-dom does not model Blink's textarea dirty-value flag and that flag is the entire mechanism here. Driving a real Chromium textarea over CDP with Input.imeSetComposition:

Also confirmed React 19 writes element.defaultValue unconditionally on every render (react-dom-client.development.js:1842), and that repeated same-value defaultValue writes do not move the caret in any dirtiness state — so streaming re-renders cannot yank it.

14 of 16 mutations killed, one equivalent, one real gap.

Two corrections to the PR's claims

"Zero proven P0/P1/P2 findings" does not survive. There is a P2 with a runnable repro — filed as #17359. The structured send clears the draft asynchronously, so a clear landing during a composition is silently dropped and onImeSettled then adopts a DOM value still holding the sent text. The composer ends up carrying a message it already sent.

Not blocking: on main the same sequence loses the new preedit instead, so this is a different failure rather than a strictly worse one, and both sit behind the same experimental Chat UI. Worth noting the obvious fix is not free — clearing optimistically before awaiting would trade this for draft loss on a rejected send, since the path guards on if (!accepted) return. The settle-side fix is the better shape.

"Never focus from the deferred flush" has no working test. use-native-chat-composer-attachments.test.tsx:211-212 asserts it, but the focus call is inside requestAnimationFrame and the test never advances a frame. Mutating the flush to applyResolvedPaths(paths, true, ...) steals focus with all 50 tests still green. A three-line rAF flush before those assertions kills it.

On size — one coherent change, correctly not split

660 lines, but ~120 are production and ~540 are tests, and the three parts are causally chained rather than bundled:

  1. The ownership fix — uncontrolled textarea plus idle-only sync. The actual [Bug]: Hangul/CJK composition is overwritten in the Native Chat composer while a turn streams #16911 fix.
  2. The attachment queue — created by part 1. Once React cannot write during composition, insertFileReferences's setDraft would be silently swallowed mid-composition, so shipping 1 without 2 regresses async attach. Fallout, not scope creep.
  3. key={paneKey} — the most separable (~30 lines), but it is what resets composition ownership across panes and removes render-phase setState the repo's own guidance discourages.

Only the NATIVE_FILE_DROP_MAX_PATHS cap is true scope creep, at ~12 lines.

On the latched flag

composing in useImeEnterGestureOwnership is a latch, not derived-and-expiring like terminal-ime-composition-tracker.ts. It is unchanged by this PR (ime-composition-keyboard-event.ts has a zero-line diff) and Chromium delivers compositionend before blur on every abandonment path tested, so it is bounded by the focus session rather than the app session — materially different from the Pinyin candidate-digit P0, where an engine switch moved no focus and emitted no events.

But this PR escalates its blast radius: a stuck composing used to only keep the Enter guard armed; now it also means React draft writes never reach the DOM. #17359 is exactly that harm materialising through a legitimate, non-stuck composition. Worth stating in the PR rather than leaving implied.

Two smaller things, not blocking

  • The PR deleted 'rescopes attachments when the scope key changes' and replaced it only with a DOM-identity assertion in a file where the attachments hook is mocked. Nothing now proves pane 2 cannot inherit pane 1's chips — true mechanically via the remount, but unguarded.
  • use-native-chat-draft.ts:19-24 still carries its render-phase scope-key reload, which key={paneKey} makes unreachable. The PR removed the two sibling render-phase adjustments and left this one.

Where this runs

No CI job anywhere exercises a real IME against this composer. The PR gate is ubuntu-only (plus package (windows)), unit tests run under happy-dom, and terminal-ime-e2e.yml — the only job with a real IME — is nightly/dispatch and targets the terminal. The Electron proof in the body and the Chromium CDP runs above are the only real-engine evidence, and neither gates this PR.

enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
…lyai#17169)

* fix(native-chat): preserve IME composition during streaming

* fix(native-chat): remount composer when draft owner changes

* fix(native-chat): reset composition across draft owners

* fix(native-chat): key pane-owned composer state

* fix(native-chat): preserve resolved attachments through IME

* fix(native-chat): bound deferred attachment paths

* fix(native-chat): add pending attachment overflow translation

* test(native-chat): publish attachment probe after render

* fix(native-chat): repair localization file newline

* fix(native-chat): use effect for attachment probe publication
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Hangul/CJK composition is overwritten in the Native Chat composer while a turn streams

1 participant