[nwparker]

Split out of #17170 at review request. #17170 now carries only the xterm composition-overlay work; this carries the Codex/Claude placeholder masking, which is a different domain at a different layer.

Stacked on #17170. The CSS here keys off the .xterm-composition-remainder class that PR adds; the dependency is one-way. GitHub shows the combined diff against main until #17170 merges — the reviewable change is the last commit only (5c7d3958d50).

What changed

• Classify a Codex/Claude stock placeholder row and, while a composition owns it, hide the overlay's rendered row tail through CSS.
• Split detectTerminalComposer so detectTerminalComposerDraft and the new hasTerminalComposerPlaceholder share one classifier.
• Move readTerminalCursorLineContext from src/main/daemon to src/shared so the renderer can use the same reader the daemon does. Import-only move.

Why

Codex and Claude draw an all-dim, full-row ghost placeholder. #17170 makes the opaque preedit overlay reproduce the committed row tail it covers — correct for ordinary text, wrong for a ghost, which would then be repeated to the right of the composing syllable rather than staying masked.

Masking is gated on structure, not on the placeholder string: a bold prompt glyph plus a dimmed model footer below a blank gap for Codex, a frame line above the prompt for Claude. Arbitrary dim output, shell prompts that happen to use the same glyph, and any row carrying typed text all keep their tail visible.

Verification

Run on head 5c7d3958d50:

• pnpm tc: passed.
• src/renderer/src/components/terminal-pane/ + terminal-composer-draft.test.ts + terminal-cursor-line-context.test.ts: 411 files / 3,983 tests passed.
• pnpm run check:reliability-gates: 99 gates valid.

Mutation results

Every line this PR adds or changes was broken, the suite re-run, and the line restored.

Mutation	Result
placeholder: !text && placeholder to placeholder	killed — 2 failed
handleBlur no longer clears activeSessionId	killed — 1 failed

Both were surviving mutations in the original #17170 and are the reason these two tests exist.

The blur test dispatches on terminal.element with a synthetic session start. That matters: a real textarea blur also ends the composition session, so ownership would clear through handleSessionEnd and the test would pass whether or not the blur listener does anything at all.

The !text guard test covers a composer row that has typed text and still matches the stock-placeholder heuristic. Without the guard, that row is classified as a placeholder — hiding a row containing real user text with everything still green.

What runs where

The renderer-unit slice is deterministic and runs on every PR. The rendered Electron/CDP arm (terminal-korean-midline-preedit-occlusion.spec.ts) drives composition through CDP, not a real IME. No CI runner exercises a real OS IME, and the PR gate has no macOS runner. The gate records that as a standing gap rather than claiming coverage.

AI disclosure

AI-assisted implementation, test construction, and review.

[coderabbitai]

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: 1f2eb722-508b-4427-9c8a-930a4284cab1

📥 Commits

Reviewing files that changed from the base of the PR and between 7f822a7 and 1f5fcec808668373591e33f04c31395aea1fece9.

📒 Files selected for processing (12)

• config/reliability-gates.jsonc
• src/main/daemon/headless-emulator.ts
• src/main/daemon/terminal-cursor-line-context.test.ts
• src/renderer/src/assets/terminal.css
• src/renderer/src/components/terminal-pane/terminal-ime-composer-placeholder-mask.test.ts
• src/renderer/src/components/terminal-pane/terminal-ime-composer-placeholder-mask.ts
• src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts
• src/shared/terminal-composer-draft.test.ts
• src/shared/terminal-composer-draft.ts
• src/shared/terminal-cursor-line-context.ts
• tests/e2e/terminal-ime-midline-occlusion-probe.ts
• tests/e2e/terminal-korean-midline-preedit-occlusion.spec.ts

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

• src/main/daemon/terminal-cursor-line-context.test.ts
• src/main/daemon/headless-emulator.ts
• src/renderer/src/assets/terminal.css
• src/shared/terminal-composer-draft.ts
• tests/e2e/terminal-korean-midline-preedit-occlusion.spec.ts
• src/shared/terminal-composer-draft.test.ts
• src/renderer/src/components/terminal-pane/terminal-ime-composer-placeholder-mask.ts
• src/renderer/src/components/terminal-pane/terminal-ime-composer-placeholder-mask.test.ts
• config/reliability-gates.jsonc
• src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts
• src/shared/terminal-cursor-line-context.ts

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

---

📝 Walkthrough

Walkthrough

The change adds structural terminal cursor-context types and symmetric row-window reads. It separates composer placeholder detection from draft detection. Renderer panes now mask eligible Codex and Claude placeholders during active IME composition and remove the mask on session end, blur, or disposal. CSS hides the composition remainder under the mask. Unit tests, Korean preedit e2e coverage, and reliability gates validate masking, typed text protection, cleanup, and bounded session handling.

Merge Risk: 🔵 Low · up to 1f5fc

The PR safely narrows placeholder masking to recognized empty rows and includes targeted validation, but two new reliability gates are not backed by corresponding recorded runs, leaving the new coverage audit trail incomplete; the change is mergeable with explicit owner follow-up.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the change, rationale, implementation scope, dependency, testing, mutation results, and AI use. It omits required template content, including a linked issue, visual proof or a…	Add the missing template sections. Provide a linked issue after “Fixes #”. Attach before-and-after visual proof, or write N/A with a reason. Complete the testing checklist with platforms tested. Complete the review, boundary, notes, and che…
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 17 files. (2 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary change: masking the agent composer placeholder during preedit.
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.

Full details: Description check

Explanation

The description explains the change, rationale, implementation scope, dependency, testing, mutation results, and AI use. It omits required template content, including a linked issue, visual proof or an explicit N/A explanation, testing checklist details, review notes, and the final checklist.

Resolution

Add the missing template sections. Provide a linked issue after “Fixes #”. Attach before-and-after visual proof, or write N/A with a reason. Complete the testing checklist with platforms tested. Complete the review, boundary, notes, and checklist sections, or mark each item as not applicable with an explanation.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 17 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

---

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

[greptile-apps]

Greptile Summary

This PR masks structurally recognized Codex and Claude placeholder tails while an IME preedit owns the composer row.

• Adds shared placeholder classification while preserving typed composer drafts.
• Adds renderer lifecycle handling and CSS masking for active composition sessions.
• Moves the terminal cursor-context reader into shared code and expands focused unit and E2E coverage.

Confidence Score: 5/5

The PR appears safe to merge, with no blocking failure remaining in the current review scope.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/renderer/src/components/terminal-pane/terminal-ime-composer-placeholder-mask.ts	Adds bounded composition-session ownership, render-time reclassification, and blur/disposal cleanup for placeholder masking.
src/shared/terminal-composer-draft.ts	Refactors composer detection into a shared classifier and exposes stock-placeholder detection without suppressing rows containing typed text.
src/shared/terminal-cursor-line-context.ts	Moves the cursor-context reader into shared code and generalizes its terminal source interface for renderer and headless use.
src/renderer/src/assets/terminal.css	Hides only the patched xterm composition remainder while the terminal owns a verified composer placeholder.
src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts	Installs and disposes the placeholder-mask binding with each terminal pane.
tests/e2e/terminal-korean-midline-preedit-occlusion.spec.ts	Extends rendered geometry coverage to verify that a Codex placeholder remains masked during Korean preedit.

Sequence Diagram

sequenceDiagram
    participant IME as IME / xterm
    participant Mask as Placeholder mask binding
    participant Reader as Cursor context reader
    participant Classifier as Composer classifier
    participant DOM as Terminal DOM
    IME->>Mask: composition session start
    Mask->>Reader: read visible cursor context
    Reader->>Classifier: classify composer row
    Classifier-->>Mask: stock placeholder and no typed text
    Mask->>DOM: add ownership class
    DOM->>DOM: hide composition remainder
    IME->>Mask: render updates
    Mask->>Reader: re-evaluate ownership
    IME->>Mask: session end or blur
    Mask->>DOM: remove ownership class

Loading

_{Reviews (4): Last reviewed commit: "fix(terminal): mask the agent composer's..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 12c50b2b-d205-42ab-806d-626ac07a57b9

📥 Commits

Reviewing files that changed from the base of the PR and between 58a52a8 and 0ec925ce49bb886ae1cc027aadd0cca484404982.

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (21)

• config/patches/@xterm__xterm@6.1.0-beta.287.patch
• config/patches/xterm-src/@xterm__xterm@6.1.0-beta.287.src.patch
• config/reliability-gates.jsonc
• src/main/daemon/headless-emulator.ts
• src/main/daemon/terminal-cursor-line-context.test.ts
• src/renderer/src/assets/terminal.css
• src/renderer/src/components/terminal-pane/terminal-ime-composer-placeholder-mask.test.ts
• src/renderer/src/components/terminal-pane/terminal-ime-composer-placeholder-mask.ts
• src/renderer/src/components/terminal-pane/terminal-ime-xterm-midline-preedit-tail.test.ts
• src/renderer/src/components/terminal-pane/terminal-ime-xterm-resumed-preedit-visibility.test.ts
• src/renderer/src/components/terminal-pane/terminal-ime-xterm-trailing-preedit-occlusion.test.ts
• src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts
• src/renderer/src/lib/pane-manager/terminal-ime-candidate-anchor.test.ts
• src/renderer/src/lib/pane-manager/terminal-ime-candidate-anchor.ts
• src/shared/terminal-composer-draft.test.ts
• src/shared/terminal-composer-draft.ts
• src/shared/terminal-cursor-line-context.ts
• tests/e2e/terminal-ime-midline-occlusion-probe.ts
• tests/e2e/terminal-ime-pane-arena.ts
• tests/e2e/terminal-korean-endofrow-preedit-cell-span.spec.ts
• tests/e2e/terminal-korean-midline-preedit-occlusion.spec.ts

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

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Add evidenceRuns entries for the two newly registered commands.

This range adds two new commands to the terminal-input.ime-and-synthetic-forwarding gate:

• A 7-file vitest command covering terminal-ime-xterm-midline-preedit-tail.test.ts, terminal-ime-xterm-resumed-preedit-visibility.test.ts, terminal-ime-xterm-trailing-preedit-occlusion.test.ts, terminal-ime-candidate-anchor.test.ts, terminal-ime-composer-placeholder-mask.test.ts, terminal-composer-draft.test.ts, and terminal-cursor-line-context.test.ts.
• An e2e command for terminal-korean-endofrow-preedit-cell-span.spec.ts and terminal-korean-midline-preedit-occlusion.spec.ts.

The evidenceRuns array for this gate has no entry matching either command. Every other command in this gate has at least one matching evidenceRuns entry with a date, platform, and result. Add evidenceRuns entries that record an actual pass for both new commands. Without them, this reliability gate claims coverage for the new preedit-overlay, candidate-anchor, and placeholder-mask behavior with no recorded evidence that the commands were executed against this build.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not classify undimmed draft text as a placeholder.

Line 113 accepts context.afterCursor when it matches a placeholder phrase. That value contains typed, undimmed text. If a user moves the cursor before a draft beginning with Try " or Ask Codex to do anything, this branch clears the draft and can mask its row during composition.

Require an empty context.afterCursor before classifying rawAfterCursor as a stock placeholder. Add a regression test with typed matching text to the right of the cursor.

Proposed fix

-  const placeholder = isStockPlaceholder(afterCursor, continuationRows)
+  const placeholder =
+    context.afterCursor.length === 0 &&
+    isStockPlaceholder(afterCursor, continuationRows)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const placeholder = isStockPlaceholder(afterCursor, continuationRows)
	const placeholder =
	context.afterCursor.length === 0 &&
	isStockPlaceholder(afterCursor, continuationRows)

[pullfrog]

✅ No new issues found.

Reviewed changes

• Shared classifier split — detectTerminalComposerDraft in src/shared/terminal-composer-draft.ts is refactored into an internal detectTerminalComposer returning a placeholder flag, with detectTerminalComposerDraft (filters placeholders, preserving the prior daemon/runtime contract) and hasTerminalComposerPlaceholder as the two public entry points. The placeholder: !text && placeholder guard ensures a row carrying typed text is never masked even when the stock placeholder is still rendered to its right.
• Cursor-context reader move — readTerminalCursorLineContext moves from src/main/daemon to src/shared/terminal-cursor-line-context.ts, switching from the @xterm/headless Terminal type to the structural TerminalCursorContextSource so both headless and browser terminals satisfy it. Daemon call sites are import-only; the parameter is renamed rowsAbove → rowsAroundCursor to match its existing symmetric use.
• Renderer mask module — installTerminalImeComposerPlaceholderMask toggles the orca-ime-composer-placeholder class on the terminal element while a composition session owns a structurally verified Codex/Claude placeholder, re-evaluating on onRender, and clearing on session end, blur, and dispose.
• CSS — .xterm.orca-ime-composer-placeholder .xterm-composition-remainder { visibility: hidden; } hides the reproduced row tail while preserving layout space.
• Tests and gates — new renderer unit test, shared draft test coverage, e2e assertion via remainderVisibility, and reliability-gate entries for the new files.

I verified the changed unit tests pass (3 files / 39 tests) and check:reliability-gates validates all 99 gates. The event names and .xterm-composition-remainder class the CSS keys off match what the stacked #17170 patch emits, and the mask's session-id validation mirrors terminal-ime-composition-route.ts.

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

[pullfrog]

Important

The two candidate-anchor writers still disagree for a preedit wider than the whole row — see inline.

Reviewed changes (since prior pullfrog review)

• Pure rebase of this PR's commit — 5c7d3958d5 is byte-identical to the previously reviewed 0ec925ce49 (same 12 files, 567 insertions / 55 deletions). No new findings in the placeholder-mask work itself.
• Stacked candidate-anchor agreement change — 0bb0b58ba7 moves the right-edge clamp into xterm's patched CompositionHelper.updateCompositionElements so it applies on the onRender path too, and keeps Orca's terminal-ime-candidate-anchor.ts applying the "same" clamp on composition events; a new render-path test replaces the old setTimeout-recovery test.

The placeholder-mask unit tests still pass on this head (3 files / 39 tests) and check:reliability-gates still validates 99 gates.

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

[pullfrog]

This clamp is missing the Math.max(0, …) floor that terminal-ime-candidate-anchor.ts:87 applies. xterm's cursorLeft + Math.min(0, maxWidth - anchorBounds.width) reduces to min(cursorLeft, cols·cellWidth − width), so for a preedit wider than the whole row (width > cols·cellWidth) it writes a negative left, while Orca's Math.max(0, Math.min(…)) clamps to 0. Because this path also runs from onRender (the last writer), the negative value can persist and push the candidate window off the left edge — the exact invariant pinned by terminal-ime-candidate-anchor.test.ts ("never anchors left of the terminal when the preedit outgrows the whole row").

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.