Repository navigation
iOS: stop terminal zoom + push-up during keyboard transitions - #8907
Merged
Merged
Conversation
Dogfood of the kbpin build showed two glitches synchronized with the software keyboard: the terminal font visibly zoomed when the keyboard closed (then snapped back), and rows were shoved off the top of the screen mid-transition before sliding back down. Three causes, all in the shared-grid negotiation around a keyboard container change: 1. The stretch-to-fill auto-fit ran on every geometry pass, including the pass right after the keyboard target changed, while effectiveGrid still held the PREVIOUS grant (the phone itself just invalidated it). It stretched the rendered font toward filling the new container with the stale row count and decayed one RPC round-trip later. The fit is now deferred until the negotiation is settled: no keyboard animation in flight, no report debouncing, the newest report's echo confirmed, and the pass's capacity equal to the last reported grid. The settle paths (animation completion, echo confirmation, retry exhaustion) each schedule one final sync so exactly one fit runs on the settled grant. 2. The render rect bottom-pinned to the LIVE viewport unconditionally. During a dismissal the surface is already sized for the taller target viewport, so pinning to the still-small live bottom pushed the top rows off screen by renderHeight - liveHeight (renderRect y hit -344pt in the captured logs) and they slid back as the keyboard left. TerminalLetterboxGeometry.renderPinnedBottomEdge now caps the clip at the settled amount, and while the negotiation is unsettled a provisionally pinned render holds its top edge instead of riding the departing keyboard down and snapping back up on the fresh grant. Settled letterbox boxes and the keyboard-rise path keep the legacy live-edge ride. 3. Capacity reports normalized the measured cell size with the main-actor liveFontSize read at apply time. A font change queued between the measurement and the apply broke the base-font normalization by the zoom ratio, reporting a grid several times too small and feeding bogus grants back into the loop (the 10-row grants visible in the field recording). The geometry pass now captures the font it measured with and the report/fit use that paired value. Verified on cmux-kbpin-sim against a live Mac kbpin instance: debug log shows zoom.autofit.deferred during transitions, renderRect pinned at y=0 through the dismissal (previously -344), no font change across the whole cycle, and frame analysis of the recorded dance shows the text top and pitch constant through both transitions.
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
📝 WalkthroughWalkthroughViewport negotiation now waits for the latest daemon echo before stretch-to-fill fitting, preserves the font size used during geometry measurement, and pins rendering through keyboard and layout transitions using shared bottom-edge geometry. ChangesViewport stabilization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GhosttySurfaceView
participant Daemon
participant TerminalViewportSnapshot
participant TerminalLetterboxGeometry
GhosttySurfaceView->>Daemon: send latest natural-grid report
GhosttySurfaceView->>TerminalViewportSnapshot: mark negotiation unsettled
Daemon-->>GhosttySurfaceView: echo report.id
GhosttySurfaceView->>GhosttySurfaceView: release deferred auto-fit
GhosttySurfaceView->>TerminalViewportSnapshot: apply settled geometry
TerminalViewportSnapshot->>TerminalLetterboxGeometry: calculate pinned bottom edge
TerminalLetterboxGeometry-->>TerminalViewportSnapshot: return render position
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryThis PR stabilizes iOS terminal geometry during software-keyboard transitions.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Sequence DiagramsequenceDiagram
participant UIKit as Keyboard transition
participant Surface as GhosttySurfaceView
participant Mac as Paired Mac
UIKit->>Surface: Update target viewport
Surface->>Surface: Hold provisional render position
Surface->>Mac: Report settled base-font capacity
Mac-->>Surface: Echo effective grid
Surface->>Surface: Confirm newest report
Surface->>Surface: Apply final geometry and font fit
Reviews (2): Last reviewed commit: "iOS: defer local shrink resize until the..." | Re-trigger Greptile |
The keyboard-rise direction still showed a momentary "all rows pushed up": the local mirror resized to the smaller container immediately, and its reflow keeps the bottom of the SCREEN (trailing blank rows included), so the visible content collapsed to the tail of the old screen jumped to the top until the remote reflow landed one round-trip later. While the negotiation is unsettled and the container shrank at the same width (keyboard rising), the geometry pass now skips the local set_size and letterbox fit: the old render keeps its size and the bottom-pinned render rect slides it up with the keyboard, prompt glued to the keyboard top. The capacity report is pure container/cell math, so the negotiation still starts immediately, and the settle pass (echo confirmed, or retries exhausted) applies ONE resize whose result matches the remote's reflowed content. Deferred passes also skip re-stamping the render's source-layout height so the stale-live clamp cannot snap the old render to the target viewport mid-ride, and the applied-container tracker resets with the render pipeline. Width changes (rotation, split) and growth keep the immediate resize.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift (1)
3607-3644: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not auto-fit a deferred geometry result.
A deferred pass sets
result.appliedResize == false, but this branch only checks the current negotiation flags. If those flags clear before the queued result reaches the MainActor, stale pre-resize geometry can triggerautoFitFontToEffectiveRowsbefore the settle pass callsghostty_surface_set_size, reintroducing font oscillation.Require
result.appliedResizeand usesnapshot.viewportNegotiationUnsettledas the single settled-state source.Proposed fix
- if keyboardHeightAnimation == nil, - pendingViewportReport == nil, - !awaitingViewportEcho, + if result.appliedResize, + !snapshot.viewportNegotiationUnsettled, reportGrid == lastReportedSize {As per path instructions, correctness-critical viewport negotiation should use one reliable source of truth and fail closed rather than relying on a second potentially stale state calculation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift` around lines 3607 - 3644, Update the auto-fit guard around autoFitFontToEffectiveRows so it requires result.appliedResize and uses snapshot.viewportNegotiationUnsettled as the sole settled-state check, negated. Remove the duplicated keyboard/report negotiation flag checks from this decision and preserve the existing deferred diagnostic logging as appropriate, ensuring deferred or unsettled geometry cannot trigger auto-fitting.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`:
- Around line 3607-3644: Update the auto-fit guard around
autoFitFontToEffectiveRows so it requires result.appliedResize and uses
snapshot.viewportNegotiationUnsettled as the sole settled-state check, negated.
Remove the duplicated keyboard/report negotiation flag checks from this decision
and preserve the existing deferred diagnostic logging as appropriate, ensuring
deferred or unsettled geometry cannot trigger auto-fitting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0ace9bc5-558f-490c-8d5d-edcedd2f7e1b
📒 Files selected for processing (2)
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView+RenderRecovery.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
Collaborator
Author
|
Round 3 (66e897c), from follow-up dogfood: the keyboard-RISE direction still showed a momentary "all rows pushed up". The local mirror resized to the smaller container immediately, and its reflow keeps the bottom of the screen including trailing blank rows, so the visible content collapsed to the tail of the old screen at the top until the remote reflow landed one round-trip later. The geometry pass now defers the local Kit tests green (110). Visual sim verification of this round was blocked by a wedged sim-to-Mac iroh link in the verification rig; rounds 1/2 evidence and the round-3 mechanism are covered above, and the rise path needs a device check (raise keyboard: rows should squeeze/ride, never flash the screen tail at the top). 🤖 Generated with Claude Code |
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Dogfood feedback on the kbpin build (#8899, since merged): the terminal font visibly zoomed when the software keyboard closed and snapped back a moment later, and rows were pushed off the top of the screen mid-transition before sliding back down. Field recording analysis plus a simulator repro against a live Mac narrowed it to three defects in the shared-grid negotiation around a keyboard container change.
Stale-grant font stretch. The stretch-to-fill auto-fit ran on the geometry pass right after the keyboard target changed, while
effectiveGridstill held the previous grant (zoom.autofit eff=72x34 baseGrid=72x65 font 10.0->18.98captured live at dismissal start). It stretched the rendered font toward filling the new container with the stale row count and decayed one RPC round-trip later — the "text zooms in when the keyboard closes" glitch. The fit now waits for a settled negotiation (no keyboard animation in flight, no debouncing report, newest report echo confirmed, pass capacity equal to the last reported grid); the settle paths (animation completion, echo confirmation, retry exhaustion) each schedule one final sync so exactly one fit runs on the settled grant.Live-viewport bottom-pin overshoot. The render rect bottom-pinned to the live viewport unconditionally. During a dismissal the surface is already sized for the taller target viewport, so the pin pushed the top rows off screen by
renderHeight - liveHeight(renderRect=440x766@-344in the captured log) and they slid back down as the keyboard left.TerminalLetterboxGeometry.renderPinnedBottomEdgenow caps the clip at the settled amount, and a provisionally pinned render (negotiation unsettled) holds its top edge instead of riding the departing keyboard down and snapping back up when the fresh grant unpins it. Settled letterbox boxes and the keyboard-rise path keep the legacy live-edge ride.Incoherent cell/font pairs in capacity reports. Reports normalized the measured cell size with
liveFontSizeread at apply time; a font change queued between the measurement and the apply skews the base-font normalization by the zoom ratio and under-reports the grid several-fold (the ~10-row grants visible in the field recording, which seed the oscillation). The geometry pass now captures the font it measured with, and the report/fit use that paired value.Verification:
CmuxMobileTerminalKithost tests green (110 tests, includes newrenderPinnedBottomEdgecases); simulator E2E dance (keyboard raise + dismiss over a live attach) recorded and frame-analyzed on this branch — text top position and row pitch constant through both transitions; debug log showszoom.autofit.deferredduring transitions,renderRect=440x397@0held through the dismissal, then the echoed grant growing the render in place with zero motion. A host-runnable behavior test for defects 1/3 is not practical (they live in the iOS-onlyCmuxMobileTerminaltarget, which does not resolve on macOS hosts); the Kit pin math is unit-tested and the rest is covered by the recorded E2E evidence.No user-facing strings changed (debug log lines only), so no localization updates.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes iOS keyboard transition glitches: stops the terminal font from zooming on keyboard close and prevents rows from being pushed off-screen during show/hide. Grid negotiation, render pinning, and local resizing now stay stable throughout keyboard animations.
awaitingViewportEcho,markViewportReportConfirmed(reportID:), and a final geometry sync on settle/timeout.TerminalLetterboxGeometry.renderPinnedBottomEdge(..., holdsProvisionalPin:); on keyboard rise, deferred local shrink resize (skipset_sizeand letterbox fit), kept the old render, avoided re-stamping source-layout height, and applied one resize on settle. Tracked the last applied container size and reset it on pipeline recreation; width changes and growth still resize immediately. Added unit tests inCmuxMobileTerminalKit.measuredFontSizein the geometry pass and used it incapacityReportGridandautoFitFontToEffectiveRowsto prevent bogus small-grid grants.Written for commit 66e897c. Summary will update on new commits.
Summary by CodeRabbit