Repository navigation
fix: reduce interactive pane echo latency - #5149
Conversation
|
@coderabbitai review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue was identified. The change is mergeable after normal checks. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 39.13% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 7 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Description check The description directly explains the interactive rendering changes, input conditions, validation, performance trade-offs, and known measurement limits. Linked Issues check The description references issue #4884, which provides a linked issue for the stated latency fix.Out of Scope Changes check The changes support the stated latency objective. Accepted-input priority and unchanged cursor-write suppression are also documented as implementation goals. Title check The title clearly and concisely identifies the main change: reducing interactive pane echo latency.
- Fix all pre-merge checks with AI
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
✅ Action performedReview finished.
|
JJLiebig
added
the
ai-review
|
@coderabbitai review |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
@greptileai review @coderabbitai review Please review the latest head |
|
|
@greptileai full review |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
Upstream commits brought in: - fix: detect codex startup update chooser (herdrdev#5174) - fix: confirm closing a workspace's last pane in the tui (herdrdev#5173) - fix: keep sgr pixel mouse units consistent (herdrdev#4971) - fix: reject workspace numbers past the last workspace (herdrdev#5171) - fix: reject oversized canonical input on macos (herdrdev#4988) - fix: save sessions before agents die during macos restart (herdrdev#5092) - fix: preserve hermes plugin list structure (herdrdev#4984) - fix: restore keyboard protocol flags after a terminal reconnect (herdrdev#5088) - fix: reduce interactive pane echo latency (herdrdev#5149) - fix: flush session saves before replacing persisted files (herdrdev#5152) Conflict resolutions: - client shell close-pane confirmation: take upstream's request_pane_close, keep the fork's SetPaneStatus menu action and closes_group confirmation field; a group-root close still confirms client-side and closes the whole group (the fork has no server confirmation_required path, 2d839ce), so upstream's close_pane group test is adapted like close_tab.rs. - pane input: keep the fork's backpressure-preserving send paths and add upstream's keyboard_forwarded tracking; keep send_bytes_after and write_user_input next to upstream's validate_input_submission. - pane mouse: take upstream's merged encoder/position helper. - satisfy the fork's redundant_clone deny lint in the new macos input tests. Sets UPSTREAM_HERDR_COMMIT to 6d697e1 and bumps bora to 0.52.3.
* fix: reduce interactive pane echo latency refs herdrdev#4884 * fix: preserve accepted input priority and skip unchanged cursor writes refs herdrdev#4884
Ordinary PowerShell redraws arrive as separate cursor/style, glyph, and prompt updates. The server's 16 ms cadence can delay each later part, and Windows timer overshoot amplifies that wait. Native Windows probes reproduced roughly 30 ms character and Ctrl+C prompt response on the unchanged base.
Give visible PTY output from a pane or popup receiving successfully forwarded keyboard/text input a 32 ms opportunity to render early. A server-wide budget permits at most 16 extra attempts per 16 ms; more keys cannot replenish that budget. This covers the observed split PSReadLine redraw, including escape-only updates before its glyph/prompt. Ordinary output retains the existing cadence. Hidden-only output, synchronized-output boundaries, retained rendering, and slow-client recovery keep their existing paths. Mouse events, releases, host scrolling, and paste do not arm early rendering.
The final revision also preserves priority for accepted keys when a later input event fails, and skips empty ANSI writes when both the cursor and its display policy are unchanged. Content revisions and the global budget are retained.
Current head:
c9a0014e53c32d31b4356d9d57dac75e00e0016f. Refs #4884.Validation
3eca9a688977734176c4410b65c1e01272820d98, base2563803dca97c040beaf3dc3acdcb5a3221b4238.just checkpassed: formatting, Clippy, 3,402 Rust tests passed / 12 skipped, Python maintenance checks, UI architecture checks, integration assets, docs contracts, and debug build. Pre-commitjust lintpassed.just check.just build, identical locked dependencies, Rust 1.96.1, Zig 0.16.0, and the verified app-local ConPTY bundle. Diagnostic tracing was disabled for final measurements. A reboot interrupted validation; rebuilding its corrupt generated PDB recovered the linker, and the complete final run passed.Initial workstation render benchmark
Ran canonical
just bench-render-scaletwice per variant, sequentially in the same isolated worktree, at 120x40 geometry with populated histories and 1/15/50 panes. Every command passed all six selected profiles. Values below are combined server surface + client composition median microseconds, with both passes shown.These profiles measure render cost/scaling, not input latency. Before samples were captured before the workstation reboot, after samples afterward; sampled total-machine CPU was 23-35% before and 9-21% after. Lower after times are not evidence of a causal render-speed improvement. Active 15-pane/1-pane median ratios after were 1.13/1.15; background ratios were 1.03/0.95. No added per-pane renderer work is introduced.
Initial workstation response comparison
An isolated outer ConPTY at 120x30 injects
Z, Ctrl+C, and Enter into PowerShell 7 (-NoLogo -NoProfile). A blocking reader timestamps the first bytes containing the target glyph or completedPS4884>prompt. Four fresh sessions per variant with eight samples each = 32 samples per action, using before/after/after/before order twice. Valid configuration, no diagnostic tracing, and identical terminal capabilities were used on both sides. P95 uses nearest rank.^CfeedbackAfter 155 Enter events at 31 Hz, sentinel latency was 15.64-33.27 ms before and 13.79-19.42 ms after across four sessions. This measures output bytes, not displayed pixels. The completed Ctrl+C prompt still has tail latency; the bounded allowance is not a deadline guarantee for arbitrary packet counts or shell execution time.
Initial workstation sustained-output cost
One visible pane repeatedly writes a carriage-return counter. Each phase runs for 60 seconds: continuous output alone, then the same output with 1,800
xkeys (30 Hz). CPU sums Herdr server + client process time as percent of one logical core, excluding PowerShell and ConPTY. Frame counts are completed synchronized-output markers at the outer pipe, not all internal render attempts.The trade-off is +9.22 percentage points of one core while continuously typing into busy output, with more delivered updates; output without typing remains near baseline. The global budget bounds extra attempts even with repeated keys or multiple input panes. This load probe used one pane/client; live multi-client, 15-pane busy-output scheduling, and displayed-pixel latency were not measured. Multi-pane render scaling and existing hidden-output/backpressure/synchronized-output tests passed, but do not substitute for those live workloads.
Full benchmark/check logs, sample/chunk JSON, probe drivers, and binary hashes are retained locally under
%TEMP%\herdr-4884-validation. Before binary SHA-256:8c95f746a37ad7de74042d4f012f826ade889efe62731a57958b92f5e2011fe2; after:dd193151e71f621f4234b3894bb1e573bf7bc36bd143485d420485673a02dea2.Laptop follow-up at
c9a0014e53c32d31b4356d9d57dac75e00e0016fThe final revision keeps priority for earlier accepted keyboard/text events when a later event in the same batch fails (for example, when the runtime input queue fills). Both pane and popup callers preserve that success flag and still log the error. The existing server regression fills three of four queue slots, submits A then B, and verifies A is accepted, B is rejected, and early rendering remains armed.
The ANSI encoder now remembers the committed cursor suppression policy. An empty-row patch with unchanged cursor state and unchanged policy emits no bytes, including when the native cursor is visible. The existing regression covers unchanged visible cursors and both native/client-drawn cursor transitions. Content revisions still propagate for selection, copy/search, and links; the global render budget is unchanged.
All follow-up validation ran on
JJLAPTOP; no workstation tests, builds, or benchmarks were run. The exact four-file diff was recovered from the workstation, then checked and measured on the laptop.origin/masterremained2563803dca97c040beaf3dc3acdcb5a3221b4238before publication. Laptopjust checkpassed: 3,402 Rust tests passed / 12 skipped, formatting, Clippy, maintenance and architecture checks, integration assets, docs contracts, and debug build. All six profiles passed in each of fourjust bench-render-scaleruns (twice per variant). Both release variants were freshly built on the laptop using Rust 1.96.1, Zig 0.16.0, locked dependencies, and the app-local ConPTY bundle. The published candidate3eca9a6is the baseline for the tables below; the earlier workstation tables above describe that candidate against master, not the final follow-up.Laptop latency: published PR candidate versus final revision
Same outer-ConPTY protocol and 120x30 geometry as above. Original/optimized/optimized/original order twice, four fresh sessions per variant, eight samples per session, 32 per action. Values are pooled median / nearest-rank P95 in milliseconds.
Character and completed Ctrl+C medians remained near 15 ms. Enter median increased 0.34 ms (2.2%). Held-Enter sentinel ranges were 14.74-16.52 ms versus 15.18-20.21 ms, including one slower final-revision sample. This is a small sample, not a guarantee about tail latency or displayed pixels.
Laptop sustained output: two 60-second runs per variant
Run order was original/optimized/optimized/original. Each fresh session measured output alone, then output with 1,800 keys at 30 Hz. CPU includes Herdr server + client and is percent of one logical core. Ranges show the two runs; medians are the midpoint of those two observations.
The busy-typing CPU median was lower in this comparison, while output-only CPU remained near baseline. Different frame counts and run-to-run variance mean this does not establish a general 11.2% CPU reduction, nor isolate all of it to skipped writes. The deterministic regression proves redundant writes are removed; broader scheduler changes are outside this fix.
Laptop render scaling
Combined server surface + client composition median microseconds, fixed 120x40 geometry and populated histories. Each cell shows both passes.
These render profiles are supporting scaling evidence, not a measurement of the ANSI write optimization. Active-pane medians changed by +2.4%, -1.0%, and +1.0% at 1/15/50 panes; background medians changed by +3.7%, -7.8%, and -10.7%. No causal render-speed claim is made.
Laptop evidence, raw samples, scripts, complete checks/benchmark logs, and the preserved working diff are under
%TEMP%\herdr-4884-validation\laptop-*. Laptop original release SHA-256:51950f432d295ada26be6c3b2add14d576a188fc6a4eba1b23a7515be9626404; final release:01d97e83cd556fa0711ea4fb3998e7762aac6bcbcc1b73ffece81034c898f561.Live multi-client and multi-pane busy-output scheduling, and displayed-pixel latency, remain unmeasured. The main Herdr session was untouched; disposable probe sessions were stopped and deleted.