Fix iOS terminal scrolling at history ends - #8399
Closed
azooz2003-bit wants to merge 5 commits into
Closed
azooz2003-bit wants to merge 5 commits into
azooz2003-bit wants to merge 5 commits into
Conversation
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. |
📝 WalkthroughWalkthroughFull render-grid replacements now preserve local scroll position, while output chunks carry replacement metadata to Ghostty. Mobile scroll gestures route by active screen, use bounded prefetch windows, and record server response limits. ChangesMobile terminal behavior
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MobileShellComposite
participant MobileTerminalOutputChunk
participant GhosttySurfaceRepresentable
participant GhosttySurfaceView
MobileShellComposite->>MobileTerminalOutputChunk: propagate isFullReplacement
GhosttySurfaceRepresentable->>GhosttySurfaceView: process full replacement
GhosttySurfaceView->>GhosttySurfaceView: preserve scroll distance
GhosttySurfaceView-->>GhosttySurfaceRepresentable: return processing result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`:
- Around line 2132-2137: Separate scroll restoration status from output
application success in the processOutput flow around scrollPositionPreserved and
GhosttySurfaceScrollPosition.restore. Keep restoration results for scroll
behavior only, and update the completion call to return true whenever the output
is applied to the current surface generation, even if the exact scroll offset
cannot be restored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 198ad846-63bf-4086-a8f2-5487da562010
📒 Files selected for processing (17)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalOutputDelivery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalScrollDelivery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/TerminalOutputDelivery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/TerminalScrollDelivery.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/TerminalOutputDeliveryQueueTests.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/TerminalScrollDeliveryQueueTests.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileTerminalOutputSinking.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttyRuntime.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView+LocalScrollbackScroll.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView+ThemeOutput.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalScrollBoundary.swiftPackages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/FullReplacementScrollPositionTests.swiftPackages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/TerminalScrollBoundaryTests.swiftSources/TerminalController+MobileScrollPrefetch.swiftSources/TerminalController.swift
Contributor
Greptile SummaryThis PR rearchitects iOS terminal scrolling so the phone owns its primary-screen viewport locally, asking the Mac only for deeper history snapshots, while preserving the user's scroll offset across full render-grid replacements and suppressing outward momentum at the loaded top and bottom boundaries.
Confidence Score: 5/5Safe to merge. The changes are well-scoped to the iOS scroll pipeline with no cross-cutting mutations to shared Mac-side state. The viewport-preservation path correctly decouples scroll-restoration failure from output-application result. The No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant DL as Display Link
participant GSV as GhosttySurfaceView (Main)
participant OQ as outputQueue (Background)
participant GK as GhosttyKit C API
participant MSC as MobileShellComposite
participant Mac as Mac RPC
Note over DL,GSV: Normal primary-screen gesture
DL->>GSV: flushPendingScrollIfNeeded()
GSV->>GK: ghostty_surface_scrollbar() boundary
alt boundary suppresses at top or bottom
GK-->>GSV: suppressed
else within range
GK-->>GSV: boundary ok
GSV->>GK: ghostty_surface_mouse_scroll()
GSV->>MSC: didScrollLines prefetch check
MSC->>Mac: "scroll RPC lines=0 maxScrollbackRows=N"
Mac-->>MSC: renderGrid snapshot
MSC->>MSC: recordResponse grow or stop window
MSC->>GSV: deliverAuthoritativeTerminalRenderGrid()
end
Note over DL,GSV: Full replacement output arrives
GSV->>GSV: beginFullReplacementOutput
DL->>GSV: flushPendingScrollIfNeeded BLOCKED
GSV->>OQ: "processOutput preserving=true async"
OQ->>GK: ghostty_surface_scrollbar distanceFromBottom
OQ->>GK: ghostty_surface_process_output full VT replay
OQ->>GK: ghostty_surface_scroll_to_row_if_revision CAS
GK-->>OQ: positioned
OQ-->>GSV: "completion applied=true via main.async"
GSV->>GSV: "finishFullReplacementOutput applied=true"
GSV->>GSV: flushPendingScrollIfNeeded buffered deltas applied
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant DL as Display Link
participant GSV as GhosttySurfaceView (Main)
participant OQ as outputQueue (Background)
participant GK as GhosttyKit C API
participant MSC as MobileShellComposite
participant Mac as Mac RPC
Note over DL,GSV: Normal primary-screen gesture
DL->>GSV: flushPendingScrollIfNeeded()
GSV->>GK: ghostty_surface_scrollbar() boundary
alt boundary suppresses at top or bottom
GK-->>GSV: suppressed
else within range
GK-->>GSV: boundary ok
GSV->>GK: ghostty_surface_mouse_scroll()
GSV->>MSC: didScrollLines prefetch check
MSC->>Mac: "scroll RPC lines=0 maxScrollbackRows=N"
Mac-->>MSC: renderGrid snapshot
MSC->>MSC: recordResponse grow or stop window
MSC->>GSV: deliverAuthoritativeTerminalRenderGrid()
end
Note over DL,GSV: Full replacement output arrives
GSV->>GSV: beginFullReplacementOutput
DL->>GSV: flushPendingScrollIfNeeded BLOCKED
GSV->>OQ: "processOutput preserving=true async"
OQ->>GK: ghostty_surface_scrollbar distanceFromBottom
OQ->>GK: ghostty_surface_process_output full VT replay
OQ->>GK: ghostty_surface_scroll_to_row_if_revision CAS
GK-->>OQ: positioned
OQ-->>GSV: "completion applied=true via main.async"
GSV->>GSV: "finishFullReplacementOutput applied=true"
GSV->>GSV: flushPendingScrollIfNeeded buffered deltas applied
Reviews (3): Last reviewed commit: "Verify full replay row-space replacement" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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. |
Collaborator
Author
|
Superseded by #8451, which fixes the reported workspace-list scroll path. This PR targeted terminal-detail scrolling and does not match the corrected scope. |
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.
Summary
Verification
Notes
The iOS package convention check reports two pre-existing violations in CmxIrohTCPFirstActivation.swift and CmuxPopoverMutation.swift. Neither file changed in this branch.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes iOS terminal scroll bounce at history ends and makes primary‑screen scrolling phone‑local. Preserves the viewport across full render‑grid replays with compare‑and‑swap restoration that waits for a row‑space rebuild, and fetches deeper history in pages up to 4,800 rows.
TerminalScrollBoundary(fails open without scrollback).Written for commit c71a77f. Summary will update on new commits.
Summary by CodeRabbit