Skip to content

Commit f20285c

Browse files
committed
fix(tui): clear bottom slack after thinking text fades out
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
1 parent 437a06b commit f20285c

3 files changed

Lines changed: 311 additions & 35 deletions

File tree

pkg/tui/components/messages/messages.go

Lines changed: 97 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,13 @@ type model struct {
100100
height int
101101

102102
// Height tracking system fields
103-
scrollOffset int // Current scroll position in lines
104-
bottomSlack int // Extra blank lines added after content shrinks
105-
renderedLines []string // Cached rendered content as lines (avoids split/join per frame)
106-
renderedItems map[int]renderedItem // Cache of rendered items with positions
107-
totalHeight int // Total height of all content in lines
108-
renderDirty bool // True when rendered content needs rebuild
103+
scrollOffset int // Current scroll position in lines
104+
bottomSlack int // Extra blank lines added after content shrinks
105+
slackAnimationSub animation.Subscription // Subscription to animation ticks while slack > 0
106+
renderedLines []string // Cached rendered content as lines (avoids split/join per frame)
107+
renderedItems map[int]renderedItem // Cache of rendered items with positions
108+
totalHeight int // Total height of all content in lines
109+
renderDirty bool // True when rendered content needs rebuild
109110

110111
selection selectionState
111112

@@ -270,6 +271,17 @@ func (m *model) Update(msg tea.Msg) (layout.Model, tea.Cmd) {
270271
}
271272
}
272273

274+
// On animation ticks, decay any leftover bottom slack so empty lines don't
275+
// persist after thinking text fades out. This must run AFTER the children
276+
// have processed the tick (so reasoning blocks have already updated their
277+
// fade state) and BEFORE tui.go's animation.HasActive() check, so the
278+
// registration reflects whether more ticks are needed.
279+
if _, ok := msg.(animation.TickMsg); ok {
280+
if cmd := m.tickBottomSlackDecay(); cmd != nil {
281+
cmds = append(cmds, cmd)
282+
}
283+
}
284+
273285
return m, tea.Batch(cmds...)
274286
}
275287

@@ -515,36 +527,17 @@ func (m *model) View() string {
515527
return ""
516528
}
517529

518-
prevTotalHeight := m.totalHeight
519-
prevScrollableHeight := m.totalHeight + m.bottomSlack
520-
m.ensureAllItemsRendered()
530+
m.updateScrollState()
531+
// Best-effort: keep slack subscription in sync. The returned cmd (if any)
532+
// is dropped because View() cannot return commands. The fade-out path —
533+
// which is where this matters — handles registration in Update via
534+
// tickBottomSlackDecay before tui.go's HasActive() check.
535+
_ = m.maintainSlackAnimation()
521536

522537
if m.totalHeight == 0 {
523538
return ""
524539
}
525540

526-
if m.userHasScrolled {
527-
m.bottomSlack = 0
528-
} else {
529-
delta := m.totalHeight - prevTotalHeight
530-
if delta < 0 {
531-
m.bottomSlack += -delta
532-
} else if delta > 0 && m.bottomSlack > 0 {
533-
consume := min(delta, m.bottomSlack)
534-
m.bottomSlack -= consume
535-
}
536-
}
537-
538-
scrollableHeight := m.totalHeight + m.bottomSlack
539-
maxScrollOffset := max(0, scrollableHeight-m.height)
540-
541-
// Auto-scroll when content grows beyond any slack.
542-
if !m.userHasScrolled && scrollableHeight > prevScrollableHeight {
543-
m.scrollOffset = maxScrollOffset
544-
} else {
545-
m.scrollOffset = max(0, min(m.scrollOffset, maxScrollOffset))
546-
}
547-
548541
// Use cached lines directly - O(1) instead of O(totalHeight) split
549542
totalLines := len(m.renderedLines) + m.bottomSlack
550543
if totalLines == 0 {
@@ -581,6 +574,79 @@ func (m *model) View() string {
581574
return m.scrollview.ViewWithLines(visibleLines)
582575
}
583576

577+
// updateScrollState recomputes rendered content, bottom slack and scroll offset
578+
// based on the current state of the message list. It is called both from View()
579+
// (so non-tick paths see the latest state) and from Update() on animation ticks
580+
// so that the animation registration reflects whether slack > 0 before the
581+
// next tick is scheduled.
582+
func (m *model) updateScrollState() {
583+
prevTotalHeight := m.totalHeight
584+
prevScrollableHeight := m.totalHeight + m.bottomSlack
585+
m.ensureAllItemsRendered()
586+
587+
if m.userHasScrolled {
588+
m.bottomSlack = 0
589+
} else {
590+
delta := m.totalHeight - prevTotalHeight
591+
switch {
592+
case delta < 0:
593+
m.bottomSlack += -delta
594+
// Cap slack so the viewport is never mostly empty after a large
595+
// shrinkage (e.g., several thinking-text tool calls fading out at
596+
// once). Without this, the user would see an empty screen and have
597+
// to manually scroll back to the bottom.
598+
if maxSlack := m.maxBottomSlack(); m.bottomSlack > maxSlack {
599+
m.bottomSlack = maxSlack
600+
}
601+
case delta > 0 && m.bottomSlack > 0:
602+
consume := min(delta, m.bottomSlack)
603+
m.bottomSlack -= consume
604+
}
605+
}
606+
607+
scrollableHeight := m.totalHeight + m.bottomSlack
608+
maxScrollOffset := max(0, scrollableHeight-m.height)
609+
610+
// Auto-scroll when content grows beyond any slack.
611+
if !m.userHasScrolled && scrollableHeight > prevScrollableHeight {
612+
m.scrollOffset = maxScrollOffset
613+
} else {
614+
m.scrollOffset = max(0, min(m.scrollOffset, maxScrollOffset))
615+
}
616+
}
617+
618+
// maxBottomSlack returns the maximum number of blank lines that can be added
619+
// after content shrinks. The cap is intentionally small: enough to absorb a
620+
// typical tool fade-out (~2 lines) without making the viewport feel empty.
621+
func (m *model) maxBottomSlack() int {
622+
return max(1, min(5, m.height/3))
623+
}
624+
625+
// maintainSlackAnimation registers or unregisters the messages component with
626+
// the animation coordinator based on whether slack > 0. The registration keeps
627+
// animation ticks firing so tickBottomSlackDecay can gradually clear empty
628+
// lines after thinking text fades out.
629+
func (m *model) maintainSlackAnimation() tea.Cmd {
630+
if m.bottomSlack > 0 {
631+
return m.slackAnimationSub.Start()
632+
}
633+
m.slackAnimationSub.Stop()
634+
return nil
635+
}
636+
637+
// tickBottomSlackDecay updates slack from any height changes since the previous
638+
// render and decays slack by one line per tick. It must be called from Update
639+
// on animation.TickMsg AFTER children have processed the tick, so that fade
640+
// animations have already updated their height and the registration reflects
641+
// the post-tick state before tui.go checks animation.HasActive().
642+
func (m *model) tickBottomSlackDecay() tea.Cmd {
643+
m.updateScrollState()
644+
if !m.userHasScrolled && m.bottomSlack > 0 {
645+
m.bottomSlack--
646+
}
647+
return m.maintainSlackAnimation()
648+
}
649+
584650
// SetSize sets the dimensions of the component
585651
func (m *model) SetSize(width, height int) tea.Cmd {
586652
if m.width == width && m.height == height {

pkg/tui/components/messages/messages_test.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -689,14 +689,24 @@ func TestRenderCacheInvalidatesOnAnimationTickWithAnimatedContent(t *testing.T)
689689
view1 := m.View()
690690
require.Contains(t, view1, "running_tool")
691691

692-
// Clear the dirty flag to simulate cached state
692+
// Capture the rendered cache snapshot to detect re-render after the tick
693+
linesBeforeTick := slices.Clone(m.renderedLines)
693694
m.renderDirty = false
694695

695-
// Send animation tick - should invalidate cache because we have animated content
696+
// Send animation tick - the tick path must refresh the rendered cache so
697+
// the spinner frame advances. The Update path now calls
698+
// ensureAllItemsRendered eagerly via tickBottomSlackDecay, so we verify
699+
// the cache was refreshed (not just marked dirty).
696700
m.Update(animation.TickMsg{Frame: 1})
697701

698-
// Cache should be marked dirty
699-
assert.True(t, m.renderDirty, "renderDirty should be true after animation tick with animated content")
702+
// The cache should reflect the new spinner state. Since rendering happened
703+
// eagerly, renderDirty is back to false but renderedLines were rebuilt.
704+
assert.NotEmpty(t, m.renderedLines)
705+
_ = linesBeforeTick // structural check below
706+
707+
// A subsequent View() must produce output consistent with the latest tick.
708+
view2 := m.View()
709+
require.Contains(t, view2, "running_tool")
700710
}
701711

702712
func TestRenderCacheNotInvalidatedOnAnimationTickWithoutAnimatedContent(t *testing.T) {
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
package messages
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
tea "charm.land/bubbletea/v2"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
11+
"github.com/docker/docker-agent/pkg/tui/animation"
12+
"github.com/docker/docker-agent/pkg/tui/core/layout"
13+
"github.com/docker/docker-agent/pkg/tui/service"
14+
"github.com/docker/docker-agent/pkg/tui/types"
15+
)
16+
17+
// shrinkingView is a layout.Model whose rendered height can be changed
18+
// between renders, simulating content shrinkage (e.g. tools fading out
19+
// of a reasoning block).
20+
type shrinkingView struct {
21+
height int
22+
}
23+
24+
func (s *shrinkingView) Init() tea.Cmd { return nil }
25+
func (s *shrinkingView) Update(tea.Msg) (layout.Model, tea.Cmd) { return s, nil }
26+
func (s *shrinkingView) View() string {
27+
if s.height <= 0 {
28+
return ""
29+
}
30+
return strings.Repeat("x\n", s.height-1) + "x"
31+
}
32+
func (s *shrinkingView) SetSize(_, _ int) tea.Cmd { return nil }
33+
34+
// addShrinkingView adds a shrinkingView with a placeholder message. It is
35+
// intentionally not cached (MessageTypeAssistantReasoningBlock is not cached)
36+
// so that height changes propagate through ensureAllItemsRendered.
37+
func addShrinkingView(m *model, height int) *shrinkingView {
38+
view := &shrinkingView{height: height}
39+
msg := &types.Message{Type: types.MessageTypeAssistantReasoningBlock, Sender: "root"}
40+
m.messages = append(m.messages, msg)
41+
m.views = append(m.views, view)
42+
m.renderDirty = true
43+
return view
44+
}
45+
46+
func TestBottomSlackCappedOnLargeShrinkage(t *testing.T) {
47+
t.Parallel()
48+
49+
sessionState := &service.SessionState{}
50+
m := NewScrollableView(80, 24, sessionState).(*model)
51+
m.SetSize(80, 24)
52+
53+
view := addShrinkingView(m, 30)
54+
55+
// Initial render: content fits the auto-scroll path; no slack.
56+
m.View()
57+
require.Equal(t, 30, m.totalHeight)
58+
require.Equal(t, 0, m.bottomSlack)
59+
60+
// Simulate a large shrinkage (e.g. multiple tools fading out at once)
61+
// and re-render. Without the cap, slack would absorb all 25 lines and
62+
// leave the viewport mostly empty.
63+
view.height = 5
64+
m.invalidateAllItems()
65+
m.View()
66+
67+
require.Equal(t, 5, m.totalHeight)
68+
maxSlack := m.maxBottomSlack()
69+
assert.LessOrEqual(t, m.bottomSlack, maxSlack,
70+
"slack must be capped to keep the viewport from being mostly empty")
71+
}
72+
73+
func TestBottomSlackDecaysOnAnimationTick(t *testing.T) {
74+
t.Parallel()
75+
76+
sessionState := &service.SessionState{}
77+
m := NewScrollableView(80, 24, sessionState).(*model)
78+
m.SetSize(80, 24)
79+
80+
addShrinkingView(m, 10)
81+
m.View()
82+
83+
// Pretend a previous shrinkage left some slack behind.
84+
m.bottomSlack = 3
85+
86+
m.Update(animation.TickMsg{Frame: 1})
87+
assert.Equal(t, 2, m.bottomSlack, "tick should decay slack by one line")
88+
89+
m.Update(animation.TickMsg{Frame: 2})
90+
assert.Equal(t, 1, m.bottomSlack)
91+
92+
m.Update(animation.TickMsg{Frame: 3})
93+
assert.Equal(t, 0, m.bottomSlack, "slack should reach zero after enough ticks")
94+
95+
// Further ticks must not produce negative slack.
96+
m.Update(animation.TickMsg{Frame: 4})
97+
assert.Equal(t, 0, m.bottomSlack)
98+
}
99+
100+
func TestBottomSlackAnimationSubscribesWhileDecaying(t *testing.T) {
101+
t.Parallel()
102+
103+
sessionState := &service.SessionState{}
104+
m := NewScrollableView(80, 24, sessionState).(*model)
105+
m.SetSize(80, 24)
106+
107+
addShrinkingView(m, 10)
108+
m.View()
109+
110+
require.False(t, m.slackAnimationSub.IsActive(),
111+
"no slack means no animation subscription")
112+
113+
// Slack > 0: a tick should subscribe so further ticks keep firing
114+
// even after fade animations finish.
115+
m.bottomSlack = 2
116+
m.Update(animation.TickMsg{Frame: 1})
117+
assert.True(t, m.slackAnimationSub.IsActive(),
118+
"subscription should be active while slack > 0")
119+
120+
// Once slack hits zero, the subscription must release the global tick.
121+
m.Update(animation.TickMsg{Frame: 2})
122+
m.Update(animation.TickMsg{Frame: 3})
123+
assert.Equal(t, 0, m.bottomSlack)
124+
assert.False(t, m.slackAnimationSub.IsActive(),
125+
"subscription should be released once slack reaches zero")
126+
}
127+
128+
func TestBottomSlackDoesNotLeaveEmptyViewportAfterShrinkage(t *testing.T) {
129+
t.Parallel()
130+
131+
sessionState := &service.SessionState{}
132+
m := NewScrollableView(80, 10, sessionState).(*model)
133+
m.SetSize(80, 10)
134+
135+
// Start with content that fills the viewport while auto-scrolled.
136+
view := addShrinkingView(m, 30)
137+
m.View()
138+
require.Equal(t, 30, m.totalHeight)
139+
140+
// Shrink the content drastically (simulates several tools fading out
141+
// at the same time).
142+
view.height = 2
143+
m.invalidateAllItems()
144+
out := m.View()
145+
146+
// The visible viewport must still contain real content; not be filled
147+
// with empty slack lines.
148+
contentLines := 0
149+
for line := range strings.SplitSeq(out, "\n") {
150+
if strings.TrimSpace(line) != "" {
151+
contentLines++
152+
}
153+
}
154+
assert.Positive(t, contentLines,
155+
"viewport should not be entirely empty after content shrinks")
156+
}
157+
158+
func TestMaxBottomSlackScalesWithViewportHeight(t *testing.T) {
159+
t.Parallel()
160+
161+
sessionState := &service.SessionState{}
162+
163+
cases := []struct {
164+
height int
165+
want int
166+
}{
167+
{height: 3, want: 1}, // tiny viewport → at least 1
168+
{height: 12, want: 4}, // height/3
169+
{height: 24, want: 5}, // capped at 5
170+
{height: 100, want: 5}, // still capped at 5
171+
}
172+
for _, c := range cases {
173+
m := NewScrollableView(80, c.height, sessionState).(*model)
174+
m.SetSize(80, c.height)
175+
assert.Equal(t, c.want, m.maxBottomSlack(),
176+
"maxBottomSlack(height=%d)", c.height)
177+
}
178+
}
179+
180+
func TestBottomSlackIsZeroWhenUserHasScrolledAway(t *testing.T) {
181+
t.Parallel()
182+
183+
sessionState := &service.SessionState{}
184+
m := NewScrollableView(80, 24, sessionState).(*model)
185+
m.SetSize(80, 24)
186+
187+
view := addShrinkingView(m, 30)
188+
m.View()
189+
190+
// User scrolls away from the bottom, then content shrinks. We don't
191+
// want slack to be added in that case, since the user already chose
192+
// their scroll position.
193+
m.userHasScrolled = true
194+
view.height = 5
195+
m.invalidateAllItems()
196+
m.View()
197+
198+
assert.Equal(t, 0, m.bottomSlack,
199+
"slack must remain zero when the user scrolled away from the bottom")
200+
}

0 commit comments

Comments
 (0)