Skip to content

Fix Korean IME composition over dim terminal placeholders - #17105

Closed
silbaram wants to merge 1 commit into
stablyai:mainfrom
silbaram:fix/codex-ime-placeholder
Closed

silbaram wants to merge 1 commit into
stablyai:mainfrom
silbaram:fix/codex-ime-placeholder

Conversation

@silbaram

Copy link
Copy Markdown

ELI5

When Korean input is still being composed, Codex has not received it yet and cannot clear its initial hint. Orca was drawing that hint again after the composing syllable. This keeps the hint covered until the IME commits, so the user sees only the Korean text being composed.

What Changed

  • Detect when every visible terminal cell after the cursor is dim-styled application ghost text.
  • Keep the ghost text span's width inside xterm's opaque composition overlay, but hide the span itself so the underlying prompt stays masked without being repeated.
  • Re-evaluate the dim-only state when a TUI repaints during an open composition.
  • Add unit and Electron CDP regression coverage for the Codex placeholder while preserving ordinary mid-line and end-of-row composition behavior.
  • Regenerate the canonical and distributable xterm patches and update the lockfile patch hash.

Why

The Codex Ask Codex to do anything prompt is real dim-styled terminal buffer content, not an HTML placeholder. Korean IME preedit text does not reach the PTY until composition commits, so Codex cannot redraw that prompt when the first syllable appears. Orca's existing mid-line composition fix copied the buffer remainder into the overlay, causing the dim prompt to be visibly appended to the Korean preedit.

Using the existing terminal style metadata keeps the fix generic to dim application ghost text and avoids coupling xterm to a Codex-specific string. Ordinary committed row content remains visible and continues to be shifted after the preedit.

Linked Issue

Fixes #17104

Related to #12545.

Visual Proof

Before:

아Ask Codex to do anything

After:

아

The Electron CDP test measures the active composition overlay against the real xterm grid and asserts that the dim placeholder is covered while the only visible overlay text is 아. A native-keyboard screenshot is not included in this draft.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Validated with:

  • pnpm test src/renderer/src/components/terminal-pane/terminal-ime-xterm-midline-preedit-tail.test.ts src/renderer/src/components/terminal-pane/terminal-ime-xterm-trailing-preedit-occlusion.test.ts — 14 passed
  • SKIP_BUILD=1 pnpm run test:e2e tests/e2e/terminal-korean-midline-preedit-occlusion.spec.ts — 3 passed
  • pnpm tc
  • pnpm run check:code-quality:changed
  • node config/scripts/regenerate-xterm-patches.mjs --check --work-dir=/tmp/orca-xterm-ime-check
  • pnpm exec electron-vite build --mode e2e

Tested through Electron CDP on Linux. Native Korean IME validation on Windows/macOS remains for reviewer confirmation. The change is renderer-side xterm behavior with no platform, filesystem, Git provider, folder-workspace, remote-wire, or SSH execution-path changes.

AI Disclosure

Implemented and reviewed with OpenAI Codex (GPT-5).

Review

Please focus on whether treating an all-dim row remainder as application ghost text is sufficiently narrow, and whether preserving a hidden span's width is the right masking behavior for xterm's opaque preedit overlay.

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

The source xterm patch was regenerated through config/scripts/regenerate-xterm-patches.mjs; the generated bundle patch was not edited by hand.

Checklist

  • This PR is small and focused
  • I explained what changed and why (including ELI5)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (targeted tests, full typecheck, changed-code quality, patch reproduction, and E2E build pass; CI will cover the full suites)

@nwparker

Copy link
Copy Markdown
Contributor

Closing as superseded: issue #17104 is fixed by merged PR #17170 (commit 17170), with acceptance coverage documented in #17172.

@nwparker nwparker closed this Aug 31, 2026
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]: Korean IME preedit repeats the Codex placeholder on first input

2 participants