From 17b892b196e60047ad870344315ff899b5e58e70 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sun, 28 Dec 2025 16:33:40 -0800 Subject: [PATCH] fix: Code review fixes for TUI feed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Return *Model from NewModel() so SetEventChannel works - Use sync.Once for safe channel close on quit - Use pointer receivers consistently throughout - Capture channels in closure to prevent race - Fix indent panic in renderAgent when indent < 2 - Remove unused filterActive and err fields 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/tui/feed/model.go | 35 +++++++++++++++++++---------------- internal/tui/feed/view.go | 28 +++++++++++++++------------- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/internal/tui/feed/model.go b/internal/tui/feed/model.go index 4fed69ad..d0e43278 100644 --- a/internal/tui/feed/model.go +++ b/internal/tui/feed/model.go @@ -1,6 +1,7 @@ package feed import ( + "sync" "time" "github.com/charmbracelet/bubbles/help" @@ -64,24 +65,23 @@ type Model struct { events []Event // UI state - keys KeyMap - help help.Model - showHelp bool - filter string - filterActive bool - err error + keys KeyMap + help help.Model + showHelp bool + filter string // Event source eventChan <-chan Event done chan struct{} + closeOnce sync.Once } // NewModel creates a new feed TUI model -func NewModel() Model { +func NewModel() *Model { h := help.New() h.ShowAll = false - return Model{ + return &Model{ focusedPanel: PanelTree, treeViewport: viewport.New(0, 0), feedViewport: viewport.New(0, 0), @@ -94,7 +94,7 @@ func NewModel() Model { } // Init initializes the model -func (m Model) Init() tea.Cmd { +func (m *Model) Init() tea.Cmd { return tea.Batch( m.listenForEvents(), tea.SetWindowTitle("GT Feed"), @@ -108,18 +108,21 @@ type eventMsg Event type tickMsg time.Time // listenForEvents returns a command that listens for events -func (m Model) listenForEvents() tea.Cmd { +func (m *Model) listenForEvents() tea.Cmd { if m.eventChan == nil { return nil } + // Capture channels to avoid race with Model mutations + eventChan := m.eventChan + done := m.done return func() tea.Msg { select { - case event, ok := <-m.eventChan: + case event, ok := <-eventChan: if !ok { return nil } return eventMsg(event) - case <-m.done: + case <-done: return nil } } @@ -133,7 +136,7 @@ func tick() tea.Cmd { } // Update handles messages -func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { +func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { var cmds []tea.Cmd switch msg := msg.(type) { @@ -166,10 +169,10 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } // handleKey processes key presses -func (m Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { +func (m *Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { switch { case key.Matches(msg, m.keys.Quit): - close(m.done) + m.closeOnce.Do(func() { close(m.done) }) return m, tea.Quit case key.Matches(msg, m.keys.Help): @@ -293,6 +296,6 @@ func (m *Model) SetEventChannel(ch <-chan Event) { } // View renders the TUI -func (m Model) View() string { +func (m *Model) View() string { return m.render() } diff --git a/internal/tui/feed/view.go b/internal/tui/feed/view.go index 899e07dd..2a3cd6f7 100644 --- a/internal/tui/feed/view.go +++ b/internal/tui/feed/view.go @@ -10,7 +10,7 @@ import ( ) // render produces the full TUI output -func (m Model) render() string { +func (m *Model) render() string { if m.width == 0 || m.height == 0 { return "Loading..." } @@ -40,7 +40,7 @@ func (m Model) render() string { } // renderHeader renders the top header bar -func (m Model) renderHeader() string { +func (m *Model) renderHeader() string { title := TitleStyle.Render("GT Feed") filter := "" @@ -60,7 +60,7 @@ func (m Model) renderHeader() string { } // renderTreePanel renders the agent tree panel with border -func (m Model) renderTreePanel() string { +func (m *Model) renderTreePanel() string { style := TreePanelStyle if m.focusedPanel == PanelTree { style = FocusedBorderStyle @@ -69,7 +69,7 @@ func (m Model) renderTreePanel() string { } // renderFeedPanel renders the event feed panel with border -func (m Model) renderFeedPanel() string { +func (m *Model) renderFeedPanel() string { style := StreamPanelStyle if m.focusedPanel == PanelFeed { style = FocusedBorderStyle @@ -78,7 +78,7 @@ func (m Model) renderFeedPanel() string { } // renderTree renders the agent tree content -func (m Model) renderTree() string { +func (m *Model) renderTree() string { if len(m.rigs) == 0 { return AgentIdleStyle.Render("No agents active") } @@ -131,7 +131,7 @@ func (m Model) renderTree() string { } // groupAgentsByRole groups agents by their role -func (m Model) groupAgentsByRole(agents map[string]*Agent) map[string][]*Agent { +func (m *Model) groupAgentsByRole(agents map[string]*Agent) map[string][]*Agent { result := make(map[string][]*Agent) for _, agent := range agents { role := agent.Role @@ -152,7 +152,7 @@ func (m Model) groupAgentsByRole(agents map[string]*Agent) map[string][]*Agent { } // renderAgentGroup renders a group of agents (crew or polecats) -func (m Model) renderAgentGroup(icon, role string, agents []*Agent) string { +func (m *Model) renderAgentGroup(icon, role string, agents []*Agent) string { var lines []string // Group header @@ -172,10 +172,12 @@ func (m Model) renderAgentGroup(icon, role string, agents []*Agent) string { } // renderAgent renders a single agent line -func (m Model) renderAgent(icon string, agent *Agent, indent int) string { +func (m *Model) renderAgent(icon string, agent *Agent, indent int) string { prefix := strings.Repeat(" ", indent) - if icon != "" { + if icon != "" && indent >= 2 { prefix = strings.Repeat(" ", indent-2) + icon + " " + } else if icon != "" { + prefix = icon + " " } // Name with status indicator @@ -208,7 +210,7 @@ func (m Model) renderAgent(icon string, agent *Agent, indent int) string { } // renderFeed renders the event feed content -func (m Model) renderFeed() string { +func (m *Model) renderFeed() string { if len(m.events) == 0 { return AgentIdleStyle.Render("No events yet") } @@ -230,7 +232,7 @@ func (m Model) renderFeed() string { } // renderEvent renders a single event line -func (m Model) renderEvent(e Event) string { +func (m *Model) renderEvent(e Event) string { // Timestamp ts := TimestampStyle.Render(fmt.Sprintf("[%s]", e.Time.Format("15:04:05"))) @@ -282,7 +284,7 @@ func (m Model) renderEvent(e Event) string { } // renderStatusBar renders the bottom status bar -func (m Model) renderStatusBar() string { +func (m *Model) renderStatusBar() string { // Panel indicator panelName := "tree" if m.focusedPanel == PanelFeed { @@ -307,7 +309,7 @@ func (m Model) renderStatusBar() string { } // renderShortHelp renders abbreviated key hints -func (m Model) renderShortHelp() string { +func (m *Model) renderShortHelp() string { hints := []string{ HelpKeyStyle.Render("j/k") + HelpDescStyle.Render(":scroll"), HelpKeyStyle.Render("tab") + HelpDescStyle.Render(":switch"),