diff --git a/cmd/entire/cli/review/tui_detail.go b/cmd/entire/cli/review/tui_detail.go index 1aab2318e..d5e7e121e 100644 --- a/cmd/entire/cli/review/tui_detail.go +++ b/cmd/entire/cli/review/tui_detail.go @@ -1,10 +1,11 @@ // Package review — see env.go for package-level rationale. // -// tui_detail.go provides detailView, the pure-function renderer for the -// alt-screen drill-in view. It renders one agent's live event buffer with -// header/footer chrome and pads to exactly termHeight lines so every frame -// has the same line count (avoids ghost rows in the Bubble Tea alt-screen -// frame diff). +// tui_detail.go provides the alt-screen drill-in renderer. The body content is +// produced by [eventLines] (one or more wrapped lines per event) and fed into +// a bubbles/v2/viewport on [reviewTUIModel]; this file's [detailFrame] is the +// pure-function chrome (header + body + footer) that wraps the viewport's +// pre-rendered output and pads to exactly termHeight lines so every frame has +// the same line count (avoids ghost rows in Bubble Tea's alt-screen diff). package review import ( @@ -14,16 +15,22 @@ import ( reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types" ) -// eventLine converts a single Event to a single display line for the detail -// view. The line is stripped of control sequences and truncated to maxWidth -// display cells. -func eventLine(ev reviewtypes.Event, maxWidth int) string { +// eventLines converts a single Event to one or more display lines for the +// detail view, wrapped to maxWidth display cells. AssistantText preserves +// embedded '\n' as paragraph breaks; other event types render as a single +// sanitized line that wraps only on width overflow. +func eventLines(ev reviewtypes.Event, maxWidth int) []string { + if maxWidth <= 0 { + return nil + } var raw string switch e := ev.(type) { case reviewtypes.Started: raw = "[started]" case reviewtypes.AssistantText: - raw = e.Text + // AssistantText is the only event that can contain meaningful + // multi-line content; wrapDisplayWidth honors embedded newlines. + return wrapDisplayWidth(e.Text, maxWidth) case reviewtypes.ToolCall: raw = fmt.Sprintf("[tool: %s] %s", e.Name, e.Args) case reviewtypes.Tokens: @@ -44,22 +51,34 @@ func eventLine(ev reviewtypes.Event, maxWidth int) string { raw = "[unknown event]" } - return truncateDisplayWidth(sanitizeDisplayText(raw), maxWidth) + return wrapDisplayWidth(raw, maxWidth) } -// detailView renders the alt-screen drill-in for one agent. row is the agentRow -// being inspected. termWidth/termHeight come from WindowSizeMsg. +// buildEventLines returns every wrapped body line for the supplied event +// buffer, in order. The result is suitable for feeding into a viewport via +// SetContentLines. +func buildEventLines(buffer []reviewtypes.Event, maxWidth int) []string { + if len(buffer) == 0 || maxWidth <= 0 { + return nil + } + out := make([]string, 0, len(buffer)) + for _, ev := range buffer { + out = append(out, eventLines(ev, maxWidth)...) + } + return out +} + +// detailFrame renders the alt-screen drill-in chrome around a body string. The +// body is the viewport's already-rendered view (already clipped to bodyHeight +// lines by the viewport itself). detailFrame adds: // -// Rendering: // 1. Header line: "─── ( events) ─────────────" (filled to termWidth) -// 2. Body: events from row.buffer scrolled to detailScroll, one line each, -// sanitized and truncated to termWidth display cells. -// 3. Footer line: "←/→ switch agent · Esc back · ↑/↓ scroll" +// 2. Body: trimmed/padded to exactly bodyHeight lines, each padded to termWidth. +// 3. Footer line: "←/→ switch agent · Esc back · scroll: PgUp/PgDn/↑/↓/Home/End" // -// CRITICAL: the rendered string is padded to exactly termHeight lines so every -// frame has the same line count. Bubble Tea's alt-screen frame diff leaves ghost -// rows if the line count varies between frames. -func detailView(row agentRow, detailScroll, termWidth, termHeight int) string { +// CRITICAL: the rendered string is exactly termHeight lines. Bubble Tea's +// alt-screen diff leaves ghost rows if the line count varies between frames. +func detailFrame(row agentRow, body string, termWidth, termHeight int) string { if termWidth < 1 { termWidth = 80 } @@ -67,77 +86,52 @@ func detailView(row agentRow, detailScroll, termWidth, termHeight int) string { termHeight = 3 } - // Reserve 1 line for header, 1 for footer; the body fills the rest. bodyHeight := termHeight - 2 if bodyHeight < 0 { bodyHeight = 0 } - // 1. Header line. headerContent := fmt.Sprintf("─── %s (%d events) ", sanitizeDisplayText(row.name), len(row.buffer)) header := padDisplayWidthWith(headerContent, termWidth, "─") - // 2. Body lines. - lines := buildBodyLines(row.buffer, detailScroll, bodyHeight, termWidth) - - // Pad body to exactly bodyHeight lines. - for len(lines) < bodyHeight { - lines = append(lines, strings.Repeat(" ", termWidth)) - } + // Normalize the viewport body to exactly bodyHeight lines, each padded to + // termWidth so frame width is stable. + bodyLines := splitBodyToHeight(body, bodyHeight, termWidth) - // 3. Footer line. - footerText := "←/→ switch agent · Esc back · ↑/↓ scroll" + footerText := "←/→ switch agent · Esc back · scroll: PgUp/PgDn/↑/↓/Home/End" footer := padDisplayWidth(footerText, termWidth) - // Assemble: header + body + footer = termHeight lines total. var b strings.Builder b.WriteString(header) b.WriteString("\n") - for _, line := range lines { + for _, line := range bodyLines { b.WriteString(line) b.WriteString("\n") } b.WriteString(footer) - // No trailing newline after footer — the caller (View) adds its own. - return b.String() } -// buildBodyLines computes the visible body lines for the detail view. -// It takes the full event buffer, a scroll offset, the maximum number of lines -// to show, and the column width. Returns at most bodyHeight lines. -func buildBodyLines(buffer []reviewtypes.Event, scroll, bodyHeight, termWidth int) []string { - if len(buffer) == 0 || bodyHeight <= 0 { +// splitBodyToHeight normalizes a multi-line body string to exactly bodyHeight +// lines, each truncated and padded to termWidth. Missing lines are padded +// with spaces. The bodyHeight cap is a defensive guard: viewport.View() +// should already clip to its Height(), so the overflow-truncation path is +// not expected to trigger in normal use. +func splitBodyToHeight(body string, bodyHeight, termWidth int) []string { + if bodyHeight <= 0 { return nil } - - // Clamp scroll to valid range. - if scroll < 0 { - scroll = 0 - } - if scroll >= len(buffer) { - scroll = len(buffer) - 1 - } - - // Determine window: scroll is the index of the LAST visible line so the - // user sees the most-recent events when auto-scrolling. Work backwards. - end := scroll + 1 // exclusive upper bound - start := end - bodyHeight - if start < 0 { - start = 0 + var raw []string + if body != "" { + raw = strings.Split(strings.TrimRight(body, "\n"), "\n") } - // Clamp end to buffer length. - if end > len(buffer) { - end = len(buffer) - start = end - bodyHeight - if start < 0 { - start = 0 + lines := make([]string, 0, bodyHeight) + for i := range bodyHeight { + if i < len(raw) { + lines = append(lines, padDisplayWidth(raw[i], termWidth)) + } else { + lines = append(lines, strings.Repeat(" ", termWidth)) } } - - lines := make([]string, 0, end-start) - for i := start; i < end; i++ { - lines = append(lines, eventLine(buffer[i], termWidth)) - } return lines } diff --git a/cmd/entire/cli/review/tui_detail_test.go b/cmd/entire/cli/review/tui_detail_test.go index 0d5245bbb..010ec3b5e 100644 --- a/cmd/entire/cli/review/tui_detail_test.go +++ b/cmd/entire/cli/review/tui_detail_test.go @@ -7,6 +7,7 @@ import ( "testing" "unicode/utf8" + tea "charm.land/bubbletea/v2" "github.com/charmbracelet/x/ansi" reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types" @@ -18,29 +19,40 @@ func countLines(s string) int { if s == "" { return 0 } - // detailView ends without a trailing newline after footer; lines are - // separated by \n. Count \n occurrences + 1 (for the final segment). + // detailFrame ends without a trailing newline after footer; lines are + // separated by \n. Count \n occurrences + 1 (for the final segment). return strings.Count(s, "\n") + 1 } -func makeBuffer(texts ...string) []reviewtypes.Event { - buf := make([]reviewtypes.Event, len(texts)) - for i, t := range texts { - buf[i] = reviewtypes.AssistantText{Text: t} - } - return buf +// renderDetailViaModel builds a reviewTUIModel populated with the supplied +// events, sized to termWidth × termHeight, and returns the rendered detail +// frame. The viewport is the source of truth for body lines, so we exercise +// rendering through the model rather than calling detailFrame in isolation. +func renderDetailViaModel(t *testing.T, name string, buffer []reviewtypes.Event, termWidth, termHeight int) string { + t.Helper() + m := newReviewTUIModel([]string{name}, nil) + updated, _ := m.Update(tea.WindowSizeMsg{Width: termWidth, Height: termHeight}) + m = mustModel(t, updated) + for _, ev := range buffer { + updated, _ := m.Update(agentEventMsg{agent: name, ev: ev}) + m = mustModel(t, updated) + } + // Enter detail mode so refreshDetailContent runs. + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) + return m.View().Content } -func TestDetailView_PadsToTermHeight(t *testing.T) { +func TestDetailFrame_PadsToTermHeight(t *testing.T) { t.Parallel() for _, termHeight := range []int{5, 10, 20, 24} { t.Run("", func(t *testing.T) { t.Parallel() - row := agentRow{ - name: "agent-a", - buffer: makeBuffer("line1", "line2"), - } - out := detailView(row, 0, 80, termHeight) + out := renderDetailViaModel(t, "agent-a", + []reviewtypes.Event{ + reviewtypes.AssistantText{Text: "line1"}, + reviewtypes.AssistantText{Text: "line2"}, + }, 80, termHeight) got := countLines(out) if got != termHeight { t.Errorf("termHeight=%d: expected %d lines, got %d\noutput:\n%s", @@ -50,24 +62,24 @@ func TestDetailView_PadsToTermHeight(t *testing.T) { } } -func TestDetailView_EmptyBuffer_PadsToTermHeight(t *testing.T) { +func TestDetailFrame_EmptyBuffer_PadsToTermHeight(t *testing.T) { t.Parallel() - row := agentRow{name: "agent-a", buffer: nil} termHeight := 10 - out := detailView(row, 0, 80, termHeight) + out := renderDetailViaModel(t, "agent-a", nil, 80, termHeight) got := countLines(out) if got != termHeight { t.Errorf("empty buffer: expected %d lines, got %d", termHeight, got) } } -func TestDetailView_HeaderContainsAgentNameAndCount(t *testing.T) { +func TestDetailFrame_HeaderContainsAgentNameAndCount(t *testing.T) { t.Parallel() - row := agentRow{ - name: "claude-code", - buffer: makeBuffer("a", "b", "c"), - } - out := detailView(row, 0, 80, 10) + out := renderDetailViaModel(t, "claude-code", + []reviewtypes.Event{ + reviewtypes.AssistantText{Text: "a"}, + reviewtypes.AssistantText{Text: "b"}, + reviewtypes.AssistantText{Text: "c"}, + }, 80, 10) firstLine := strings.SplitN(out, "\n", 2)[0] if !strings.Contains(firstLine, "claude-code") { t.Errorf("header missing agent name: %q", firstLine) @@ -80,10 +92,10 @@ func TestDetailView_HeaderContainsAgentNameAndCount(t *testing.T) { } } -func TestDetailView_FooterPresent(t *testing.T) { +func TestDetailFrame_FooterPresent(t *testing.T) { t.Parallel() - row := agentRow{name: "agent-a", buffer: makeBuffer("x")} - out := detailView(row, 0, 80, 8) + out := renderDetailViaModel(t, "agent-a", + []reviewtypes.Event{reviewtypes.AssistantText{Text: "x"}}, 80, 8) lines := strings.Split(out, "\n") lastLine := lines[len(lines)-1] if !strings.Contains(lastLine, "Esc back") { @@ -91,40 +103,18 @@ func TestDetailView_FooterPresent(t *testing.T) { } } -func TestDetailView_LineTruncation_RuneSafe(t *testing.T) { - t.Parallel() - // Use a multi-byte UTF-8 string: each '日' is 3 bytes but 1 rune. - multibyte := strings.Repeat("日", 20) // 20 runes, 60 bytes - row := agentRow{ - name: "agent-a", - buffer: []reviewtypes.Event{reviewtypes.AssistantText{Text: multibyte}}, - } - termWidth := 10 - out := detailView(row, 0, termWidth, 5) - // Each body line must not exceed termWidth runes. - for i, line := range strings.Split(out, "\n") { - runes := utf8.RuneCountInString(line) - if runes > termWidth { - t.Errorf("line %d has %d runes (>%d): %q", i, runes, termWidth, line) - } - } -} - -func TestDetailView_LinesFitTerminalWidth(t *testing.T) { +func TestDetailFrame_LinesFitTerminalWidth(t *testing.T) { t.Parallel() - row := agentRow{ - name: "claude-code-with-a-very-wide-name", - buffer: []reviewtypes.Event{ - reviewtypes.AssistantText{Text: strings.Repeat("界", 20)}, - reviewtypes.ToolCall{Name: "wide", Args: strings.Repeat("🚀", 20)}, - reviewtypes.RunError{Err: errors.New(strings.Repeat("日", 20))}, - }, + buffer := []reviewtypes.Event{ + reviewtypes.AssistantText{Text: strings.Repeat("界", 20)}, + reviewtypes.ToolCall{Name: "wide", Args: strings.Repeat("🚀", 20)}, + reviewtypes.RunError{Err: errors.New(strings.Repeat("日", 20))}, } - for _, width := range []int{1, 2, 5, 10, 20, 40, 80} { + for _, width := range []int{10, 20, 40, 80} { t.Run(fmt.Sprintf("width %d", width), func(t *testing.T) { t.Parallel() - out := detailView(row, 2, width, 8) + out := renderDetailViaModel(t, "claude-code-with-a-very-wide-name", buffer, width, 12) for i, line := range strings.Split(out, "\n") { if got := ansi.StringWidth(line); got > width { t.Fatalf("line %d width = %d, want <= %d:\n%q", i, got, width, line) @@ -134,15 +124,26 @@ func TestDetailView_LinesFitTerminalWidth(t *testing.T) { } } -func TestDetailView_ANSIStripped(t *testing.T) { +func TestDetailFrame_MultibyteRuneSafe(t *testing.T) { + t.Parallel() + multibyte := strings.Repeat("日", 20) // 20 runes, 60 bytes + termWidth := 10 + out := renderDetailViaModel(t, "agent-a", + []reviewtypes.Event{reviewtypes.AssistantText{Text: multibyte}}, termWidth, 8) + for i, line := range strings.Split(out, "\n") { + runes := utf8.RuneCountInString(line) + if runes > termWidth { + t.Errorf("line %d has %d runes (>%d): %q", i, runes, termWidth, line) + } + } +} + +func TestDetailFrame_ANSIStripped(t *testing.T) { t.Parallel() // Include CSI sequences that codex emits (cursor-hide / cursor-show). ansiText := "hello\x1b[?25lworld\x1b[?25h" - row := agentRow{ - name: "agent-a", - buffer: []reviewtypes.Event{reviewtypes.AssistantText{Text: ansiText}}, - } - out := detailView(row, 0, 80, 5) + out := renderDetailViaModel(t, "agent-a", + []reviewtypes.Event{reviewtypes.AssistantText{Text: ansiText}}, 80, 6) if strings.Contains(out, "\x1b") { t.Error("output should have ANSI sequences stripped") } @@ -151,46 +152,74 @@ func TestDetailView_ANSIStripped(t *testing.T) { } } -func TestDetailView_Scrolling_LeadingLinesHidden(t *testing.T) { +func TestDetailFrame_EventTypes_Rendered(t *testing.T) { t.Parallel() - // 5 events; termHeight=6 (1 header + 3 body + 1 footer = 5; we set 6 to leave 4 body lines). - texts := []string{"line0", "line1", "line2", "line3", "line4"} - row := agentRow{name: "agent-a", buffer: makeBuffer(texts...)} + buffer := []reviewtypes.Event{ + reviewtypes.Started{}, + reviewtypes.ToolCall{Name: "read_file", Args: "foo.go"}, + reviewtypes.Tokens{In: 100, Out: 50}, + reviewtypes.Finished{Success: true}, + reviewtypes.RunError{Err: errors.New("oops")}, + } + out := renderDetailViaModel(t, "agent-a", buffer, 120, 10) + checks := []string{"[started]", "[tool: read_file]", "in=100", "[finished: success]", "[error: oops]"} + for _, want := range checks { + if !strings.Contains(out, want) { + t.Errorf("expected %q in output; got:\n%s", want, out) + } + } +} - // scroll=4 (max): shows events 1-4 in body (if bodyHeight=4). - termHeight := 6 - out := detailView(row, 4, 80, termHeight) - if strings.Contains(out, "line0") { - t.Error("line0 should not appear when scrolled to the bottom with 4 body lines") +// TestDetailFrame_WrapsLongAssistantText is load-bearing: a long AssistantText +// must wrap across multiple visible body lines rather than being truncated. +// This is the whole point of switching the drill-in body to a viewport. +func TestDetailFrame_WrapsLongAssistantText(t *testing.T) { + t.Parallel() + // 200+ character AssistantText (space-separated tokens so word wrap can fire). + long := strings.TrimSpace(strings.Repeat("word ", 50)) // 50 words × 5 chars - trailing space ≈ 249 chars + if utf8.RuneCountInString(long) < 200 { + t.Fatalf("test setup: expected >= 200 runes, got %d", utf8.RuneCountInString(long)) } - if !strings.Contains(out, "line4") { - t.Errorf("line4 should appear at scroll=4; output:\n%s", out) + termWidth := 40 + termHeight := 20 // ample body height + out := renderDetailViaModel(t, "agent-a", + []reviewtypes.Event{reviewtypes.AssistantText{Text: long}}, termWidth, termHeight) + + // Every line fits within termWidth. + lines := strings.Split(out, "\n") + for i, line := range lines { + if got := ansi.StringWidth(line); got > termWidth { + t.Fatalf("line %d width = %d, want <= %d:\n%q", i, got, termWidth, line) + } } - // scroll=0: shows first bodyHeight events. - out0 := detailView(row, 0, 80, termHeight) - if !strings.Contains(out0, "line0") { - t.Errorf("line0 should appear at scroll=0; output:\n%s", out0) + // The body must contain more than one line of "word" content. A single + // truncated line would only show one fragment; wrapping should produce + // several body lines beyond the header. + wordLineCount := 0 + for _, line := range lines { + if strings.Contains(line, "word") { + wordLineCount++ + } + } + if wordLineCount < 4 { + t.Errorf("expected long AssistantText to wrap onto multiple visible lines (got %d body lines with 'word'); output:\n%s", + wordLineCount, out) } } -func TestDetailView_EventTypes_Rendered(t *testing.T) { +// TestDetailFrame_PreservesAssistantTextNewlines pins that embedded newlines in +// AssistantText act as paragraph breaks rather than being collapsed to spaces. +// Multi-paragraph review findings must remain readable after the wrap helper +// turns them into multiple body lines. +func TestDetailFrame_PreservesAssistantTextNewlines(t *testing.T) { t.Parallel() - row := agentRow{ - name: "agent-a", - buffer: []reviewtypes.Event{ - reviewtypes.Started{}, - reviewtypes.ToolCall{Name: "read_file", Args: "foo.go"}, - reviewtypes.Tokens{In: 100, Out: 50}, - reviewtypes.Finished{Success: true}, - reviewtypes.RunError{Err: errors.New("oops")}, - }, - } - out := detailView(row, 4, 120, 10) - checks := []string{"[started]", "[tool: read_file]", "in=100", "[finished: success]", "[error: oops]"} - for _, want := range checks { + text := "first paragraph here\nsecond paragraph here\nthird paragraph here" + out := renderDetailViaModel(t, "agent-a", + []reviewtypes.Event{reviewtypes.AssistantText{Text: text}}, 80, 12) + for _, want := range []string{"first paragraph here", "second paragraph here", "third paragraph here"} { if !strings.Contains(out, want) { - t.Errorf("expected %q in output; got:\n%s", want, out) + t.Errorf("expected paragraph %q to survive in detail output; got:\n%s", want, out) } } } diff --git a/cmd/entire/cli/review/tui_model.go b/cmd/entire/cli/review/tui_model.go index 540084fa8..9c8770715 100644 --- a/cmd/entire/cli/review/tui_model.go +++ b/cmd/entire/cli/review/tui_model.go @@ -16,6 +16,7 @@ import ( "time" "charm.land/bubbles/v2/spinner" + "charm.land/bubbles/v2/viewport" tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" @@ -23,6 +24,16 @@ import ( "github.com/entireio/cli/cmd/entire/cli/stringutil" ) +// Default terminal dimensions used before the first tea.WindowSizeMsg +// arrives. Shared by the constructor and the dashboardWidth / +// detailViewportWidth fallbacks so both views render at the same width when +// termWidth is uninitialized — a 1-cell viewport falls back here would +// collapse to a single column, which is not what we want. +const ( + defaultTermWidth = 80 + defaultTermHeight = 24 +) + // agentRow holds per-agent live state during the TUI run. type agentRow struct { name string @@ -51,11 +62,14 @@ type tickMsg time.Time // reviewTUIModel is the Bubble Tea model for the review dashboard. type reviewTUIModel struct { - rows []agentRow - rowIdx map[string]int // agent name → row index (O(1) lookup) - detailMode bool - detailIdx int // which agent is shown in drill-in - detailScroll int + rows []agentRow + rowIdx map[string]int // agent name → row index (O(1) lookup) + detailMode bool + detailIdx int // which agent is shown in drill-in + // detail is the pager backing the drill-in body. Width/Height are kept + // in sync with termWidth/termHeight (minus header+footer). Scroll + // position is internal state; AtBottom drives auto-tail. + detail viewport.Model cancel context.CancelFunc cancelOnce *sync.Once @@ -66,6 +80,13 @@ type reviewTUIModel struct { finished bool summary reviewtypes.RunSummary + + // cancelling tracks whether a Ctrl+C-initiated cancellation is in flight. + // Set true on the first Ctrl+C (in tandem with cancelOnce firing the shared + // CancelFunc); the dashboard switches to a "cancelling" indicator and the + // footer offers a force-quit hint. A second Ctrl+C while cancelling=true + // force-quits without waiting for agents to drain. + cancelling bool } // newReviewTUIModel builds an initial model pre-populated with one row per @@ -85,14 +106,21 @@ func newReviewTUIModel(agents []string, cancel context.CancelFunc) reviewTUIMode } rowIdx[name] = i } + // Seed viewport with defaults that match termWidth/termHeight so an + // immediate Ctrl+O before any WindowSizeMsg still renders. + vp := viewport.New( + viewport.WithWidth(defaultTermWidth), + viewport.WithHeight(defaultTermHeight-2), + ) return reviewTUIModel{ rows: rows, rowIdx: rowIdx, + detail: vp, cancel: cancel, cancelOnce: &sync.Once{}, spinner: sp, - termWidth: 80, - termHeight: 24, + termWidth: defaultTermWidth, + termHeight: defaultTermHeight, } } @@ -167,17 +195,77 @@ func (m reviewTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, spinCmd case tea.WindowSizeMsg: + wasAtBottom := m.detail.AtBottom() m.termWidth = msg.Width m.termHeight = msg.Height - m = m.clampScroll() + m.detail.SetWidth(m.detailViewportWidth()) + m.detail.SetHeight(m.detailViewportHeight()) + m = m.refreshDetailContentWithAutoTail(wasAtBottom) return m, nil case tea.KeyPressMsg: return m.handleKey(msg) + + case tea.MouseWheelMsg, tea.MouseClickMsg, tea.MouseReleaseMsg, tea.MouseMotionMsg: + // Mouse events: only meaningful inside the drill-in viewport, which + // handles tea.MouseWheelMsg natively. + // Without this delegation the events arrive at the Program (because + // View.MouseMode = MouseModeCellMotion is set during detail mode) + // but fall through Update unhandled — the user sees no scroll + // response to the wheel. + if m.detailMode { + var cmd tea.Cmd + m.detail, cmd = m.detail.Update(msg) + return m, cmd + } + return m, nil } return m, nil } +// detailViewportWidth returns the viewport's width, mirroring termWidth. +func (m reviewTUIModel) detailViewportWidth() int { + width, _ := m.currentTerminalSize() + return width +} + +// detailViewportHeight returns the viewport's body height, reserving one line +// for the header and one for the footer. +func (m reviewTUIModel) detailViewportHeight() int { + _, termHeight := m.currentTerminalSize() + h := termHeight - 2 + if h < 1 { + return 1 + } + return h +} + +// refreshDetailContent re-renders the focused agent's events into the +// viewport. It preserves auto-tail: if the viewport was sitting at the bottom +// (or has no scrollable content), it jumps to the new bottom after the content +// is replaced; otherwise the user's scroll position is left untouched. +// +// reviewTUIModel uses value receivers throughout (matching the Bubble Tea +// idiom of returning an updated tea.Model from Update); the viewport is +// mutated in place on the returned copy and the caller assigns the result +// back. +func (m reviewTUIModel) refreshDetailContent() reviewTUIModel { + return m.refreshDetailContentWithAutoTail(m.detail.AtBottom()) +} + +func (m reviewTUIModel) refreshDetailContentWithAutoTail(wasAtBottom bool) reviewTUIModel { + if len(m.rows) == 0 || m.detailIdx < 0 || m.detailIdx >= len(m.rows) { + m.detail.SetContentLines(nil) + return m + } + lines := buildEventLines(m.rows[m.detailIdx].buffer, m.detailViewportWidth()) + m.detail.SetContentLines(lines) + if wasAtBottom { + m.detail.GotoBottom() + } + return m +} + // handleAgentEvent processes an agentEventMsg, updating the relevant row. func (m reviewTUIModel) handleAgentEvent(msg agentEventMsg) (tea.Model, tea.Cmd) { idx, ok := m.rowIdx[msg.agent] @@ -227,15 +315,11 @@ func (m reviewTUIModel) handleAgentEvent(msg agentEventMsg) (tea.Model, tea.Cmd) // No visible state update for tool calls in the dashboard. } - // Auto-follow ONLY when the user is already at the bottom. This lets a - // user scroll up to inspect older events without each new event yanking - // them back to the tail. The pre-append max-scroll was for buffer - // length-1; if detailScroll was at-or-past that, the user was tailing. + // Re-render the focused agent's viewport content when a new event lands + // for it. refreshDetailContent's AtBottom check preserves user scroll if + // they've scrolled up; auto-tails otherwise. if m.detailMode && m.detailIdx == idx { - preAppendMax := len(row.buffer) - 2 // -1 for the just-appended event, -1 for max-index - if preAppendMax < 0 || m.detailScroll >= preAppendMax { - m.detailScroll = m.maxDetailScroll() - } + m = m.refreshDetailContent() } return m, nil @@ -243,19 +327,61 @@ func (m reviewTUIModel) handleAgentEvent(msg agentEventMsg) (tea.Model, tea.Cmd) // handleKey processes keyboard input. func (m reviewTUIModel) handleKey(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { - // Any key after finished dismisses. + // Post-finish: explicit exit keys dismiss the TUI from either the + // dashboard or detail mode. Esc retains its "back to dashboard" meaning + // while drilled in so the user can return to the finished dashboard + // before quitting. Other keys (including Ctrl+O and scroll keys) fall + // through to normal handling so post-mortem inspection still works. if m.finished { - return m, tea.Quit + switch { + case msg.Mod == 0 && msg.Code == 'q': + return m, tea.Quit + case msg.Mod == 0 && msg.Code == tea.KeyEnter: + return m, tea.Quit + case msg.Mod == tea.ModCtrl && msg.Code == 'c': + return m, tea.Quit + case msg.Mod == 0 && (msg.Code == tea.KeyEscape): + if !m.detailMode { + return m, tea.Quit + } + // Detail mode: fall through to main switch where Esc returns to dashboard. + } } switch { case msg.Code == 'c' && msg.Mod == tea.ModCtrl: + if m.cancelling { + // Second Ctrl+C while a cancellation is already in flight + // force-quits without waiting for agents to drain. Checked + // before m.detailMode so the force-quit escape hatch works + // from drill-in too — the dashboard footer hint promises this + // path and a user drilled into a hanging agent's buffer is + // exactly when they need force-quit most. cancelOnce guards + // CancelFunc against the duplicate-firing case. + return m, tea.Quit + } + if m.allAgentsTerminal() { + // Race window: every agent emitted a terminal event but + // runFinishedMsg hasn't arrived yet. There's nothing left to + // cancel — quit immediately instead of flashing the + // "Cancelling agents..." footer until the runFinishedMsg lands. + // Checked before m.detailMode so the user reading a finished + // agent's buffer doesn't have to press Esc first to dismiss. + return m, tea.Quit + } if m.detailMode { - // In drill-in: Ctrl+C is intentionally ignored; Esc first. + // Idle drill-in with at least one agent still running: Ctrl+C + // is intentionally ignored so the user reading content can't + // accidentally fire a cancel. Esc first to return to the + // dashboard. return m, nil } + m.cancelling = true m.cancelOnce.Do(m.cancel) - return m, tea.Quit + // Do NOT quit on the first Ctrl+C: leave the TUI up so the user sees + // the cancelling indicator while agents drain. Natural finish + // (runFinishedMsg) or a second Ctrl+C dismisses the TUI. + return m, nil case msg.Code == 'o' && msg.Mod == tea.ModCtrl: if m.detailMode { @@ -266,10 +392,16 @@ func (m reviewTUIModel) handleKey(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { if len(m.rows) > 0 && (m.detailIdx < 0 || m.detailIdx >= len(m.rows)) { m.detailIdx = 0 } - m.detailScroll = m.maxDetailScroll() + // Resize viewport in case termWidth/termHeight have changed since + // last detail-mode entry, then load the focused agent's events and + // tail to the bottom. + m.detail.SetWidth(m.detailViewportWidth()) + m.detail.SetHeight(m.detailViewportHeight()) + m = m.refreshDetailContent() + m.detail.GotoBottom() return m, nil - case msg.Code == tea.KeyEscape || msg.Code == tea.KeyEsc: + case msg.Code == tea.KeyEscape: if m.detailMode { m.detailMode = false return m, nil @@ -279,69 +411,70 @@ func (m reviewTUIModel) handleKey(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { case msg.Code == tea.KeyLeft: if m.detailMode && len(m.rows) > 0 { m.detailIdx = (m.detailIdx - 1 + len(m.rows)) % len(m.rows) - m.detailScroll = m.maxDetailScroll() + m = m.refreshDetailContent() + m.detail.GotoBottom() } return m, nil case msg.Code == tea.KeyRight: if m.detailMode && len(m.rows) > 0 { m.detailIdx = (m.detailIdx + 1) % len(m.rows) - m.detailScroll = m.maxDetailScroll() - } - return m, nil - - case msg.Code == tea.KeyUp: - if m.detailMode && m.detailScroll > 0 { - m.detailScroll-- - } - return m, nil - - case msg.Code == tea.KeyDown: - if m.detailMode { - if maxScroll := m.maxDetailScroll(); m.detailScroll < maxScroll { - m.detailScroll++ - } + m = m.refreshDetailContent() + m.detail.GotoBottom() } return m, nil } + // Delegate any unhandled key to the viewport so PgUp/PgDn/Home/End/↑/↓ + // reach its internal keymap. Only meaningful in detail mode — on the + // dashboard the viewport is inert. + if m.detailMode { + var cmd tea.Cmd + m.detail, cmd = m.detail.Update(msg) + return m, cmd + } return m, nil } -// maxDetailScroll returns the largest valid detailScroll value for the current -// agent's buffer (0 when the buffer is empty or no rows exist). -func (m reviewTUIModel) maxDetailScroll() int { +// allAgentsTerminal reports whether every agent row has reached a terminal +// status. Used to short-circuit Ctrl+C cancellation in the race window between +// the last agent emitting Finished/RunError and runFinishedMsg arriving — at +// that point CancelFunc has nothing to do and the "Cancelling agents..." +// footer would only flash briefly before runFinishedMsg dismisses it anyway. +func (m reviewTUIModel) allAgentsTerminal() bool { if len(m.rows) == 0 { - return 0 - } - n := len(m.rows[m.detailIdx].buffer) - if n == 0 { - return 0 + return false } - return n - 1 -} - -// clampScroll returns a copy of m with detailScroll clamped to valid bounds. -// Used after resize or index change. -func (m reviewTUIModel) clampScroll() reviewTUIModel { - maxScroll := m.maxDetailScroll() - if m.detailScroll > maxScroll { - m.detailScroll = maxScroll + for _, r := range m.rows { + if r.status == reviewtypes.AgentStatusUnknown { + return false + } } - return m + return true } // View renders the current state. +// +// In detail mode we enable [tea.MouseModeCellMotion] so the embedded +// [viewport.Model] receives mouse-wheel events natively — the viewport handles +// them as scroll, but only if the Bubble Tea program is configured to deliver +// mouse messages. Bubble Tea v2 expresses that config as a per-view field +// rather than a Program option, so it lives here next to AltScreen. +// Dashboard mode leaves the default [tea.MouseModeNone] in place so normal +// terminal mouse selection still works on the summary table. func (m reviewTUIModel) View() tea.View { var content string termWidth, termHeight := m.currentTerminalSize() if m.detailMode && len(m.rows) > 0 { - content = detailView(m.rows[m.detailIdx], m.detailScroll, termWidth, termHeight) + content = detailFrame(m.rows[m.detailIdx], m.detail.View(), termWidth, termHeight) } else { content = m.dashboardView() } v := tea.NewView(content) v.AltScreen = true + if m.detailMode { + v.MouseMode = tea.MouseModeCellMotion + } return v } @@ -356,10 +489,13 @@ func (m reviewTUIModel) dashboardView() string { } b.WriteString("\n") - if m.finished { + switch { + case m.finished: m.writeDashboardLine(&b, m.countsLine()) - m.writeDashboardLine(&b, "Press any key to exit.") - } else { + m.writeDashboardLine(&b, "Ctrl+O: drill in · q/Esc/Enter: exit") + case m.cancelling: + m.writeDashboardLine(&b, "Cancelling agents... · Ctrl+C again: force quit") + default: m.writeDashboardLine(&b, "Ctrl+O: drill in · Ctrl+C: cancel") } return b.String() @@ -379,10 +515,10 @@ func (m reviewTUIModel) currentTerminalSize() (int, int) { width := m.termWidth height := m.termHeight if width <= 0 { - width = 80 + width = defaultTermWidth } if height <= 0 { - height = 24 + height = defaultTermHeight } return width, height } @@ -405,9 +541,15 @@ func (m reviewTUIModel) renderRow(row agentRow) string { case reviewtypes.AgentStatusCancelled: statusStr = "— cancel" case reviewtypes.AgentStatusUnknown: - if row.runStart.IsZero() { + switch { + case m.cancelling: + // In-flight cancellation: distinct from the terminal Cancelled + // state ("— cancel") so the user can see that the cancel signal + // has been sent but the agent is still draining. + statusStr = "cancelling" + case row.runStart.IsZero(): statusStr = "queued" - } else { + default: statusStr = m.spinner.View() + " running" } } diff --git a/cmd/entire/cli/review/tui_model_test.go b/cmd/entire/cli/review/tui_model_test.go index f0accfe97..99ff586f6 100644 --- a/cmd/entire/cli/review/tui_model_test.go +++ b/cmd/entire/cli/review/tui_model_test.go @@ -191,23 +191,28 @@ func TestTUIModel_DashboardErrorPreviewYieldsToAssistantTextBeforeFailure(t *tes } } -func TestTUIModel_KeyCtrlC_NotDetailMode_CancelsAndQuits(t *testing.T) { +func TestTUIModel_KeyCtrlC_NotDetailMode_CancelsAndMarksCancelling(t *testing.T) { t.Parallel() var called atomic.Bool cancel := func() { called.Store(true) } m := newTestModel([]string{"agent-a"}, cancel) - _, cmd := m.Update(testCtrlKey('c')) + updated, cmd := m.Update(testCtrlKey('c')) if !called.Load() { t.Error("expected cancel to be called on Ctrl+C outside detail mode") } - if cmd == nil { - t.Error("expected a quit command to be returned") + m2 := mustModel(t, updated) + if !m2.cancelling { + t.Error("expected m.cancelling=true after first Ctrl+C") } - // Verify it's the quit command by running it. - msg := cmd() - if _, ok := msg.(tea.QuitMsg); !ok { - t.Errorf("expected tea.QuitMsg from Ctrl+C cmd, got %T", msg) + // First Ctrl+C must NOT quit — the TUI stays up so agents can drain + // visibly. Only a second Ctrl+C (or natural finish) dismisses. + if cmd != nil { + if msg := cmd(); msg != nil { + if _, ok := msg.(tea.QuitMsg); ok { + t.Error("first Ctrl+C must NOT send a quit command; it should wait for agents to drain") + } + } } } @@ -261,6 +266,9 @@ func TestTUIModel_KeyCtrlO_EntersDrillIn(t *testing.T) { if !m2.View().AltScreen { t.Error("expected View().AltScreen=true in detail mode") } + if got := m2.View().MouseMode; got != tea.MouseModeCellMotion { + t.Errorf("expected View().MouseMode=MouseModeCellMotion in detail mode (so viewport gets wheel events); got %v", got) + } } func TestTUIModel_KeyEsc_ExitsDrillIn(t *testing.T) { @@ -279,6 +287,9 @@ func TestTUIModel_KeyEsc_ExitsDrillIn(t *testing.T) { if !m2.View().AltScreen { t.Error("expected View().AltScreen=true outside detail mode") } + if got := m2.View().MouseMode; got != tea.MouseModeNone { + t.Errorf("expected View().MouseMode=MouseModeNone outside detail mode (preserve normal terminal selection on dashboard); got %v", got) + } } func TestTUIModel_DashboardUsesAltScreen(t *testing.T) { @@ -330,35 +341,75 @@ func TestTUIModel_LeftRight_CycleDetailIdx(t *testing.T) { } } -func TestTUIModel_UpDown_Scroll(t *testing.T) { +// TestTUIModel_InitializesDetailViewport pins that the viewport widget on the +// model starts with non-zero default dimensions so that an immediate Ctrl+O +// before a WindowSizeMsg still renders without panicking. +func TestTUIModel_InitializesDetailViewport(t *testing.T) { t.Parallel() m := newTestModel([]string{"agent-a"}, func() {}) - m.detailMode = true - // Populate buffer so max scroll > 0. + if w := m.detail.Width(); w <= 0 { + t.Errorf("expected detail viewport width > 0, got %d", w) + } + if h := m.detail.Height(); h <= 0 { + t.Errorf("expected detail viewport height > 0, got %d", h) + } +} + +// TestTUIModel_DelegatesMouseWheelToViewport pins that mouse-wheel events in +// detail mode reach the viewport's MouseWheelMsg handler. The bug this guards +// against: setting View.MouseMode = MouseModeCellMotion on entry to detail +// mode caused the Program to receive mouse events, but the Update switch +// only routed tea.KeyPressMsg — mouse events fell through unhandled and the +// viewport never got a chance to scroll. +func TestTUIModel_DelegatesMouseWheelToViewport(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a"}, func() {}) + updated, _ := m.Update(tea.WindowSizeMsg{Width: 40, Height: 10}) + m = mustModel(t, updated) + long := strings.Repeat("paragraph of text ", 20) for range 5 { - m.rows[0].buffer = append(m.rows[0].buffer, reviewtypes.Started{}) + updated, _ = m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.AssistantText{Text: long}}) + m = mustModel(t, updated) } - m.detailScroll = 4 // at max + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) + m.detail.GotoTop() + startOffset := m.detail.YOffset() - // Down when at max: clamp. - updated, _ := m.Update(testKey(tea.KeyDown)) + updated, _ = m.Update(tea.MouseWheelMsg{Button: tea.MouseWheelDown}) m = mustModel(t, updated) - if m.detailScroll != 4 { - t.Errorf("scroll should stay at max on Down; got %d", m.detailScroll) + if m.detail.YOffset() <= startOffset { + t.Errorf("expected mouse-wheel-down to advance viewport YOffset beyond %d; got %d", startOffset, m.detail.YOffset()) } +} - // Up: 4 → 3. - updated, _ = m.Update(testKey(tea.KeyUp)) +// TestTUIModel_DelegatesScrollKeysToViewport pins that pager keys (PgDn) in +// detail mode reach the viewport's internal keymap and advance YOffset rather +// than being swallowed by the model's switch. +func TestTUIModel_DelegatesScrollKeysToViewport(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a"}, func() {}) + // Size the window so the viewport has a sensible height. + updated, _ := m.Update(tea.WindowSizeMsg{Width: 40, Height: 10}) m = mustModel(t, updated) - if m.detailScroll != 3 { - t.Errorf("expected scroll=3 after Up; got %d", m.detailScroll) + // Populate enough content to scroll. AssistantText with long text wraps + // to many viewport lines. + long := strings.Repeat("paragraph of text ", 20) + for range 5 { + updated, _ = m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.AssistantText{Text: long}}) + m = mustModel(t, updated) } + // Enter detail mode (auto-tails to bottom). + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) + // Jump to top so PgDn has somewhere to scroll into. + m.detail.GotoTop() + startOffset := m.detail.YOffset() - // Down again: 3 → 4. - updated, _ = m.Update(testKey(tea.KeyDown)) + updated, _ = m.Update(tea.KeyPressMsg{Code: tea.KeyPgDown}) m = mustModel(t, updated) - if m.detailScroll != 4 { - t.Errorf("expected scroll=4 after Down; got %d", m.detailScroll) + if m.detail.YOffset() <= startOffset { + t.Errorf("expected PgDn to advance viewport YOffset beyond %d; got %d", startOffset, m.detail.YOffset()) } } @@ -383,7 +434,7 @@ func TestTUIModel_TickMsg_ReSchedulesTick(t *testing.T) { } } -func TestTUIModel_RunFinishedMsg_AnyKeyQuits(t *testing.T) { +func TestTUIModel_RunFinishedMsg_MarksFinished(t *testing.T) { t.Parallel() m := newTestModel([]string{"agent-a"}, func() {}) @@ -392,33 +443,292 @@ func TestTUIModel_RunFinishedMsg_AnyKeyQuits(t *testing.T) { if !m2.finished { t.Error("model should be finished after runFinishedMsg") } +} + +// TestTUIModel_PostFinishCtrlOEntersDetailMode pins that Ctrl+O still enters +// drill-in after both agents finish so the user can inspect completed output. +// Previously any-key-quits behavior swallowed Ctrl+O on the post-finish frame. +func TestTUIModel_PostFinishCtrlOEntersDetailMode(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a", "agent-b"}, func() {}) + updated, _ := m.Update(runFinishedMsg{summary: reviewtypes.RunSummary{}}) + m = mustModel(t, updated) + if !m.finished { + t.Fatal("setup: expected model finished after runFinishedMsg") + } + + updated, cmd := m.Update(testCtrlKey('o')) + m2 := mustModel(t, updated) + if !m2.detailMode { + t.Error("expected Ctrl+O post-finish to enter detail mode") + } + // Ctrl+O must NOT quit post-finish. + if cmd != nil { + if msg := cmd(); msg != nil { + if _, ok := msg.(tea.QuitMsg); ok { + t.Error("Ctrl+O post-finish must not produce a quit command") + } + } + } +} + +// TestTUIModel_PostFinishQuitsOnExplicitKeys pins that q, Esc, Enter, and Ctrl+C +// each produce tea.QuitMsg when finished and in dashboard mode. +func TestTUIModel_PostFinishQuitsOnExplicitKeys(t *testing.T) { + t.Parallel() + cases := []struct { + name string + key tea.KeyPressMsg + }{ + {"q", testKey('q')}, + {"Esc", testKey(tea.KeyEscape)}, + {"KeyEsc", testKey(tea.KeyEsc)}, + {"Enter", testKey(tea.KeyEnter)}, + {"Ctrl+C", testCtrlKey('c')}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a"}, func() {}) + updated, _ := m.Update(runFinishedMsg{summary: reviewtypes.RunSummary{}}) + m = mustModel(t, updated) + + _, cmd := m.Update(tc.key) + if cmd == nil { + t.Fatalf("expected a command for explicit-exit key %q post-finish", tc.name) + } + msg := cmd() + if _, ok := msg.(tea.QuitMsg); !ok { + t.Errorf("expected tea.QuitMsg for key %q post-finish, got %T", tc.name, msg) + } + }) + } +} + +// TestTUIModel_PostFinishIgnoresRandomKeys pins that non-exit keys (e.g. 'x', +// arrow keys) do NOT quit when finished on the dashboard. They fall through to +// normal handling instead of the old any-key-quits shortcut. +func TestTUIModel_PostFinishIgnoresRandomKeys(t *testing.T) { + t.Parallel() + cases := []struct { + name string + key tea.KeyPressMsg + }{ + {"x", testKey('x')}, + {"Right", testKey(tea.KeyRight)}, + {"Left", testKey(tea.KeyLeft)}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a", "agent-b"}, func() {}) + updated, _ := m.Update(runFinishedMsg{summary: reviewtypes.RunSummary{}}) + m = mustModel(t, updated) + + _, cmd := m.Update(tc.key) + if cmd != nil { + if msg := cmd(); msg != nil { + if _, ok := msg.(tea.QuitMsg); ok { + t.Errorf("key %q must not quit post-finish on dashboard", tc.name) + } + } + } + }) + } +} + +// TestTUIModel_PostFinishFooterUsesExplicitExitKeys pins the dashboard footer +// switches from the old "Press any key to exit." prompt to an explicit-keys +// hint that mentions Ctrl+O and the named exit keys. +func TestTUIModel_PostFinishFooterUsesExplicitExitKeys(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a"}, func() {}) + m.termWidth = 120 + updated, _ := m.Update(runFinishedMsg{summary: reviewtypes.RunSummary{}}) + m = mustModel(t, updated) + + out := m.dashboardView() + if strings.Contains(out, "Press any key to exit.") { + t.Errorf("post-finish footer must not show legacy 'Press any key to exit.' line:\n%s", out) + } + if !strings.Contains(out, "Ctrl+O") { + t.Errorf("post-finish footer should mention Ctrl+O for drill-in:\n%s", out) + } + if !strings.Contains(out, "q/Esc/Enter") { + t.Errorf("post-finish footer should list q/Esc/Enter as exit keys:\n%s", out) + } +} + +// TestTUIModel_CtrlCMarksAgentsCancelling pins that the first Ctrl+C while +// agents are still running sets m.cancelling and renderRow reflects an +// in-flight cancellation indicator (distinct from the terminal Cancelled +// state). +func TestTUIModel_CtrlCMarksAgentsCancelling(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a"}, func() {}) + // Stamp runStart so the row is in the running branch of renderRow. + updated, _ := m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.Started{}}) + m = mustModel(t, updated) + m.termWidth = 120 + + // Before Ctrl+C the running row should say "running". + if status := m.renderRow(m.rows[0]); !strings.Contains(status, "running") { + t.Fatalf("setup: expected running indicator pre-Ctrl+C; got %q", status) + } + + updated, _ = m.Update(testCtrlKey('c')) + m = mustModel(t, updated) + if !m.cancelling { + t.Fatal("expected m.cancelling=true after first Ctrl+C") + } + + got := m.renderRow(m.rows[0]) + if strings.Contains(got, "running") { + t.Errorf("renderRow should drop the 'running' indicator once cancelling; got %q", got) + } + if !strings.Contains(got, "cancel") { + t.Errorf("renderRow should show a cancelling indicator; got %q", got) + } +} + +// TestTUIModel_FooterDuringCancellation pins that the dashboard footer changes +// while a cancel is in flight (cancelling && !finished) to signal the user +// that draining is in progress and a second Ctrl+C will force quit. +func TestTUIModel_FooterDuringCancellation(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a"}, func() {}) + m.termWidth = 120 + updated, _ := m.Update(testCtrlKey('c')) + m = mustModel(t, updated) + + out := m.dashboardView() + if !strings.Contains(out, "Cancelling agents") { + t.Errorf("expected footer to announce cancellation in progress; got:\n%s", out) + } + if !strings.Contains(out, "Ctrl+C again") { + t.Errorf("expected footer to mention force-quit hint; got:\n%s", out) + } +} + +// TestTUIModel_SecondCtrlCForceQuits pins that once cancelling is in flight, +// a second Ctrl+C emits tea.QuitMsg immediately rather than waiting for +// agents to drain. +func TestTUIModel_SecondCtrlCForceQuits(t *testing.T) { + t.Parallel() + var count atomic.Int32 + cancel := func() { count.Add(1) } + + m := newTestModel([]string{"agent-a"}, cancel) + updated, _ := m.Update(testCtrlKey('c')) + m = mustModel(t, updated) + if !m.cancelling { + t.Fatal("setup: expected cancelling=true after first Ctrl+C") + } + + _, cmd := m.Update(testCtrlKey('c')) + if cmd == nil { + t.Fatal("expected a command from second Ctrl+C") + } + msg := cmd() + if _, ok := msg.(tea.QuitMsg); !ok { + t.Errorf("expected tea.QuitMsg from second Ctrl+C, got %T", msg) + } + // Shared CancelFunc still fires at most once thanks to cancelOnce. + if got := count.Load(); got != 1 { + t.Errorf("CancelFunc should fire exactly once across both Ctrl+Cs; got %d", got) + } +} + +// TestTUIModel_SecondCtrlCForceQuits_FromDetailMode locks the force-quit +// escape hatch from inside the drill-in view. When cancelling is already +// in flight, the dashboard footer promises "Ctrl+C again: force quit" — +// a user who drilled into a hanging agent's buffer to diagnose the hang +// needs that promise to hold from drill-in too. Without the cancelling- +// before-detailMode precedence in handleKey, Ctrl+C in this state was +// silently swallowed. +func TestTUIModel_SecondCtrlCForceQuits_FromDetailMode(t *testing.T) { + t.Parallel() + var count atomic.Int32 + cancel := func() { count.Add(1) } + + m := newTestModel([]string{"agent-a"}, cancel) + // First Ctrl+C on dashboard initiates cancellation. + updated, _ := m.Update(testCtrlKey('c')) + m = mustModel(t, updated) + if !m.cancelling { + t.Fatal("setup: expected cancelling=true after first Ctrl+C") + } + // Drill in to inspect the hanging agent. + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) + if !m.detailMode { + t.Fatal("setup: expected detailMode=true after Ctrl+O") + } - // Any key should now quit. - _, cmd := m2.Update(testKey(tea.KeyEnter)) + // Second Ctrl+C while cancelling AND in drill-in must force-quit. + _, cmd := m.Update(testCtrlKey('c')) if cmd == nil { - t.Error("expected quit command after finished + any key") + t.Fatal("expected a command from second Ctrl+C in detail mode while cancelling") + } + msg := cmd() + if _, ok := msg.(tea.QuitMsg); !ok { + t.Errorf("expected tea.QuitMsg from second Ctrl+C in detail mode while cancelling, got %T", msg) } - if msg := cmd(); msg == nil { - t.Error("expected non-nil quit msg") + if got := count.Load(); got != 1 { + t.Errorf("CancelFunc should fire exactly once; got %d", got) } } +// TestTUIModel_AutoFollow_DetailMode pins that when the viewport is sitting +// at the bottom and a new event arrives, the model snaps back to bottom so +// the user keeps seeing the tail. The viewport's AtBottom() drives this. func TestTUIModel_AutoFollow_DetailMode(t *testing.T) { t.Parallel() m := newTestModel([]string{"agent-a"}, func() {}) - m.detailMode = true - m.detailIdx = 0 - m.detailScroll = 0 - - // Send 5 events; model should auto-scroll to bottom each time. - current := m - for i := range 5 { - updated, _ := current.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.Started{}}) - current = mustModel(t, updated) - wantScroll := i // buffer has i+1 events; max scroll is i - if current.detailScroll != wantScroll { - t.Errorf("event %d: want detailScroll=%d, got %d", i, wantScroll, current.detailScroll) - } + updated, _ := m.Update(tea.WindowSizeMsg{Width: 40, Height: 8}) + m = mustModel(t, updated) + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) + + // Send events that produce content taller than the viewport so a tail + // exists below the visible window. + long := strings.Repeat("review finding ", 10) + for range 6 { + updated, _ = m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.AssistantText{Text: long}}) + m = mustModel(t, updated) + } + if !m.detail.AtBottom() { + t.Errorf("expected viewport to track bottom after each event (auto-follow); YOffset=%d, total=%d", + m.detail.YOffset(), m.detail.TotalLineCount()) + } +} + +// TestTUIModel_AutoFollow_ResizePreservesBottomWhenTailing pins that a user who +// is tailing the detail viewport remains at the bottom after a resize changes +// wrapping and increases the viewport's maximum scroll offset. +func TestTUIModel_AutoFollow_ResizePreservesBottomWhenTailing(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a"}, func() {}) + updated, _ := m.Update(tea.WindowSizeMsg{Width: 80, Height: 12}) + m = mustModel(t, updated) + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) + + long := strings.Repeat("resize-sensitive review finding ", 8) + for range 8 { + updated, _ = m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.AssistantText{Text: long}}) + m = mustModel(t, updated) + } + m.detail.GotoBottom() + if !m.detail.AtBottom() { + t.Fatal("setup: expected viewport to be at bottom before resize") + } + + updated, _ = m.Update(tea.WindowSizeMsg{Width: 30, Height: 8}) + m = mustModel(t, updated) + if !m.detail.AtBottom() { + t.Errorf("expected viewport to stay at bottom after resize; YOffset=%d, total=%d", + m.detail.YOffset(), m.detail.TotalLineCount()) } } @@ -535,41 +845,46 @@ func assertDashboardFitsWidthAt(t *testing.T, m reviewTUIModel, width int) { } // TestTUIModel_AutoFollow_PreservesUserScroll pins the contract that new -// agent events should NOT yank the user back to the tail when they have +// agent events must NOT yank the user back to the tail when they have // scrolled up to inspect older events. Auto-follow only re-engages when -// the user is already at the bottom. +// the viewport is already at the bottom. func TestTUIModel_AutoFollow_PreservesUserScroll(t *testing.T) { t.Parallel() m := newReviewTUIModel([]string{"agent-a"}, nil) - m.termHeight = 10 - m.detailMode = true - m.detailIdx = 0 + updated, _ := m.Update(tea.WindowSizeMsg{Width: 40, Height: 10}) + m = mustModel(t, updated) + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) - // Build up a buffer of 20 events. + // Build up enough wrapped content to overflow the viewport several times. + long := strings.Repeat("line of review text ", 10) for range 20 { - updated, _ := m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.AssistantText{Text: "line"}}) + updated, _ = m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.AssistantText{Text: long}}) m = mustModel(t, updated) } - // Snap to bottom, then scroll up by 5. - m.detailScroll = m.maxDetailScroll() - 5 - scrollBeforeNewEvent := m.detailScroll + // Scroll up away from the bottom. + m.detail.GotoTop() + startOffset := m.detail.YOffset() + if m.detail.AtBottom() { + t.Fatal("test setup: viewport unexpectedly at bottom after GotoTop") + } - // Send another event. The user is NOT at the bottom — scroll should not move. - updated, _ := m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.AssistantText{Text: "new line"}}) + // Send another event — the user is NOT at the bottom, so YOffset must not move. + updated, _ = m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.AssistantText{Text: long}}) m = mustModel(t, updated) - if m.detailScroll != scrollBeforeNewEvent { - t.Errorf("expected detailScroll to stay at %d (user scrolled up), got %d (auto-follow yanked back)", - scrollBeforeNewEvent, m.detailScroll) + if m.detail.YOffset() != startOffset { + t.Errorf("expected viewport YOffset to stay at %d (user scrolled up); got %d (auto-follow yanked back)", + startOffset, m.detail.YOffset()) } - // Now scroll to bottom and send another event — should auto-follow. - m.detailScroll = m.maxDetailScroll() - updated, _ = m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.AssistantText{Text: "another"}}) + // Now jump to bottom and send another event — should auto-follow. + m.detail.GotoBottom() + updated, _ = m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.AssistantText{Text: long}}) m = mustModel(t, updated) - if m.detailScroll != m.maxDetailScroll() { - t.Errorf("expected auto-follow to track bottom (got detailScroll=%d, max=%d)", - m.detailScroll, m.maxDetailScroll()) + if !m.detail.AtBottom() { + t.Errorf("expected auto-follow to track bottom after event; YOffset=%d, total=%d", + m.detail.YOffset(), m.detail.TotalLineCount()) } } @@ -673,3 +988,236 @@ func TestTUIModel_RunErrorSticky_FinishedDoesNotFlipToSucceeded(t *testing.T) { t.Errorf("expected Failed to stick (RunError is sticky), got %v", m.rows[0].status) } } + +// TestTUIModel_DelegatesNonWheelMouseEventsToViewport pins that the Update +// case-arm routing mouse events to the viewport covers Click, Release, and +// Motion in addition to Wheel. The viewport's selection support (click-drag +// highlight in terminals that emit cell-motion events) needs the full event +// stream, not just scroll. A future refactor that narrowed the case-arm to +// wheel-only would silently break selection while still passing +// TestTUIModel_DelegatesMouseWheelToViewport. +// +// Coverage limitation: the viewport's internal selection state is not +// exposed via a public getter, so this test asserts the weaker invariant +// that the events do not panic and that detailMode survives. That catches +// the regression where the case-arm is removed entirely (and the events +// fall through Update's catch-all to return m, nil); it does not catch +// finer-grained changes to selection semantics inside the viewport. +func TestTUIModel_DelegatesNonWheelMouseEventsToViewport(t *testing.T) { + t.Parallel() + cases := []struct { + name string + msg tea.Msg + }{ + {"click", tea.MouseClickMsg{}}, + {"release", tea.MouseReleaseMsg{}}, + {"motion", tea.MouseMotionMsg{}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a"}, func() {}) + updated, _ := m.Update(tea.WindowSizeMsg{Width: 40, Height: 10}) + m = mustModel(t, updated) + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) + if !m.detailMode { + t.Fatalf("setup: expected detailMode=true after Ctrl+O") + } + + updated, _ = m.Update(tc.msg) + m = mustModel(t, updated) + if !m.detailMode { + t.Errorf("detailMode should remain true after mouse %s event", tc.name) + } + }) + } +} + +// TestTUIModel_CancellingIndicatorOnlyAffectsRunningRows pins that the +// "cancelling" status indicator in renderRow only replaces "running" — rows +// already in a terminal status keep their own indicator. Without this gate +// the post-Ctrl+C frame would briefly show every row as "cancelling" before +// the dashboard transitioned to its post-finish state, including agents +// that had already succeeded. +func TestTUIModel_CancellingIndicatorOnlyAffectsRunningRows(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a", "agent-b"}, func() {}) + m.termWidth = 120 + // agent-a has already succeeded; agent-b is still running. + updated, _ := m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.Started{}}) + m = mustModel(t, updated) + updated, _ = m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.Finished{Success: true}}) + m = mustModel(t, updated) + updated, _ = m.Update(agentEventMsg{agent: "agent-b", ev: reviewtypes.Started{}}) + m = mustModel(t, updated) + + // Flip cancelling without going through Ctrl+C so the + // allAgentsTerminal() short-circuit doesn't fire. + m.cancelling = true + + gotA := m.renderRow(m.rows[0]) + if !strings.Contains(gotA, "done") { + t.Errorf("succeeded row should retain ✓ done indicator while cancelling; got %q", gotA) + } + if strings.Contains(gotA, "cancelling") { + t.Errorf("succeeded row must not show 'cancelling' indicator; got %q", gotA) + } + + gotB := m.renderRow(m.rows[1]) + if !strings.Contains(gotB, "cancelling") { + t.Errorf("running row should show cancelling indicator; got %q", gotB) + } +} + +// TestTUIModel_CtrlCAfterAllRowsTerminalQuitsImmediately pins that Ctrl+C +// during the race window between an agent's terminal event (Finished or +// RunError) and the orchestrator's runFinishedMsg short-circuits to tea.Quit +// instead of flashing the "Cancelling agents..." indicator. The pre-fix +// behavior fired CancelFunc and set m.cancelling=true even though every +// agent had already finished — confusing the user for the brief window +// before the dashboard transitioned to its post-finish state. +func TestTUIModel_CtrlCAfterAllRowsTerminalQuitsImmediately(t *testing.T) { + t.Parallel() + var called atomic.Bool + cancel := func() { called.Store(true) } + m := newTestModel([]string{"agent-a", "agent-b"}, cancel) + + // Drive both rows to a terminal status via events. runFinishedMsg is NOT + // sent — that's the race window we're testing. + updated, _ := m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.Finished{Success: true}}) + m = mustModel(t, updated) + updated, _ = m.Update(agentEventMsg{agent: "agent-b", ev: reviewtypes.Finished{Success: true}}) + m = mustModel(t, updated) + if m.finished { + t.Fatal("setup: m.finished should be false (runFinishedMsg not sent)") + } + + updated, cmd := m.Update(testCtrlKey('c')) + m = mustModel(t, updated) + if m.cancelling { + t.Error("Ctrl+C with all rows already terminal must NOT set cancelling=true") + } + if called.Load() { + t.Error("CancelFunc must not fire when there's nothing left to cancel") + } + if cmd == nil { + t.Fatal("expected a quit command when Ctrl+C arrives with all rows terminal") + } + if _, ok := cmd().(tea.QuitMsg); !ok { + t.Errorf("expected tea.QuitMsg, got %T", cmd()) + } +} + +// TestTUIModel_CtrlCInDetailModeAfterAllRowsTerminalQuits pins that Ctrl+C +// pressed inside drill-in during the narrow race window where every agent +// has emitted a terminal event but the orchestrator's runFinishedMsg has +// not arrived yet quits the TUI instead of being swallowed. Without the +// detail-mode gate on allAgentsTerminal(), the user reading a completed +// agent's buffer would have to press Esc first to return to the dashboard +// before Ctrl+C took effect — needlessly two-step when there is nothing +// left to cancel. Companion to +// [TestTUIModel_CtrlCAfterAllRowsTerminalQuitsImmediately] for the +// dashboard-mode case. +func TestTUIModel_CtrlCInDetailModeAfterAllRowsTerminalQuits(t *testing.T) { + t.Parallel() + var called atomic.Bool + cancel := func() { called.Store(true) } + m := newTestModel([]string{"agent-a", "agent-b"}, cancel) + + updated, _ := m.Update(agentEventMsg{agent: "agent-a", ev: reviewtypes.Finished{Success: true}}) + m = mustModel(t, updated) + updated, _ = m.Update(agentEventMsg{agent: "agent-b", ev: reviewtypes.Finished{Success: true}}) + m = mustModel(t, updated) + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) + if !m.detailMode { + t.Fatal("setup: expected detailMode=true after Ctrl+O") + } + if m.finished { + t.Fatal("setup: m.finished should be false (runFinishedMsg not sent)") + } + + updated, cmd := m.Update(testCtrlKey('c')) + m = mustModel(t, updated) + if m.cancelling { + t.Error("Ctrl+C in detail mode with all rows terminal must NOT set cancelling=true") + } + if called.Load() { + t.Error("CancelFunc must not fire when there is nothing left to cancel") + } + if cmd == nil { + t.Fatal("expected a quit command when Ctrl+C arrives in detail mode with all rows terminal") + } + if _, ok := cmd().(tea.QuitMsg); !ok { + t.Errorf("expected tea.QuitMsg, got %T", cmd()) + } +} + +// TestTUIModel_PostFinishInDetailMode_ExitKeysQuit pins that q/Enter/Ctrl+C +// pressed while drilled in AFTER the run finished dismiss the TUI directly +// instead of being swallowed by the viewport (which has no quit binding). +// Pre-fix the user had to Esc out of detail mode first, then press an exit +// key — a two-step dismissal with no on-screen hint that q/Enter were inert. +func TestTUIModel_PostFinishInDetailMode_ExitKeysQuit(t *testing.T) { + t.Parallel() + cases := []struct { + name string + key tea.KeyPressMsg + }{ + {"q", testKey('q')}, + {"Enter", testKey(tea.KeyEnter)}, + {"Ctrl+C", testCtrlKey('c')}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a"}, func() {}) + updated, _ := m.Update(runFinishedMsg{summary: reviewtypes.RunSummary{}}) + m = mustModel(t, updated) + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) + if !m.detailMode { + t.Fatalf("setup: expected detailMode=true after Ctrl+O") + } + + _, cmd := m.Update(tc.key) + if cmd == nil { + t.Fatalf("expected a command for key %q post-finish in detail mode", tc.name) + } + if _, ok := cmd().(tea.QuitMsg); !ok { + t.Errorf("expected tea.QuitMsg for key %q post-finish in detail mode, got %T", tc.name, cmd()) + } + }) + } +} + +// TestTUIModel_PostFinishInDetailMode_EscReturnsToDashboard pins that Esc +// preserves its "back to dashboard" meaning even after the run finishes, +// rather than quitting outright. The user can then dismiss from the +// dashboard via q/Esc/Enter/Ctrl+C. Regression guard for the post-finish +// detail-mode dismissal change. +func TestTUIModel_PostFinishInDetailMode_EscReturnsToDashboard(t *testing.T) { + t.Parallel() + m := newTestModel([]string{"agent-a"}, func() {}) + updated, _ := m.Update(runFinishedMsg{summary: reviewtypes.RunSummary{}}) + m = mustModel(t, updated) + updated, _ = m.Update(testCtrlKey('o')) + m = mustModel(t, updated) + if !m.detailMode { + t.Fatalf("setup: expected detailMode=true after Ctrl+O") + } + + updated, cmd := m.Update(testKey(tea.KeyEscape)) + m = mustModel(t, updated) + if m.detailMode { + t.Error("Esc post-finish in detail mode must return to dashboard, not quit") + } + if cmd != nil { + if msg := cmd(); msg != nil { + if _, ok := msg.(tea.QuitMsg); ok { + t.Error("Esc post-finish in detail mode must NOT quit; it returns to dashboard") + } + } + } +} diff --git a/cmd/entire/cli/review/tui_sink.go b/cmd/entire/cli/review/tui_sink.go index b85f1bb1e..eca87aace 100644 --- a/cmd/entire/cli/review/tui_sink.go +++ b/cmd/entire/cli/review/tui_sink.go @@ -25,10 +25,12 @@ import ( // orchestrator's dispatch goroutine. // // Cancellation: cancel is the same context.CancelFunc that controls the -// orchestrator's run context. KeyCtrlC in the dashboard calls this function -// (gated by a sync.Once in the model). Out-of-TUI SIGINT routes through the -// cobra root's context, which cancels the same function — no parallel -// signal.Notify goroutine is needed here. +// orchestrator's run context. The first KeyCtrlC in the dashboard fires this +// function (guarded by a sync.Once in the model) and switches the dashboard +// to a "Cancelling agents..." indicator while agents drain; a second KeyCtrlC +// force-quits without waiting. Out-of-TUI SIGINT routes through the cobra +// root's context, which cancels the same function — no parallel signal.Notify +// goroutine is needed here. type TUISink struct { program *tea.Program @@ -145,11 +147,15 @@ func (s *TUISink) AgentEvent(agent string, ev reviewtypes.Event) { // RunFinished (Sink interface): mark the run complete and send the final // summary message. The TUI shows the dashboard one more frame with the -// terminal statuses and waits for the user to press any key to dismiss. +// terminal statuses, then waits for the user to dismiss it. After a run +// completes, dismissal requires an explicit exit key (q/Esc/Enter/Ctrl+C); +// Ctrl+O still drills into agent buffers for post-mortem inspection. Other +// keys are no-ops so users can navigate the completed run without +// accidentally dismissing. // -// IMPORTANT: RunFinished blocks until the user dismisses (presses any key) -// so that post-run sinks (e.g. DumpSink) render their narrative AFTER the -// TUI has exited and the terminal is back in normal mode. +// IMPORTANT: RunFinished blocks until the user dismisses so that post-run +// sinks (e.g. DumpSink) render their narrative AFTER the TUI has exited +// and the terminal is back in normal mode. func (s *TUISink) RunFinished(summary reviewtypes.RunSummary) { s.mu.Lock() if s.finished { @@ -160,8 +166,8 @@ func (s *TUISink) RunFinished(summary reviewtypes.RunSummary) { s.mu.Unlock() s.program.Send(runFinishedMsg{summary: summary}) - // Block until the Bubble Tea program exits (user presses any key after - // seeing the final dashboard, or Ctrl+C was already received and the - // program already quit). + // Block until the Bubble Tea program exits — user pressed an explicit + // exit key (q/Esc/Enter/Ctrl+C) after seeing the final dashboard, or + // Ctrl+C was received during the run and the program already quit. s.Wait() } diff --git a/cmd/entire/cli/review/tui_sink_test.go b/cmd/entire/cli/review/tui_sink_test.go index 5abdc8bb7..b7c84f4a0 100644 --- a/cmd/entire/cli/review/tui_sink_test.go +++ b/cmd/entire/cli/review/tui_sink_test.go @@ -28,7 +28,9 @@ func finishAndDismissTUI(t *testing.T, sink *TUISink, summary reviewtypes.RunSum case <-done: return case <-ticker.C: - sink.program.Send(tea.KeyPressMsg(tea.Key{Code: 'x', Text: "x"})) + // 'q' is an explicit post-finish exit key. (Any-key-quits was + // removed so the user can still Ctrl+O into completed output.) + sink.program.Send(tea.KeyPressMsg(tea.Key{Code: 'q', Text: "q"})) case <-timeout: t.Fatal("RunFinished() did not return within 10 seconds") } @@ -98,7 +100,7 @@ func TestTUISink_AgentEvent_BeforeStart_IsNoOp(t *testing.T) { } // TestTUISink_RunFinished_EventuallyUnblocks verifies that RunFinished unblocks -// once the finished TUI receives the same any-key dismissal used by a user. +// once the finished TUI receives an explicit exit key (q) like a user would press. func TestTUISink_RunFinished_EventuallyUnblocks(t *testing.T) { t.Parallel() var buf bytes.Buffer diff --git a/cmd/entire/cli/review/tui_text.go b/cmd/entire/cli/review/tui_text.go index 36b9ba302..243f168bb 100644 --- a/cmd/entire/cli/review/tui_text.go +++ b/cmd/entire/cli/review/tui_text.go @@ -56,3 +56,36 @@ func truncateDisplayWidth(s string, width int) string { } return ansi.Truncate(s, width, "…") } + +// wrapDisplayWidth wraps s to lines no wider than width display cells. Embedded +// '\n' characters are honored as paragraph boundaries: each paragraph is +// sanitized (ANSI/control stripped) and wrapped independently. A paragraph that +// wraps to nothing still contributes an empty line, preserving blank-line +// structure between paragraphs. +// +// Trailing newlines are stripped before splitting so "text\n" yields a single +// line, not a phantom blank tail — matching how splitBodyToHeight trims its +// input. +// +// Returns nil for width <= 0 or input that is empty (or only newlines). +func wrapDisplayWidth(s string, width int) []string { + if width <= 0 { + return nil + } + s = strings.TrimRight(s, "\n") + if s == "" { + return nil + } + paragraphs := strings.Split(s, "\n") + out := make([]string, 0, len(paragraphs)) + for _, p := range paragraphs { + clean := sanitizeDisplayText(p) + if clean == "" { + out = append(out, "") + continue + } + wrapped := ansi.Wrap(clean, width, "") + out = append(out, strings.Split(wrapped, "\n")...) + } + return out +} diff --git a/cmd/entire/cli/review/tui_text_test.go b/cmd/entire/cli/review/tui_text_test.go new file mode 100644 index 000000000..275dccb62 --- /dev/null +++ b/cmd/entire/cli/review/tui_text_test.go @@ -0,0 +1,86 @@ +package review + +import ( + "reflect" + "testing" +) + +func TestWrapDisplayWidth(t *testing.T) { + t.Parallel() + tests := []struct { + name string + in string + width int + want []string + }{ + { + name: "empty input returns nil", + in: "", + width: 80, + want: nil, + }, + { + name: "zero width returns nil", + in: "anything", + width: 0, + want: nil, + }, + { + name: "negative width returns nil", + in: "anything", + width: -10, + want: nil, + }, + { + name: "short single line fits", + in: "hello", + width: 80, + want: []string{"hello"}, + }, + { + name: "long line wraps to width", + in: "aaaa bbbb cccc", + width: 5, + want: []string{"aaaa", "bbbb", "cccc"}, + }, + { + name: "embedded newline preserved as paragraph break", + in: "a\n\nb", + width: 80, + want: []string{"a", "", "b"}, + }, + { + name: "trailing newline does not produce phantom blank line", + in: "text\n", + width: 80, + want: []string{"text"}, + }, + { + name: "multiple trailing newlines collapsed", + in: "text\n\n\n", + width: 80, + want: []string{"text"}, + }, + { + name: "ANSI escape stripped from output", + in: "\x1b[31mred\x1b[0m text", + width: 80, + want: []string{"red text"}, + }, + { + name: "control chars stripped", + in: "a\x07b", + width: 80, + want: []string{"ab"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := wrapDisplayWidth(tt.in, tt.width) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("wrapDisplayWidth(%q, %d) = %#v, want %#v", tt.in, tt.width, got, tt.want) + } + }) + } +}