Skip to content

Commit d2d671f

Browse files
mariusvniekerkcodexwesm
authored
fix: preserve requested model provenance on rerun (#581)
## Summary - add `requested_model` / `requested_provider` alongside effective `model` / `provider` - recompute effective model/provider on rerun from requested intent plus current config/defaults - expose requested model/provider as optional hidden queue columns in the TUI - extend SQLite, PostgreSQL, and sync paths for the new provenance fields ## Why Issue #556 came from reruns replaying stale effective OpenCode model values. The old schema only stored the effective model/provider, so rerun could not tell whether a value was explicitly requested or merely resolved from config/defaults. ## Ambiguous migration choice Existing rows are ambiguous: the historical `model` / `provider` values do not tell us whether they were explicitly requested or implicitly resolved. I considered storing a per-row creation/model-schema version marker so rerun could special-case older rows, but that adds more schema and branching complexity than the problem warrants. This change instead chooses the simpler behavior: - migrate existing rows with `requested_model = NULL` and `requested_provider = NULL` - do **not** backfill requested fields from historical effective values - treat `NULL` requested fields as "no explicit request recorded; reevaluate" That means pre-migration rows will rerun against current defaults/config rather than freezing an old ambiguous effective value. ## Validation - `go test ./internal/storage ./internal/daemon ./cmd/roborev/tui` - `go vet ./...` ## Exact scenario checked I also re-ran the exact configuration discussed in #556: - `default_agent = "opencode"` - `default_model = "opencode/nemotron-3-super-free"` Before this change, that config stored the resolved effective model in `review_jobs.model`, which rerun then replayed as if it were original intent. After this change, explicit requested intent is tracked separately, and config-derived runs leave requested fields empty so rerun can reevaluate. --------- Co-authored-by: OpenAI Codex <noreply@openai.com> Co-authored-by: Wes McKinney <wesmckinn+git@gmail.com>
1 parent ea575f3 commit d2d671f

File tree

19 files changed

+747
-324
lines changed

19 files changed

+747
-324
lines changed

cmd/roborev/tui/queue_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2200,12 +2200,22 @@ func TestParseColumnOrderAppendsMissing(t *testing.T) {
22002200
assert.True(t, slices.Equal(got[:len(wantPrefix)], wantPrefix))
22012201

22022202
pfCount := 0
2203+
requestedModelCount := 0
2204+
requestedProviderCount := 0
22032205
for _, c := range got {
22042206
if c == colPF {
22052207
pfCount++
22062208
}
2209+
if c == colRequestedModel {
2210+
requestedModelCount++
2211+
}
2212+
if c == colRequestedProvider {
2213+
requestedProviderCount++
2214+
}
22072215
}
22082216
assert.Equal(t, 1, pfCount)
2217+
assert.Equal(t, 1, requestedModelCount)
2218+
assert.Equal(t, 1, requestedProviderCount)
22092219
}
22102220

22112221
func TestDefaultColumnOrderDetection(t *testing.T) {
@@ -2221,3 +2231,10 @@ func TestDefaultColumnOrderDetection(t *testing.T) {
22212231

22222232
assert.False(t, slices.Equal(customOrder, toggleableColumns))
22232233
}
2234+
2235+
func TestDefaultHiddenColumnsIncludeRequestedFields(t *testing.T) {
2236+
hidden := parseHiddenColumns(nil)
2237+
assert.True(t, hidden[colSessionID])
2238+
assert.True(t, hidden[colRequestedModel])
2239+
assert.True(t, hidden[colRequestedProvider])
2240+
}

cmd/roborev/tui/render_queue.go

Lines changed: 60 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -100,19 +100,21 @@ func (m model) getVisibleSelectedIdx() int {
100100

101101
// Queue table column indices.
102102
const (
103-
colSel = iota // "> " selection indicator
104-
colJobID // Job ID
105-
colRef // Git ref (short SHA or range)
106-
colBranch // Branch name
107-
colRepo // Repository display name
108-
colAgent // Agent name
109-
colQueued // Enqueue timestamp
110-
colElapsed // Elapsed time
111-
colStatus // Job status
112-
colPF // Pass/Fail verdict
113-
colHandled // Done status
114-
colSessionID // Session ID
115-
colCount // total number of columns
103+
colSel = iota // "> " selection indicator
104+
colJobID // Job ID
105+
colRef // Git ref (short SHA or range)
106+
colBranch // Branch name
107+
colRepo // Repository display name
108+
colAgent // Agent name
109+
colQueued // Enqueue timestamp
110+
colElapsed // Elapsed time
111+
colStatus // Job status
112+
colPF // Pass/Fail verdict
113+
colHandled // Done status
114+
colSessionID // Session ID
115+
colRequestedModel // Explicitly requested model
116+
colRequestedProvider // Explicitly requested provider
117+
colCount // total number of columns
116118
)
117119

118120
func (m model) renderQueueView() string {
@@ -309,15 +311,17 @@ func (m model) renderQueueView() string {
309311

310312
// Fixed-width columns: exact sizes (content + padding, not counting inter-column spacing)
311313
fixedWidth := map[int]int{
312-
colSel: 2,
313-
colJobID: idWidth,
314-
colStatus: max(contentWidth[colStatus], 6), // "Status" header = 6, auto-sizes to content
315-
colQueued: 12,
316-
colElapsed: 8,
317-
colPF: 3, // "P/F" header = 3
318-
colHandled: max(contentWidth[colHandled], 6), // "Closed" header = 6
319-
colAgent: min(max(contentWidth[colAgent], 5), 12), // "Agent" header = 5, cap at 12
320-
colSessionID: min(max(contentWidth[colSessionID], 7), 12), // "Session" header = 7, cap at 12
314+
colSel: 2,
315+
colJobID: idWidth,
316+
colStatus: max(contentWidth[colStatus], 6), // "Status" header = 6, auto-sizes to content
317+
colQueued: 12,
318+
colElapsed: 8,
319+
colPF: 3, // "P/F" header = 3
320+
colHandled: max(contentWidth[colHandled], 6), // "Closed" header = 6
321+
colAgent: min(max(contentWidth[colAgent], 5), 12), // "Agent" header = 5, cap at 12
322+
colSessionID: min(max(contentWidth[colSessionID], 7), 12), // "Session" header = 7, cap at 12
323+
colRequestedModel: min(max(contentWidth[colRequestedModel], 15), 24), // "Req Model" header = 9
324+
colRequestedProvider: min(max(contentWidth[colRequestedProvider], 18), 24), // "Req Provider" header = 12
321325
}
322326

323327
// Flexible columns absorb excess space
@@ -593,8 +597,8 @@ func (m model) renderQueueView() string {
593597
}
594598

595599
// jobCells returns plain text cell values for a job row.
596-
// Order: ref, branch, repo, agent, queued, elapsed, status, pf, handled
597-
// (colRef through colHandled, 9 values).
600+
// Order: ref, branch, repo, agent, queued, elapsed, status, pf, handled,
601+
// session, requested model, requested provider.
598602
func (m model) jobCells(job storage.ReviewJob) []string {
599603
ref := shortJobRef(job)
600604
if !config.IsDefaultReviewType(job.ReviewType) {
@@ -645,7 +649,10 @@ func (m model) jobCells(job storage.ReviewJob) []string {
645649
sessionID = string(runes[:12])
646650
}
647651

648-
return []string{ref, branch, repo, agentName, enqueued, elapsed, status, verdict, handled, sessionID}
652+
requestedModel := stripControlChars(job.RequestedModel)
653+
requestedProvider := stripControlChars(job.RequestedProvider)
654+
655+
return []string{ref, branch, repo, agentName, enqueued, elapsed, status, verdict, handled, sessionID, requestedModel, requestedProvider}
649656
}
650657

651658
// statusLabel returns a capitalized display label for the job status.
@@ -770,34 +777,38 @@ func migrateColumnConfig(cfg *config.Config) bool {
770777

771778
// toggleableColumns is the ordered list of columns the user can show/hide.
772779
// colSel and colJobID are always visible and not included here.
773-
var toggleableColumns = []int{colRef, colBranch, colRepo, colAgent, colQueued, colElapsed, colStatus, colPF, colHandled, colSessionID}
780+
var toggleableColumns = []int{colRef, colBranch, colRepo, colAgent, colQueued, colElapsed, colStatus, colPF, colHandled, colSessionID, colRequestedModel, colRequestedProvider}
774781

775782
// columnNames maps column constants to display names.
776783
var columnNames = map[int]string{
777-
colRef: "Ref",
778-
colBranch: "Branch",
779-
colRepo: "Repo",
780-
colAgent: "Agent",
781-
colStatus: "Status",
782-
colQueued: "Queued",
783-
colElapsed: "Elapsed",
784-
colPF: "P/F",
785-
colHandled: "Closed",
786-
colSessionID: "Session",
784+
colRef: "Ref",
785+
colBranch: "Branch",
786+
colRepo: "Repo",
787+
colAgent: "Agent",
788+
colStatus: "Status",
789+
colQueued: "Queued",
790+
colElapsed: "Elapsed",
791+
colPF: "P/F",
792+
colHandled: "Closed",
793+
colSessionID: "Session",
794+
colRequestedModel: "Req Model",
795+
colRequestedProvider: "Req Provider",
787796
}
788797

789798
// columnConfigNames maps column constants to config file names (lowercase).
790799
var columnConfigNames = map[int]string{
791-
colRef: "ref",
792-
colBranch: "branch",
793-
colRepo: "repo",
794-
colAgent: "agent",
795-
colStatus: "status",
796-
colQueued: "queued",
797-
colElapsed: "elapsed",
798-
colPF: "pf",
799-
colHandled: "closed",
800-
colSessionID: "session_id",
800+
colRef: "ref",
801+
colBranch: "branch",
802+
colRepo: "repo",
803+
colAgent: "agent",
804+
colStatus: "status",
805+
colQueued: "queued",
806+
colElapsed: "elapsed",
807+
colPF: "pf",
808+
colHandled: "closed",
809+
colSessionID: "session_id",
810+
colRequestedModel: "requested_model",
811+
colRequestedProvider: "requested_provider",
801812
}
802813

803814
// drainFlexOverflow reduces flex column widths to absorb overflow,
@@ -838,7 +849,9 @@ func columnDisplayName(col int) string {
838849
// defaultHiddenColumns lists columns that are hidden by default.
839850
// Users can enable them via the column options modal.
840851
var defaultHiddenColumns = map[int]bool{
841-
colSessionID: true,
852+
colSessionID: true,
853+
colRequestedModel: true,
854+
colRequestedProvider: true,
842855
}
843856

844857
// parseHiddenColumns converts config hidden_columns strings to column ID set.

cmd/roborev/tui/tui.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ func newModel(ep daemon.DaemonEndpoint, opts ...option) model {
453453
tabWidth := 2
454454
columnBorders := false
455455
tasksEnabled := false
456-
hiddenCols := map[int]bool{}
456+
hiddenCols := parseHiddenColumns(nil)
457457
colOrder := parseColumnOrder(nil)
458458
taskColOrder := parseTaskColumnOrder(nil)
459459
var cwdRepoRoot, cwdBranch string

cmd/roborev/tui/types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ const (
8989
)
9090

9191
// columnOption represents an item in the column options modal.
92-
// id is a column constant (colRef..colHandled) or a sentinel option ID.
92+
// id is a queue column constant or a sentinel option ID.
9393
type columnOption struct {
9494
id int // column constant or sentinel option ID
9595
name string // display label

0 commit comments

Comments
 (0)