From 25690ffbe13b4d6391988465dd3e7b40387c5938 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 4 Mar 2026 18:18:54 -0600 Subject: [PATCH 1/5] Restore separate P/F verdict column in TUI queue table The combined Status column (showing Pass/Fail/Done/etc.) was confusing. Split it back into separate Status and P/F columns: Status shows the job state (Queued/Running/Done/Error/Canceled), P/F shows the verdict (P/F/-). The P/F column sits between Status and Closed. Co-Authored-By: Claude Opus 4.6 --- cmd/roborev/tui/queue_test.go | 86 ++++++++----------------- cmd/roborev/tui/render_queue.go | 109 ++++++++++++++++---------------- cmd/roborev/tui/tui.go | 1 - 3 files changed, 82 insertions(+), 114 deletions(-) diff --git a/cmd/roborev/tui/queue_test.go b/cmd/roborev/tui/queue_test.go index 8f736989..0638abfd 100644 --- a/cmd/roborev/tui/queue_test.go +++ b/cmd/roborev/tui/queue_test.go @@ -628,7 +628,7 @@ func TestTUIJobCellsContent(t *testing.T) { ) cells := m.jobCells(job) - // cells order: ref, branch, repo, agent, queued, elapsed, status, closed + // cells order: ref, branch, repo, agent, queued, elapsed, status, pf, closed if !strings.Contains(cells[0], "abc1234") { t.Errorf("Expected ref to contain abc1234, got %q", cells[0]) } @@ -641,6 +641,9 @@ func TestTUIJobCellsContent(t *testing.T) { if cells[6] != "Done" { t.Errorf("Expected status 'Done', got %q", cells[6]) } + if cells[7] != "-" { + t.Errorf("Expected verdict '-', got %q", cells[7]) + } }) t.Run("claude-code normalizes to claude", func(t *testing.T) { @@ -659,11 +662,14 @@ func TestTUIJobCellsContent(t *testing.T) { job.Closed = &handled cells := m.jobCells(job) - if cells[6] != "Pass" { - t.Errorf("Expected status 'Pass', got %q", cells[6]) + if cells[6] != "Done" { + t.Errorf("Expected status 'Done', got %q", cells[6]) + } + if cells[7] != "P" { + t.Errorf("Expected verdict 'P', got %q", cells[7]) } - if cells[7] != "yes" { - t.Errorf("Expected closed 'yes', got %q", cells[7]) + if cells[8] != "yes" { + t.Errorf("Expected closed 'yes', got %q", cells[8]) } }) } @@ -2198,9 +2204,7 @@ func TestTaskColWidthCacheReuse(t *testing.T) { } } -func TestCombinedStatus(t *testing.T) { - strPtr := func(s string) *string { return &s } - +func TestStatusLabel(t *testing.T) { tests := []struct { name string job storage.ReviewJob @@ -2210,63 +2214,21 @@ func TestCombinedStatus(t *testing.T) { {"running", storage.ReviewJob{Status: storage.JobStatusRunning}, "Running"}, {"failed", storage.ReviewJob{Status: storage.JobStatusFailed}, "Error"}, {"canceled", storage.ReviewJob{Status: storage.JobStatusCanceled}, "Canceled"}, - {"done pass", storage.ReviewJob{Status: storage.JobStatusDone, Verdict: strPtr("P")}, "Pass"}, - {"done fail", storage.ReviewJob{Status: storage.JobStatusDone, Verdict: strPtr("F")}, "Fail"}, - {"done unexpected verdict", storage.ReviewJob{Status: storage.JobStatusDone, Verdict: strPtr("X")}, "Fail"}, - {"done nil verdict", storage.ReviewJob{Status: storage.JobStatusDone}, "Done"}, - {"applied pass", storage.ReviewJob{Status: storage.JobStatusApplied, Verdict: strPtr("P")}, "Pass"}, - {"rebased fail", storage.ReviewJob{Status: storage.JobStatusRebased, Verdict: strPtr("F")}, "Fail"}, + {"done", storage.ReviewJob{Status: storage.JobStatusDone}, "Done"}, + {"applied", storage.ReviewJob{Status: storage.JobStatusApplied}, "Done"}, + {"rebased", storage.ReviewJob{Status: storage.JobStatusRebased}, "Done"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := combinedStatus(tt.job) + got := statusLabel(tt.job) if got != tt.want { - t.Errorf("combinedStatus() = %q, want %q", got, tt.want) + t.Errorf("statusLabel() = %q, want %q", got, tt.want) } }) } } -func TestCombinedStatusColor(t *testing.T) { - strPtr := func(s string) *string { return &s } - - tests := []struct { - name string - job storage.ReviewJob - wantStyle lipgloss.Style - }{ - {"queued", storage.ReviewJob{Status: storage.JobStatusQueued}, queuedStyle}, - {"running", storage.ReviewJob{Status: storage.JobStatusRunning}, runningStyle}, - {"failed", storage.ReviewJob{Status: storage.JobStatusFailed}, failedStyle}, - {"canceled", storage.ReviewJob{Status: storage.JobStatusCanceled}, canceledStyle}, - {"done pass", storage.ReviewJob{Status: storage.JobStatusDone, Verdict: strPtr("P")}, passStyle}, - {"done fail", storage.ReviewJob{Status: storage.JobStatusDone, Verdict: strPtr("F")}, failStyle}, - {"done unexpected verdict", storage.ReviewJob{Status: storage.JobStatusDone, Verdict: strPtr("X")}, failStyle}, - {"done nil verdict", storage.ReviewJob{Status: storage.JobStatusDone}, readyStyle}, - {"applied nil verdict", storage.ReviewJob{Status: storage.JobStatusApplied}, readyStyle}, - {"rebased nil verdict", storage.ReviewJob{Status: storage.JobStatusRebased}, readyStyle}, - {"unknown status", storage.ReviewJob{Status: "unknown"}, queuedStyle}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := combinedStatusColor(tt.job) - want := tt.wantStyle.GetForeground() - if got != want { - t.Errorf("combinedStatusColor() = %v, want %v", got, want) - } - }) - } - - // Verify Error (failedStyle) and Fail (failStyle) use different colors - errorColor := failedStyle.GetForeground() - failColor := failStyle.GetForeground() - if errorColor == failColor { - t.Errorf("Error and Fail should have distinct colors, both are %v", errorColor) - } -} - func TestClosedKeyShortcut(t *testing.T) { boolPtr := func(b bool) *bool { return &b } @@ -2339,17 +2301,23 @@ func TestMigrateColumnConfig(t *testing.T) { wantDirty: true, wantColOrder: nil, }, + { + name: "combined status default order resets", + columnOrder: []string{"ref", "branch", "repo", "agent", "queued", "elapsed", "status", "closed"}, + wantDirty: true, + wantColOrder: nil, + }, { name: "custom order preserved", - columnOrder: []string{"repo", "ref", "agent", "status", "queued", "elapsed", "branch", "closed"}, + columnOrder: []string{"repo", "ref", "agent", "status", "pf", "queued", "elapsed", "branch", "closed"}, wantDirty: false, - wantColOrder: []string{"repo", "ref", "agent", "status", "queued", "elapsed", "branch", "closed"}, + wantColOrder: []string{"repo", "ref", "agent", "status", "pf", "queued", "elapsed", "branch", "closed"}, }, { name: "current default order preserved", - columnOrder: []string{"ref", "branch", "repo", "agent", "queued", "elapsed", "status", "closed"}, + columnOrder: []string{"ref", "branch", "repo", "agent", "queued", "elapsed", "status", "pf", "closed"}, wantDirty: false, - wantColOrder: []string{"ref", "branch", "repo", "agent", "queued", "elapsed", "status", "closed"}, + wantColOrder: []string{"ref", "branch", "repo", "agent", "queued", "elapsed", "status", "pf", "closed"}, }, } diff --git a/cmd/roborev/tui/render_queue.go b/cmd/roborev/tui/render_queue.go index 83a61249..22ab4435 100644 --- a/cmd/roborev/tui/render_queue.go +++ b/cmd/roborev/tui/render_queue.go @@ -97,7 +97,8 @@ const ( colAgent // Agent name colQueued // Enqueue timestamp colElapsed // Elapsed time - colStatus // Job status (combined with verdict) + colStatus // Job status + colPF // Pass/Fail verdict colHandled // Done status colCount // total number of columns ) @@ -248,7 +249,7 @@ func (m model) renderQueueView() string { visCols := m.visibleColumns() // Compute per-column max content widths, using cache when data hasn't changed. - allHeaders := [colCount]string{"", "JobID", "Ref", "Branch", "Repo", "Agent", "Queued", "Elapsed", "Status", "Closed"} + allHeaders := [colCount]string{"", "JobID", "Ref", "Branch", "Repo", "Agent", "Queued", "Elapsed", "Status", "P/F", "Closed"} allFullRows := make([][]string, len(visibleJobList)) for i, job := range visibleJobList { cells := m.jobCells(job) @@ -301,6 +302,7 @@ func (m model) renderQueueView() string { colStatus: 8, // fits "Canceled" (8), "Running" (7), etc. colQueued: 12, colElapsed: 8, + colPF: 3, // "P/F" header = 3 colHandled: max(contentWidth[colHandled], 6), // "Closed" header = 6 colAgent: min(max(contentWidth[colAgent], 5), 12), // "Agent" header = 5, cap at 12 } @@ -491,7 +493,28 @@ func (m model) renderQueueView() string { job := windowJobs[row] switch logicalCol { case colStatus: - s = s.Foreground(combinedStatusColor(job)) + switch job.Status { + case storage.JobStatusQueued: + s = s.Foreground(queuedStyle.GetForeground()) + case storage.JobStatusRunning: + s = s.Foreground(runningStyle.GetForeground()) + case storage.JobStatusDone, + storage.JobStatusApplied, + storage.JobStatusRebased: + s = s.Foreground(doneStyle.GetForeground()) + case storage.JobStatusFailed: + s = s.Foreground(failedStyle.GetForeground()) + case storage.JobStatusCanceled: + s = s.Foreground(canceledStyle.GetForeground()) + } + case colPF: + if job.Verdict != nil { + if *job.Verdict == "P" { + s = s.Foreground(passStyle.GetForeground()) + } else { + s = s.Foreground(failStyle.GetForeground()) + } + } case colHandled: if job.Closed != nil { if *job.Closed { @@ -575,8 +598,8 @@ func (m model) renderQueueView() string { } // jobCells returns plain text cell values for a job row. -// Order: ref, branch, repo, agent, status, queued, elapsed, handled -// (colRef through colHandled, 8 values). +// Order: ref, branch, repo, agent, queued, elapsed, status, pf, handled +// (colRef through colHandled, 9 values). func (m model) jobCells(job storage.ReviewJob) []string { ref := shortJobRef(job) if !config.IsDefaultReviewType(job.ReviewType) { @@ -606,7 +629,12 @@ func (m model) jobCells(job storage.ReviewJob) []string { } } - status := combinedStatus(job) + status := statusLabel(job) + + verdict := "-" + if job.Verdict != nil { + verdict = *job.Verdict + } handled := "" if job.Closed != nil { @@ -617,13 +645,11 @@ func (m model) jobCells(job storage.ReviewJob) []string { } } - return []string{ref, branch, repo, agentName, enqueued, elapsed, status, handled} + return []string{ref, branch, repo, agentName, enqueued, elapsed, status, verdict, handled} } -// combinedStatus returns a display string that merges job status -// with verdict: Queued, Running, Error, Canceled, Pass, Fail, or -// Done (for task/fix jobs that have no verdict). -func combinedStatus(job storage.ReviewJob) string { +// statusLabel returns a capitalized display label for the job status. +func statusLabel(job storage.ReviewJob) string { switch job.Status { case storage.JobStatusQueued: return "Queued" @@ -635,46 +661,12 @@ func combinedStatus(job storage.ReviewJob) string { return "Canceled" case storage.JobStatusDone, storage.JobStatusApplied, storage.JobStatusRebased: - if job.Verdict != nil { - if *job.Verdict == "P" { - return "Pass" - } - return "Fail" - } return "Done" default: return string(job.Status) } } -// combinedStatusColor returns the foreground color for the -// combined status column based on job state and verdict. -func combinedStatusColor( - job storage.ReviewJob, -) lipgloss.TerminalColor { - switch job.Status { - case storage.JobStatusQueued: - return queuedStyle.GetForeground() - case storage.JobStatusRunning: - return runningStyle.GetForeground() - case storage.JobStatusFailed: - return failedStyle.GetForeground() - case storage.JobStatusCanceled: - return canceledStyle.GetForeground() - case storage.JobStatusDone, storage.JobStatusApplied, - storage.JobStatusRebased: - if job.Verdict == nil { - return readyStyle.GetForeground() - } - if *job.Verdict != "P" { - return failStyle.GetForeground() - } - return passStyle.GetForeground() - default: - return queuedStyle.GetForeground() - } -} - // commandLineForJob computes the representative agent command line from job parameters. // Returns empty string if the agent is not available. func commandLineForJob(job *storage.ReviewJob) string { @@ -722,21 +714,28 @@ func migrateColumnConfig(cfg *config.Config) bool { cfg.HiddenColumns = nil dirty = true } - // Old default order (status before queued) → reset - oldDefault := []string{ - "ref", "branch", "repo", "agent", - "status", "queued", "elapsed", "closed", - } - if slices.Equal(cfg.ColumnOrder, oldDefault) { - cfg.ColumnOrder = nil - dirty = true + // Old default orders → reset + oldDefaults := [][]string{ + // status before queued (pre-rename) + {"ref", "branch", "repo", "agent", + "status", "queued", "elapsed", "closed"}, + // combined status without pf column + {"ref", "branch", "repo", "agent", + "queued", "elapsed", "status", "closed"}, + } + for _, old := range oldDefaults { + if slices.Equal(cfg.ColumnOrder, old) { + cfg.ColumnOrder = nil + dirty = true + break + } } return dirty } // toggleableColumns is the ordered list of columns the user can show/hide. // colSel and colJobID are always visible and not included here. -var toggleableColumns = []int{colRef, colBranch, colRepo, colAgent, colQueued, colElapsed, colStatus, colHandled} +var toggleableColumns = []int{colRef, colBranch, colRepo, colAgent, colQueued, colElapsed, colStatus, colPF, colHandled} // columnNames maps column constants to display names. var columnNames = map[int]string{ @@ -747,6 +746,7 @@ var columnNames = map[int]string{ colStatus: "Status", colQueued: "Queued", colElapsed: "Elapsed", + colPF: "P/F", colHandled: "Closed", } @@ -759,6 +759,7 @@ var columnConfigNames = map[int]string{ colStatus: "status", colQueued: "queued", colElapsed: "elapsed", + colPF: "pf", colHandled: "closed", } diff --git a/cmd/roborev/tui/tui.go b/cmd/roborev/tui/tui.go index 73f96c66..8e6d1e7b 100644 --- a/cmd/roborev/tui/tui.go +++ b/cmd/roborev/tui/tui.go @@ -51,7 +51,6 @@ var ( passStyle = lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "28", Dark: "46"}) // Green failStyle = lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "124", Dark: "196"}) // Red (review found issues) - readyStyle = lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "27", Dark: "39"}) // Blue (done, no verdict) closedStyle = lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "30", Dark: "51"}) // Cyan helpStyle = lipgloss.NewStyle(). From ecd63843d62a729ef08454077ac573d0b159e817 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 4 Mar 2026 18:29:14 -0600 Subject: [PATCH 2/5] Add color tests for Status/P/F columns, change Done to blue Extract statusColor and verdictColor helpers from the inline StyleFunc to make them unit-testable. Add TestStatusColor, TestVerdictColor, and TestParseColumnOrderAppendsMissing tests. Change doneStyle from green (same as passStyle) to blue so Done and Pass are visually distinct. Co-Authored-By: Claude Opus 4.6 --- cmd/roborev/tui/queue_test.go | 80 +++++++++++++++++++++++++++++++++ cmd/roborev/tui/render_queue.go | 58 +++++++++++++++--------- cmd/roborev/tui/tui.go | 2 +- 3 files changed, 119 insertions(+), 21 deletions(-) diff --git a/cmd/roborev/tui/queue_test.go b/cmd/roborev/tui/queue_test.go index 0638abfd..55532b28 100644 --- a/cmd/roborev/tui/queue_test.go +++ b/cmd/roborev/tui/queue_test.go @@ -2229,6 +2229,62 @@ func TestStatusLabel(t *testing.T) { } } +func TestStatusColor(t *testing.T) { + tests := []struct { + name string + status storage.JobStatus + wantStyle lipgloss.Style + }{ + {"queued", storage.JobStatusQueued, queuedStyle}, + {"running", storage.JobStatusRunning, runningStyle}, + {"done", storage.JobStatusDone, doneStyle}, + {"applied", storage.JobStatusApplied, doneStyle}, + {"rebased", storage.JobStatusRebased, doneStyle}, + {"failed", storage.JobStatusFailed, failedStyle}, + {"canceled", storage.JobStatusCanceled, canceledStyle}, + {"unknown", storage.JobStatus("unknown"), queuedStyle}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := statusColor(tt.status) + want := tt.wantStyle.GetForeground() + if got != want { + t.Errorf("statusColor(%q) = %v, want %v", tt.status, got, want) + } + }) + } + + // Error (failedStyle/orange) and Fail (failStyle/red) must be distinct + if failedStyle.GetForeground() == failStyle.GetForeground() { + t.Error("Error and Fail should have distinct colors") + } +} + +func TestVerdictColor(t *testing.T) { + strPtr := func(s string) *string { return &s } + + tests := []struct { + name string + verdict *string + want lipgloss.TerminalColor + }{ + {"pass", strPtr("P"), passStyle.GetForeground()}, + {"fail", strPtr("F"), failStyle.GetForeground()}, + {"unexpected", strPtr("X"), failStyle.GetForeground()}, + {"nil", nil, nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := verdictColor(tt.verdict) + if got != tt.want { + t.Errorf("verdictColor() = %v, want %v", got, tt.want) + } + }) + } +} + func TestClosedKeyShortcut(t *testing.T) { boolPtr := func(b bool) *bool { return &b } @@ -2341,6 +2397,30 @@ func TestMigrateColumnConfig(t *testing.T) { } } +func TestParseColumnOrderAppendsMissing(t *testing.T) { + // A custom order saved before the pf column existed should get + // pf appended automatically by resolveColumnOrder. + oldCustom := []string{"repo", "ref", "agent", "status", "queued", "elapsed", "branch", "closed"} + got := parseColumnOrder(oldCustom) + + // Verify existing columns are in the user's order + wantPrefix := []int{colRepo, colRef, colAgent, colStatus, colQueued, colElapsed, colBranch, colHandled} + if !slices.Equal(got[:len(wantPrefix)], wantPrefix) { + t.Errorf("prefix = %v, want %v", got[:len(wantPrefix)], wantPrefix) + } + + // pf must be appended exactly once + pfCount := 0 + for _, c := range got { + if c == colPF { + pfCount++ + } + } + if pfCount != 1 { + t.Errorf("expected pf to appear once, got %d in %v", pfCount, got) + } +} + func TestDefaultColumnOrderDetection(t *testing.T) { // Verify the slices.Equal check that saveColumnOptions uses // to decide whether to persist column order: default order diff --git a/cmd/roborev/tui/render_queue.go b/cmd/roborev/tui/render_queue.go index 22ab4435..8519929c 100644 --- a/cmd/roborev/tui/render_queue.go +++ b/cmd/roborev/tui/render_queue.go @@ -493,27 +493,10 @@ func (m model) renderQueueView() string { job := windowJobs[row] switch logicalCol { case colStatus: - switch job.Status { - case storage.JobStatusQueued: - s = s.Foreground(queuedStyle.GetForeground()) - case storage.JobStatusRunning: - s = s.Foreground(runningStyle.GetForeground()) - case storage.JobStatusDone, - storage.JobStatusApplied, - storage.JobStatusRebased: - s = s.Foreground(doneStyle.GetForeground()) - case storage.JobStatusFailed: - s = s.Foreground(failedStyle.GetForeground()) - case storage.JobStatusCanceled: - s = s.Foreground(canceledStyle.GetForeground()) - } + s = s.Foreground(statusColor(job.Status)) case colPF: - if job.Verdict != nil { - if *job.Verdict == "P" { - s = s.Foreground(passStyle.GetForeground()) - } else { - s = s.Foreground(failStyle.GetForeground()) - } + if c := verdictColor(job.Verdict); c != nil { + s = s.Foreground(c) } case colHandled: if job.Closed != nil { @@ -667,6 +650,41 @@ func statusLabel(job storage.ReviewJob) string { } } +// statusColor returns the foreground color for the Status column. +func statusColor( + status storage.JobStatus, +) lipgloss.TerminalColor { + switch status { + case storage.JobStatusQueued: + return queuedStyle.GetForeground() + case storage.JobStatusRunning: + return runningStyle.GetForeground() + case storage.JobStatusDone, storage.JobStatusApplied, + storage.JobStatusRebased: + return doneStyle.GetForeground() + case storage.JobStatusFailed: + return failedStyle.GetForeground() + case storage.JobStatusCanceled: + return canceledStyle.GetForeground() + default: + return queuedStyle.GetForeground() + } +} + +// verdictColor returns the foreground color for the P/F column. +// Returns nil when no color should be applied (nil verdict). +func verdictColor( + verdict *string, +) lipgloss.TerminalColor { + if verdict == nil { + return nil + } + if *verdict == "P" { + return passStyle.GetForeground() + } + return failStyle.GetForeground() +} + // commandLineForJob computes the representative agent command line from job parameters. // Returns empty string if the agent is not available. func commandLineForJob(job *storage.ReviewJob) string { diff --git a/cmd/roborev/tui/tui.go b/cmd/roborev/tui/tui.go index 8e6d1e7b..6bdb83fd 100644 --- a/cmd/roborev/tui/tui.go +++ b/cmd/roborev/tui/tui.go @@ -45,7 +45,7 @@ var ( queuedStyle = lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "136", Dark: "226"}) // Yellow/Gold runningStyle = lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "25", Dark: "33"}) // Blue - doneStyle = lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "28", Dark: "46"}) // Green + doneStyle = lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "27", Dark: "39"}) // Blue (completed) failedStyle = lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "166", Dark: "208"}) // Orange (job error) canceledStyle = lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "243", Dark: "245"}) // Gray From f855c1701decbe85370fd7cf0321cd161426def1 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 4 Mar 2026 18:31:24 -0600 Subject: [PATCH 3/5] Return nil for unknown status color instead of queuedStyle Unknown job statuses previously got queuedStyle (yellow), which could mislead users into thinking the job is queued. Return nil so no foreground color is applied, matching the behavior before the refactor. Co-Authored-By: Claude Opus 4.6 --- cmd/roborev/tui/queue_test.go | 27 +++++++++++++-------------- cmd/roborev/tui/render_queue.go | 6 ++++-- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/cmd/roborev/tui/queue_test.go b/cmd/roborev/tui/queue_test.go index 55532b28..27ad973d 100644 --- a/cmd/roborev/tui/queue_test.go +++ b/cmd/roborev/tui/queue_test.go @@ -2231,26 +2231,25 @@ func TestStatusLabel(t *testing.T) { func TestStatusColor(t *testing.T) { tests := []struct { - name string - status storage.JobStatus - wantStyle lipgloss.Style + name string + status storage.JobStatus + want lipgloss.TerminalColor }{ - {"queued", storage.JobStatusQueued, queuedStyle}, - {"running", storage.JobStatusRunning, runningStyle}, - {"done", storage.JobStatusDone, doneStyle}, - {"applied", storage.JobStatusApplied, doneStyle}, - {"rebased", storage.JobStatusRebased, doneStyle}, - {"failed", storage.JobStatusFailed, failedStyle}, - {"canceled", storage.JobStatusCanceled, canceledStyle}, - {"unknown", storage.JobStatus("unknown"), queuedStyle}, + {"queued", storage.JobStatusQueued, queuedStyle.GetForeground()}, + {"running", storage.JobStatusRunning, runningStyle.GetForeground()}, + {"done", storage.JobStatusDone, doneStyle.GetForeground()}, + {"applied", storage.JobStatusApplied, doneStyle.GetForeground()}, + {"rebased", storage.JobStatusRebased, doneStyle.GetForeground()}, + {"failed", storage.JobStatusFailed, failedStyle.GetForeground()}, + {"canceled", storage.JobStatusCanceled, canceledStyle.GetForeground()}, + {"unknown", storage.JobStatus("unknown"), nil}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := statusColor(tt.status) - want := tt.wantStyle.GetForeground() - if got != want { - t.Errorf("statusColor(%q) = %v, want %v", tt.status, got, want) + if got != tt.want { + t.Errorf("statusColor(%q) = %v, want %v", tt.status, got, tt.want) } }) } diff --git a/cmd/roborev/tui/render_queue.go b/cmd/roborev/tui/render_queue.go index 8519929c..2ac00895 100644 --- a/cmd/roborev/tui/render_queue.go +++ b/cmd/roborev/tui/render_queue.go @@ -493,7 +493,9 @@ func (m model) renderQueueView() string { job := windowJobs[row] switch logicalCol { case colStatus: - s = s.Foreground(statusColor(job.Status)) + if c := statusColor(job.Status); c != nil { + s = s.Foreground(c) + } case colPF: if c := verdictColor(job.Verdict); c != nil { s = s.Foreground(c) @@ -667,7 +669,7 @@ func statusColor( case storage.JobStatusCanceled: return canceledStyle.GetForeground() default: - return queuedStyle.GetForeground() + return nil } } From 2303adda67fd408044dbe80c8f1e6669538b98dc Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 4 Mar 2026 18:31:55 -0600 Subject: [PATCH 4/5] Auto-size Status column width based on visible content Instead of hardcoding 8 chars (to fit "Canceled"), size the Status column to the widest status string actually present. Saves 2 chars when no canceled jobs are visible (e.g. "Done" = 4, "Queued" = 6). Co-Authored-By: Claude Opus 4.6 --- cmd/roborev/tui/render_queue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/roborev/tui/render_queue.go b/cmd/roborev/tui/render_queue.go index 2ac00895..b9f171c0 100644 --- a/cmd/roborev/tui/render_queue.go +++ b/cmd/roborev/tui/render_queue.go @@ -299,7 +299,7 @@ func (m model) renderQueueView() string { fixedWidth := map[int]int{ colSel: 2, colJobID: idWidth, - colStatus: 8, // fits "Canceled" (8), "Running" (7), etc. + colStatus: max(contentWidth[colStatus], 6), // "Status" header = 6, auto-sizes to content colQueued: 12, colElapsed: 8, colPF: 3, // "P/F" header = 3 From c374f722f185cd9ec9cb3447738f6630bc98c71b Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 4 Mar 2026 18:37:15 -0600 Subject: [PATCH 5/5] Add TestStatusColumnAutoWidth for content-driven status sizing Verifies the Status column auto-sizes to the widest label present: 6 for Done/Queued (header floor), 7 for Running, 8 for Canceled. Co-Authored-By: Claude Opus 4.6 --- cmd/roborev/tui/queue_test.go | 65 +++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/cmd/roborev/tui/queue_test.go b/cmd/roborev/tui/queue_test.go index 27ad973d..4701c1b4 100644 --- a/cmd/roborev/tui/queue_test.go +++ b/cmd/roborev/tui/queue_test.go @@ -739,6 +739,71 @@ func TestTUIQueueTableRendersWithinWidth(t *testing.T) { } } +func TestStatusColumnAutoWidth(t *testing.T) { + // The Status column should auto-size to the widest status label + // present in the visible jobs, with a floor of 6 (the "Status" + // header width). This saves horizontal space when no wide status + // labels (e.g. "Canceled") are present. + tests := []struct { + name string + statuses []storage.JobStatus + wantWidth int // expected content width (header included) + }{ + {"done only", []storage.JobStatus{storage.JobStatusDone}, 6}, // "Done"=4, header=6 + {"queued only", []storage.JobStatus{storage.JobStatusQueued}, 6}, // "Queued"=6, header=6 + {"running", []storage.JobStatus{storage.JobStatusRunning}, 7}, // "Running"=7 + {"canceled", []storage.JobStatus{storage.JobStatusCanceled}, 8}, // "Canceled"=8 + {"mixed done and error", []storage.JobStatus{storage.JobStatusDone, storage.JobStatusFailed}, 6}, // max("Done"=4,"Error"=5,header=6)=6 + {"mixed done and canceled", []storage.JobStatus{storage.JobStatusDone, storage.JobStatusCanceled}, 8}, // max("Done"=4,"Canceled"=8)=8 + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := newModel("http://localhost", withExternalIODisabled()) + m.width = 200 + m.height = 30 + + jobs := make([]storage.ReviewJob, len(tt.statuses)) + for i, s := range tt.statuses { + jobs[i] = makeJob(int64(i+1), withStatus(s), withRef("abc1234"), withRepoName("repo"), withAgent("test")) + } + m.jobs = jobs + m.selectedIdx = 0 + m.selectedJobID = 1 + + output := m.renderQueueView() + lines := strings.Split(output, "\n") + + // Find the header line (contains "Status" and "P/F") + var headerLine string + for _, line := range lines { + stripped := stripTestANSI(line) + if strings.Contains(stripped, "Status") && strings.Contains(stripped, "P/F") { + headerLine = stripped + break + } + } + if headerLine == "" { + t.Fatal("could not find header line with Status and P/F") + } + + statusIdx := strings.Index(headerLine, "Status") + pfIdx := strings.Index(headerLine, "P/F") + if statusIdx < 0 || pfIdx < 0 || pfIdx <= statusIdx { + t.Fatalf("unexpected header layout: %q", headerLine) + } + + // The gap between "Status" start and "P/F" start is + // the column width + inter-column spacing (1 char padding). + gap := pfIdx - statusIdx + gotWidth := gap - 1 // subtract 1 for inter-column spacing + if gotWidth != tt.wantWidth { + t.Errorf("Status column width = %d, want %d (header: %q)", gotWidth, tt.wantWidth, headerLine) + } + }) + } +} + func TestTUIPaginationAppendMode(t *testing.T) { m := newModel("http://localhost", withExternalIODisabled())