fix(client): coalesce pending semantic frames - #3167
Conversation
📝 WalkthroughWalkthroughThe server reader now coalesces graphics-free semantic frames. The client loop processes the newest pending frame and ignores stale wake-ups. Control, resize, and graphics messages clear pending state and preserve event order. Tests cover coalescing and ordering barriers. ChangesSemantic frame coalescing
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to The client may process a newer screen frame before an already queued resize event, producing incorrect event ordering and transient terminal state. The change is not merge-ready until this ordering risk is fixed or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant ServerReader
participant EventQueue
participant ClientLoop
participant PendingFrame
ServerReader->>PendingFrame: store newest graphics-free frame
ServerReader->>EventQueue: emit one SemanticFrame wake-up
ServerReader->>PendingFrame: replace older pending frame
EventQueue->>ClientLoop: deliver SemanticFrame event
ClientLoop->>PendingFrame: consume latest frame
PendingFrame-->>ClientLoop: return ServerMessage::Frame
ServerReader->>EventQueue: enqueue barrier message
ClientLoop->>ClientLoop: process events in order
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
42ae9f7 to
3eefc2a
Compare
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: 594d292d-63b1-4047-bbd5-c6ea44eb048a
📒 Files selected for processing (1)
src/client/mod.rs
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
Greptile SummaryThe PR coalesces consecutive graphics-free semantic frames in the thin client so slow terminal painting does not leave stale full frames ahead of input and control events.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/client/mod.rs | Adds pending-slot semantic-frame coalescing while retaining FIFO barriers for control and graphics traffic, with focused ordering tests. |
Sequence Diagram
sequenceDiagram
participant S as Server reader
participant P as Pending frame slot
participant Q as Client event queue
participant L as Client loop
S->>P: Store semantic frame A
S->>Q: Queue one SemanticFrame wake
S->>P: Replace A with newer frame B
Q->>L: Deliver wake
L->>P: Take frame B
L->>L: Render newest frame
S->>Q: Queue control/graphics barrier
S->>P: Create new slot for later frames
Reviews (2): Last reviewed commit: "Merge branch 'master' into fix/client-fr..." | Re-trigger Greptile
ogulcancelik
left a comment
There was a problem hiding this comment.
reviewed the frame coalescing and ordering boundaries. complete semantic frames are safely replaceable, graphics and control messages remain barriers, and the pending-slot wakeup cannot lose the newest frame. no blocking issues found.
What changed
Herdr's server already keeps only the latest pending render frame. Once frames reached the thin client though, every semantic frame still went through the same 256-entry queue as keyboard, mouse, and resize events.
If the host terminal fell behind while painting, superseded full frames could queue ahead of input. The client then diffed, wrote, and flushed each stale frame before processing the next event.
This keeps only the latest graphics-free semantic frame in each consecutive run. Control messages and frames with terminal graphics remain ordering barriers. Terminal ANSI output stays ordered.
Why
I went chasing some pretty severe Herdr-only lag on Windows. At the worst points, typing and changing spaces could take seconds while the same Codex session in a normal Windows Terminal was fine.
One active Codex pane produced about 122 KB/s of pane output in 37 bursts per second. That expanded to roughly 1.6 MB/s of complete client frame traffic. Across the live session, the client received about 0.4 to 3.8 MB/s at 6 to 57 writes per second.
The server queue was already bounded to one pending frame, so the backlog happened after the socket. Slow host painting allowed complete frames to pile up in the client event queue ahead of input.
Semantic frames describe the complete current screen. Once a newer frame exists, rendering older ones adds no value. Keeping the latest frame lets input and control events proceed without replaying stale screen history.
This is client-only and does not change the protocol. I have been dogfooding the patched client against the existing server on Windows, seemed fine, but I wouldn't mind some other tests tbh.
Testing
just check