diff --git a/cmd/roborev/tui/control_handlers.go b/cmd/roborev/tui/control_handlers.go index 22c2fe93..53210ab8 100644 --- a/cmd/roborev/tui/control_handlers.go +++ b/cmd/roborev/tui/control_handlers.go @@ -254,6 +254,7 @@ func (m model) handleCtrlSetFilter( m.fetchSeq++ m.queueColGen++ m.loadingJobs = true + m.recomputeClassifyEffective() return m, controlResponse{OK: true}, m.fetchJobs() } @@ -299,6 +300,7 @@ func (m model) handleCtrlClearFilter( m.fetchSeq++ m.queueColGen++ m.loadingJobs = true + m.recomputeClassifyEffective() return m, controlResponse{OK: true}, m.fetchJobs() } diff --git a/cmd/roborev/tui/fetch.go b/cmd/roborev/tui/fetch.go index 967b2985..357cdae1 100644 --- a/cmd/roborev/tui/fetch.go +++ b/cmd/roborev/tui/fetch.go @@ -149,6 +149,10 @@ func listJobsParams(values neturl.Values) daemonclient.ListJobsParams { } setStringParam("job_type", ¶ms.JobType) setStringParam("exclude_job_type", ¶ms.ExcludeJobType) + if value := values.Get("hide_classify_jobs"); value != "" { + typed := daemonclient.ListJobsParamsHideClassifyJobs(value) + params.HideClassifyJobs = &typed + } setStringParam("repo_prefix", ¶ms.RepoPrefix) setIntParam("limit", ¶ms.Limit) setIntParam("offset", ¶ms.Offset) @@ -198,6 +202,13 @@ func (m model) fetchJobs() tea.Cmd { // Exclude fix jobs — they belong in the Tasks view, not the queue params.Set("exclude_job_type", "fix") + // Hide auto-design-router byproducts (classify rows + skipped design + // rows) unless the user opted in via show_classify_jobs. Resolved at + // fetch time so single-repo filters honor that repo's override. + if !m.shouldShowClassifyJobs() { + params.Set("hide_classify_jobs", "true") + } + // Set limit: use pagination unless we need client-side filtering (multi-repo) if needsAllJobs { params.Set("limit", "0") @@ -240,6 +251,9 @@ func (m model) fetchMoreJobs() tea.Cmd { params.Set("closed", "false") } params.Set("exclude_job_type", "fix") + if !m.shouldShowClassifyJobs() { + params.Set("hide_classify_jobs", "true") + } result, err := m.loadJobsPage(params) if err != nil { return paginationErrMsg{ diff --git a/cmd/roborev/tui/filter.go b/cmd/roborev/tui/filter.go index 3172fdff..fcf4c3b1 100644 --- a/cmd/roborev/tui/filter.go +++ b/cmd/roborev/tui/filter.go @@ -255,6 +255,7 @@ func (m *model) reconcileAutoRepoFilter() bool { m.fetchSeq++ m.queueColGen++ m.loadingJobs = true + m.recomputeClassifyEffective() return true } @@ -319,6 +320,7 @@ func (m *model) popFilter() string { case filterTypeRepo: m.activeRepoFilter = nil m.autoRepoFilter = false + m.recomputeClassifyEffective() case filterTypeBranch: m.activeBranchFilter = "" } diff --git a/cmd/roborev/tui/handlers.go b/cmd/roborev/tui/handlers.go index f397bbf4..267d46c6 100644 --- a/cmd/roborev/tui/handlers.go +++ b/cmd/roborev/tui/handlers.go @@ -129,6 +129,8 @@ func (m model) handleGlobalKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return m.handleBranchFilterOpenKey() case "h": return m.handleHideClosedKey() + case "s": + return m.handleToggleClassifyKey() case "c": return m.handleCommentOpenKey() case "y": diff --git a/cmd/roborev/tui/handlers_modal.go b/cmd/roborev/tui/handlers_modal.go index 1b076826..6bc9ab09 100644 --- a/cmd/roborev/tui/handlers_modal.go +++ b/cmd/roborev/tui/handlers_modal.go @@ -172,6 +172,10 @@ func (m model) handleFilterKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { m.fetchSeq++ m.queueColGen++ m.loadingJobs = true + // activeRepoFilter may have changed; refresh the cached + // classify visibility so the next render/fetch sees the + // repo-scoped value without hitting disk on the hot path. + m.recomputeClassifyEffective() return m, m.fetchJobs() case "backspace": if len(m.filterSearch) > 0 { diff --git a/cmd/roborev/tui/handlers_msg.go b/cmd/roborev/tui/handlers_msg.go index 85feb88d..dd18d6b2 100644 --- a/cmd/roborev/tui/handlers_msg.go +++ b/cmd/roborev/tui/handlers_msg.go @@ -546,8 +546,22 @@ func (m model) handleLogOutputMsg( } if msg.err != nil { if errors.Is(msg.err, errNoLog) { + // Auto-design-router rows (running classify, terminal + // skipped, failed/canceled classify) never produce a + // streamed agent log because the classifier is a + // one-shot SchemaAgent.Decide call. Stay in the log + // view so renderLogView's classifyReasoningLines + // header can show the verdict, skip reason, and any + // classifier error — bouncing back to the queue + // would hide that information behind a flash. + job := m.logViewLookupJob() + if job != nil && len(classifyReasoningLines(job, m.width)) > 0 { + m.logLines = []logLine{} + m.logStreaming = false + return m, nil + } flash := "No log available for this job" - if job := m.logViewLookupJob(); job != nil && + if job != nil && job.Status == storage.JobStatusFailed && job.Error != "" { flash = fmt.Sprintf( diff --git a/cmd/roborev/tui/handlers_queue.go b/cmd/roborev/tui/handlers_queue.go index 83cf18c5..f75d7aa5 100644 --- a/cmd/roborev/tui/handlers_queue.go +++ b/cmd/roborev/tui/handlers_queue.go @@ -372,3 +372,20 @@ func (m model) handleHideClosedKey() (tea.Model, tea.Cmd) { m.loadingJobs = true return m, m.fetchJobs() } + +// handleToggleClassifyKey flips visibility of auto-design-router +// classifier rows and skipped design rows in the queue. The toggle +// is session-only; it overrides show_classify_jobs config until the +// TUI is restarted. +func (m model) handleToggleClassifyKey() (tea.Model, tea.Cmd) { + if m.currentView != viewQueue { + return m, nil + } + next := !m.shouldShowClassifyJobs() + m.classifyOverride = &next + m.recomputeClassifyEffective() + m.queueColGen++ + m.fetchSeq++ + m.loadingJobs = true + return m, m.fetchJobs() +} diff --git a/cmd/roborev/tui/helpers_test.go b/cmd/roborev/tui/helpers_test.go index 59dd9703..de3908f1 100644 --- a/cmd/roborev/tui/helpers_test.go +++ b/cmd/roborev/tui/helpers_test.go @@ -495,7 +495,7 @@ func TestRenderHelpTableLinesWithinWidth(t *testing.T) { helpSets := map[string][][]helpItem{ "queue": { {{"x", "cancel"}, {"r", "rerun"}, {"l", "log"}, {"p", "prompt"}, {"c", "comment"}, {"y", "copy"}, {"m", "commit"}, {"F", "fix"}}, - {{"↑/↓", "nav"}, {"enter", "review"}, {"a", "closed"}, {"f", "filter"}, {"h", "hide"}, {"T", "tasks"}, {"?", "help"}, {"q", "quit"}}, + {{"↑/↓", "nav"}, {"enter", "review"}, {"a", "closed"}, {"f", "filter"}, {"h", "hide"}, {"s", "show classify"}, {"T", "tasks"}, {"?", "help"}, {"q", "quit"}}, }, "review": { {{"p", "prompt"}, {"c", "comment"}, {"m", "commit"}, {"a", "closed"}, {"y", "copy"}, {"F", "fix"}}, diff --git a/cmd/roborev/tui/nav.go b/cmd/roborev/tui/nav.go index a14ff979..32013b67 100644 --- a/cmd/roborev/tui/nav.go +++ b/cmd/roborev/tui/nav.go @@ -144,17 +144,17 @@ func (m *model) logViewLookupJob() *storage.ReviewJob { } // logVisibleLines returns the number of content lines visible in the -// log view, accounting for title, optional command line, separator, -// status, and help bar. +// log view, accounting for title, optional command line, optional +// classify-reasoning lines, separator, status, and help bar. func (m *model) logVisibleLines() int { // title + separator + status + help(N) helpRows := m.logHelpRows() reserved := 3 + len(reflowHelpRows(helpRows, m.width)) - // Check if command line header is shown if job := m.logViewLookupJob(); job != nil { if commandLineForJob(job) != "" { reserved++ } + reserved += len(classifyReasoningLines(job, m.width)) } return max(m.height-reserved, 1) } diff --git a/cmd/roborev/tui/render_log.go b/cmd/roborev/tui/render_log.go index c9179dd7..02789022 100644 --- a/cmd/roborev/tui/render_log.go +++ b/cmd/roborev/tui/render_log.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/mattn/go-runewidth" + "github.com/roborev-dev/roborev/internal/storage" ) func (m model) renderLogView() string { @@ -42,6 +43,19 @@ func (m model) renderLogView() string { headerLines++ } + // For auto-design-router rows (classify-typed, or terminal skipped + // design reviews) the streamed agent log is empty — the classifier + // runs as a one-shot SchemaAgent.Decide call, not a streaming chat. + // Surface the classifier's verdict and reasoning here so 'l' on a + // classify/skipped row actually shows something useful. Truncated + // to m.width so each returned string occupies exactly one terminal + // row, keeping the log content area aligned with logVisibleLines. + for _, line := range classifyReasoningLines(job, m.width) { + b.WriteString(line) + b.WriteString("\x1b[K\n") + headerLines++ + } + // Calculate visible area (must match logVisibleLines()) logHelp := m.logHelpRows() logHelpLines := len(reflowHelpRows(logHelp, m.width)) @@ -105,6 +119,119 @@ func (m model) renderLogView() string { return b.String() } +// classifyReasoningLines returns pre-styled header lines describing the +// auto-design-router classifier's verdict for a classify-typed row or a +// terminal skipped design row. Returns nil for jobs that aren't part of +// the auto-design pipeline so the log view leaves their layout alone. +// +// Both classify and skipped triggers require source='auto_design'; a +// future pipeline that adopts the classify job_type or the skipped +// status for unrelated reasons should not be mislabeled as +// auto-design output. +// +// Each returned string is truncated to fit width so it occupies exactly +// one terminal row. width <= 0 disables truncation (useful for tests or +// when the caller doesn't know the terminal width yet); the styled +// output reflects the raw text in that case. +func classifyReasoningLines(job *storage.ReviewJob, width int) []string { + if job == nil || job.Source != "auto_design" { + return nil + } + isClassify := job.JobType == storage.JobTypeClassify + isAutoDesignSkipped := job.Status == storage.JobStatusSkipped + if !isClassify && !isAutoDesignSkipped { + return nil + } + + var verdict string + switch { + case isAutoDesignSkipped: + // completeClassifyAsSkip converts a classifier execution + // failure into a skipped row with job.Error populated. A + // clean "no design review needed" verdict comes from + // applyClassifyVerdict and leaves job.Error empty. Don't + // label a degraded skip as a clean verdict — operators + // reading the log header need to see that the classifier + // errored. + if job.Error != "" { + verdict = "Auto-design classifier failed (skipped)" + } else { + verdict = "Auto-design verdict: no design review needed" + } + case isClassify: + // classify rows can sit in queued/running, or end in + // failed/canceled when something went wrong before the + // classifier could MarkClassifyAsSkippedDesign or + // PromoteClassifyToDesignReview the row. Distinguish these so + // 'l' on a failed classifier doesn't claim it's still running. + switch job.Status { + case storage.JobStatusQueued, storage.JobStatusRunning: + verdict = "Auto-design classifier in progress" + case storage.JobStatusFailed: + verdict = "Auto-design classifier failed" + case storage.JobStatusCanceled: + verdict = "Auto-design classifier canceled" + default: + verdict = "Auto-design classifier: " + string(job.Status) + } + default: + verdict = "Auto-design classifier" + } + + render := func(text string) string { + // Fold embedded newlines/tabs to single spaces so each + // rendered line occupies one terminal row. job.Error stores + // raw classifier stderr/error text and routinely contains + // '\n'; without this, multiline blobs would wrap across + // terminal rows while logVisibleLines reserves only one row + // per returned line, pushing status/help off the screen. + text = foldWhitespace(text) + if width > 0 && runewidth.StringWidth(text) > width { + text = runewidth.Truncate(text, width, "…") + } + return statusStyle.Render(text) + } + + var lines []string + lines = append(lines, render(verdict)) + if job.SkipReason != "" { + lines = append(lines, render("Reason: "+job.SkipReason)) + } + if job.Error != "" { + lines = append(lines, render("Detail: "+job.Error)) + } + return lines +} + +// foldWhitespace replaces embedded newlines, carriage returns, and +// tabs with a single space and collapses runs of resulting whitespace. +// Used to keep one-row-per-line invariants when rendering raw error or +// reason text into the log view header. +func foldWhitespace(s string) string { + if !strings.ContainsAny(s, "\n\r\t") { + return s + } + var b strings.Builder + b.Grow(len(s)) + prevSpace := false + for _, r := range s { + switch r { + case '\n', '\r', '\t': + r = ' ' + } + if r == ' ' { + if prevSpace { + continue + } + prevSpace = true + } else { + prevSpace = false + } + b.WriteRune(r) + } + return b.String() +} + func formatHelpLine(key, desc string) string { return fmt.Sprintf(" %-14s %s", key, desc) } @@ -147,6 +274,7 @@ func helpLines(tasksEnabled, noQuit bool) []string { keys: []struct{ key, desc string }{ {"f", "Filter by repository/branch"}, {"h", "Toggle hide closed/failed"}, + {"s", "Toggle classify rows (auto-design router)"}, {"esc", "Clear filters (one at a time)"}, }, }, diff --git a/cmd/roborev/tui/render_log_classify_test.go b/cmd/roborev/tui/render_log_classify_test.go new file mode 100644 index 00000000..f3f8d2bf --- /dev/null +++ b/cmd/roborev/tui/render_log_classify_test.go @@ -0,0 +1,230 @@ +package tui + +import ( + "strings" + "testing" + + "github.com/mattn/go-runewidth" + "github.com/muesli/ansi" + "github.com/roborev-dev/roborev/internal/storage" + "github.com/stretchr/testify/assert" +) + +func TestClassifyReasoningLines(t *testing.T) { + tests := []struct { + name string + job *storage.ReviewJob + want []string + notWant []string + wantEmpty bool + }{ + { + name: "nil job", + job: nil, + wantEmpty: true, + }, + { + name: "plain review job", + job: &storage.ReviewJob{ + ID: 1, + JobType: storage.JobTypeReview, + Status: storage.JobStatusDone, + }, + wantEmpty: true, + }, + { + name: "running classify", + job: &storage.ReviewJob{ + ID: 2, + JobType: storage.JobTypeClassify, + Status: storage.JobStatusRunning, + Source: "auto_design", + }, + want: []string{"in progress"}, + }, + { + name: "queued classify", + job: &storage.ReviewJob{ + ID: 20, + JobType: storage.JobTypeClassify, + Status: storage.JobStatusQueued, + Source: "auto_design", + }, + want: []string{"in progress"}, + }, + { + // Terminal classify failures must not claim "in progress"; + // the operator pressing 'l' needs the actual status. + name: "failed classify", + job: &storage.ReviewJob{ + ID: 21, + JobType: storage.JobTypeClassify, + Status: storage.JobStatusFailed, + Source: "auto_design", + Error: "exec: classifier: timeout after 30s", + }, + want: []string{"failed", "timeout"}, + notWant: []string{ + "in progress", + "no design review needed", + }, + }, + { + name: "canceled classify", + job: &storage.ReviewJob{ + ID: 22, + JobType: storage.JobTypeClassify, + Status: storage.JobStatusCanceled, + Source: "auto_design", + }, + want: []string{"canceled"}, + notWant: []string{"in progress"}, + }, + { + // Clean classifier verdict (applyClassifyVerdict path): + // MarkClassifyAsSkippedDesign with empty errorDetail. + name: "skipped auto_design with reason", + job: &storage.ReviewJob{ + ID: 3, + JobType: storage.JobTypeReview, + Status: storage.JobStatusSkipped, + Source: "auto_design", + SkipReason: "trivial diff", + }, + want: []string{"no design review needed", "trivial diff"}, + notWant: []string{"failed"}, + }, + { + // Daemon path: classifier execution fails → + // completeClassifyAsSkip → MarkClassifyAsSkippedDesign + // produces a row with status=skipped, source=auto_design, + // AND a non-empty Error. Must NOT be labeled as a clean + // "no design review needed" verdict. + name: "skipped auto_design with classifier error", + job: &storage.ReviewJob{ + ID: 4, + JobType: storage.JobTypeReview, + Status: storage.JobStatusSkipped, + Source: "auto_design", + SkipReason: "classifier unavailable", + Error: "classify_agent \"codex\" not registered", + }, + want: []string{ + "classifier failed", + "classifier unavailable", + "not registered", + }, + notWant: []string{ + "no design review needed", + }, + }, + { + name: "skipped non-auto_design row leaves log alone", + job: &storage.ReviewJob{ + ID: 5, + JobType: storage.JobTypeReview, + Status: storage.JobStatusSkipped, + Source: "", + SkipReason: "unrelated reason", + }, + wantEmpty: true, + }, + { + // Future pipeline could use classify job_type for unrelated + // routing; mislabeling those as auto-design output would be + // wrong. + name: "non-auto_design classify row leaves log alone", + job: &storage.ReviewJob{ + ID: 6, + JobType: storage.JobTypeClassify, + Status: storage.JobStatusRunning, + Source: "", + }, + wantEmpty: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Width 0 = no truncation; tests use raw content + // substrings. + got := classifyReasoningLines(tt.job, 0) + if tt.wantEmpty { + assert.Empty(t, got, + "expected no header lines for non-classify job") + return + } + joined := strings.Join(got, "\n") + for _, want := range tt.want { + assert.Contains(t, joined, want, + "reasoning header should mention %q", want) + } + for _, notWant := range tt.notWant { + assert.NotContains(t, joined, notWant, + "reasoning header should NOT mention %q", notWant) + } + }) + } +} + +// TestClassifyReasoningLinesFoldsNewlines locks in that embedded +// newlines/tabs in raw classifier error text are folded to spaces so +// each returned line stays on a single terminal row. job.Error is +// stored raw (sanitization only applies to skip_reason at storage +// entry), and a multiline blob would break the row-reservation +// invariant in logVisibleLines. +func TestClassifyReasoningLinesFoldsNewlines(t *testing.T) { + job := &storage.ReviewJob{ + ID: 7, + JobType: storage.JobTypeReview, + Status: storage.JobStatusSkipped, + Source: "auto_design", + SkipReason: "line one\nline two", + Error: "exec: classifier\n\tstderr=timeout\n\tcode=124", + } + got := classifyReasoningLines(job, 0) + assert.Len(t, got, 3) + for i, line := range got { + assert.NotContains(t, line, "\n", + "line %d contains a newline: %q", i, line) + assert.NotContains(t, line, "\r", + "line %d contains a carriage return: %q", i, line) + assert.NotContains(t, line, "\t", + "line %d contains a tab: %q", i, line) + } +} + +// TestClassifyReasoningLinesTruncation locks in the contract that +// renderLogView and logVisibleLines both rely on: each returned line +// occupies exactly one terminal row. If the lines could wrap, the +// reserved-space calculation would be wrong and the log content area +// would misalign. +func TestClassifyReasoningLinesTruncation(t *testing.T) { + longReason := strings.Repeat("very long classifier reason ", 8) + longError := strings.Repeat("internal error blob ", 12) + job := &storage.ReviewJob{ + ID: 6, + JobType: storage.JobTypeReview, + Status: storage.JobStatusSkipped, + Source: "auto_design", + SkipReason: longReason, + Error: longError, + } + const width = 80 + + got := classifyReasoningLines(job, width) + assert.Len(t, got, 3, "expected verdict + reason + detail rows") + for i, line := range got { + // Strip ANSI styling to measure printable width. + stripped := ansi.PrintableRuneWidth(line) + // runewidth.StringWidth on the rendered line includes ANSI + // codes so use ansi.PrintableRuneWidth for the assertion; + // fall back to a runewidth check on a stripped copy if + // PrintableRuneWidth returns 0 (older versions). + if stripped == 0 { + stripped = runewidth.StringWidth(line) + } + assert.LessOrEqual(t, stripped, width, + "line %d (%q) exceeds width %d", i, line, width) + } +} diff --git a/cmd/roborev/tui/render_queue.go b/cmd/roborev/tui/render_queue.go index a2ff6821..26896e9e 100644 --- a/cmd/roborev/tui/render_queue.go +++ b/cmd/roborev/tui/render_queue.go @@ -43,7 +43,13 @@ func (m model) queueHelpRows() [][]helpItem { if !m.lockedRepoFilter || !m.lockedBranchFilter { row2 = append(row2, helpItem{"f", "filter"}) } - row2 = append(row2, helpItem{"h", "hide"}, helpItem{"D", "focus"}) + row2 = append(row2, helpItem{"h", "hide"}) + if m.shouldShowClassifyJobs() { + row2 = append(row2, helpItem{"s", "hide classify"}) + } else { + row2 = append(row2, helpItem{"s", "show classify"}) + } + row2 = append(row2, helpItem{"D", "focus"}) if m.tasksWorkflowEnabled() { row2 = append(row2, helpItem{"T", "tasks"}) } diff --git a/cmd/roborev/tui/tui.go b/cmd/roborev/tui/tui.go index 156b3ca9..39b74ede 100644 --- a/cmd/roborev/tui/tui.go +++ b/cmd/roborev/tui/tui.go @@ -316,6 +316,20 @@ type model struct { activeBranchFilter string // Empty = show all, otherwise branch name to filter by filterStack []string // Order of applied filters: "repo", "branch" - for escape to pop in order hideClosed bool // When true, hide jobs with closed reviews + // globalCfg is cached at startup and consulted at fetch time so that + // show_classify_jobs can be resolved against whichever repo is the + // currently active single-repo filter (rather than baked in once + // from the cwd at launch). + globalCfg *config.Config + // classifyOverride is a session-level toggle that overrides the + // config-derived show_classify_jobs value. nil = follow config; + // non-nil = use the override regardless of config. + classifyOverride *bool + // classifyEffective caches the resolved show-classify-jobs value + // so render and fetch paths don't hit disk via LoadRepoConfig on + // every paint or event. Recomputed by recomputeClassifyEffective + // whenever classifyOverride or activeRepoFilter changes. + classifyEffective bool // Display name cache (keyed by repo path) displayNames map[string]string @@ -482,6 +496,7 @@ func newModel(ep daemon.DaemonEndpoint, opts ...option) model { colOrder := parseColumnOrder(nil) taskColOrder := parseTaskColumnOrder(nil) var cwdRepoRoot, cwdRepoIdentity, cwdBranch string + var globalCfg *config.Config if !opt.disableExternalIO { // Read daemon version from runtime file @@ -491,6 +506,7 @@ func newModel(ep daemon.DaemonEndpoint, opts ...option) model { // Load preferences from config if cfg, err := config.LoadGlobal(); err == nil { + globalCfg = cfg hideClosed = cfg.HideClosedByDefault autoFilterRepo = cfg.AutoFilterRepo autoFilterBranch = cfg.AutoFilterBranch @@ -583,7 +599,7 @@ func newModel(ep daemon.DaemonEndpoint, opts ...option) model { } httpClient := ep.HTTPClient(10 * time.Second) - return model{ + m := model{ endpoint: ep, daemonVersion: daemonVersion, client: httpClient, @@ -596,6 +612,7 @@ func newModel(ep daemon.DaemonEndpoint, opts ...option) model { loadingJobs: true, // Init() calls fetchJobs, so mark as loading loadingStatus: true, // Init() calls fetchStatus, so mark as loading hideClosed: hideClosed, + globalCfg: globalCfg, activeRepoFilter: activeRepoFilter, autoRepoFilter: autoRepoFilterActive, activeBranchFilter: activeBranchFilter, @@ -624,6 +641,10 @@ func newModel(ep daemon.DaemonEndpoint, opts ...option) model { queueColCache: &colWidthCache{gen: -1}, taskColCache: &colWidthCache{gen: -1}, } + // Seed the cached classify-visibility decision once so render and + // fetch can read m.classifyEffective without hitting disk. + m.recomputeClassifyEffective() + return m } func (m model) Init() tea.Cmd { @@ -650,6 +671,42 @@ func (m model) tasksDisabledMessage() string { return "Tasks workflow disabled. Set advanced.tasks_enabled=true in global config to enable it." } +// shouldShowClassifyJobs returns the cached effective value computed +// by recomputeClassifyEffective. Hot path: callable from render and +// fetch without filesystem I/O. +func (m model) shouldShowClassifyJobs() bool { + return m.classifyEffective +} + +// recomputeClassifyEffective resolves whether the queue should include +// auto-design-router classifier rows and skipped design rows, and +// caches the result in m.classifyEffective. +// +// Precedence: a session-level toggle (the 's' key) takes priority over +// config. When unset, falls back to the per-repo override if the queue +// is filtered to a single repo, otherwise the global setting. This +// avoids the surprise of one repo's override leaking into a multi-repo +// view while still letting the user flip the toggle on the fly. +// +// Call this from any handler that mutates classifyOverride or +// activeRepoFilter. It only loads repo config from disk under the +// single-repo-filter branch, so amortizing it to filter/toggle changes +// keeps the render and fetch paths I/O-free. +func (m *model) recomputeClassifyEffective() { + switch { + case m.classifyOverride != nil: + m.classifyEffective = *m.classifyOverride + case len(m.activeRepoFilter) == 1: + m.classifyEffective = config.ResolveShowClassifyJobs( + m.activeRepoFilter[0], m.globalCfg, + ) + case m.globalCfg != nil: + m.classifyEffective = m.globalCfg.ShowClassifyJobs + default: + m.classifyEffective = false + } +} + // getDisplayName returns the display name for a repo, using the cache. // Falls back to loading from config on cache miss, then to the provided default name. func (m *model) getDisplayName(repoPath, defaultName string) string { diff --git a/internal/config/config.go b/internal/config/config.go index 875c4977..b4defdb8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -197,6 +197,7 @@ type Config struct { HideAddressedByDefault bool `toml:"hide_addressed_by_default"` // deprecated: use hide_closed_by_default AutoFilterRepo bool `toml:"auto_filter_repo" comment:"Automatically filter the TUI queue to the current repo."` AutoFilterBranch bool `toml:"auto_filter_branch" comment:"Automatically filter the TUI queue to the current branch."` + ShowClassifyJobs bool `toml:"show_classify_jobs" comment:"Show auto-design-review classifier rows (and skipped design rows) in the TUI queue. Off by default to reduce noise."` MouseEnabled bool `toml:"mouse_enabled" comment:"Enable mouse support in the TUI."` // Enable mouse capture and mouse-driven TUI interactions TabWidth int `toml:"tab_width"` // Tab expansion width for TUI rendering (default: 2) HiddenColumns []string `toml:"hidden_columns" comment:"Queue columns to hide in the TUI."` // Column names to hide in queue table (e.g. ["branch", "agent"]) @@ -1070,6 +1071,7 @@ type RepoConfig struct { // Behavior AutoClosePassingReviews *bool `toml:"auto_close_passing_reviews" comment:"Automatically close reviews that pass with no findings in this repo."` + ShowClassifyJobs *bool `toml:"show_classify_jobs" comment:"Override whether the TUI queue shows auto-design-review classifier rows for this repo. Omit to inherit."` // Hooks configuration (per-repo) Hooks []HookConfig `toml:"hooks"` @@ -1542,6 +1544,23 @@ func ResolveAutoClosePassingReviews(repoPath string, globalCfg *Config) bool { return resolveBool(globalVal, repoVal) } +// ResolveShowClassifyJobs returns whether the TUI queue should display +// auto-design-review classifier rows (job_type=classify) and skipped +// design rows (status=skipped). Off by default to reduce queue noise +// when the auto-design router is enabled. Per-repo config overrides +// global. +func ResolveShowClassifyJobs(repoPath string, globalCfg *Config) bool { + var repoVal *bool + if repoCfg, err := LoadRepoConfig(repoPath); err == nil && repoCfg != nil { + repoVal = repoCfg.ShowClassifyJobs + } + var globalVal bool + if globalCfg != nil { + globalVal = globalCfg.ShowClassifyJobs + } + return resolveBool(globalVal, repoVal) +} + // ResolveExcludePatterns returns the merged exclude patterns from // repo config and global config. Repo patterns are read from the // default branch (like review guidelines) to prevent untrusted diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 69460a73..6f84c09b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -387,6 +387,45 @@ func TestResolveAutoClosePassingReviews(t *testing.T) { } } +func TestResolveShowClassifyJobs(t *testing.T) { + tests := []struct { + name string + repoConfig string + globalConfig *Config + want bool + }{ + { + name: "default false", + want: false, + }, + { + name: "global enabled", + globalConfig: &Config{ShowClassifyJobs: true}, + want: true, + }, + { + name: "repo overrides global to true", + repoConfig: `show_classify_jobs = true`, + globalConfig: &Config{ShowClassifyJobs: false}, + want: true, + }, + { + name: "repo overrides global to false", + repoConfig: `show_classify_jobs = false`, + globalConfig: &Config{ShowClassifyJobs: true}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := newTempRepo(t, tt.repoConfig) + got := ResolveShowClassifyJobs(tmpDir, tt.globalConfig) + assert.Equal(t, tt.want, got) + }) + } +} + func TestResolveReasoning(t *testing.T) { type resolverFunc func(explicit string, dir string, globalCfg *Config) (string, error) diff --git a/internal/daemon/server.go b/internal/daemon/server.go index ad4196af..2657a8b2 100644 --- a/internal/daemon/server.go +++ b/internal/daemon/server.go @@ -889,6 +889,9 @@ func (s *Server) humaListJobs( storage.WithExcludeJobType(input.ExcludeJobType), ) } + if input.HideClassifyJobs == "true" { + listOpts = append(listOpts, storage.WithHideClassifyJobs()) + } if repoPrefix != "" && repo == "" { listOpts = append( listOpts, storage.WithRepoPrefix(repoPrefix), diff --git a/internal/daemon/server_jobs_test.go b/internal/daemon/server_jobs_test.go index 92c61b09..c1ab746f 100644 --- a/internal/daemon/server_jobs_test.go +++ b/internal/daemon/server_jobs_test.go @@ -2367,6 +2367,52 @@ func TestHandleListJobsJobTypeFilter(t *testing.T) { }, "Expected non-fix job, got fix") } }) + + t.Run("hide_classify_jobs=true hides classify and skipped rows", func(t *testing.T) { + // Add a running classify row and a skipped design row alongside + // the existing review + fix jobs. The (repo_id, commit_id, + // review_type='design') partial unique index on auto_design rows + // means each auto_design row needs a distinct commit. + classifyCommit, err := db.GetOrCreateCommit(repo.ID, "jt-classify", "Author", "Subject", time.Now()) + require.NoError(t, err) + _, err = db.Exec(` + INSERT INTO review_jobs + (repo_id, commit_id, git_ref, agent, status, job_type, review_type, source, worker_id, started_at, enqueued_at, updated_at) + VALUES (?, ?, 'jt-classify', 'test', 'running', 'classify', 'design', 'auto_design', 'w1', datetime('now'), datetime('now'), datetime('now')) + `, repo.ID, classifyCommit.ID) + require.NoError(t, err) + + skippedCommit, err := db.GetOrCreateCommit(repo.ID, "jt-skipped", "Author", "Subject", time.Now()) + require.NoError(t, err) + _, err = db.Exec(` + INSERT INTO review_jobs + (repo_id, commit_id, git_ref, agent, status, job_type, review_type, source, skip_reason, enqueued_at, finished_at, updated_at) + VALUES (?, ?, 'jt-skipped', 'test', 'skipped', 'review', 'design', 'auto_design', 'trivial diff', datetime('now'), datetime('now'), datetime('now')) + `, repo.ID, skippedCommit.ID) + require.NoError(t, err) + + req := httptest.NewRequest( + http.MethodGet, "/api/jobs?hide_classify_jobs=true", nil, + ) + w := httptest.NewRecorder() + server.httpServer.Handler.ServeHTTP(w, req) + testutil.AssertStatusCode(t, w, http.StatusOK) + + var resp struct { + Jobs []storage.ReviewJob `json:"jobs"` + } + testutil.DecodeJSON(t, w, &resp) + + // Should see the original review + fix (2 jobs), not the classify + // or skipped rows. + assert.Len(t, resp.Jobs, 2, "expected only review and fix jobs") + for _, j := range resp.Jobs { + assert.NotEqual(t, storage.JobTypeClassify, j.JobType, + "classify rows should be hidden") + assert.NotEqual(t, storage.JobStatusSkipped, j.Status, + "skipped rows should be hidden") + } + }) } func TestHandleListJobsRepoPrefixFilter(t *testing.T) { diff --git a/internal/daemon/types.go b/internal/daemon/types.go index 857334cf..31642a74 100644 --- a/internal/daemon/types.go +++ b/internal/daemon/types.go @@ -65,6 +65,7 @@ type ListJobsInput struct { Closed string `query:"closed" doc:"Filter by review closed state" enum:"true,false,"` JobType string `query:"job_type" doc:"Filter by job type"` ExcludeJobType string `query:"exclude_job_type" doc:"Exclude jobs of this type"` + HideClassifyJobs string `query:"hide_classify_jobs" doc:"Hide auto-design-router rows (job_type=classify and status=skipped)" enum:"true,false,"` RepoPrefix string `query:"repo_prefix" doc:"Filter repos by path prefix"` Limit int `query:"limit" default:"-999999" doc:"Max results (default 50, 0=unlimited, max 10000)"` Offset int `query:"offset" default:"-1" doc:"Skip N results (requires limit>0)"` diff --git a/internal/daemon_client/client.gen.go b/internal/daemon_client/client.gen.go index ddb91966..523a58b4 100644 --- a/internal/daemon_client/client.gen.go +++ b/internal/daemon_client/client.gen.go @@ -59,18 +59,39 @@ func (e ListJobsParamsClosed) Valid() bool { } } +// Defines values for ListJobsParamsHideClassifyJobs. +const ( + ListJobsParamsHideClassifyJobsEmpty ListJobsParamsHideClassifyJobs = "" + ListJobsParamsHideClassifyJobsFalse ListJobsParamsHideClassifyJobs = "false" + ListJobsParamsHideClassifyJobsTrue ListJobsParamsHideClassifyJobs = "true" +) + +// Valid indicates whether the value is a known member of the ListJobsParamsHideClassifyJobs enum. +func (e ListJobsParamsHideClassifyJobs) Valid() bool { + switch e { + case ListJobsParamsHideClassifyJobsEmpty: + return true + case ListJobsParamsHideClassifyJobsFalse: + return true + case ListJobsParamsHideClassifyJobsTrue: + return true + default: + return false + } +} + // Defines values for GetSummaryParamsAll. const ( - False GetSummaryParamsAll = "false" - True GetSummaryParamsAll = "true" + GetSummaryParamsAllFalse GetSummaryParamsAll = "false" + GetSummaryParamsAllTrue GetSummaryParamsAll = "true" ) // Valid indicates whether the value is a known member of the GetSummaryParamsAll enum. func (e GetSummaryParamsAll) Valid() bool { switch e { - case False: + case GetSummaryParamsAllFalse: return true - case True: + case GetSummaryParamsAllTrue: return true default: return false @@ -465,9 +486,10 @@ type RepoSummary struct { // RepoWithCount defines model for RepoWithCount. type RepoWithCount struct { - Count int64 `json:"count"` - Name string `json:"name"` - RootPath string `json:"root_path"` + Count int64 `json:"count"` + Identity *string `json:"identity,omitempty"` + Name string `json:"name"` + RootPath string `json:"root_path"` } // RerunJobOutputBody defines model for RerunJobOutputBody. @@ -696,6 +718,9 @@ type ListJobsParams struct { // ExcludeJobType Exclude jobs of this type ExcludeJobType *string `form:"exclude_job_type,omitempty" json:"exclude_job_type,omitempty"` + // HideClassifyJobs Hide auto-design-router rows (job_type=classify and status=skipped) + HideClassifyJobs *ListJobsParamsHideClassifyJobs `form:"hide_classify_jobs,omitempty" json:"hide_classify_jobs,omitempty"` + // RepoPrefix Filter repos by path prefix RepoPrefix *string `form:"repo_prefix,omitempty" json:"repo_prefix,omitempty"` @@ -715,6 +740,9 @@ type ListJobsParamsBranchIncludeEmpty string // ListJobsParamsClosed defines parameters for ListJobs. type ListJobsParamsClosed string +// ListJobsParamsHideClassifyJobs defines parameters for ListJobs. +type ListJobsParamsHideClassifyJobs string + // ListReposParams defines parameters for ListRepos. type ListReposParams struct { // Branch Filter to repos with jobs on this branch @@ -2332,6 +2360,22 @@ func NewListJobsRequest(server string, params *ListJobsParams) (*http.Request, e } + if params.HideClassifyJobs != nil { + + if queryFrag, err := runtime.StyleParamWithOptions("form", false, "hide_classify_jobs", *params.HideClassifyJobs, runtime.StyleParamOptions{ParamLocation: runtime.ParamLocationQuery, Type: "string", Format: ""}); err != nil { + return nil, err + } else if parsed, err := url.ParseQuery(queryFrag); err != nil { + return nil, err + } else { + for k, v := range parsed { + for _, v2 := range v { + queryValues.Add(k, v2) + } + } + } + + } + if params.RepoPrefix != nil { if queryFrag, err := runtime.StyleParamWithOptions("form", false, "repo_prefix", *params.RepoPrefix, runtime.StyleParamOptions{ParamLocation: runtime.ParamLocationQuery, Type: "string", Format: ""}); err != nil { diff --git a/internal/daemon_client/openapi-3.0.json b/internal/daemon_client/openapi-3.0.json index b4553153..40af8f45 100644 --- a/internal/daemon_client/openapi-3.0.json +++ b/internal/daemon_client/openapi-3.0.json @@ -1221,6 +1221,9 @@ "format": "int64", "type": "integer" }, + "identity": { + "type": "string" + }, "name": { "type": "string" }, @@ -2569,6 +2572,21 @@ "type": "string" } }, + { + "description": "Hide auto-design-router rows (job_type=classify and status=skipped)", + "explode": false, + "in": "query", + "name": "hide_classify_jobs", + "schema": { + "description": "Hide auto-design-router rows (job_type=classify and status=skipped)", + "enum": [ + "true", + "false", + "" + ], + "type": "string" + } + }, { "description": "Filter repos by path prefix", "explode": false, diff --git a/internal/storage/db_filter_test.go b/internal/storage/db_filter_test.go index 035eb6cb..e1f82bb4 100644 --- a/internal/storage/db_filter_test.go +++ b/internal/storage/db_filter_test.go @@ -741,6 +741,113 @@ func TestListJobsWithJobTypeFilter(t *testing.T) { } } +func TestListJobsWithHideClassifyJobs(t *testing.T) { + db := openTestDB(t) + defer db.Close() + + // Plain queued review — should always be visible. + repo, _, _ := createJobChain(t, db, "/tmp/repo-hide-classify", "review-sha") + + // Running classify row (auto-design router decision in flight). The + // (repo_id, commit_id, review_type='design') partial unique index on + // source='auto_design' rows means each auto_design row needs a + // distinct commit. + classifyCommit := createCommit(t, db, repo.ID, "classify-sha") + var classifyID int64 + require.NoError(t, db.QueryRow(` + INSERT INTO review_jobs + (repo_id, commit_id, git_ref, status, job_type, review_type, source, worker_id, started_at, enqueued_at, updated_at) + VALUES (?, ?, 'classify-sha', 'running', 'classify', 'design', 'auto_design', 'w1', datetime('now'), datetime('now'), datetime('now')) + RETURNING id + `, repo.ID, classifyCommit.ID).Scan(&classifyID)) + + // Skipped design row (classifier or heuristic decided no design + // review needed). + skippedCommit := createCommit(t, db, repo.ID, "skipped-sha") + var skippedID int64 + require.NoError(t, db.QueryRow(` + INSERT INTO review_jobs + (repo_id, commit_id, git_ref, status, job_type, review_type, source, skip_reason, enqueued_at, finished_at, updated_at) + VALUES (?, ?, 'skipped-sha', 'skipped', 'review', 'design', 'auto_design', 'trivial diff', datetime('now'), datetime('now'), datetime('now')) + RETURNING id + `, repo.ID, skippedCommit.ID).Scan(&skippedID)) + + // Skipped row that did NOT come from the auto-design router (source + // set to empty string). A future pipeline could adopt + // status='skipped' for some other reason; this filter must leave + // such rows alone. + otherSkippedCommit := createCommit(t, db, repo.ID, "other-skipped-sha") + var otherSkippedID int64 + require.NoError(t, db.QueryRow(` + INSERT INTO review_jobs + (repo_id, commit_id, git_ref, status, job_type, review_type, source, skip_reason, enqueued_at, finished_at, updated_at) + VALUES (?, ?, 'other-skipped-sha', 'skipped', 'review', '', '', 'unrelated reason', datetime('now'), datetime('now'), datetime('now')) + RETURNING id + `, repo.ID, otherSkippedCommit.ID).Scan(&otherSkippedID)) + + // Skipped row with source IS NULL (the column is nullable, and + // EnqueueJob never sets it for plain reviews). This case is + // load-bearing: a NULL-naive predicate would silently drop these + // rows from results. + nullSkippedCommit := createCommit(t, db, repo.ID, "null-skipped-sha") + var nullSkippedID int64 + require.NoError(t, db.QueryRow(` + INSERT INTO review_jobs + (repo_id, commit_id, git_ref, status, job_type, review_type, skip_reason, enqueued_at, finished_at, updated_at) + VALUES (?, ?, 'null-skipped-sha', 'skipped', 'review', '', 'unrelated reason', datetime('now'), datetime('now'), datetime('now')) + RETURNING id + `, repo.ID, nullSkippedCommit.ID).Scan(&nullSkippedID)) + + // Classify-typed row that did NOT come from the auto-design router + // (source=''). A future pipeline could use the classify job_type + // for unrelated routing; the filter must leave such rows alone. + otherClassifyCommit := createCommit(t, db, repo.ID, "other-classify-sha") + var otherClassifyID int64 + require.NoError(t, db.QueryRow(` + INSERT INTO review_jobs + (repo_id, commit_id, git_ref, status, job_type, review_type, source, worker_id, started_at, enqueued_at, updated_at) + VALUES (?, ?, 'other-classify-sha', 'running', 'classify', 'review', '', 'w2', datetime('now'), datetime('now'), datetime('now')) + RETURNING id + `, repo.ID, otherClassifyCommit.ID).Scan(&otherClassifyID)) + + t.Run("default returns all rows", func(t *testing.T) { + jobs, err := db.ListJobs("", "", 50, 0) + require.NoError(t, err) + assert.Len(t, jobs, 6, + "default should include both classify variants, all three skipped variants, and the plain review row") + }) + + t.Run("hide_classify_jobs only hides auto_design rows", func(t *testing.T) { + jobs, err := db.ListJobs("", "", 50, 0, WithHideClassifyJobs()) + require.NoError(t, err) + // Should keep: plain review, source='' skipped, source IS NULL + // skipped, non-auto_design classify. Should hide: classify row + // with source='auto_design', skipped row with source='auto_design'. + assert.Len(t, jobs, 4, "should hide only auto-design router rows") + var sawOtherSkipped, sawNullSkipped, sawOtherClassify bool + for _, j := range jobs { + assert.NotEqual(t, classifyID, j.ID, + "auto_design classify row should be hidden") + assert.NotEqual(t, skippedID, j.ID, + "auto_design skipped row should be hidden") + switch j.ID { + case otherSkippedID: + sawOtherSkipped = true + case nullSkippedID: + sawNullSkipped = true + case otherClassifyID: + sawOtherClassify = true + } + } + assert.True(t, sawOtherSkipped, + "skipped rows with source='' must remain visible") + assert.True(t, sawNullSkipped, + "skipped rows with source IS NULL must remain visible") + assert.True(t, sawOtherClassify, + "classify rows with source != 'auto_design' must remain visible") + }) +} + func TestEscapeLike(t *testing.T) { tests := []struct { input string diff --git a/internal/storage/jobs.go b/internal/storage/jobs.go index 758b9926..e89685a0 100644 --- a/internal/storage/jobs.go +++ b/internal/storage/jobs.go @@ -686,6 +686,7 @@ type listJobsOptions struct { closed *bool jobType string excludeJobType string + hideClassifyJobs bool repoPrefix string beforeCursor *int64 } @@ -724,6 +725,16 @@ func WithExcludeJobType(jobType string) ListJobsOption { return func(o *listJobsOptions) { o.excludeJobType = jobType } } +// WithHideClassifyJobs excludes auto-design-router byproducts: rows +// with job_type='classify' (the routing decision itself) and rows +// with status='skipped' (the terminal state for skipped design +// reviews). Both clauses are scoped to source='auto_design' so a +// future pipeline that creates classify jobs or skipped jobs for a +// different reason is not silently swallowed by this filter. +func WithHideClassifyJobs() ListJobsOption { + return func(o *listJobsOptions) { o.hideClassifyJobs = true } +} + // WithBeforeCursor filters jobs to those with ID < cursor (for cursor pagination). func WithBeforeCursor(id int64) ListJobsOption { return func(o *listJobsOptions) { o.beforeCursor = &id } @@ -799,6 +810,20 @@ func buildJobFilterClause(statusFilter, repoFilter string, o listJobsOptions) (s conditions = append(conditions, "j.job_type != ?") args = append(args, o.excludeJobType) } + if o.hideClassifyJobs { + // source is nullable, so guard the comparison with COALESCE. + // Without it, j.source = 'auto_design' returns NULL for rows + // where source IS NULL, NOT (TRUE AND NULL) is also NULL, and + // the row gets silently dropped from results. + // + // Both predicates are scoped to source='auto_design' so a + // future pipeline that adopts classify jobs or status='skipped' + // for an unrelated reason isn't silently hidden. + conditions = append(conditions, + "NOT (COALESCE(j.source, '') = 'auto_design' AND (j.job_type = ? OR j.status = ?))") + args = append(args, + JobTypeClassify, string(JobStatusSkipped)) + } if o.beforeCursor != nil { conditions = append(conditions, "j.id < ?") args = append(args, *o.beforeCursor) @@ -818,7 +843,8 @@ func (db *DB) ListJobs(statusFilter string, repoFilter string, limit, offset int COALESCE(j.agentic, 0), r.root_path, r.name, c.subject, rv.closed, rv.output, rv.verdict_bool, j.source_machine_id, j.uuid, j.model, j.job_type, j.review_type, j.patch_id, j.parent_job_id, j.provider, j.requested_model, j.requested_provider, j.token_usage, COALESCE(j.worktree_path, ''), - j.command_line, COALESCE(j.min_severity, '') + j.command_line, COALESCE(j.min_severity, ''), + j.skip_reason, j.source FROM review_jobs j JOIN repos r ON r.id = j.repo_id LEFT JOIN commits c ON c.id = j.commit_id @@ -857,7 +883,8 @@ func (db *DB) ListJobs(statusFilter string, repoFilter string, limit, offset int &fields.Agentic, &j.RepoPath, &j.RepoName, &fields.CommitSubject, &fields.Closed, &output, &verdictBool, &fields.SourceMachineID, &fields.UUID, &fields.Model, &fields.JobType, &fields.ReviewType, &fields.PatchID, &fields.ParentJobID, &fields.Provider, &fields.RequestedModel, &fields.RequestedProvider, &fields.TokenUsage, &fields.WorktreePath, - &fields.CommandLine, &fields.MinSeverity) + &fields.CommandLine, &fields.MinSeverity, + &fields.SkipReason, &fields.Source) if err != nil { return nil, err }