fix(tui): clear bottom slack after thinking text fades out#2543
Merged
dgageot merged 4 commits intodocker:mainfrom Apr 27, 2026
Merged
Conversation
Tools fading out of a reasoning block were leaving the viewport mostly empty until the user manually reset the scroll bar. The bottomSlack mechanism kept absorbing the height shrinkage but never released it because nothing decayed the slack once content settled. - Cap bottomSlack at min(5, height/3) so a large simultaneous shrinkage cannot fill the visible window with blank lines. - Decay bottomSlack by one line per animation tick so leftover empty lines clear quickly after a fade. - Self-subscribe to the animation coordinator while slack > 0 so the tick stream keeps firing after the originating fades end. - Move the slack/scroll bookkeeping into updateScrollState() and call it from both View() and Update() on animation.TickMsg, so the new subscription is registered before tui.go's HasActive() check and the next tick is correctly scheduled. Includes regression tests covering capping, decay, subscription lifecycle, and that the viewport always contains real content after a shrinkage. Assisted-By: docker-agent
Tighten the bottom-slack code without changing behaviour: - Inline the maintainSlackAnimation helper into onAnimationTick (its only meaningful caller). View() now just calls Stop() when slack reaches zero, which avoids a tea.Cmd that would have been silently dropped — and removes the subtle "subscription marked active but no tick scheduled" failure mode that came with it. - Rename tickBottomSlackDecay to onAnimationTick to match Bubbletea's on<Event> naming. - Collapse the slack add/consume branches in updateScrollState to a single line each using min/max. - Drop the redundant nil-cmd check around onAnimationTick (tea.Batch already filters nils). - Tighten the surrounding comments. Also simplify the spinner-tick test that the previous commit updated: the dead linesBeforeTick capture is removed and the assertion reads top-down. Assisted-By: docker-agent
Apply review feedback on the bottom-slack work: - Rename onAnimationTick to handleAnimationTick to match the surrounding handle* convention (handleMouseClick, handleKeyPress, etc.). - Apply maxBottomSlack() to the public AdjustBottomSlack API so external callers cannot bypass the cap that protects the viewport from going mostly empty. - Strengthen TestBottomSlackAnimationSubscribesWhileDecaying so it also asserts that Update returns a non-nil tick command — without it, the subscription would be marked active but no tick would ever be scheduled. - Add TestBottomSlackDecayPausesWhenUserScrollsAway covering the case where the user scrolls away mid-decay. - Add TestAdjustBottomSlackRespectsCapAndFloor covering the new cap and the existing floor on AdjustBottomSlack. Assisted-By: docker-agent
TestBottomSlackAnimationSubscribesWhileDecaying was flaky when run in parallel with other tests touching the animation coordinator. animation.Subscription.Start only returns a non-nil tea.Cmd for the first global subscriber: subsequent subscribers piggy-back on the already-running tick stream. So when other parallel tests had already registered an animation, our Start returned nil even though our subscription was correctly registered, and the assert.NotNil(t, cmd, ...) addition from the previous commit failed roughly 1 in 5 runs. The local IsActive() assertion is sufficient to verify that handleAnimationTick registered the subscription. Asserting on the global tick command is racy without exposing test-only resets in the animation package, which isn't worth it. Verified stable across 20 -race iterations. Assisted-By: docker-agent
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.
Problem
In the TUI, when thinking-text tools fade out of a reasoning block, the viewport is sometimes left mostly empty until the user manually resets the scroll bar.
Root cause
The
bottomSlackmechanism adds blank lines to absorb height shrinkage so the UI doesn't jump when tool calls in reasoning blocks appear/disappear. But slack was only ever consumed by subsequent growth — there was no decay — and it could grow unbounded. When several tools fade out at once with no follow-up content, slack ended up filling the visible window, leaving an empty screen.Fix
bottomSlackatmin(5, height/3)so the viewport can never be mostly empty after a large simultaneous shrinkage.bottomSlackby one line per animation tick so leftover empty lines clear quickly after a fade.bottomSlack > 0so the tick stream keeps firing after the originating fades end.updateScrollState()and call it from bothView()and fromUpdate()onanimation.TickMsg, so the new subscription is registered beforetui.go'sHasActive()check and the next tick is correctly scheduled.AdjustBottomSlackAPI too, so external callers cannot bypass it.Tests
New
pkg/tui/components/messages/slack_test.gocovering:maxBottomSlackscales sensibly with viewport heightAdjustBottomSlackrespects both the cap and the floorVerified stable across 20
-raceiterations.Commits
fix(tui): clear bottom slack after thinking text fades out— the fix itself.refactor(tui): simplify slack subscription handling— small simplifications (inline a one-shot helper, rename to match the codebase, collapse min/max branches).fix(tui): address slack-handling review findings— naming consistency (onAnimationTick→handleAnimationTick),AdjustBottomSlackcap enforcement, extra tests.test(tui): drop racy tick-cmd assertion from slack subscription test— a follow-up review-suggested assertion turned out to be racy when run in parallel with other animation-touching tests; the local subscription state assertion is sufficient.Assisted-By: docker-agent