Uh oh!
There was an error while loading. Please reload this page.
fix(fleet): stop the TUI freezing at a human gate - #448
Merged
Conversation
Opening the gate options modal (`g`) left the Runs screen animating underneath it. A covered screen is still composited, so its ~10fps repaints kept re-blending the modal on top of it: on one 160x45 terminal sitting at an open gate that was roughly 2.5x the escape sequences and ~40% more CPU than the same screen with no modal up. On a terminal that cannot absorb the stream -- over SSH, in a multiplexer, on a slow emulator -- keystrokes queue behind the redraw and the modal appears to freeze. Guard `_tick` and `_update_gate_detail` on `Screen.is_active`, and hold the animation timer so it can be paused on `ScreenSuspend` and restarted on `ScreenResume`. The guards are the load-bearing half, not the pause: `App.push_screen` appends to the screen stack synchronously but *posts* `ScreenSuspend`, and the timer invokes its callback from its own task rather than through the message pump, so frames keep arriving until the pump drains -- which takes longest exactly when the pump is backed up. The pause is what stops a 10fps task waking for a screen nobody reads. `Screen.is_current` cannot express the condition: it means "still being composited", which is the state being suppressed. Opacity is irrelevant to that -- `App._background_screens` appends the screen below the top before testing background alpha, so it is true under an opaque screen too. The ~2s data poll is deliberately left running, so gate-entry and run-failure notifications still fire while a modal is up; only the render is suppressed. Hide the preview pane unconditionally on the empty-fleet branch, which otherwise paired the "no runs" empty state with a preview still offering `g` for a run that had gone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`SplashScreen._dismiss` pops whatever is on top, so it must only fire while the splash itself is on top. It guarded on `Screen.is_current`, which means "still being composited" and stays true for a covered screen -- so a splash covered at the moment its self-dismiss timer fired would have popped the screen covering it instead of itself. Not reachable today, since nothing is pushed over the splash. This is the same misreading of `is_current` fixed on the Runs screen in the previous commit, so it is corrected while the distinction is in view rather than left as a trap for whoever first pushes a screen there. `is_active` still guards the race the original comment describes -- the timer and a keypress both dismiss, and popping an already-popped screen raises -- because a popped screen is neither active nor current. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Jason Robert (jrob5756)
marked this pull request as ready for review
August 16, 2026 14:21
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 16, 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
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.
The bug
Opening the gate options modal (
g) inconductor fleetsometimes made the TUI stop responding.Cause
The Runs screen kept animating underneath the modal. Pushing a screen in Textual does not stop the covered screen's timers, and a covered screen is still composited — so its ~10fps repaints kept re-blending the modal sitting on top of it.
Measured on a synthetic gated run (one live run, 60 KB gate prompt):
Absolute figures are machine- and emulator-specific, but the direction is structural. On a terminal that cannot absorb that stream — over SSH, in a multiplexer, on a slow emulator — keystrokes queue behind the redraw and the modal appears frozen.
The fix
Guard
_tickand_update_gate_detailonScreen.is_active, and hold the animation timer so it can be paused onScreenSuspendand restarted onScreenResume.The guards are the load-bearing half, not the pause.
App.push_screenappends to the screen stack synchronously but postsScreenSuspendas a message, and the timer invokes its callback from its own asyncio task rather than through the screen's message pump — so frames keep arriving until the pump drains, which takes longest exactly when the pump is backed up. That is the failure being fixed. The pause is what stops a 10fps task waking for a screen nobody is reading.Screen.is_currentcannot express this condition: it means still being composited, which is the state being suppressed — Textual gates its whole update cycle on it, which is why a covered screen repaints at all. Opacity is irrelevant, sinceApp._background_screensappends the screen below the top before testing background alpha, so it isTrueunder an opaque screen too.Two deliberate non-changes:
gfor a run that had gone.This also stops the screen animating under the splash at launch, and under the run-detail, history, providers, registries, and new-run screens.
Tests
Five tests, one per mechanism. All seven mutations below are killed:
_tickguard_update_gate_detailguardis_active→is_current(either guard)on_screen_resume's repaintrefresh_runstoo (would kill notifications)Two details worth knowing, both found in review:
_tickinvocations, not frames._tick's own guard already stops the frames, so a frame-counting assertion holds whether or not the timer was ever paused — only an invocation count separates "paused" from "called, returned early".DataTable.RowHighlightedrepaints on focus regain and a call-count assertion passes even when the handler's repaint is deleted.ruff check,ruff format --check,ty checkclean;tests/test_fleet/+tests/test_cli/test_markup_guards.py= 650 passed.Second commit: the same misreading on the splash
SplashScreen._dismisspops whatever is on top, so it must only fire while the splash itself is on top. It guarded onis_current, which stays true for a covered screen — so a splash covered at the moment its self-dismiss timer fired would have popped the screen covering it instead of itself.Not reachable today (nothing is pushed over the splash), so its test is a guard on the guard: it pins the predicate rather than a reachable path. Reverting to
is_currentfails it, with the splash left on top and the modal popped.Fixed here rather than left alone because it is the identical misreading, and the distinction is in view.
Notes
No existing issue covers this. #437 is a different fleet TUI performance problem (blocking I/O on the event loop).
Reviewed with the
code-reviewskill (7 agents). Findings addressed before this PR was opened, including replacing a hand-rolled_is_on_topproperty that reimplementedScreen.is_activewith a narrowerexceptclause.