diff --git a/docs/changes/unreleased/1898-task-model-visibility.md b/docs/changes/unreleased/1898-task-model-visibility.md new file mode 100644 index 000000000..9e7c7688e --- /dev/null +++ b/docs/changes/unreleased/1898-task-model-visibility.md @@ -0,0 +1,13 @@ +--- +kind: fixed +title: task pages keep saved crew models visible without a priced call +pr: 1898 +surface: [chat, engine, remote, docs] +invalidates: + - "A run task page needed a saved spend row to name a model. It now also reads the run's saved worker, planner and checker assignments, including for archived children after an interruption; those assignments are separate from historical usage." + - "Task links and arrow navigation could open an ordinary task room for a run and offer a model picker that could not apply. Known run tasks now open the same stored page as sidebar clicks, with explicit editing limits." + - "A program's declared model pool was visible only in the footer. Task setup now lists its models and thinking level; missing history is shown as not recorded instead of silently omitting the model row." +--- + +This changes visibility and navigation. Run and program models remain fixed for +an existing run; ordinary task model controls retain their existing scope. diff --git a/internal/manual/chat/task-controls.md b/internal/manual/chat/task-controls.md index 709bf56be..1bff6947b 100644 --- a/internal/manual/chat/task-controls.md +++ b/internal/manual/chat/task-controls.md @@ -36,6 +36,19 @@ name, `…` replaces outer connectors; breadcrumbs still open the actual ancesto Long sections show their position; resizing clamps each section to its available rows. Short or narrow terminals retain the compact column and existing controls. +## Why task models are missing, and which task models can be changed + +On a run-backed task, Task setup shows **Assigned crew**: the saved worker, +planner and checker, including on child tasks and after an interruption before +any priced call was saved. `Used model` is a separate historical reading from +the spend ledger, not necessarily the worker's current assignment. Missing old +records say `Model · not recorded`; the conversation's model is never guessed. +Run models cannot be changed here. `/crew` sets the next run's crew; it does +not move an existing run. A program such as senior-dev lists the models it +declared and its thinking level here; those are fixed for that run. Choose +models when starting it again. Ordinary tasks retain their task model picker, +even when their earlier model was not recorded. + ## Stop this task — /stop and the Stop task button Click `Stop task…` under Task setup, click `Stop` in the compact task header, or type diff --git a/internal/remote/client_test.go b/internal/remote/client_test.go index 2455e71a7..94c1c99e3 100644 --- a/internal/remote/client_test.go +++ b/internal/remote/client_test.go @@ -908,6 +908,7 @@ func TestPlanTasksAndPlanTaskPageCrossWhole(t *testing.T) { } page := session.PlanTaskPage{ Row: row, Description: "the complete work order", + Models: []session.PlanTaskModel{{Seat: "worker", Model: "saved/worker"}, {Seat: "planner", Model: "saved/planner"}, {Seat: "checker", Model: "saved/checker"}}, Notes: []session.PlanTaskNote{{Author: "worker", Person: true, Body: "note body", At: started}}, Steps: []session.PlanStep{{Kind: "bash", Step: 7, Command: "go test", Observation: "PASS", FullOutput: "/tmp/out", Writes: []string{"wire.go"}, Children: []string{"t-child"}}}, Live: row.Live, diff --git a/internal/session/plandb_models.go b/internal/session/plandb_models.go new file mode 100644 index 000000000..cf3c6e30b --- /dev/null +++ b/internal/session/plandb_models.go @@ -0,0 +1,47 @@ +package session + +import ( + "strings" + + "github.com/Agent-Field/codeaf/internal/crewroute" +) + +// The ledger cannot name a model until a priced call was saved. The run's +// checkpoint already records its accepted seats, including when recovery was +// refused after an interrupted call. Read that policy without rerouting it or +// borrowing the profile's current settings. Children share their run's crew. +func (a *Agent) planTaskModels(root string) []PlanTaskModel { + g := a.tasker() + if g == nil { + return nil + } + g.mu.Lock() + defer g.mu.Unlock() + for _, notice := range g.runRowsLocked() { + if notice.PlanTask != planStoreID(root) { + continue + } + var out []PlanTaskModel + for _, seat := range crewroute.Seats { + model := "" + if notice.Crew != nil { + model = notice.Crew.Seat(seat).Model + } + if saved := notice.CrewState; saved != nil && model == "" { + if saved.Routed { + model = saved.Decision.Seat(seat).Model + } else { + model = map[crewroute.Seat]string{crewroute.Worker: saved.Work, crewroute.Planner: saved.Plan, crewroute.Checker: saved.Check}[seat] + } + } + if seat == crewroute.Worker && model == "" { + model = notice.Model + } + if model = strings.TrimSpace(model); model != "" { + out = append(out, PlanTaskModel{Seat: string(seat), Model: model}) + } + } + return out + } + return nil +} diff --git a/internal/session/plandb_models_test.go b/internal/session/plandb_models_test.go new file mode 100644 index 000000000..95e070ff0 --- /dev/null +++ b/internal/session/plandb_models_test.go @@ -0,0 +1,89 @@ +package session + +import ( + "path/filepath" + "reflect" + "testing" + + "github.com/Agent-Field/codeaf/internal/crewroute" + "github.com/Agent-Field/codeaf/internal/plandb" +) + +func TestPlanTaskPageKeepsAssignedCrewWithoutAPricedCall(t *testing.T) { + dir, chat := t.TempDir(), "model-page" + path := filepath.Join(dir, planStoreFilename) + seedPlanStore(t, path, chat, plandb.TaskSpec{ID: "child", Title: "Child"}) + a, _ := newTestAgent(t, &scriptedCompleter{}, nil) + armPlanStore(t, a, path, chat) + decision := crewroute.Decision{Crew: []crewroute.Pick{ + {Seat: crewroute.Worker, Model: "saved/worker"}, + {Seat: crewroute.Planner, Model: "saved/planner"}, + {Seat: crewroute.Checker, Model: "saved/checker"}, + }} + // The screenshot's shape: recovery refused an unpriced interrupted call, + // but the durable row still carries every accepted seat. No ledger rows. + a.graph().keepRunRows(7, []TaskNotice{{ID: 7, PlanTask: planStoreID(planRootID), State: TaskInterrupted, + CrewState: &TaskCrewRecord{Version: 1, Routed: true, Decision: decision, InFlight: 1}}}) + want := []PlanTaskModel{{"worker", "saved/worker"}, {"planner", "saved/planner"}, {"checker", "saved/checker"}} + for _, id := range []string{planRootID, "child"} { + page, ok := a.PlanTaskPage(id) + if !ok || !reflect.DeepEqual(page.Models, want) { + t.Fatalf("%s lost accepted crew: %+v", id, page.Models) + } + if page.Row.Model != "" || page.Row.Tokens != 0 || page.Row.USD != 0 { + t.Fatalf("assignment invented usage: %+v", page.Row) + } + page.Models[0].Model = "changed by reader" + } + // A different run and the current profile cannot supply missing history. + a.graph().keepRunRows(7, []TaskNotice{{ID: 7, PlanTask: "t-elsewhere", Model: "other/worker", Crew: &decision}}) + page, _ := a.PlanTaskPage("child") + if len(page.Models) != 0 { + t.Fatalf("another run supplied models: %+v", page.Models) + } +} + +func TestPlanTaskModelsUsesThePublishedCrewAndUnroutedCheckpoint(t *testing.T) { + a, _ := newTestAgent(t, &scriptedCompleter{}, nil) + g := a.graph() + saved := &TaskCrewRecord{Version: 1, Work: "saved/work", Plan: "saved/plan", Check: "saved/check"} + g.keepRunRows(7, []TaskNotice{{ID: 7, PlanTask: "t-7", CrewState: saved}}) + if got := a.planTaskModels("7"); !reflect.DeepEqual(got, []PlanTaskModel{{"worker", "saved/work"}, {"planner", "saved/plan"}, {"checker", "saved/check"}}) { + t.Fatalf("unrouted seats: %+v", got) + } + decision := &crewroute.Decision{Crew: []crewroute.Pick{{Seat: crewroute.Worker, Model: "current/work"}}} + g.keepRunRows(7, []TaskNotice{{ID: 7, PlanTask: "t-7", Crew: decision, Model: "old/work", CrewState: saved}}) + if got := a.planTaskModels("7"); len(got) != 3 || got[0].Model != "current/work" { + t.Fatalf("published worker lost to old history: %+v", got) + } +} + +func TestArchivedTaskModelsSurviveReopeningTheConversation(t *testing.T) { + dir, chat := t.TempDir(), "archived-models" + path := filepath.Join(dir, planStoreFilename) + seedPlanStore(t, path+".1", chat, plandb.TaskSpec{ID: "child", Title: "Archived child"}) + open := func() *Agent { + a, _ := newTestAgent(t, &scriptedCompleter{}, func(c *Config) { + c.SessionFile = filepath.Join(dir, "transcript.jsonl") + }) + armPlanStore(t, a, path, chat) + return a + } + first := open() + first.graph().mu.Lock() + first.graph().seq = 7 + first.graph().mu.Unlock() + first.graph().keepRunRows(7, []TaskNotice{{ID: 7, PlanTask: planStoreID(planRootID), State: TaskInterrupted, + CrewState: &TaskCrewRecord{Version: 1, Work: "accepted/worker", Plan: "accepted/planner", Check: "accepted/checker", InFlight: 1}}}) + if err := first.Close(); err != nil { + t.Fatal(err) + } + second := open() + page, ok := second.PlanTaskPage("child") + if !ok || !reflect.DeepEqual(page.Models, []PlanTaskModel{{"worker", "accepted/worker"}, {"planner", "accepted/planner"}, {"checker", "accepted/checker"}}) { + t.Fatalf("reopening lost the archived assignment: %+v (found %v)", page.Models, ok) + } + if page.Row.Model != "" { + t.Fatalf("a saved assignment became usage: %+v", page.Row) + } +} diff --git a/internal/session/plandb_tasks.go b/internal/session/plandb_tasks.go index f2c94ccb0..801f56d18 100644 --- a/internal/session/plandb_tasks.go +++ b/internal/session/plandb_tasks.go @@ -178,7 +178,10 @@ type PlanTaskNote struct { // the description that is its work order, every note left on it, and the steps // its worker recorded. type PlanTaskPage struct { - Row PlanTaskRow + Row PlanTaskRow + // Models are the run's accepted seats, including on a child page and before + // any spend was saved. Row.Model remains the ledger's historical reading. + Models []PlanTaskModel `json:",omitempty"` Description string Result string Checks []string @@ -207,6 +210,12 @@ type PlanTaskPage struct { Program *PlanProgram } +// PlanTaskModel names an assigned seat, not proof that it answered a call. +type PlanTaskModel struct { + Seat string + Model string +} + // PlanStep is one line of a task's trajectory — one command the worker ran and // the head of what came back. IT MIRRORS internal/run's Step, which is where // the record is written: this package cannot import that one, because it @@ -404,6 +413,7 @@ func (a *Agent) PlanTaskPage(id string) (PlanTaskPage, bool) { } return PlanTaskPage{ Row: pageRow, + Models: a.planTaskModels(store.RootID()), Description: task.Description, Result: task.Result, Checks: append([]string(nil), task.Checks...), diff --git a/internal/tui3/room.go b/internal/tui3/room.go index 80c840dbd..ede9001bc 100644 --- a/internal/tui3/room.go +++ b/internal/tui3/room.go @@ -709,6 +709,17 @@ func (a *app) openRoom(id uint64, title string) { a.openProgramRoom(id, title, a.heldProgramPage(id)) return } + // Tabs, task links and the arrow key must read the same stored page as a + // sidebar click. A run row has no ordinary node to retarget or journal to + // stream; opening that room hid its crew and offered a picker that refused. + if node := a.tasks[id]; node != nil && node.planTask != "" && a.roomPageAsked != id { + a.roomPump = a.openRailPlan(node.planTask, func() tea.Cmd { + a.roomPageAsked = id + a.openRoom(id, title) + return a.takeRoomPump() + }) + return + } doors, ok := a.roomDoors() if !ok { // Local engine windows use this same door without a remote host label. diff --git a/internal/tui3/roommodels.go b/internal/tui3/roommodels.go new file mode 100644 index 000000000..e0b1bc097 --- /dev/null +++ b/internal/tui3/roommodels.go @@ -0,0 +1,52 @@ +package tui3 + +import "strings" + +// Run-backed pages have no retargeting door. Keep their recorded assignment +// visible independently of the spend ledger, and say where a future choice is +// made instead of presenting a picker that would change another conversation. +func (a *app) roomRunModelRows(width int, row func(string, string) railLine) []railLine { + var out []railLine + if p := a.programOf(); p != nil { + if program := p.page.Program; program != nil { + for _, model := range program.Models { + out = append(out, row("Model · "+modelBase(model), "")) + } + } + if len(out) == 0 { + out = append(out, row("Models · not recorded", "")) + } + if program := p.page.Program; program != nil && program.Effort != "" { + out = append(out, row("Thinking · "+program.Effort, "")) + } + return append(out, a.roomModelLimitRows(width, "Models are fixed for this run.", "Choose models when starting it again.")...) + } + if plan := a.roomPlan(); plan != nil { + for _, seat := range plan.page.Models { + label := seat.Seat + if label != "" { + label = strings.ToUpper(label[:1]) + label[1:] + } + out = append(out, row(label+" · "+modelBase(seat.Model), "")) + } + if len(out) > 0 { + out = append([]railLine{{text: a.pal.dim(fit("Assigned crew", width)), entry: -1}}, out...) + } + if used := plan.page.Row.Model; used != "" { + out = append(out, row("Used model · "+modelBase(used), "")) + } else if len(out) == 0 { + out = append(out, row("Model · not recorded", "")) + } + } + return append(out, a.roomModelLimitRows(width, "Run models cannot be changed here.", "/crew sets the next run's crew.")...) +} + +func (a *app) roomModelLimitRows(width int, words ...string) []railLine { + var out []railLine + for _, word := range words { + for _, line := range wrap(word, max(width, 1)) { + out = append(out, railLine{text: a.pal.dim(line), entry: -1}) + } + } + return out +} diff --git a/internal/tui3/roommodels_test.go b/internal/tui3/roommodels_test.go new file mode 100644 index 000000000..36de6d203 --- /dev/null +++ b/internal/tui3/roommodels_test.go @@ -0,0 +1,86 @@ +package tui3 + +import ( + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/session" +) + +func roomSetupText(a *app) string { + var words []string + for _, line := range a.roomControlRows(a.railRoom()) { + words = append(words, plain(line.text)) + } + return strings.Join(words, "\n") +} + +func TestRunRoomShowsAssignedSeatsWithoutUsageAndExplainsEditing(t *testing.T) { + row := session.PlanTaskRow{ID: "t-7", Title: "Interrupted work", Status: "failed", Interrupted: true} + page := session.PlanTaskPage{Row: row, Models: []session.PlanTaskModel{ + {Seat: "worker", Model: "saved/worker-model"}, {Seat: "planner", Model: "saved/planner-model"}, {Seat: "checker", Model: "saved/checker-model"}, + }} + a, _ := planAppWith(t, []session.PlanTaskRow{row}, map[string]session.PlanTaskPage{row.ID: page}) + a.width, a.height = 160, 48 + openPlanRoomNow(t, a, row.ID) + text := roomSetupText(a) + for _, word := range []string{"Assigned crew", "Worker · worker-model", "Planner · planner-model", "Checker · checker-model", "Run models cannot be changed here.", "/crew sets the next run's crew."} { + if !strings.Contains(text, word) { + t.Fatalf("missing %q: %s", word, text) + } + } + if strings.Contains(text, "Used model") || a.roomModelMovable() { + t.Fatalf("assignment claimed usage or editing: %s", text) + } + for _, line := range a.roomControlRows(a.railRoom()) { + if line.roomAction == "model" { + t.Fatal("run model offered the ordinary task picker") + } + } + page.Models = nil + a.planRoomTake(page) + if text = roomSetupText(a); !strings.Contains(text, "Model · not recorded") || strings.Contains(text, a.model) { + t.Fatalf("missing history was hidden or borrowed: %s", text) + } +} + +func TestProgramSetupShowsEveryDeclaredModelAndItsThinking(t *testing.T) { + a, agent := programRoomApp(t, 160, 48) + page := agent.planFake.pages["7"] + page.Program.Models = []string{"saved/first-model", "saved/second-model"} + page.Program.Effort = "high" + agent.planFake.setPage("7", page) + openProgramRoomNow(t, a) + text := roomSetupText(a) + for _, word := range []string{"Model · first-model", "Model · second-model", "Thinking · high", "Models are fixed for this run.", "Choose models when starting it again."} { + if !strings.Contains(text, word) { + t.Fatalf("missing %q: %s", word, text) + } + } +} + +func TestAnOrdinaryTaskWithNoRecordedModelKeepsItsPicker(t *testing.T) { + a, _ := roomModelApp(t, "") + a.width, a.height = 160, 48 + if text := roomSetupText(a); !strings.Contains(text, "Model · not recorded") { + t.Fatalf("unknown model vanished: %s", text) + } + panelRow(t, a, "model") +} + +func TestRunTaskLinksOpenTheSameModelPageAsTheSidebar(t *testing.T) { + a, fake := railTaskPageApp(t, true) + a.width, a.height = 160, 48 + page := fake.planFake.pages["2"] + page.Models = []session.PlanTaskModel{{Seat: "worker", Model: "saved/worker-model"}, {Seat: "planner", Model: "saved/planner-model"}} + fake.planFake.setPage("t-2", page) + a.tasks[2].planTask = "t-2" + a.openRoomFor(2, a.tasks[2].title) + drain(t, a, a.takeRoomPump()) + if a.roomPlan() == nil || a.roomModelMovable() { + t.Fatal("a run task link opened the ordinary task picker") + } + if text := roomSetupText(a); !strings.Contains(text, "Planner · planner-model") { + t.Fatalf("the task link lost its saved seats: %s", text) + } +} diff --git a/internal/tui3/roompanel.go b/internal/tui3/roompanel.go index c813eb09f..a632c1656 100644 --- a/internal/tui3/roompanel.go +++ b/internal/tui3/roompanel.go @@ -137,12 +137,18 @@ func (a *app) roomControlRows(width int) []railLine { return railLine{text: text, entry: -1, roomAction: action} } heading := "Task setup" - if taskSetupLater(a.roomNode()) { + if taskSetupLater(a.roomNode()) && a.roomModelMovable() { heading = "Next run setup" } out := []railLine{{entry: -1}, {text: a.pal.dim(fit(heading, width)), entry: -1}} - if n := a.roomNode(); n != nil && n.model != "" { - action, word := "", "Model · "+modelBase(firstNonEmpty(n.nextModel, n.model)) + if a.programOf() != nil || a.roomPlan() != nil { + out = append(out, a.roomRunModelRows(width, row)...) + } else if n := a.roomNode(); n != nil { + model := modelBase(firstNonEmpty(n.nextModel, n.model)) + if model == "" { + model = "not recorded" + } + action, word := "", "Model · "+model if a.roomModelMovable() { action = "model" word = fit(word, max(width-2, 0)) + " ▾" @@ -403,7 +409,7 @@ func taskSetupLater(node *taskNode) bool { func taskSetupAvailable(node *taskNode) bool { // A ROW LENT TO A RUN'S TASK HAS NO NODE TO SET UP: its worker is the run's, // and no door here reaches it (planroom.go). - if node == nil || node.run != "" || node.planRow != nil { + if node == nil || node.run != "" || node.planRow != nil || node.planTask != "" { return false } if node.state == session.TaskRunning || node.state == session.TaskQueued {