Merge pull request #2543 from dgageot/board/fix-tui-empty-screen-after-thinking-text-a93f3184

fix(tui): clear bottom slack after thinking text fades out

Author: dgageot

pkg/tui/components/messages/messages.go

@@ -100,12 +100,13 @@ type model struct {
 	height   int
 
 	// Height tracking system fields
-	scrollOffset  int                  // Current scroll position in lines
-	bottomSlack   int                  // Extra blank lines added after content shrinks
-	renderedLines []string             // Cached rendered content as lines (avoids split/join per frame)
-	renderedItems map[int]renderedItem // Cache of rendered items with positions
-	totalHeight   int                  // Total height of all content in lines
-	renderDirty   bool                 // True when rendered content needs rebuild
+	scrollOffset      int                    // Current scroll position in lines
+	bottomSlack       int                    // Extra blank lines added after content shrinks
+	slackAnimationSub animation.Subscription // Subscription to animation ticks while slack > 0
+	renderedLines     []string               // Cached rendered content as lines (avoids split/join per frame)
+	renderedItems     map[int]renderedItem   // Cache of rendered items with positions
+	totalHeight       int                    // Total height of all content in lines
+	renderDirty       bool                   // True when rendered content needs rebuild
 
 	selection selectionState
 
@@ -270,6 +271,15 @@ func (m *model) Update(msg tea.Msg) (layout.Model, tea.Cmd) {
 		}
 	}
 
+	// On animation ticks, decay leftover bottom slack and keep the slack
+	// subscription in sync so empty lines don't persist after thinking text
+	// fades out. Must run after children update so reasoning blocks have
+	// applied their fade state, and before tui.go's HasActive() check so the
+	// subscription is registered when the next tick is scheduled.
+	if _, ok := msg.(animation.TickMsg); ok {
+		cmds = append(cmds, m.handleAnimationTick())
+	}
+
 	return m, tea.Batch(cmds...)
 }
 
@@ -515,36 +525,18 @@ func (m *model) View() string {
 		return ""
 	}
 
-	prevTotalHeight := m.totalHeight
-	prevScrollableHeight := m.totalHeight + m.bottomSlack
-	m.ensureAllItemsRendered()
+	m.updateScrollState()
+	// Release the slack subscription once it's no longer needed. Starting it
+	// is only done from Update via handleAnimationTick, where the returned
+	// tea.Cmd can be propagated to actually schedule the next tick.
+	if m.bottomSlack == 0 {
+		m.slackAnimationSub.Stop()
+	}
 
 	if m.totalHeight == 0 {
 		return ""
 	}
 
-	if m.userHasScrolled {
-		m.bottomSlack = 0
-	} else {
-		delta := m.totalHeight - prevTotalHeight
-		if delta < 0 {
-			m.bottomSlack += -delta
-		} else if delta > 0 && m.bottomSlack > 0 {
-			consume := min(delta, m.bottomSlack)
-			m.bottomSlack -= consume
-		}
-	}
-
-	scrollableHeight := m.totalHeight + m.bottomSlack
-	maxScrollOffset := max(0, scrollableHeight-m.height)
-
-	// Auto-scroll when content grows beyond any slack.
-	if !m.userHasScrolled && scrollableHeight > prevScrollableHeight {
-		m.scrollOffset = maxScrollOffset
-	} else {
-		m.scrollOffset = max(0, min(m.scrollOffset, maxScrollOffset))
-	}
-
 	// Use cached lines directly - O(1) instead of O(totalHeight) split
 	totalLines := len(m.renderedLines) + m.bottomSlack
 	if totalLines == 0 {
@@ -581,6 +573,63 @@ func (m *model) View() string {
 	return m.scrollview.ViewWithLines(visibleLines)
 }
 
+// updateScrollState recomputes rendered content, bottom slack and scroll
+// offset from the current state of the message list. Called both from View()
+// and from Update() on animation ticks so that the slack subscription is
+// registered before tui.go schedules the next tick.
+func (m *model) updateScrollState() {
+	prevTotalHeight := m.totalHeight
+	prevScrollableHeight := m.totalHeight + m.bottomSlack
+	m.ensureAllItemsRendered()
+
+	if m.userHasScrolled {
+		m.bottomSlack = 0
+	} else {
+		delta := m.totalHeight - prevTotalHeight
+		switch {
+		case delta < 0:
+			// Cap so the viewport is never mostly empty after a large
+			// shrinkage (e.g., several tool calls fading out at once).
+			m.bottomSlack = min(m.bottomSlack-delta, m.maxBottomSlack())
+		case delta > 0 && m.bottomSlack > 0:
+			m.bottomSlack = max(0, m.bottomSlack-delta)
+		}
+	}
+
+	scrollableHeight := m.totalHeight + m.bottomSlack
+	maxScrollOffset := max(0, scrollableHeight-m.height)
+
+	// Auto-scroll when content grows beyond any slack.
+	if !m.userHasScrolled && scrollableHeight > prevScrollableHeight {
+		m.scrollOffset = maxScrollOffset
+	} else {
+		m.scrollOffset = max(0, min(m.scrollOffset, maxScrollOffset))
+	}
+}
+
+// maxBottomSlack returns the maximum blank lines added after content shrinks.
+// Small enough that the viewport never feels empty, large enough to absorb a
+// typical tool fade-out (~2 lines) without a visible jump.
+func (m *model) maxBottomSlack() int {
+	return max(1, min(5, m.height/3))
+}
+
+// handleAnimationTick refreshes scroll state, decays any leftover slack by
+// one line, and keeps the slack subscription alive while slack > 0 so
+// further ticks fire even after fade animations finish. Returns the command
+// to schedule the next tick when the subscription transitions to active.
+func (m *model) handleAnimationTick() tea.Cmd {
+	m.updateScrollState()
+	if !m.userHasScrolled && m.bottomSlack > 0 {
+		m.bottomSlack--
+	}
+	if m.bottomSlack > 0 {
+		return m.slackAnimationSub.Start()
+	}
+	m.slackAnimationSub.Stop()
+	return nil
+}
+
 // SetSize sets the dimensions of the component
 func (m *model) SetSize(width, height int) tea.Cmd {
 	if m.width == width && m.height == height {
@@ -1535,7 +1584,7 @@ func (m *model) AdjustBottomSlack(delta int) {
 	if delta == 0 {
 		return
 	}
-	m.bottomSlack = max(0, m.bottomSlack+delta)
+	m.bottomSlack = max(0, min(m.bottomSlack+delta, m.maxBottomSlack()))
 }
 
 // contentWidth returns the width available for content.

pkg/tui/components/messages/messages_test.go

@@ -685,18 +685,17 @@ func TestRenderCacheInvalidatesOnAnimationTickWithAnimatedContent(t *testing.T)
 	m.views = append(m.views, m.createToolCallView(toolMsg))
 	m.renderDirty = true
 
-	// First render
-	view1 := m.View()
-	require.Contains(t, view1, "running_tool")
-
-	// Clear the dirty flag to simulate cached state
+	// First render populates the cache.
+	require.Contains(t, m.View(), "running_tool")
 	m.renderDirty = false
 
-	// Send animation tick - should invalidate cache because we have animated content
+	// An animation tick must refresh the cache so the spinner frame advances.
+	// onAnimationTick now re-renders eagerly inside Update, so the resulting
+	// View() output stays consistent with the latest tick.
 	m.Update(animation.TickMsg{Frame: 1})
 
-	// Cache should be marked dirty
-	assert.True(t, m.renderDirty, "renderDirty should be true after animation tick with animated content")
+	require.NotEmpty(t, m.renderedLines)
+	require.Contains(t, m.View(), "running_tool")
 }
 
 func TestRenderCacheNotInvalidatedOnAnimationTickWithoutAnimatedContent(t *testing.T) {