diff --git a/AGENTS.md b/AGENTS.md index f651ca486..a9f105095 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1014,6 +1014,14 @@ task push-image # Build and push multi-platform 5. **Telemetry adds overhead**: Disable with `TELEMETRY_ENABLED=false` for benchmarking +### TUI General Guidelines + +- Never use commands to send messages when you can directly mutate children or state. +- Keep things simple; do not overcomplicate. +- Create files if needed to separate logic; do not nest models. +- Never do IO or expensive work in `Update`; always use a `tea.Cmd`. +- Never change the model state inside of a command use messages and than update the state in the main loop + ## Quick Reference: Key Files | File | Purpose | diff --git a/pkg/tui/components/sidebar/sidebar.go b/pkg/tui/components/sidebar/sidebar.go index 72f1b0617..8f248a220 100644 --- a/pkg/tui/components/sidebar/sidebar.go +++ b/pkg/tui/components/sidebar/sidebar.go @@ -47,7 +47,7 @@ type Model interface { SetTokenUsage(event *runtime.TokenUsageEvent) SetTodos(result *tools.ToolCallResult) error SetMode(mode Mode) - SetAgentInfo(agentName, model, description string) + SetAgentInfo(agentName, model, description string) tea.Cmd SetTeamInfo(availableAgents []runtime.AgentDetails) SetAgentSwitching(switching bool) SetToolsetInfo(availableTools int, loading bool) @@ -93,6 +93,8 @@ type Model interface { ScrollByWheel(delta int) // IsScrollbarDragging returns true when the scrollbar thumb is being dragged. IsScrollbarDragging() bool + // Cleanup cancels any in-flight async operations. + Cleanup() } // ragIndexingState tracks per-strategy indexing progress @@ -143,6 +145,8 @@ type model struct { titleInput textinput.Model lastTitleClickTime time.Time // for double-click detection on title + cancelReasoningCheck context.CancelFunc // cancels the in-flight ModelSupportsReasoning call + // Render cache to avoid re-rendering sections on every frame during scroll cachedLines []string // Cached rendered lines cachedWidth int // Width used for cached render @@ -248,14 +252,31 @@ func (m *model) SetTodos(result *tools.ToolCallResult) error { return m.todoComp.SetTodos(result) } +// reasoningSupportResultMsg carries the async result of a ModelSupportsReasoning check. +type reasoningSupportResultMsg struct { + modelID string + supported bool +} + +// checkReasoningSupportCmd returns a tea.Cmd that checks reasoning support asynchronously. +func checkReasoningSupportCmd(ctx context.Context, modelID string) tea.Cmd { + return func() tea.Msg { + supported := modelsdev.ModelSupportsReasoning(ctx, modelID) + return reasoningSupportResultMsg{modelID: modelID, supported: supported} + } +} + // SetAgentInfo sets the current agent information and updates the model in availableAgents -func (m *model) SetAgentInfo(agentName, modelID, description string) { +func (m *model) SetAgentInfo(agentName, modelID, description string) tea.Cmd { m.currentAgent = agentName m.agentModel = modelID m.agentDescription = description - // TODO: this can block for up to 30s on the first call if the cache is cold, - // which freezes the TUI. Move to an async command. - m.reasoningSupported = modelsdev.ModelSupportsReasoning(context.TODO(), modelID) + + if m.cancelReasoningCheck != nil { + m.cancelReasoningCheck() + } + ctx, cancel := context.WithCancel(context.Background()) + m.cancelReasoningCheck = cancel // Update the provider and model in availableAgents for the current agent. // This is important when fallback models from different providers are used. @@ -274,6 +295,7 @@ func (m *model) SetAgentInfo(agentName, modelID, description string) { } } m.invalidateCache() + return checkReasoningSupportCmd(ctx, modelID) } // SetTeamInfo sets the available agents in the team @@ -596,9 +618,15 @@ func (m *model) Update(msg tea.Msg) (layout.Model, tea.Cmd) { m.invalidateCache() m.stopSpinner() // Will only stop if no other state needs it return m, nil - case *runtime.AgentInfoEvent: - m.SetAgentInfo(msg.AgentName, msg.Model, msg.Description) + case reasoningSupportResultMsg: + if msg.modelID == m.agentModel { + m.reasoningSupported = msg.supported + m.invalidateCache() + } return m, nil + case *runtime.AgentInfoEvent: + cmd := m.SetAgentInfo(msg.AgentName, msg.Model, msg.Description) + return m, cmd case *runtime.TeamInfoEvent: m.SetTeamInfo(msg.AvailableAgents) return m, nil @@ -1348,3 +1376,11 @@ func (m *model) UpdateTitleInput(msg tea.Msg) tea.Cmd { m.invalidateCache() // Input changes affect rendering return cmd } + +// Cleanup cancels any in-flight async operations. +func (m *model) Cleanup() { + if m.cancelReasoningCheck != nil { + m.cancelReasoningCheck() + m.cancelReasoningCheck = nil + } +} diff --git a/pkg/tui/handlers.go b/pkg/tui/handlers.go index 515688921..a3377281f 100644 --- a/pkg/tui/handlers.go +++ b/pkg/tui/handlers.go @@ -328,11 +328,21 @@ func (a *appModel) handleToggleYolo() (tea.Model, tea.Cmd) { } func (a *appModel) handleToggleThinking() (tea.Model, tea.Cmd) { - // Check if the current model supports reasoning + if a.cancelThinkingCheck != nil { + a.cancelThinkingCheck() + } + ctx, cancel := context.WithCancel(context.Background()) + a.cancelThinkingCheck = cancel + currentModel := a.application.CurrentAgentModel() - // TODO: this can block for up to 30s on the first call if the cache is cold, - // which freezes the TUI. Move to an async command. - if !modelsdev.ModelSupportsReasoning(context.TODO(), currentModel) { + return a, func() tea.Msg { + supported := modelsdev.ModelSupportsReasoning(ctx, currentModel) + return messages.ToggleThinkingResultMsg{Supported: supported} + } +} + +func (a *appModel) handleToggleThinkingResult(msg messages.ToggleThinkingResultMsg) (tea.Model, tea.Cmd) { + if !msg.Supported { return a, notification.InfoCmd("Thinking/reasoning is not supported for the current model") } @@ -340,20 +350,19 @@ func (a *appModel) handleToggleThinking() (tea.Model, tea.Cmd) { sess.Thinking = !sess.Thinking a.sessionState.SetThinking(sess.Thinking) - // Persist the change to the database immediately if store := a.application.SessionStore(); store != nil { if err := store.UpdateSession(context.Background(), sess); err != nil { return a, notification.ErrorCmd(fmt.Sprintf("Failed to save session: %v", err)) } } - var msg string + var infoMsg string if sess.Thinking { - msg = "Thinking/reasoning enabled for this session" + infoMsg = "Thinking/reasoning enabled for this session" } else { - msg = "Thinking/reasoning disabled for this session" + infoMsg = "Thinking/reasoning disabled for this session" } - return a, notification.InfoCmd(msg) + return a, notification.InfoCmd(infoMsg) } func (a *appModel) handleToggleHideToolResults() (tea.Model, tea.Cmd) { diff --git a/pkg/tui/messages/toggle.go b/pkg/tui/messages/toggle.go index cd6802025..99a5a0408 100644 --- a/pkg/tui/messages/toggle.go +++ b/pkg/tui/messages/toggle.go @@ -8,6 +8,12 @@ type ( // ToggleThinkingMsg toggles extended thinking mode. ToggleThinkingMsg struct{} + // ToggleThinkingResultMsg carries the async result of reasoning support check. + // If Supported is true, thinking is toggled; otherwise a notification is shown. + ToggleThinkingResultMsg struct { + Supported bool + } + // ToggleHideToolResultsMsg toggles hiding of tool results. ToggleHideToolResultsMsg struct{} diff --git a/pkg/tui/page/chat/chat.go b/pkg/tui/page/chat/chat.go index 34b77fcae..20e9db3fb 100644 --- a/pkg/tui/page/chat/chat.go +++ b/pkg/tui/page/chat/chat.go @@ -1171,6 +1171,7 @@ func (p *chatPage) CompactSession(additionalPrompt string) tea.Cmd { func (p *chatPage) Cleanup() { p.stopProgressBar() + p.sidebar.Cleanup() p.editor.Cleanup() } diff --git a/pkg/tui/page/chat/runtime_events.go b/pkg/tui/page/chat/runtime_events.go index 28071cb31..cb74f1ca9 100644 --- a/pkg/tui/page/chat/runtime_events.go +++ b/pkg/tui/page/chat/runtime_events.go @@ -58,10 +58,10 @@ func (p *chatPage) handleRuntimeEvent(msg tea.Msg) (bool, tea.Cmd) { case *runtime.ModelFallbackEvent: // Update sidebar with the fallback model immediately so it reflects the switch - p.sidebar.SetAgentInfo(msg.AgentName, msg.FallbackModel, "") + sidebarCmd := p.sidebar.SetAgentInfo(msg.AgentName, msg.FallbackModel, "") // Notify user when switching to a fallback model, include the reason fallbackMsg := fmt.Sprintf("Model %s failed (%s), switching to %s", msg.FailedModel, msg.Reason, msg.FallbackModel) - return true, notification.WarningCmd(fallbackMsg) + return true, tea.Batch(sidebarCmd, notification.WarningCmd(fallbackMsg)) // ===== Stream Lifecycle Events ===== case *runtime.StreamStartedEvent: @@ -102,9 +102,9 @@ func (p *chatPage) handleRuntimeEvent(msg tea.Msg) (bool, tea.Cmd) { return true, nil case *runtime.AgentInfoEvent: - p.sidebar.SetAgentInfo(msg.AgentName, msg.Model, msg.Description) + sidebarCmd := p.sidebar.SetAgentInfo(msg.AgentName, msg.Model, msg.Description) p.messages.AddWelcomeMessage(msg.WelcomeMessage) - return true, nil + return true, sidebarCmd case *runtime.TeamInfoEvent: p.sidebar.SetTeamInfo(msg.AvailableAgents) diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index ced3b6566..72a9ff480 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -49,6 +49,8 @@ type appModel struct { transcriber *transcribe.Transcriber + cancelThinkingCheck context.CancelFunc // cancels the in-flight thinking toggle check + // External event subscriptions (Elm Architecture pattern) themeWatcher *styles.ThemeWatcher themeSubscription *subscription.ChannelSubscription[string] // Listens for theme file changes @@ -160,7 +162,7 @@ func New(ctx context.Context, a *app.App) tea.Model { // Make sure to stop the progress bar and theme watcher when the app quits abruptly. go func() { <-ctx.Done() - t.chatPage.Cleanup() + t.cleanup() t.themeWatcher.Stop() }() @@ -168,6 +170,14 @@ func New(ctx context.Context, a *app.App) tea.Model { } // Init initializes the application +func (a *appModel) cleanup() { + if a.cancelThinkingCheck != nil { + a.cancelThinkingCheck() + a.cancelThinkingCheck = nil + } + a.chatPage.Cleanup() +} + func (a *appModel) Init() tea.Cmd { cmds := []tea.Cmd{ a.dialog.Init(), @@ -350,7 +360,7 @@ func (a *appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case messages.ExitSessionMsg: // /exit command exits immediately without confirmation - a.chatPage.Cleanup() + a.cleanup() return a, tea.Quit case messages.NewSessionMsg: @@ -407,6 +417,9 @@ func (a *appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case messages.ToggleThinkingMsg: return a.handleToggleThinking() + case messages.ToggleThinkingResultMsg: + return a.handleToggleThinkingResult(msg) + case messages.ToggleHideToolResultsMsg: return a.handleToggleHideToolResults() @@ -525,11 +538,11 @@ func (a *appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return a, nil case dialog.ExitConfirmedMsg: - a.chatPage.Cleanup() + a.cleanup() return a, tea.Quit case messages.ExitAfterFirstResponseMsg: - a.chatPage.Cleanup() + a.cleanup() return a, tea.Quit case chat.EditorHeightChangedMsg: