[xungua]

ELI5

IME preedit covers placeholder hints cleanly.

What Changed

Mask wrapped placeholders with the terminal background; preserve trailing user text. Add discriminating wrap tests and remove duplicate repaint reads.

Why

Placeholder hints must not be treated as user-authored input.

Linked Issue

Fixes #17104.

Visual Proof

Isolated Electron/xterm fixture with CDP composition input. Before recreates the old mask adapter on the patched runtime; after uses the final adapter. Synthetic content, not full-app screenshots.

Before

After

Testing

427 tests passed on macOS; reverting either reviewed fix fails its regression. Renderer/Node typechecks, xterm consistency, lint and formatting passed. Synthetic Electron/CDP composition and cancellation passed.

Full macOS validation (Node 24, pnpm 12): pnpm lint, pnpm typecheck, pnpm test --maxWorkers=4, and pnpm build all executed. Typecheck and complete desktop/Web/native build passed.

Full suite at f2fc9a3365: 76,615 passed / 7 failed / 391 skipped. Final-HEAD serial follow-ups: 22 passed; performance/timeout and registry-install failures cleared without relaxing assertions or policies.

Still failing: 4 real Claude CLI startup checks (5/10-second initialization deadlines) and 1 English runtime-catalog test. All reproduced on base bba68b1. Full lint fails on the same 7 catalog entries; later extraction/coverage gates passed separately. Full lint/test are not green.

• Automated regressions added/updated.
• Manual native Windows/Linux and live SSH QA: not run.

AI Disclosure

OpenAI Codex.

Review

Self-reviewed; Pullfrog reports no new issues. Human review pending.

Agent skill upstream boundary

• N/A: no upstream agent-skill material copied.

Notes

No new permissions, platform-specific paths/shortcuts, Git commands or wire fields. Folder workspaces and remote ownership considered; full mobile/platform QA remains unrun.

Checklist

• Small, focused change; problem and rationale explained.
• Before/after proof attached, or N/A explained.
• Correctness, security and performance reviewed.
• Cross-platform, SSH and compatibility impact considered.
• Full repository lint/typecheck/test/build executed; results above.
• Full typecheck and build passed.
• Full lint/test green: baseline failures documented above.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: a793180b-f8fe-4cef-a8fd-2fe3caeaa1c0

📥 Commits

Reviewing files that changed from the base of the PR and between 2b1ec81 and f2fc9a3.

📒 Files selected for processing (2)

• src/renderer/src/components/terminal-pane/terminal-ime-composer-placeholder-mask.ts
• src/shared/terminal-composer-draft.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/shared/terminal-composer-draft.ts
• src/renderer/src/components/terminal-pane/terminal-ime-composer-placeholder-mask.ts

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

---

📝 Walkthrough

Walkthrough

The patch updates CompositionHelper to reconcile IME composition, keypress, input, cancellation, and deferred finalization events. Composition rendering now supports themed colors, cell masks, caret updates, and suppressible row remainders. The placeholder mask handles remainder events and disposal. Placeholder detection preserves soft-wrapped text behavior. Tests cover masking, colors, committed input, repaint classification, context reads, and disposal.

Priority: ➖ Normal — Schedule the IME rendering fix because it addresses a Windows Korean composition defect affecting placeholder masking and committed text in Codex and Claude terminals.

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to f2fc9

This change masks terminal placeholders during IME composition while preserving trailing text and restores normal rendering when composition ends. No concrete merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue #17104. They mask placeholder text during IME composition, preserve placeholder width and background coverage across wrapped rows, retain ordinary committed text, and remove …
Out of Scope Changes check	✅ Passed	The xterm composition changes, placeholder mask updates, CSS change, placeholder detection fix, and regression tests all support the linked IME rendering objective. No unrelated code changes are evide…
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files.
Title check	✅ Passed	The title clearly identifies the main change: fixing IME preedit rendering over Codex and Claude placeholders.
Description check	✅ Passed	The description covers the required sections, explains the change and rationale, links issue #17104, includes visual proof, documents testing and known baseline failures, and records unrun platform QA…

---

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.}

[pullfrog]

ℹ️ No critical issues — a focused, well-tested fix with two minor suggestions below.

Reviewed changes

• xterm CompositionHelper patch: placeholder mask — during preedit the patched helper dispatches a cancelable xterm-composition-remainder event and, when the terminal owns a stock Codex/Claude placeholder, covers the row remainder (including soft-wrapped continuation rows) with an opaque mask built from per-cell background colors, replacing the old visibility: hidden CSS.
• Orca wiring — installTerminalImeComposerPlaceholderMask gains a xterm-composition-remainder listener that preventDefaults for structurally-verified placeholders; the .xterm-composition-remainder { visibility: hidden } rule is removed from terminal.css.
• Placeholder detection — isStockPlaceholder joins continuation rows with a space only between non-wrapped rows, so a soft-wrapped placeholder reconstructs to its exact stock string and is detected.
• Tests — the mask suite now asserts RGB / light-RGB / palette / inverse background resolution, wrapped-row masking, and that no input reaches the pty during or after the masked composition.

ℹ️ Native Windows/Linux IME validation is still outstanding

The PR body lists before/after visual proof and native Windows/Linux IME validation as pending. The change is renderer-only (a DOM overlay), but IME event ordering and composition rendering differ meaningfully across the three platforms, so the mask should be exercised on Windows and Linux before this lands rather than macOS-only.

ℹ️ Nitpicks

• src/shared/terminal-composer-draft.test.ts:254 ("keeps a wrapped stock placeholder out of draft metadata") models the continuation as rowsBelow: [' anything'] without rowsBelowWrapped: [true], so it exercises the space-inserting (hard-newline) path — not the soft-wrap path this PR's isStockPlaceholder change actually fixes. The soft-wrap case is covered end-to-end by the resize test in the mask suite; a direct unit case with rowsBelowWrapped: [true] would pin the regression at its source.
• During composition the new xterm-composition-remainder handler and the existing onRender handler each call syncPlaceholderOwnership(), so readTerminalCursorLineContext runs twice per repaint while composing. Bounded and minor, but the duplicate read is avoidable.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

• Removed the redundant onRender ownership sync — installTerminalImeComposerPlaceholderMask no longer re-reads the cursor line context on every render during composition. xterm's own onRender handler invokes updateCompositionElements, which dispatches the cancelable xterm-composition-remainder event handled here, so ownership now syncs exactly once per render-time decision.
• Added a unit test for soft-wrapped placeholder detection — terminal-composer-draft.test.ts pins the mid-word soft-wrap case at its source (rowsBelowWrapped: [true, …] → placeholder; [false, …] → draft with a newline).
• Added a repaint-read regression test — the mask suite asserts readTerminalCursorLineContext is called exactly once per remainder decision and never after disposal.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[nwparker]

Thank you for this, and I am sorry about the outcome — the core problem it fixes was already resolved and shipped about a week before you opened this, so most of the change now duplicates what is on main. That is our fault for not closing the loop on the issue sooner, and it cost you real work.

I want to be specific about what you found that the earlier fix did not, because it is still broken today.

src/shared/terminal-composer-draft.ts stitches soft-wrapped rows back together with an unconditional space. So a hint that wraps — …to do any + thing — becomes to do any thing and stops matching. That is live on main right now.

It also has a consequence past input handling that I do not think you claimed, and which makes it matter more: once we stop recognising the wrapped hint, we treat it as text the user typed. So a stock prompt can leak into places that are only ever supposed to hold real input.

Would you open a small PR with just that piece — the change and its test, the two files under the shared directory? It is about ten lines, touches nothing else, and I will merge it quickly. Full credit stays with you.

The reason I cannot take the rest: it also rewrites our bundled terminal component, and that file has been substantially reworked since you branched — the same region was rebuilt for character-width handling a few weeks ago. Bringing your version forward would not be a merge so much as writing it again against different code, and that is not a fair thing to ask of you.

On the masking work itself: what landed covers the contract this issue set — one composition session owns the mask at a time, a newer start supersedes an older one, a stale end cannot clear the current owner, the placeholder is identified structurally rather than by how it looks, and end, blur and disposal all clear it. Your approach reached the same place by a different mechanism. If you think there is a case the shipped version still gets wrong, say which one and I will look — I would rather reopen than lose it.

One genuinely open gap you raised that I do not think is covered: wrapped placeholder rows. If you want to take that on top of the ten-line fix above, it would be welcome.

Closing this one as superseded, not as wrong. Thanks again — the wrap bug is a good catch and I would like it back as its own PR.