From 93787843f4bdf92271c106458fbdaf9b90d261e4 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 30 Oct 2026 16:08:50 -0400 Subject: [PATCH 1/2] fix(timeline): Draw a background job as tools, not as waiting on a subagent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `run_in_background` shell job is a tool still running, but the pane bucketed its time with subagent waits — the one state whose legend says "waiting on subagents" and whose row only exists when a child agent does. A session with no subagents, such as 86f71ae7, therefore showed green with nothing to point at: both of its jobs (11:50-12:00, 12:37-12:57) read as subagent time. A job's span now joins the tools bucket, taken after the model so a job left running through a reply hides no model, and it counts in the tools column too. Teal means a subagent again. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/agentop/tui/timeline_layout.go | 25 +++++++++++++++--------- cmd/agentop/tui/timeline_pane.go | 5 +++-- cmd/agentop/tui/timeline_pane_test.go | 28 +++++++++++++++++++++++---- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/cmd/agentop/tui/timeline_layout.go b/cmd/agentop/tui/timeline_layout.go index 32decd96a..d047ad225 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,10 +562,17 @@ 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()), - modelTime: m.cover(from, to), toolTime: t.cover(from, to), calls: calls[a.ID]} + modelTime: m.cover(from, to), toolTime: t.cover(from, to) + jobs.cover(from, to), calls: calls[a.ID]} l, known := life[a.ID] // Gap cells are filled too, each with the whole stretch it stands for, so the marker can // say what that stretch was. @@ -595,7 +597,12 @@ 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)} + s.tool += take(jobs) + 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..d1d56ea70 100644 --- a/cmd/agentop/tui/timeline_pane_test.go +++ b/cmd/agentop/tui/timeline_pane_test.go @@ -1052,17 +1052,37 @@ 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, as the bar is. + without := buildTimelineLayout(sampleTimeline(), tlAt(0), tlAt(1800), 180, tlAt(3600)).agents[0].toolTime + if got := lay.agents[0].toolTime - without; got != 210*time.Second { + t.Errorf("tools total grew by %v, want the job's 210s", 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) + } } // A first fetch that times out is retried on a backoff and drawn as loading, a refresh never From a84380e298440bb8103a1836913223b0a2842668 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 30 Oct 2026 16:21:09 -0400 Subject: [PATCH 2/2] fix(timeline): Count a background job in the tools column as drawn A job's span and a reply overlap: Job.Start is the request that carries the job's acknowledgement, which is the next turn's own request. The bar gives that overlap to the model, but the tools column added the job's whole span, so model + tools could exceed the row's span. The column now adds the share take() gives the job in each cell, which is what the bar paints, and only in the cell a collapsed gap draws. Fixes review: - The tools total counted a job's whole span while the bar gave shared time to the model. - A job in a collapsed gap was counted three times, once per undrawn gap cell. - cmd/agentop/README.md still described a background job as teal, "waiting on its subagents". Files: - cmd/agentop/tui/timeline_layout.go: toolTime adds the job's drawn share, marker cell of a gap only - cmd/agentop/tui/timeline_pane_test.go: both job tests assert what the bar gains; a third pins the gap - cmd/agentop/README.md: the pane's prose names a background job as a tool still running Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/agentop/README.md | 11 ++++---- cmd/agentop/tui/timeline_layout.go | 10 +++++-- cmd/agentop/tui/timeline_pane_test.go | 40 +++++++++++++++++++++++++-- 3 files changed, 51 insertions(+), 10 deletions(-) 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 d047ad225..9a253f5f5 100644 --- a/cmd/agentop/tui/timeline_layout.go +++ b/cmd/agentop/tui/timeline_layout.go @@ -572,7 +572,7 @@ func tlAgentRows(tl *timeline.Timeline, ax tlAxis, from, to, now time.Time) []tl 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()), - modelTime: m.cover(from, to), toolTime: t.cover(from, to) + jobs.cover(from, to), calls: calls[a.ID]} + modelTime: m.cover(from, to), toolTime: t.cover(from, to), calls: calls[a.ID]} l, known := life[a.ID] // Gap cells are filled too, each with the whole stretch it stands for, so the marker can // say what that stretch was. @@ -600,7 +600,13 @@ func tlAgentRows(tl *timeline.Timeline, ax tlAxis, from, to, now time.Time) []tl // 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)} - s.tool += take(jobs) + 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 } diff --git a/cmd/agentop/tui/timeline_pane_test.go b/cmd/agentop/tui/timeline_pane_test.go index d1d56ea70..872b34e41 100644 --- a/cmd/agentop/tui/timeline_pane_test.go +++ b/cmd/agentop/tui/timeline_pane_test.go @@ -1064,10 +1064,11 @@ func TestTimelineWaits_BackgroundJobIsATool(t *testing.T) { 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, as the bar is. + // 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 != 210*time.Second { - t.Errorf("tools total grew by %v, want the job's 210s", got) + 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) } } @@ -1083,6 +1084,39 @@ func TestTimelineWaits_BackgroundJobDoesNotHideTheModel(t *testing.T) { 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