Revert "Fix arrow key navigation in IME candidate window during composition (#3694)"

[austinywang]

Reverts #3694 ("Fix arrow key navigation in IME candidate window during composition," merged 2026-05-08).

Why

#3694 was the source of an arrow-key regression that broke navigation for any user with a CJK / Korean / Japanese / Vietnamese IME selected as the active system input source. Two follow-on PRs (#3767, #3836) extended the same framework and made the breakage worse. Both have been reverted (#3845, #3846), but the no-marked-text branch and broad isInputMethodSource predicate introduced by #3694 itself are still live and still suppress arrow keys for IME users empirically — confirmed by a Korean user (yoonkeee@gmail.com, locale ko-KR) who reports arrows still dead on the post-#3845/#3846 nightly (a183d44be).

What this reverts

• The no-marked-text branch of shouldSuppressGhosttyKeyForwardingAfterIMEHandling (the broad isInputMethodSource(inputSourceId) gate that suppresses any non-numpad-fallback event for any IME user when AppKit claims to have handled the event).
• shouldKeepIMECompositionCommandInsideTextInput (the 15-key navigation/control case list).
• shouldRouteTextInputKeyEquivalentToKeyDown (the window-level re-route in AppDelegate.performKeyEquivalent).
• shouldOpenZhuyinCandidatesWithSyntheticSpace and shouldRememberZhuyinCandidateInteraction (the Zhuyin candidate-window synthetic-Space + interaction tracking).
• isTraditionalZhuyinInputSource, isInputMethodSource, shouldAllowDeferredNumpadIMEFallback (helpers).
• The imeSuppressedKeyUpKeyCodes / zhuyinCandidateOpenRequested IME transient state on GhosttyNSView.
• The 8-line addition to AppDelegate.swift that funneled key equivalents through the re-route.
• All 509 lines added to cmuxTests/CJKIMEMarkedSelectionTests.swift plus the test changes in cmuxTests/CJKIMEInputTests.swift.

What this re-opens

Issue #3691 — "Chinese IME (Zhuyin/Bopomofo): arrow keys cannot navigate candidate window in terminal." Re-opening it explicitly; see the updated body there for guardrails that any future fix must satisfy so we don't re-enter this loop.

Trade

State	Affected users	Severity
Pre-revert (current main)	every Korean / Japanese / Pinyin / Cangjie / IME user	arrow keys + page/home/end/space dead globally in terminal
Post-revert (this PR)	Apple Zhuyin / Bopomofo users mid-composition only	arrow keys can't navigate the Zhuyin candidate window

Strongly positive net trade.

Conflicts resolved

Sources/GhosttyTerminalView.swift had a small conflict in the DEBUG test accessor block (lines 7056–7082) because the #3767 / #3836 reverts had already partially un-done the surrounding code. Resolved by taking the pre-#3694 version (single test accessor pair, no imeSuppressedKeyUpKeyCodesForTesting / zhuyinCandidateOpenRequestedForTesting / setIMETransientStateForTesting / debugTextInputEventHandler — all of those were #3694 additions).

Post-revert sanity grep confirms zero residual references in Sources/, cmuxTests/, cmuxUITests/ to any of the #3694-introduced symbols.

Verification target

yoonkeee@gmail.com (locale ko-KR) — the post-revert nightly cut from this branch should be the test build. Comment on #3691 (or a follow-up issue) once they confirm.

---

Note

Medium Risk
Changes core keyboard/IME event handling by removing IME-specific suppression/rerouting paths and related transient state, which could affect shortcut and arrow-key behavior across input methods.

Overview
Reverts the prior IME candidate-navigation framework by removing IME-specific command/arrow handling that suppressed forwarding even when no marked text was active, along with the window-level performKeyEquivalent reroute that funneled some key equivalents back into keyDown.

Simplifies IME forwarding decisions to only suppress terminal key forwarding when marked text/selection actually changes, and deletes related transient state in GhosttyTerminalView (e.g. suppressed key-up tracking and Zhuyin candidate-open bookkeeping).

Updates tests by dropping the large Zhuyin candidate/arrow navigation suite and switching CJK IME interception from a custom debug handler to a targeted interpretKeyEvents method swizzle.

^{Reviewed by Cursor Bugbot for commit ecdf3fc. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Reverts the IME arrow-key routing from #3694 to fix a regression that disabled arrow/page/home/end/space keys for users with CJK/Korean/Japanese/Vietnamese IMEs. Normal terminal navigation is restored; Zhuyin candidate-window arrows during composition remain unfixed and are tracked in issue #3691.

• Bug Fixes
  • Removed window-level key-equivalent reroute in AppDelegate that intercepted IME arrows.
  • Rolled back broad IME gating and Zhuyin-specific logic (synthetic Space, candidate tracking) in GhosttyNSView+IMEComposition and GhosttyTerminalView.
  • Restored pre-Fix arrow key navigation in IME candidate window during composition #3694 suppression behavior to only block forwarding when marked text or its selection actually changes.
  • Dropped IME transient state and debug hooks; deleted Zhuyin/IME tests added by Fix arrow key navigation in IME candidate window during composition #3694 and simplified remaining test swizzles.

^{Written for commit ecdf3fc. Summary will update on new commits.}

Summary by CodeRabbit

• Refactor
  • Simplified IME (Input Method Editor) composition handling for keyboard input by streamlining event routing logic.
  • Removed Zhuyin candidate navigation and synthetic-space handling from IME input processing.
  • Reduced IME suppression decision-making to core marked-text state changes.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	May 11, 2026 8:45am
cmux-staging	Building	Preview, Comment	May 11, 2026 8:45am

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR simplifies the IME key-event handling pipeline by removing early text-input key-equivalent routing, narrowing the IME suppression decision to marked-text state and accumulated text, and updating test interception to Objective-C method swizzling. Keyboard layout capture becomes conditional, marked-text clearing is guarded on non-empty state, and large test suites for Zhuyin candidate navigation are removed.

Changes

IME Key-Event Handling Simplification

Layer / File(s)	Summary
IME Suppression Helper Signature Sources/GhosttyNSView+IMEComposition.swift	shouldSuppressGhosttyKeyForwardingAfterIMEHandling removes IME event metadata, input-source parameters; now depends only on marked-text state and accumulated text. Carbon.HIToolbox import removed; test wrapper updated.
Key Event Routing Sources/AppDelegate.swift, Sources/GhosttyTerminalView.swift	AppDelegate removes early text-input key-equivalent→keyDown routing. keyDown conditionally captures keyboard layout ID only when no marked text exists. IME suppression now uses narrower marked-text tuple check replacing broader Zhuyin/event-state logic.
Marked-Text State Handling Sources/GhosttyTerminalView.swift	unmarkText() guards marked-text clearing on non-empty state. keyTextAccumulatorForTesting accessor reformatted; older Zhuyin transient-state test properties removed.
Test Instrumentation & Coverage cmuxTests/CJKIMEInputTests.swift, cmuxTests/CJKIMEMarkedSelectionTests.swift	Carbon.HIToolbox imports removed. keyEvent helper narrowed to text/keyCode/windowNumber. Objective-C method swizzling replaces debugTextInputEventHandler interception for interpretKeyEvents. Large test suites for Zhuyin candidate navigation and IME suppression edge cases deleted.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• manaflow-ai/cmux#3767: Modifies the IME suppression logic in GhosttyNSView including the shouldSuppressGhosttyKeyForwardingAfterIMEHandling helper.
• manaflow-ai/cmux#3694: Modifies the same IME/key-routing code and related test hooks for GhosttyNSView.
• manaflow-ai/cmux#3836: Modifies GhosttyNSView+IMEComposition's IME-routing logic and input-source detection for Bopomofo/Zhuyin handling.

Poem

🐰 Keys now flow with simpler paths,
No early text shortcuts or aftermath,
Marked text whispers state so pure,
Suppression gates, clean and sure—
Zhuyin tests retire with grace,
Method swizzles take their place! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.79% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically describes the main change: reverting PR #3694 which fixed IME candidate window arrow navigation.
Description check	✅ Passed	The description is comprehensive and well-structured, covering why the revert is necessary, what was reverted, trade-offs, conflict resolution, and verification targets.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch revert-3694

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This reverts #3694 ("Fix arrow key navigation in IME candidate window"), which introduced a broad isInputMethodSource predicate that suppressed arrow-key forwarding for every IME user (Korean, Japanese, Pinyin, Cangjie, …) even outside composition — confirmed broken on nightly a183d44be by a Korean user. The revert restores the pre-#3694 behavior: shouldSuppressGhosttyKeyForwardingAfterIMEHandling only suppresses when marked text is actually present and either the text or selection changed, so arrow keys are always live outside composition.

• GhosttyNSView+IMEComposition.swift — all Fix arrow key navigation in IME candidate window during composition #3694 helpers removed (isInputMethodSource, shouldKeepIMECompositionCommandInsideTextInput, Zhuyin candidate helpers, shouldRouteTextInputKeyEquivalentToKeyDown); suppression function restored to its 16-line pre-Fix arrow key navigation in IME candidate window during composition #3694 form.
• GhosttyTerminalView.swift — handleTextInputKeyEvent / inputContext.handleEvent wrapper removed; interpretKeyEvents called directly; imeSuppressedKeyUpKeyCodes, zhuyinCandidateOpenRequested, textInputCommandSelectorDuringKeyDown state dropped; unmarkText semantics unchanged.
• Test infrastructure — swizzle in CJKIMEInputTests.swift reverts from the DEBUG static hook to proper method_exchangeImplementations on interpretKeyEvents(_:); 509 Zhuyin/IME integration test lines deleted from CJKIMEMarkedSelectionTests.swift.

Confidence Score: 5/5

Clean revert of a confirmed regression — restores the narrow marked-text-only suppression path that was working before #3694 and fixes broken arrow-key navigation for all Korean/Japanese/Pinyin IME users.

Every changed line either deletes code introduced by #3694 or restores an exact pre-#3694 spelling. The post-revert suppression logic is small and easy to audit: it only fires when marked text is present and text or selection actually changed, which matches the intended invariant. Conflict resolution in the DEBUG accessor block is correct and introduces no new logic. The test swizzle switch from a DEBUG static hook to standard ObjC method_exchangeImplementations is the right approach and the pattern is implemented correctly.

No files require special attention. The largest deletion is in cmuxTests/CJKIMEMarkedSelectionTests.swift, but those tests were exclusively for the reverted #3694 behavior.

Important Files Changed

Filename	Overview
Sources/GhosttyNSView+IMEComposition.swift	All #3694 additions removed: broad isInputMethodSource predicate, no-marked-text branch, Zhuyin candidate-window helpers, and shouldRouteTextInputKeyEquivalentToKeyDown. Post-revert function body is 16 lines; only suppresses forwarding when marked text is present and text or selection actually changed.
Sources/GhosttyTerminalView.swift	keyDown reverts to interpretKeyEvents directly (drops handleTextInputKeyEvent/inputContext.handleEvent); removes imeSuppressedKeyUpKeyCodes, zhuyinCandidateOpenRequested, textInputCommandSelectorDuringKeyDown state; unmarkText behavior is functionally equivalent to pre-#3694. Conflict in DEBUG test-accessor block resolved cleanly.
Sources/AppDelegate.swift	Removes the 8-line shouldRouteTextInputKeyEquivalentToKeyDown re-route from performKeyEquivalent; existing marked-text / no-Cmd guard that follows it is untouched.
cmuxTests/CJKIMEInputTests.swift	Swizzle infrastructure switched from DEBUG static hook (debugTextInputEventHandler) to proper ObjC method_exchangeImplementations on interpretKeyEvents(_:). Standard swizzle pattern; class_addMethod/replaceMethod fallback handles both direct and inherited override cases correctly.
cmuxTests/CJKIMEMarkedSelectionTests.swift	All 509 lines of #3694 Zhuyin/IME integration tests deleted; keyEvent helper signature simplified back to (text:keyCode:windowNumber:) matching pre-#3694 shape. Remaining tests unaffected.

Sequence Diagram

sequenceDiagram
    participant W as NSWindow
    participant V as GhosttyNSView.keyDown
    participant I as interpretKeyEvents
    participant S as shouldSuppressGhosttyKeyForwarding
    participant G as Ghostty terminal

    W->>V: keyDown(event)
    V->>I: interpretKeyEvents([translationEvent])
    I-->>V: marked text state updated
    V->>S: before, after, accumulatedText
    alt marked text present and text or selection changed
        S-->>V: true - suppress
    else no marked text or state unchanged
        S-->>V: false - forward
        V->>G: sendGhosttyKey(keyEvent)
    end

Loading

_{Reviews (1): Last reviewed commit: "Revert "Fix arrow key navigation in IME ..." | Re-trigger Greptile}