fix: speed up alternate-screen history reads - #2426
Conversation
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change adds PTY content-sequence tracking, sequence-stable terminal snapshots, and deadline-based alternate-screen traversal. Alternate-screen reads now coordinate output quiet periods, synchronized redraws, harvesting, and restoration with expanded test coverage. ChangesAlternate-screen read synchronization
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PTY
participant PaneRuntime
participant TerminalRuntime
participant PendingAltScreenRead
PTY->>PaneRuntime: process terminal bytes
PaneRuntime-->>TerminalRuntime: expose content_seq
TerminalRuntime-->>PendingAltScreenRead: return stable snapshot and sequence
PendingAltScreenRead->>PendingAltScreenRead: track output and deadlines
PendingAltScreenRead->>PTY: issue traversal or restore step
PTY-->>PendingAltScreenRead: produce redraw output
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b0ac5f89-a3cc-426c-8452-6fd3cda3dc97
📒 Files selected for processing (4)
src/pane.rssrc/server/alt_screen_read.rssrc/server/headless.rssrc/terminal/runtime.rs
660e2e2 to
852c33d
Compare
|
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. |
Greptile SummaryThe PR replaces fixed alternate-screen polling delays with PTY-output-driven traversal while preserving bounded traversal and viewport restoration.
Confidence Score: 4/5The PR should not merge until the outstanding alternate-screen deadline bug is fixed because delayed output can make the headless event loop busy-poll. Delayed PTY output can establish a quiet window while the separate wheel-step deadline is already expired, and the pending read's deadline feeds the single-threaded headless timer loop directly, leaving the previously reported busy-poll path unresolved. Files Needing Attention: src/server/alt_screen_read.rs, src/server/headless.rs
|
| Filename | Overview |
|---|---|
| src/server/alt_screen_read.rs | Reworks traversal into output-driven, deadline-bounded phases and adds extensive tests; the previously reported deadline-scheduling issue remains outstanding. |
| src/pane.rs | Adds an atomic sequence counter bracketing terminal processing for stable snapshot detection. |
| src/terminal/runtime.rs | Adds a bounded retry helper that returns a snapshot only when the content sequence remains stable. |
| src/server/headless.rs | Captures the initial snapshot sequence, supplies it to pending reads, and schedules their reported deadlines in the main loop. |
Sequence Diagram
sequenceDiagram
participant API as Pane read request
participant H as Headless server loop
participant R as PendingAltScreenRead
participant T as TerminalRuntime
participant P as PTY application
API->>H: request alternate-screen history
H->>T: snapshot_with_seq()
H->>R: start(snapshot, content_seq)
R->>T: send wheel batch
P-->>T: process PTY redraw bytes
T->>T: increment content_seq before/after parsing
H->>R: poll(now)
R->>T: read content_seq and stable snapshot
alt synchronized output active
R-->>H: wait for frame boundary
else output still fragmented
R-->>H: wait for quiet deadline
else stable redraw
R->>R: merge history or restore viewport
end
Reviews (2): Last reviewed commit: "fix: speed up alternate-screen history r..." | Re-trigger Greptile
852c33d to
39e52d8
Compare
|
@coderabbitai review |
|
Summary
Validation
just checkrefs #2387