Rebuild the TTY progress renderer on a model/layout/screen split - #14051
Open
ndeloof wants to merge 4 commits into
Open
Rebuild the TTY progress renderer on a model/layout/screen split#14051ndeloof wants to merge 4 commits into
ndeloof wants to merge 4 commits into
Conversation
ndeloof
force-pushed
the
tty-renderer-model-layout-screen
branch
2 times, most recently
from
August 25, 2026 09:16
eca7884 to
69d84de
Compare
ndeloof
force-pushed
the
tty-renderer-model-layout-screen
branch
2 times, most recently
from
August 25, 2026 09:50
cd7d403 to
c48995e
Compare
glours
reviewed
Aug 25, 2026
glours
left a comment
glours
left a comment
Contributor
There was a problem hiding this comment.
Some potential fixes to apply before we merge it
…t/screen split The TTY renderer accumulated display-corruption fixes (truncation of details, then progress sizes, then task ids; timer alignment; rune-based measurement) that each patched one symptom of the same structural gap: nothing guaranteed a rendered line fits the terminal, and once a line wraps, cursor arithmetic desyncs and the block corrupts. Replace it with three separable units: - tty_model.go: pure event reducer with an injected clock, preserving first-parent-wins updates, monotonic progress and header counters - tty_layout.go: pure (model, size, now) -> lines function; all widths are measured in terminal cells (go-runewidth, so CJK is correct) on plain text before coloring, and every line is clipped to the terminal width by construction, status text included - tty_screen.go: diff-based repaint; unchanged rows are skipped, identical frames write nothing, a frame is a single Write; a shrinking terminal abandons the block instead of moving the cursor over reflowed rows The writer coordinates them behind a mutex and stops the refresh goroutine through context cancellation, so Done cannot block when the operation context was cancelled first (Ctrl-C during pull). The spinner frame is derived from the clock instead of advancing on every call, and truncation can no longer split multi-byte runes. Visual output is unchanged: the snapshot test reproduces the previous renderer's golden output character for character. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
…ther row The "[+] op N/M" header was the one line bypassing renderSegs, so on a very narrow terminal it could wrap and desync the cursor arithmetic the rest of the design guarantees against. Route it through the same clip and cover degenerate widths (8, 12 cells) in the invariant test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
…14119 Transposes the applicable findings from the review of the ttyWriter fix (#14119) to the rewritten renderer: - Done waits for the refresh goroutine to exit (outside the lock), so nothing can repaint after Done returns and output printed right after an operation cannot be garbled by a stray frame - a nested Start retires the previous cycle instead of leaking its refresh goroutine until the parent context is cancelled - build suspension no longer leaks across cycles: Start resets it and starts a fresh block below whatever buildkit wrote - drop the dead strings.ToLower in startDependencies: the unexported start only reads projectName when no Project is passed The remaining findings are structural to this rewrite: Done is a single critical section, and per-cycle context cancellation replaces the doneSignal handshake. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
- layoutFrame clips its output to the terminal height: height==1 no longer overflows with header + more-marker - the two status-to-color switches merge into eventColor(status, def) - the clear-suspension sequence extracts into unsuspend() - the injected clock becomes a clockwork.Clock (already a direct dependency); tests advance a fake clock and the cancel test loses its settling sleep — Done waits for the refresh goroutine itself - roots, per-parent children and the completed count are maintained incrementally by apply(), so a 100ms tick with an unchanged tree no longer rescans every task - the frame<0 clamp in spinGlyph drops: time comes from the writer's single clock and never regresses; the invariant is documented on the model - Start's retire-previous-cycle comment spells out that cancellation only guarantees the old goroutine will exit, not that it already has Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
ndeloof
force-pushed
the
tty-renderer-model-layout-screen
branch
from
August 25, 2026 16:04
6ba9fad to
0ae1552
Compare
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What I did
Replaced the TTY progress renderer with a three-layer design that makes the recurring display corruptions impossible by construction:
tty_model.go— pure event reducer (injected clock); preserves first-parent-wins updates, monotonic progress and header counterstty_layout.go— pure(model, size, now) → linesfunction; widths measured in terminal cells (go-runewidth, CJK-correct) on plain text before coloring; every line — status text included — is clipped to the terminal width, so lines can never wrap and desync cursor arithmetictty_screen.go— diff-based repaint: unchanged rows are skipped, identical frames write nothing, oneWriteper frame; terminal shrink abandons the block instead of repainting over reflowed rowsThe writer coordinates them behind a mutex; the refresh goroutine stops through context cancellation, so
Done()can no longer deadlock when the operation context was cancelled first (Ctrl-C during pull). The spinner frame derives from the clock instead of advancing per call, and truncation can no longer split multi-byte runes.Visual output is unchanged — the snapshot test reproduces the previous renderer's golden output character for character. Net −401 lines.
Related issue
Structural follow-up to the line-overflow / cursor-desync family (#13595) and the
Done()deadlock (#13639).🤖 Generated with Claude Code