Skip to content

iOS: anchor the keyboard-rise render slide to the cursor row - #9133

Merged
azooz2003-bit merged 1 commit into
mainfrom
feat-ios-kb-rise-anchor
Jul 29, 2026
Merged

azooz2003-bit merged 1 commit into
mainfrom
feat-ios-kb-rise-anchor

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 29, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #8907 (merged as the hotfix): the "momentary push of all terminal rows" was still reproducible on main when the keyboard OPENS.

Repro on main, captured in the app debug log with a scripted simulator dance: the deferred old render is bottom-pinned to the live viewport during the rise, so with a prompt in the upper half of the screen (blank rows below it) the content slid to renderRect=440x714@-287 for ~0.3s and the settle resize dropped it back. The rows the keyboard covers are blank, so no motion is needed at all.

Fix: while the grid negotiation is unsettled and the viewport is not growing, TerminalLetterboxGeometry.renderPinnedBottomEdge anchors the slide to the cursor row instead of the screen bottom — the blank space below the cursor absorbs the keyboard intrusion first (zero motion for short content), sliding starts only once the cursor row itself would be covered, and a full-screen prompt keeps the exact legacy ride so it never hides under the keyboard. Unknown cursor falls back to the legacy ride. The anchor also holds at live == target while the deferred resize waits on the grid echo. The cursor bottom comes from the same non-blocking ghostty_surface_ime_point read the cursor overlay already performs on the main thread.

Verification, before vs after on the identical scripted dance (raise + dismiss over a live Mac attach, window recorded at 20fps):

  • main: geom.deferShrink then renderRect@-45 → @-287 → settle @0; recording shows all rows jumping up and back.
  • this branch: renderRect=440x714@0 on every geometry pass through the rise, 440x397@0 held through the dismissal; frame analysis puts the text top at the same pixel in all 384 frames.
  • CmuxMobileTerminalKit host tests: 111 pass, including new cases for cursor-anchored shrink (no-motion, exact-slide, full-screen fallback, unknown-cursor fallback, settled-shrink unchanged).

A red/green two-commit split was not practical: the failing assertion requires the new cursorBottomInRender parameter to exist, so a test-only first commit cannot compile without shipping a stub of the fix.

No user-facing strings changed; no localization impact.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Anchors the terminal render during iOS keyboard rise to the cursor row to stop the brief upward slide of all rows when the keyboard opens. Keeps the prompt visible with zero motion for short content; full-screen prompts keep the legacy ride.

  • Bug Fixes
    • Pin the provisional slide to the cursor row while the viewport shrinks; fall back to screen-bottom ride if the cursor is unknown and keep the pin when live == target until the grid settles.
    • Read cursor bottom via non-blocking ghostty_surface_ime_point and pass cursorBottomInRender through GhosttySurfaceView → TerminalViewportSnapshot → TerminalLetterboxGeometry.
    • Added tests in CmuxMobileTerminalKit for no-motion, exact-slide, full-screen fallback, and unknown-cursor cases; all host tests pass.

Written for commit d5a860b. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved terminal viewport behavior during keyboard-driven resizing.
    • Keeps the cursor row visible and prevents content from shifting prematurely while the viewport shrinks or expands.
    • Preserves previous positioning behavior when cursor location cannot be determined.
  • Tests

    • Added coverage for cursor-anchored positioning across multiple viewport and cursor scenarios.

Even with the deferred shrink resize, the keyboard rise still shoved
every terminal row up by the keyboard height: the old render is
bottom-pinned to the live viewport, and for a prompt sitting in the
upper half of the screen the rows the keyboard covers are the BLANK
rows below it. Riding the screen bottom pushed the content rows to
renderRect y=-287 in the captured repro and the settle resize dropped
them back — the "momentary push of all terminal rows".

While the negotiation is unsettled and the viewport is not growing,
the render now slides only as much as needed to keep the cursor row
visible: the blank space below the cursor absorbs the keyboard
intrusion first (zero motion for short content), and a full-screen
prompt keeps the legacy ride so it never hides under the keyboard.
The cursor bottom comes from the same non-blocking
ghostty_surface_ime_point read the cursor overlay uses. The anchor
also holds at live == target while the deferred resize waits on the
grid echo, so the final stretch of the transition cannot snap.

Verified on-device-sim with the same scripted dance as the repro:
renderRect held at 440x714@0 through the entire rise (previously
-45 -> -287) and frame analysis of the recording shows the text top
at the same pixel in all 384 frames across raise and dismiss.
@coderabbitai

coderabbitai Bot commented Jul 29, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7c57ee7f-1a56-4f87-9aa7-fca2c73a37fe

📥 Commits

Reviewing files that changed from the base of the PR and between 058dc30 and d5a860b.

📒 Files selected for processing (4)
  • Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
  • Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalViewportSnapshot.swift
  • Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalLetterboxGeometry.swift
  • Packages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalLetterboxGeometryTests.swift

📝 Walkthrough

Walkthrough

The change adds cursor-bottom measurement from Ghostty, passes it through viewport render-rectangle computation, and uses it during provisional keyboard-shrink bottom pinning. Tests cover cursor-aware, covered, last-row, unknown-cursor, and steady-state cases.

Changes

Cursor-anchored viewport rendering

Layer / File(s) Summary
Provisional pinning geometry
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalViewportSnapshot.swift, Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalLetterboxGeometry.swift, Packages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalLetterboxGeometryTests.swift
renderRect and renderPinnedBottomEdge accept cursor-bottom coordinates, and tests verify provisional shrink behavior for known and unknown cursor positions.
Cursor geometry wiring
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
Ghostty IME coordinates are converted to render-local cursor-bottom points and passed through both viewport layout and geometry-result application paths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GhosttySurfaceView
  participant TerminalViewportSnapshot
  participant TerminalLetterboxGeometry
  GhosttySurfaceView->>GhosttySurfaceView: Read Ghostty IME point
  GhosttySurfaceView->>TerminalViewportSnapshot: Pass cursorBottomInRender
  TerminalViewportSnapshot->>TerminalLetterboxGeometry: Compute pinned bottom edge
Loading

Possibly related PRs

  • manaflow-ai/cmux#8907: Both changes modify provisional bottom-edge pinning during keyboard viewport transitions.
🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: anchoring the iOS keyboard-rise render slide to the cursor row.
Description check ✅ Passed The description covers the change, rationale, testing, review trigger, and checklist context; only the demo video section is missing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Cmux Swift Actor Isolation ✅ Passed PASS: The new logic stays in main-actor UIKit paths; the added snapshot/geometry helpers are pure value math, with no new Sendable/shared-mutable or protocol isolation issues.
Cmux Swift Blocking Runtime ✅ Passed Diff only adds cursor-aware geometry and a non-blocking ime-point read; no semaphores, waits, sleeps, syncs, or locks in production Swift.
Cmux Browser Automation Off-Main ✅ Passed PASS: PR only changes iOS terminal layout/geometry files; no browser.* socket commands, worker routing, or policy coverage changes were introduced.
Cmux Expensive Synchronous Load ✅ Passed Changed paths only add cursor-position geometry math and a non-blocking Ghostty point read; no agent-history load, JSON parse, or main-actor disk scan was introduced.
Cmux Cache Substitution Correctness ✅ Passed cursorBottomInRender is read live from ghostty_surface_ime_point, not a cache, and nil falls back to legacy bottom-pin behavior.
Cmux No Hacky Sleeps ✅ Passed Only Swift files changed; runtime-no-hacky-sleeps is out of scope for Swift timing/blocking primitives.
Cmux Algorithmic Complexity ✅ Passed PASS: The diff only adds one cursor-point read and O(1) arithmetic in layout; no new scans, sorts, or nested collection work in the hot path.
Cmux Swift Concurrency ✅ Passed The diff only adds synchronous cursor-geometry plumbing and tests; no new background queues, Combine state, new completions, or fire-and-forget Tasks were introduced.
Cmux Swift @Concurrent ✅ Passed PASS: The PR only adds synchronous UIKit-bound helpers and passes a cursor metric through; no new nonisolated async work or @concurrent annotations were introduced.
Cmux Swift Package Boundaries ✅ Passed Changed logic lives in SwiftPM TerminalLetterboxGeometry; app-target code only probes Ghostty/forwards state, which the rule allows as glue.
Cmux Swiftpm Lockfiles ✅ Passed Diff only changes Swift source/tests; no Package.swift, Package.resolved, .gitignore, or workflow files were touched, so the lockfile policy is not implicated.
Cmux Swift Logging ✅ Passed The diff adds no print/debugPrint/dump/NSLog or new Logger-based diagnostics, and the test changes contain no runtime logging.
Cmux User-Facing Error Privacy ✅ Passed Only internal geometry/layout logic and tests changed; no user-facing error, alert, command output, or recovery copy was added.
Cmux Full Internationalization ✅ Passed Touched files only change render geometry and tests; no new user-facing Swift text, xcstrings, or locale data were introduced.
Cmux Swiftui State Layout ✅ Passed UIKit bridge and pure geometry only; no new SwiftUI state/layout primitives or render-time state mutation in the diff.
Cmux Architecture Rethink ✅ Passed Small local correctness fix: existing layout path gains a required cursor-bottom bridge; no sleeps, polling, new owners, or duplicate lifecycle wiring.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR only changes rendering/layout code and tests; no standalone window types or cmuxAuxiliaryWindowIdentifiers changes, so the auxiliary-window shortcut rule doesn’t apply.
Cmux Source Artifacts ✅ Passed Changed paths are only Swift source/tests; no artifact dirs, logs, screenshots, recordings, caches, or build outputs appear in the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Only production geometry logic and a private cursor helper were added; no new #if DEBUG, debug*/ForTesting seam, or test-only accessor appears in Sources/.
Cmux No Ambient Global State ✅ Passed No new file-scope funcs/vars or singletons were added; the cursor probe is a private instance method and the geometry changes stay within existing static utility code.
✨ 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 feat-ios-kb-rise-anchor

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.

@azooz2003-bit
azooz2003-bit merged commit 69d05f4 into main Jul 29, 2026
6 checks passed
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.

1 participant