Merge pull request #1497 from krissetto/global-animation-ticks

Use a global animation coordinator for animation ticks

Author: dgageot

AGENTS.md

@@ -674,6 +674,27 @@ return events
 - `AssistantMessage` - Agent response (text + tool calls)
 - `ToolMessage` - Tool execution result
 
+### TUI Animation Coordination
+
+All animated TUI components share a single tick stream via `pkg/tui/animation/`.
+
+```go
+// Init: register and maybe start tick
+func (m *MyComponent) Init() tea.Cmd {
+    return animation.StartTickIfFirst()
+}
+
+// Update: handle tick
+if tick, ok := msg.(animation.TickMsg); ok {
+    m.frame = tick.Frame
+}
+
+// When done: unregister
+animation.Unregister()
+```
+
+**Rules:** Only call from `Init()`/`Update()`, never from `Cmd` goroutines. Always `Unregister()` when animation stops.
+
 ## File Locations and Patterns
 
 ### Key Package Structure

docs/CONTRIBUTING.md

@@ -86,10 +86,6 @@ This agent is an *expert Golang developer specializing in the Docker `cagent` mu
 Ask it anything about `cagent`. It can be questions about the current code or about
 improvements to the code. It can also fix issues and implement new features!
 
-## Project Architecture
-
-More info about the architecture behind `cagent` can be found [here](/docs/architecture.md)
-
 ## Add a new model provider
 
 More details on how to add a new model provider can be found in [PROVIDERS.md](/docs/PROVIDERS.md)

pkg/tui/animation/coordinator.go

@@ -0,0 +1,97 @@
+// Package animation provides centralized animation tick management for the TUI.
+// All animated components (spinners, fades, etc.) share a single tick stream
+// to avoid tick storms and ensure synchronized animations.
+//
+// Thread safety: All exported functions are safe for concurrent use, though the
+// typical usage pattern is single-threaded via Bubble Tea's Update loop.
+package animation
+
+import (
+	"sync"
+	"time"
+
+	tea "charm.land/bubbletea/v2"
+)
+
+// TickMsg is broadcast to all animated components on each animation frame.
+// Components should handle this message to update their animation state.
+type TickMsg struct {
+	Frame int
+}
+
+// Coordinator manages a single tick stream for all animations.
+// It tracks active animations and only generates ticks when at least one is active.
+type Coordinator struct {
+	// mu guards all fields. While Bubble Tea's Update loop is single-threaded,
+	// the mutex protects against accidental misuse from Cmd goroutines and
+	// ensures StartTickIfFirst is atomic (no race between check and register).
+	mu     sync.Mutex
+	frame  int
+	active int32
+}
+
+// globalCoordinator is the singleton coordinator instance.
+var globalCoordinator = &Coordinator{}
+
+// Register increments the active animation count.
+// Call this when an animation starts.
+func Register() {
+	globalCoordinator.mu.Lock()
+	globalCoordinator.active++
+	globalCoordinator.mu.Unlock()
+}
+
+// Unregister decrements the active animation count.
+// Call this when an animation stops.
+func Unregister() {
+	globalCoordinator.mu.Lock()
+	if globalCoordinator.active > 0 {
+		globalCoordinator.active--
+	}
+	globalCoordinator.mu.Unlock()
+}
+
+// HasActive returns true if any animations are currently active.
+func HasActive() bool {
+	globalCoordinator.mu.Lock()
+	active := globalCoordinator.active > 0
+	globalCoordinator.mu.Unlock()
+	return active
+}
+
+// StartTick starts the global animation tick if any animations are active.
+// Call this after processing a TickMsg to continue the tick stream.
+func StartTick() tea.Cmd {
+	globalCoordinator.mu.Lock()
+	defer globalCoordinator.mu.Unlock()
+	if globalCoordinator.active <= 0 {
+		return nil
+	}
+	return globalCoordinator.tickLocked()
+}
+
+// StartTickIfFirst registers an animation and starts the tick if this is the first.
+// This is atomic: no race between checking and registering.
+// Returns the tick command if the tick stream was started, nil otherwise.
+func StartTickIfFirst() tea.Cmd {
+	globalCoordinator.mu.Lock()
+	defer globalCoordinator.mu.Unlock()
+	wasEmpty := globalCoordinator.active == 0
+	globalCoordinator.active++
+	if wasEmpty {
+		return globalCoordinator.tickLocked()
+	}
+	return nil
+}
+
+// tickLocked returns a tick command. Must be called with mu held.
+// 14 FPS - smooth enough for most animations without being too CPU-intensive.
+func (c *Coordinator) tickLocked() tea.Cmd {
+	return tea.Tick(time.Second/14, func(time.Time) tea.Msg {
+		c.mu.Lock()
+		c.frame++
+		frame := c.frame
+		c.mu.Unlock()
+		return TickMsg{Frame: frame}
+	})
+}

pkg/tui/animation/coordinator_test.go

@@ -0,0 +1,174 @@
+package animation
+
+import (
+	"sync"
+	"testing"
+	"time"
+
+	tea "charm.land/bubbletea/v2"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func resetGlobalCoordinator(t *testing.T) {
+	t.Helper()
+	globalCoordinator.mu.Lock()
+	globalCoordinator.active = 0
+	globalCoordinator.frame = 0
+	globalCoordinator.mu.Unlock()
+}
+
+func getActiveCount() int32 {
+	globalCoordinator.mu.Lock()
+	defer globalCoordinator.mu.Unlock()
+	return globalCoordinator.active
+}
+
+func runCmdWithTimeout(t *testing.T, cmd tea.Cmd) tea.Msg {
+	t.Helper()
+	require.NotNil(t, cmd)
+
+	done := make(chan tea.Msg, 1)
+	go func() {
+		done <- cmd()
+	}()
+
+	timeout := time.NewTimer(250 * time.Millisecond)
+	defer timeout.Stop()
+
+	select {
+	case msg := <-done:
+		return msg
+	case <-timeout.C:
+		t.Fatal("timed out waiting for tick command")
+	}
+
+	return nil
+}
+
+func runTickCmd(t *testing.T, cmd tea.Cmd) TickMsg {
+	t.Helper()
+
+	msg := runCmdWithTimeout(t, cmd)
+	tickMsg, ok := msg.(TickMsg)
+	require.True(t, ok)
+
+	return tickMsg
+}
+
+func TestGlobalCoordinatorLifecycle(t *testing.T) {
+	resetGlobalCoordinator(t)
+
+	// No active animations = no tick
+	require.Nil(t, StartTick())
+
+	// First registration starts tick
+	firstTick := StartTickIfFirst()
+	tickMsg := runTickCmd(t, firstTick)
+	assert.Equal(t, 1, tickMsg.Frame)
+
+	// Subsequent tick continues
+	nextTick := StartTick()
+	tickMsg = runTickCmd(t, nextTick)
+	assert.Equal(t, 2, tickMsg.Frame)
+
+	// Second StartTickIfFirst registers but doesn't return tick (not first)
+	cmd := StartTickIfFirst()
+	require.Nil(t, cmd)
+	assert.Equal(t, int32(2), getActiveCount())
+
+	// Unregister one, still active
+	Unregister()
+	require.True(t, HasActive())
+	require.NotNil(t, StartTick())
+
+	// Unregister last one
+	Unregister()
+	require.False(t, HasActive())
+	require.Nil(t, StartTick())
+}
+
+func TestUnregisterNeverGoesNegative(t *testing.T) {
+	resetGlobalCoordinator(t)
+
+	// Multiple unregisters when already at 0
+	Unregister()
+	Unregister()
+	Unregister()
+
+	assert.Equal(t, int32(0), getActiveCount())
+	require.False(t, HasActive())
+}
+
+func TestConcurrentRegisterUnregister(t *testing.T) {
+	resetGlobalCoordinator(t)
+
+	const goroutines = 100
+	const opsPerGoroutine = 100
+
+	var wg sync.WaitGroup
+	wg.Add(goroutines * 2)
+
+	// Half goroutines do register
+	for range goroutines {
+		go func() {
+			defer wg.Done()
+			for range opsPerGoroutine {
+				Register()
+			}
+		}()
+	}
+
+	// Half goroutines do unregister
+	for range goroutines {
+		go func() {
+			defer wg.Done()
+			for range opsPerGoroutine {
+				Unregister()
+			}
+		}()
+	}
+
+	wg.Wait()
+
+	// Should have exactly goroutines * opsPerGoroutine registers
+	// minus whatever unregisters succeeded (capped at 0)
+	// Final count should be >= 0
+	count := getActiveCount()
+	assert.GreaterOrEqual(t, count, int32(0), "active count should never be negative")
+}
+
+func TestConcurrentStartTickIfFirst(t *testing.T) {
+	resetGlobalCoordinator(t)
+
+	const goroutines = 50
+	var wg sync.WaitGroup
+	wg.Add(goroutines)
+
+	cmds := make(chan tea.Cmd, goroutines)
+
+	// Many goroutines race to be "first"
+	for range goroutines {
+		go func() {
+			defer wg.Done()
+			cmd := StartTickIfFirst()
+			cmds <- cmd
+		}()
+	}
+
+	wg.Wait()
+	close(cmds)
+
+	// Count non-nil commands (ticks started)
+	ticksStarted := 0
+	for cmd := range cmds {
+		if cmd != nil {
+			ticksStarted++
+		}
+	}
+
+	// Exactly one should have started the tick
+	assert.Equal(t, 1, ticksStarted, "exactly one goroutine should start the tick")
+	// All should have registered
+	assert.Equal(t, int32(goroutines), getActiveCount())
+}

pkg/tui/components/message/message.go

@@ -52,7 +52,7 @@ func New(msg, previous *types.Message) *messageModel {
 // Init initializes the message view
 func (mv *messageModel) Init() tea.Cmd {
 	if mv.message.Type == types.MessageTypeSpinner || mv.message.Type == types.MessageTypeLoading {
-		return mv.spinner.Tick()
+		return mv.spinner.Init()
 	}
 	return nil
 }

pkg/tui/components/messages/messages.go

@@ -17,6 +17,7 @@ import (
 	"github.com/docker/cagent/pkg/session"
 	"github.com/docker/cagent/pkg/tools"
 	"github.com/docker/cagent/pkg/tools/builtin"
+	"github.com/docker/cagent/pkg/tui/animation"
 	"github.com/docker/cagent/pkg/tui/components/message"
 	"github.com/docker/cagent/pkg/tui/components/reasoningblock"
 	"github.com/docker/cagent/pkg/tui/components/scrollbar"
@@ -189,6 +190,14 @@ func (m *model) Update(msg tea.Msg) (layout.Model, tea.Cmd) {
 	case reasoningblock.BlockMsg:
 		return m.forwardToReasoningBlock(msg.GetBlockID(), msg)
 
+	case animation.TickMsg:
+		// Invalidate render cache if there's animated content that needs redrawing.
+		// This ensures fades, spinners, etc. actually update visually on each tick.
+		if m.hasAnimatedContent() {
+			m.renderDirty = true
+		}
+		// Fall through to forward tick to all views
+
 	case tea.KeyPressMsg:
 		return m.handleKeyPress(msg)
 	}
@@ -1313,3 +1322,32 @@ func (m *model) handleScrollbarUpdate(msg tea.Msg) (layout.Model, tea.Cmd) {
 	m.scrollOffset = m.scrollbar.GetScrollOffset()
 	return m, cmd
 }
+
+// hasAnimatedContent returns true if the message list contains content that
+// requires tick-driven updates (spinners, fades, etc.). Used to decide whether
+// to invalidate the render cache on animation ticks.
+func (m *model) hasAnimatedContent() bool {
+	for i, msg := range m.messages {
+		switch msg.Type {
+		case types.MessageTypeSpinner, types.MessageTypeLoading:
+			// Spinner/loading messages always need ticks
+			return true
+		case types.MessageTypeToolCall:
+			// Tool calls with pending/running status have spinners
+			if msg.ToolStatus == types.ToolStatusPending ||
+				msg.ToolStatus == types.ToolStatusRunning {
+				return true
+			}
+		case types.MessageTypeAssistantReasoningBlock:
+			// Check if reasoning block needs tick updates
+			if i < len(m.views) {
+				if block, ok := m.views[i].(*reasoningblock.Model); ok {
+					if block.NeedsTick() {
+						return true
+					}
+				}
+			}
+		}
+	}
+	return false
+}