diff --git a/cmd/agentop/README.md b/cmd/agentop/README.md index 8c988794a..212544dc9 100644 --- a/cmd/agentop/README.md +++ b/cmd/agentop/README.md @@ -1309,12 +1309,13 @@ agentop is for, and the other four are surfaces you visit and leave. ``` An agent cell is a bar of how that cell's time divided, stacked from the - bottom: amber the agent's tools running, purple its model generating, red - retrying after a failed model call, teal waiting on its subagents or its - background shell jobs, grey waiting on you, as is an `AskUserQuestion` the + bottom: amber the agent's tools running — a background shell job is a tool + still running, and counts here — purple its model generating, red + retrying after a failed model call, teal waiting on its subagents, grey + waiting on you, as is an `AskUserQuestion` the agent put to you. Waiting is read from the gaps - between an agent's model calls: a gap that one of its subagents or background - jobs was running in, or that a subagent's result ended, is waiting on + between an agent's model calls: a gap that one of its subagents was running + in, or that a subagent's result ended, is waiting on subagents; a gap before a call that re-sent the same conversation is retrying; after a failed call it is retrying until the last failed request to the model host, then waiting on you, since Claude Code shows the error and stops. On the diff --git a/cmd/agentop/tui/timeline_layout.go b/cmd/agentop/tui/timeline_layout.go index 32decd96a..9a253f5f5 100644 --- a/cmd/agentop/tui/timeline_layout.go +++ b/cmd/agentop/tui/timeline_layout.go @@ -411,8 +411,8 @@ func (v ivs) cover(lo, hi time.Time) time.Duration { // waits classifies the gaps between an agent's consecutive turns, and the one after its last. // -// - Woken by a subagent's result: waiting on its subagents, as is any time one of them or one -// of its background jobs was running. +// - Woken by a subagent's result: waiting on its subagents, as is any time one of them was +// running. A background job is a tool still running, and is drawn as one (see build). // - The next call re-sent the conversation unchanged: retrying, the whole gap. // - After a failed call otherwise: retrying until the last failed request to a model host in // the gap, then waiting on the user, who had to act. Claude Code retries a cut stream for a @@ -428,11 +428,6 @@ func waits(tl *timeline.Timeline, agent string, children []string, life map[stri kids = append(kids, l) } } - for _, j := range tl.Jobs { - if j.Agent == agent { - kids = append(kids, [2]time.Time{j.Start, tlEndOr(j.End, now)}) - } - } woke := map[int64]bool{} for _, r := range tl.Returns { if r.Into == agent { @@ -567,6 +562,13 @@ func tlAgentRows(tl *timeline.Timeline, ax tlAxis, from, to, now time.Time) []tl build := func(a timeline.Agent, depth int) tlAgentRow { m, t := newIvs(model[a.ID]), newIvs(tool[a.ID]) + var js [][2]time.Time + for _, j := range tl.Jobs { + if j.Agent == a.ID { + js = append(js, [2]time.Time{j.Start, tlEndOr(j.End, now)}) + } + } + jobs := newIvs(js) retry, subs, user := waits(tl, a.ID, children[a.ID], life, now) user = newIvs(append(user, asking[a.ID]...)) row := tlAgentRow{agent: a, depth: depth, cells: make([]share, ax.width()), alive: make([]bool, ax.width()), @@ -595,7 +597,18 @@ func tlAgentRows(tl *timeline.Timeline, ax tlAxis, from, to, now time.Time) []tl left -= f return f } - row.cells[c] = share{tool: take(t), model: take(m), retry: take(retry), subs: take(subs), user: take(user)} + // A background job is a tool still running: it takes what the model and the + // foreground tools did not, so a job left running through a reply hides no model. + s := share{tool: take(t), model: take(m)} + job := take(jobs) + s.tool += job + // Only the marker of a collapsed gap is drawn; the cells either side share its span, + // so counting them would take the stretch off the column more than once. + if !cell.gap || cell.marker { + row.toolTime += time.Duration(job * span) + } + s.retry, s.subs, s.user = take(retry), take(subs), take(user) + row.cells[c] = s } return row } diff --git a/cmd/agentop/tui/timeline_pane.go b/cmd/agentop/tui/timeline_pane.go index 829737b9e..4be9cf619 100644 --- a/cmd/agentop/tui/timeline_pane.go +++ b/cmd/agentop/tui/timeline_pane.go @@ -21,8 +21,9 @@ import ( // draws them. // // An agent cell is a stacked bar of how that cell's time divided: tools running at the -// baseline, then the model generating, retrying after a failed call, waiting on its subagents, -// and waiting on the user. A host cell's height is how many requests were in flight at once. +// baseline (a background job is a tool still running, so it counts here), then the model +// generating, retrying after a failed call, waiting on its subagents, and waiting on the user. +// A host cell's height is how many requests were in flight at once. // timelineRefresh is the most often a live session's timeline is refetched, and how often the // layout's "now" moves for one with a call in flight. diff --git a/cmd/agentop/tui/timeline_pane_test.go b/cmd/agentop/tui/timeline_pane_test.go index e90ca8684..872b34e41 100644 --- a/cmd/agentop/tui/timeline_pane_test.go +++ b/cmd/agentop/tui/timeline_pane_test.go @@ -1052,17 +1052,71 @@ func TestTimelineWaits_MainDefaultsToYouExceptAfterALostTool(t *testing.T) { } } -// A background job the main agent left running is waiting on subagents, not on the user. -func TestTimelineWaits_BackgroundJobIsNotTheUsersTime(t *testing.T) { +// A background job is a tool still running, and its agent is never waiting on the user while +// one is: it is drawn as tools, not as waiting on subagents, which has a row to point at. +func TestTimelineWaits_BackgroundJobIsATool(t *testing.T) { tl := sampleTimeline() // main finishes a reply at 1200s; the user answers at 1500s tl.Jobs = []timeline.Job{{Agent: timeline.MainAgent, ID: "bk1", Start: tlAt(1190), End: tlPtr(1400), Seq: 29}} lay := buildTimelineLayout(tl, tlAt(0), tlAt(1800), 180, tlAt(3600)) - if c := lay.agents[0].cells[cellOf(t, lay, 1300)]; !near(c.subs, 1) { - t.Errorf("while the job ran: %+v, want waiting on subagents", c) + if c := lay.agents[0].cells[cellOf(t, lay, 1300)]; !near(c.tool, 1) { + t.Errorf("while the job ran: %+v, want tools", c) } if c := lay.agents[0].cells[cellOf(t, lay, 1450)]; !near(c.user, 1) { t.Errorf("after the job's notification: %+v, want waiting on the user", c) } + // It is in the tools total too, by what the bar gives it: the job's first 10s fall inside the + // reply that ends at 1200s, so those are the model's, not tools'. + without := buildTimelineLayout(sampleTimeline(), tlAt(0), tlAt(1800), 180, tlAt(3600)).agents[0].toolTime + if got := lay.agents[0].toolTime - without; got != 200*time.Second { + t.Errorf("tools total grew by %v, want the job's 200s outside the reply", got) + } +} + +// A job left running through a reply does not hide the model: the model claims its own time and +// the job takes what is left. +func TestTimelineWaits_BackgroundJobDoesNotHideTheModel(t *testing.T) { + tl := sampleTimeline() // main's turn 960-1200s, then the user answers at 1500s + tl.Jobs = []timeline.Job{{Agent: timeline.MainAgent, ID: "bk1", Start: tlAt(1150), End: tlPtr(1300), Seq: 29}} + lay := buildTimelineLayout(tl, tlAt(0), tlAt(1800), 180, tlAt(3600)) + if c := lay.agents[0].cells[cellOf(t, lay, 1170)]; !near(c.model, 1) || !near(c.tool, 0) { + t.Errorf("job through a turn: %+v, want model only", c) + } + if c := lay.agents[0].cells[cellOf(t, lay, 1250)]; !near(c.tool, 1) { + t.Errorf("job after the turn: %+v, want tools", c) + } + // The tools total counts the time the bar gives the job: its 50s inside the reply is the + // model's. + without := buildTimelineLayout(sampleTimeline(), tlAt(0), tlAt(1800), 180, tlAt(3600)).agents[0].toolTime + if got := lay.agents[0].toolTime - without; got != 100*time.Second { + t.Errorf("tools total grew by %v, want the job's 100s outside the reply", got) + } +} + +// A collapsed idle stretch is three cells sharing one span, and only the middle one is drawn: a +// job inside it belongs in the tools total once. +func TestTimelineWaits_BackgroundJobInACollapsedGapCountedOnce(t *testing.T) { + tl := &timeline.Timeline{ + Start: tlAt(0), End: tlAt(7360), + Agents: []timeline.Agent{{ID: timeline.MainAgent, Label: "main"}}, + Turns: []timeline.Turn{ + {Agent: timeline.MainAgent, Start: tlAt(10), End: tlPtr(60), Finish: "end_turn", Seq: 1}, + {Agent: timeline.MainAgent, Start: tlAt(7300), End: tlPtr(7360), Finish: "end_turn", Seq: 40}, + }, + Jobs: []timeline.Job{{Agent: timeline.MainAgent, ID: "bk1", Start: tlAt(120), End: tlPtr(5000), Seq: 2}}, + } + lay := buildTimelineLayout(tl, time.Time{}, time.Time{}, 40, tlAt(7360)) + markers := 0 + for _, c := range lay.axis.cells { + if c.marker { + markers++ + } + } + if markers == 0 { + t.Fatalf("axis has no collapsed gap, so this test reads nothing") + } + if got := lay.agents[0].toolTime; got != 4880*time.Second { + t.Errorf("tools total = %v, want the job's 4880s once", got) + } } // A first fetch that times out is retried on a backoff and drawn as loading, a refresh never