Uh oh!
There was an error while loading. Please reload this page.
fix(fleet): stop TUI tick from repainting preview/footer, disable anim over RDP - #464
Merged
Merged
Conversation
…m over remote sessions RunsScreen._tick rebuilt the whole preview Text and re-evaluated the footer's key bindings ~10 times a second just to animate a spinner glyph, which felt laggy over RDP/SSH. Split the preview pane so the tick only updates the spinner glyph, and auto-disable the animation clock (and Textual's animation_level) when an RDP or SSH session is detected. CONDUCTOR_FLEET_NO_ANIM still force-disables; a new CONDUCTOR_FLEET_ANIM force-enables regardless of detection. Issue #462.
Blocking: - Add tests/test_fleet/test_tui_app.py covering the remote-session detection wiring in app.py::on_mount (animation_level toggling and the notify() toast), previously unreachable from CI because the fleet conftest disables animation for every test. - Make the flagship regression test (TestFrameTickDoesNotRepaintPreviewOrFooter) deterministic instead of latently flaky by monkeypatching POLL_INTERVAL_SECONDS to 60s so the ~2s data poll can never land inside the idle-frame measurement window, and strengthen it with behaviour-level patch.object(..., wraps=...) assertions on #run-preview/ #run-preview-score so an inline rebuild bypassing _preview_text would still be caught. - Correct CHANGELOG.md, docs/fleet.md and AGENTS.md, which claimed CONDUCTOR_FLEET_NO_ANIM behaviour was unchanged / attributed the new App.animation_level = "none" wiring to detection alone. It actually fires for any path that disables animation, including the existing explicit CONDUCTOR_FLEET_NO_ANIM switch. Recommendations applied: - _animate_preview now gates on _ANIMATED_STATUSES like its sibling _tick row loop, so a paused/completed run's preview chip no longer spins forever; docstring and a new regression test updated to match. - Static.update(score, layout=False) on the animated path, since every spinner glyph is one cell wide and the widget's size cannot change. - Corrected two now-inaccurate docstrings (_update_gate_detail's ownership claim, _refresh_row_bindings' call-rate claim). - docs/cli-reference.md and docs/fleet.md: removed/reworded two other inaccuracies (sparkline motion is not gated by animations_enabled; "one frame clock" overstates the two independent timers). - Reworded the notification text to sidestep the "a RDP"/"a SSH" article mismatch. Recommendations skipped (see PR review reply for full reasoning): rebasing onto origin/main (explicitly against workflow rules -- never rebase an open PR); the remaining recommendations (unused width param, try-block scope, failure-counter/notify on repeated errors, Literal typing for RDP/SSH, shared precedence-chain extraction, additional layout-equivalence/progress-header/display-toggle tests) were judged too large relative to what they buy for a review-application pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Jason Robert (jrob5756)
marked this pull request as ready for review
August 18, 2026 19:52
added 2 commits
August 18, 2026 16:30
…k-repaint # Conflicts: # AGENTS.md
SSH detection was implemented on the assumption that any remote session pays the same repaint cost. Measurement says otherwise, and the mechanism explains why: RDP renders server-side into a framebuffer, then diffs, encodes and ships changed *pixel regions* with codecs tuned for mostly-static desktop content, so cost scales with pixels changed per second and a churning text region is its worst case. SSH ships the ANSI byte stream and the client's own terminal renders it, so cost is a few hundred bytes per frame, fire-and-forget with no per-frame round trip. The two differ by orders of magnitude. What actually warrants disabling animation is a *slow* link, and there is no signal for slow -- only "SSH at all", which is overwhelmingly a fast LAN or broadband connection. Auto-disabling on it degraded the common case and announced a problem the reader did not have. CONDUCTOR_FLEET_NO_ANIM remains the remedy for a genuinely slow link and for every remote transport with no reliable signal (VNC, Citrix, xrdp), so nothing that previously had a fix loses one. Also drops the precedence chain from four levels to three. is_remote_session() keeps its `str | None` return rather than collapsing to bool, so a second transport that later earns an automatic default needs no signature churn. The removed SSH cases become negative assertions rather than deletions, so the scoping decision is pinned where a missing test would read as an oversight and invite a "fix". Refs #462
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 19, 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 freeto 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
RunsScreen._tickno longer rebuilds the whole preview pane / re-evaluates footer bindings on every ~10fps animation tick — only the spinner glyph updates.SESSIONNAME, SSH viaSSH_CONNECTION/SSH_TTY) now auto-disable the animation clock and Textual'sanimation_level.CONDUCTOR_FLEET_NO_ANIMstill force-disables; newCONDUCTOR_FLEET_ANIMforce-enables regardless of detection.Closes#462
Testing
tests/test_fleet/test_tui_anim.pyandtests/test_fleet/test_tui_runs.py.