diff --git a/cmd/agentop/README.md b/cmd/agentop/README.md index 76ddc7259..d9e923ac9 100644 --- a/cmd/agentop/README.md +++ b/cmd/agentop/README.md @@ -1112,6 +1112,14 @@ agentop is for, and the other four are surfaces you visit and leave. toward. Sorting never changes the `#` numbers or a row's figures. It reorders the finished rows only. + Each tool call has its own row under the exchange whose response asked for it. + Its `#` cell is `┣⚒` or `┗⚒` (the last tool of the exchange). In a sorted view it + is `↳`, where `` is the `#` of that exchange. From exchange 1000 it is `` + only, because the column has no space for `↳`. The tool's DURATION is measured + from outside the agent: from the reply to the next request of the same agent. So + it includes the client's own delay, and it can miss a tool that started while the + reply still streamed. + The twelve default columns together need about 180 terminal columns, so the table drops what does not fit and the footer says how many (`→ N more columns`). Columns carry a keep rank rather than being equally expendable: DIR, DURATION, TOKENS and COST diff --git a/cmd/agentop/tui/agents_pane.go b/cmd/agentop/tui/agents_pane.go index 19a211db6..dad849fba 100644 --- a/cmd/agentop/tui/agents_pane.go +++ b/cmd/agentop/tui/agents_pane.go @@ -520,7 +520,8 @@ func (m *model) leaveAgentsPane() tea.Cmd { if m.pane == paneUsage { return m.resumeUsagePolling() } - if m.pane == paneTimeline { + // The Timeline pane, or the events and detail panes for their tool rows (#1776). + if m.timelineChainLive() { return m.resumeTimeline() } return nil diff --git a/cmd/agentop/tui/app.go b/cmd/agentop/tui/app.go index 6a86879be..68b870054 100644 --- a/cmd/agentop/tui/app.go +++ b/cmd/agentop/tui/app.go @@ -2411,7 +2411,7 @@ func (m *model) handleStreamEvent(ev apiclient.StreamEvent) { } stale: m.sessionsStale = true - if m.pane == paneTimeline && m.timeline.session == e.SessionID { + if m.timelineChainLive() && m.timeline.session == e.SessionID { m.timeline.stale = true // refetched on the pane's next tick, at most once a second } if m.pane == paneEvents && m.selectedSess == e.SessionID { @@ -2431,7 +2431,8 @@ stale: // only opening another session or erasing clears it) or the timeline's // (openTimeline does not set selectedSess; leaving the timeline pane clears // it, so a session glanced at once does not keep its stream for the rest of -// the run). This is the retention predicate for streamed events (#1542): +// the run. The events pane sets it only to selectedSess, and erasing clears +// it with selectedSess). This is the retention predicate for streamed events (#1542): // everything else arrives, updates its summary, and is dropped. func (m *model) watchingSession(id string) bool { return m.selectedSess == id || m.timeline.session == id diff --git a/cmd/agentop/tui/clear.go b/cmd/agentop/tui/clear.go index 3687f7e5b..20f661e4c 100644 --- a/cmd/agentop/tui/clear.go +++ b/cmd/agentop/tui/clear.go @@ -333,6 +333,9 @@ func (m *model) forgetSessions(ids []string) { m.detailEvent = nil m.detailPlugin = nil } + if m.timeline.session == id { + m.setTimelineSession("") // a #1542 retention key, like selectedSess + } } m.sessions = slices.DeleteFunc(m.sessions, func(s session.SessionSummary) bool { return slices.Contains(ids, s.ID) @@ -368,6 +371,7 @@ func (m *model) resetAfterClear() { m.detailEvent = nil m.detailPlugin = nil m.selectedSess = "" + m.setTimelineSession("") // a #1542 retention key, like selectedSess m.selectedEventKey = eventKey{} m.visibleRows = nil if m.pane != paneAgents { diff --git a/cmd/agentop/tui/events_columns.go b/cmd/agentop/tui/events_columns.go index 61a173004..2058c4dc1 100644 --- a/cmd/agentop/tui/events_columns.go +++ b/cmd/agentop/tui/events_columns.go @@ -307,7 +307,7 @@ var eventColumns = []eventColumn{ // 9 is the heading with a sort glyph, which is wider than anything durationCell // produces ("12.34s", "59m59s"). {id: colDuration, width: 9, defaultOn: true, keep: keepLow, rightAlign: true, - desc: "how long the exchange took", + desc: "how long the exchange took; for a tool: from the reply to the next request of the same agent", cell: func(c cellContext) string { return durationCell(*answer(c.row)) }, sortKey: durationSortKey}, // On by default since #1309. It was off because only an opaque tunnel's close row had @@ -837,3 +837,74 @@ func anyColumnSelected(sel map[eventColumnID]bool) bool { } return false } + +// toolCells is the cells of tool row tr (#1776), whose exchange is cc. The columns up to +// DURATION get the tool's own cells. At the first shown column after them, one last cell +// holds the trailing text, and the table spans it to the right edge. sr is the row's search +// row: text that no shown column holds goes in Dropped, so a search still finds it. +func toolCells(cols []eventColumn, cc cellContext, tr eventRow, sorted bool, now time.Time) (table.Row, table.SearchRow) { + t := tr.tool + // The model writes the name and the preview, so a control rune must not reach the terminal. + name, trailing := sanitizeLabel(t.Name), sanitizeLabel(t.trailing(now)) + row := make(table.Row, 0, len(cols)) + var sr table.SearchRow + named, spanned := false, false + for _, c := range cols { + var v string + switch c.id { + case colIndex: + switch { + case sorted: + if v = c.cell(cc); len(v) < c.width { + v = "↳" + v // the number wins when both do not fit + } + case tr.last: + v = "┗⚒" + default: + v = "┣⚒" + } + case colTime: + v = t.Start.Format("15:04:05.00") + case colDir: + if cc.row.event.Subagent != "" && cc.m.subagentBlocks != nil { + v = "│" + } + case colAction, colPlugin: + case colMethod: + v, named = name, true + case colStatus: + v = t.status() + case colDuration: + v = t.durationCell(now) + default: + row = append(row, trailing) + spanned = true + } + if spanned { + break + } + row = append(row, c.align(v)) + } + if !spanned && trailing != "" { + sr.Dropped = append(sr.Dropped, trailing) + } + if !named { + sr.Dropped = append(sr.Dropped, name) + } + return row, sr +} + +// toolSortKey is a tool row's sort value on column id (rule 11). A column with no tool value +// gives the zero value, with numeric as the column's other keys have it, so less compares the +// same field on every row. +func toolSortKey(id eventColumnID, t toolRow, numeric bool, now time.Time) sortValue { + switch id { + case colDuration: + return numKey(t.duration(now)) + case colTime: + return numKey(t.Start.UnixNano()) + case colMethod: + return strKey(t.Name) + } + return sortValue{numeric: numeric} +} diff --git a/cmd/agentop/tui/events_pane.go b/cmd/agentop/tui/events_pane.go index ae8327fdb..92916d716 100644 --- a/cmd/agentop/tui/events_pane.go +++ b/cmd/agentop/tui/events_pane.go @@ -27,6 +27,10 @@ type eventKey struct { direction pipeline.Direction phase pipeline.SessionPhase requestID string + // tool, toolAgent and toolSeq pin a tool row: its tool ID, agent and response seq. The seq + // tells apart two calls with one ID: some local models send call_0 on every turn. + tool, toolAgent string + toolSeq uint64 } func keyOf(e *pipeline.SessionEvent) eventKey { @@ -36,10 +40,28 @@ func keyOf(e *pipeline.SessionEvent) eventKey { return eventKey{at: e.At.UnixNano(), direction: e.Direction, phase: e.Phase, requestID: e.RequestID} } +// pinKeyOf is the pin key of row er: its tool for a tool row, else its event. +func pinKeyOf(er eventRow) eventKey { + if er.tool != nil { + return eventKey{tool: er.tool.ID, toolAgent: er.tool.Agent, toolSeq: er.tool.Seq} + } + return keyOf(er.event) +} + +// selectedPinKey is the pin key of the row under the cursor. +func (m *model) selectedPinKey() eventKey { + return pinKeyOf(m.selectedRowOn(m.eventsTbl.Cursor())) +} + // findByKey is the row that holds the event with key k, by either half (#1681). A pin can // name a response: a response-only row is keyed by it, and the row merges with its request // when the older page with the request loads. func findByKey(rows []eventRow, k eventKey) int { + if k.tool != "" { + return slices.IndexFunc(rows, func(r eventRow) bool { + return r.tool != nil && r.tool.ID == k.tool && r.tool.Agent == k.toolAgent && r.tool.Seq == k.toolSeq + }) + } return slices.IndexFunc(rows, func(r eventRow) bool { return rowHolds(r, func(e *pipeline.SessionEvent) bool { return keyOf(e) == k }) }) @@ -86,6 +108,10 @@ type eventRow struct { // pane fetched them for. raw *pipeline.RawCapture respRaw *pipeline.RawCapture + // tool is set on a tool row (#1776), which has no event. last is set on the last tool + // of its exchange. + tool *toolRow + last bool } // invocations returns every plugin invocation the row's ACTION/PLUGIN cell and the inactive @@ -259,6 +285,20 @@ func (m *model) rebuildEventsTable() { } } } + // Tool rows (#1776) join an exchange by its response's seq, only for the session the + // timeline is for: a linked bucket's events carry seqs of another session. + var tools map[uint64][]toolRow + if m.timeline.session == m.selectedSess { + tools = m.timeline.tools + } + toolsOf := func(er eventRow) []toolRow { + resp := exchangeResp(er) + if len(tools) == 0 || resp == nil || resp.Seq == 0 || eventSession(m.selectedSess, resp) != m.selectedSess { + return nil + } + return tools[resp.Seq] + } + now := m.clock() q := m.searchQuery(paneEvents) var full []table.SearchRow var droppedCols []eventColumn @@ -271,8 +311,9 @@ func (m *model) rebuildEventsTable() { // renderEventRow appends one event's cells to every parallel structure the // pane keeps — the table's rows, visibleRows, and the search rows — so the // two places a row can render from (its own loop turn, or its block's - // expansion below) cannot drift apart. - renderEventRow := func(j int) { + // expansion below) cannot drift apart. The exchange's tool rows follow it, and it + // returns them. + renderEventRow := func(j int) []toolRow { cc := ctxs[j] row := make(table.Row, 0, len(cols)) for _, c := range cols { @@ -293,6 +334,17 @@ func (m *model) rebuildEventsTable() { if q != "" { full = append(full, eventSearchRow(cols, droppedCols, cc)) } + ts := toolsOf(cc.row) + for i := range ts { + tr := eventRow{tool: &ts[i], last: i == len(ts)-1} + cells, sr := toolCells(cols, cc, tr, sortCol != nil, now) + rows = append(rows, cells) + m.visibleRows = append(m.visibleRows, tr) + if q != "" { + full = append(full, sr) + } + } + return ts } for j, cc := range ctxs { // hideInactive (the `s` toggle) is off by default — every message is @@ -347,7 +399,7 @@ func (m *model) rebuildEventsTable() { continue } - renderEventRow(j) + ts := renderEventRow(j) // Keyed from the SAME cellContext that just rendered the row, so the value // sorted on and the value displayed cannot come apart. cc.width stays zero // here — render takes its own copy — and no sortKey reads it. @@ -361,7 +413,11 @@ func (m *model) rebuildEventsTable() { // because blocks exist only while no sort is active (see the collection // above), so a row the sort needs cannot be skipped by a block. if sortCol != nil { - keys = append(keys, rowKey{val: sortCol.sortKey(cc), exchange: j}) + k := sortCol.sortKey(cc) + keys = append(keys, rowKey{val: k, exchange: j}) + for i := range ts { + keys = append(keys, rowKey{val: toolSortKey(sortCol.id, ts[i], k.numeric, now), exchange: j}) + } } } // Sort the OUTPUT, never the input. buildEventRows' tunnel fold, the backward @@ -443,7 +499,7 @@ func (m *model) rebuildEventsTable() { // zero-valued: a real pin, including one the eviction branch below deliberately // cleared, must not be overwritten. if m.sortCol != "" && m.selectedEventKey == (eventKey{}) { - m.selectedEventKey = keyOf(m.selectedEventOn(prevRow)) + m.selectedEventKey = pinKeyOf(m.selectedRowOn(prevRow)) } // An OPENING, not a refresh: the session under the table changed. This is the @@ -570,18 +626,18 @@ func sortEventRows(rows []table.Row, visible []eventRow, full []table.SearchRow, } } -// selectedEventOn returns the event at an arbitrary row of the rows just built, or -// nil when the row is out of range. +// selectedRowOn returns the row at an arbitrary row of the rows just built, or the zero +// eventRow when the row is out of range. // -// Distinct from selectedEvent, which reads the TABLE's cursor: during a rebuild the +// Distinct from selectedEventRow, which reads the TABLE's cursor: during a rebuild the // table has already been handed the new rows but the cursor has not been restored // yet, so the only meaningful row index is the one carried over from before the // rebuild. Used to seed the sort pin from that row. -func (m *model) selectedEventOn(row int) *pipeline.SessionEvent { +func (m *model) selectedRowOn(row int) eventRow { if row < 0 || row >= len(m.visibleRows) { - return nil + return eventRow{} } - return m.visibleRows[row].event + return m.visibleRows[row] } // selectedEvent returns the event at the cursor row, or nil. The cursor diff --git a/cmd/agentop/tui/events_rebuild_bench_test.go b/cmd/agentop/tui/events_rebuild_bench_test.go index 5dbe6a049..3cc82fcc5 100644 --- a/cmd/agentop/tui/events_rebuild_bench_test.go +++ b/cmd/agentop/tui/events_rebuild_bench_test.go @@ -8,6 +8,7 @@ import ( "github.com/rossoctl/cortex/core/cost/event" "github.com/rossoctl/cortex/core/pipeline" + "github.com/rossoctl/cortex/core/session/timeline" ) // BenchmarkRebuildEventsTable is one rebuild of a 2,000-event session in which every @@ -16,18 +17,22 @@ import ( // The cost record is what makes TOKENS and COST the expensive cells — each decodes it — // so a fixture without one would not show what evaluating them twice per row costs. // "searched" marks one row in ten, which costs one match per row on top of "plain": a search -// hides nothing, so every row is still rendered. +// hides nothing, so every row is still rendered. Each response also has tool rows from the +// timeline, and "sorted" sorts by DURATION, which ranks the tool rows among the exchanges. func BenchmarkRebuildEventsTable(b *testing.B) { for _, bc := range []struct { name string search string + sort eventColumnID }{ - {"plain", ""}, - {"searched", "h7.example"}, + {"plain", "", ""}, + {"searched", "h7.example", ""}, + {"sorted", "", colDuration}, } { b.Run(bc.name, func(b *testing.B) { m := rebuildBenchModel(b, 1000) m.search = map[paneID]string{paneEvents: bc.search} + m.sortCol = bc.sort b.ReportAllocs() for b.Loop() { m.rebuildEventsTable() @@ -38,7 +43,8 @@ func BenchmarkRebuildEventsTable(b *testing.B) { // rebuildBenchModel is an events pane over n inference exchanges, every response priced and // every other one carrying an estimated tool-prune saving. Hosts cycle through ten values so -// a search for one of them matches a tenth of the rows. +// a search for one of them matches a tenth of the rows. Each response asks for one tool, and +// every 20th response asks for two parallel tools; each tool ends 1s after its response. func rebuildBenchModel(b *testing.B, n int) *model { b.Helper() plain, err := json.Marshal(event.Event{CostUSD: 0.2767, Settled: true, PromptUSD: 0.2546, OutputUSD: 0.0221}) @@ -53,6 +59,7 @@ func rebuildBenchModel(b *testing.B, n int) *model { start := time.Date(2026, 10, 2, 13, 0, 0, 0, time.UTC) events := make([]pipeline.SessionEvent, 0, 2*n) + tl := &timeline.Timeline{Tools: make([]timeline.Tool, 0, n+n/20)} for i := range n { at := start.Add(time.Duration(i) * 3 * time.Second) host := fmt.Sprintf("h%d.example", i%10) @@ -63,17 +70,29 @@ func rebuildBenchModel(b *testing.B, n int) *model { } events = append(events, pipeline.SessionEvent{ At: at, SessionID: "s", Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, - Host: host, RequestID: id, + Host: host, RequestID: id, Seq: uint64(2*i + 1), Inference: &pipeline.InferenceExtension{Model: "claude-sonnet-5"}, }, pipeline.SessionEvent{ At: at.Add(2 * time.Second), SessionID: "s", Direction: pipeline.Outbound, Phase: pipeline.SessionResponse, - Host: host, StatusCode: 200, Duration: 2 * time.Second, RequestID: id, + Host: host, StatusCode: 200, Duration: 2 * time.Second, RequestID: id, Seq: uint64(2*i + 2), Inference: &pipeline.InferenceExtension{ Model: "claude-sonnet-5", InputTokens: 200_000 + i*100, OutputTokens: 400, }, Plugins: map[string]json.RawMessage{event.Key: rec}, }) + toolStart := at.Add(2 * time.Second) + toolEnd := toolStart.Add(time.Second) + tools := 1 + if i%20 == 0 { + tools = 2 + } + for k := range tools { + tl.Tools = append(tl.Tools, timeline.Tool{ + Agent: "main", ID: fmt.Sprintf("t%d-%d", i, k), Name: "Bash", + Start: toolStart, End: &toolEnd, Preview: "git -C /repo status", Seq: uint64(2*i + 2), + }) + } } m := &model{ pane: paneEvents, selectedSess: "s", @@ -81,7 +100,11 @@ func rebuildBenchModel(b *testing.B, n int) *model { events: map[string][]pipeline.SessionEvent{"s": events}, eventColumns: defaultColumnSelection(), } + m.timeline.session, m.timeline.tl, m.timeline.tools = "s", tl, indexTools(tl) m.eventsTbl = newEventsTable() m.rebuildEventsTable() + if got, want := len(m.eventsTbl.Rows()), n+len(tl.Tools); got != want { + b.Fatalf("rows = %d, want %d (each exchange and each tool)", got, want) + } return m } diff --git a/cmd/agentop/tui/events_timeline_test.go b/cmd/agentop/tui/events_timeline_test.go new file mode 100644 index 000000000..2f025ab0a --- /dev/null +++ b/cmd/agentop/tui/events_timeline_test.go @@ -0,0 +1,302 @@ +package tui + +import ( + "slices" + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/rossoctl/cortex/cmd/agentop/apiclient" + "github.com/rossoctl/cortex/core/pipeline" + "github.com/rossoctl/cortex/core/session" +) + +// eventsTimelineModel is a model on the sessions pane with sessions "a" and "b" listed and +// none open. +func eventsTimelineModel(t *testing.T) *model { + t.Helper() + m := fitModel(t, paneSessions, 120, 40, nil) + m.client = deadClient() + m.selectedSess = "" + m.sessionsData = nil + m.sessions = []session.SessionSummary{{ID: "a", UpdatedAt: tlAt(2)}, {ID: "b", UpdatedAt: tlAt(1)}} + m.timeline = timelineState{cursor: -1} + m.rebuildSessionsTable() + return m +} + +// openSession presses Enter on the sessions-pane row of id. +func openSession(t *testing.T, m *model, id string) tea.Cmd { + t.Helper() + m.pane = paneSessions + m.rebuildSessionsTable() + for i, rid := range m.sessionRowIDs { + if rid == id { + m.sessionsTbl.SetCursor(i) + return m.handleKey(tea.KeyMsg{Type: tea.KeyEnter}) + } + } + t.Fatalf("session %q is not in the sessions table: %v", id, m.sessionRowIDs) + return nil +} + +func TestEventsTimeline_ChainStartsOnEventsPane(t *testing.T) { + m := eventsTimelineModel(t) + if cmd := openSession(t, m, "a"); cmd == nil { + t.Fatalf("Enter on a session returned no cmd") + } + if m.pane != paneEvents || m.timeline.session != m.selectedSess || m.selectedSess != "a" { + t.Fatalf("pane %v timeline session %q selected %q", m.pane, m.timeline.session, m.selectedSess) + } + if cmd := m.handleTimelineTick(timelineTickMsg{gen: m.timeline.tickGen}); cmd == nil { + t.Errorf("a tick on the events pane did not schedule the next one") + } +} + +func TestEventsTimeline_StreamMarksStale(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + m.timeline.stale = false + m.handleStreamEvent(apiclient.StreamEvent{Event: &pipeline.SessionEvent{SessionID: "b", At: tlAt(3)}}) + if m.timeline.stale { + t.Errorf("an event of another session marked the timeline stale") + } + m.handleStreamEvent(apiclient.StreamEvent{Event: &pipeline.SessionEvent{SessionID: "a", At: tlAt(3)}}) + if !m.timeline.stale { + t.Errorf("an event of the open session did not mark the timeline stale") + } +} + +func TestEventsTimeline_NotFoundGivesNoError(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + m.handleTimelineLoaded(timelineLoadedMsg{req: m.timeline.reqSeq, err: apiclient.ErrNotFound}) + if m.timeline.tl != nil { + t.Errorf("a 404 left a timeline") + } + if out := strings.ToLower(stripANSI(m.View())); strings.Contains(out, "timeline") { + t.Errorf("the events view shows the timeline failure:\n%s", out) + } +} + +func TestEventsTimeline_TickStopsOffPane(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + m.pane = paneSessions + if cmd := m.handleTimelineTick(timelineTickMsg{gen: m.timeline.tickGen}); cmd != nil { + t.Errorf("a tick on the sessions pane scheduled another") + } +} + +func TestEventsTimeline_EscFromTimelineResumes(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + m.openTimeline(m.selectedSess) + if m.pane != paneTimeline { + t.Fatalf("pane %v, want the timeline", m.pane) + } + cmd := m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}) + if m.pane != paneEvents || m.timeline.session != m.selectedSess || cmd == nil { + t.Errorf("pane %v timeline session %q selected %q cmd %v", m.pane, m.timeline.session, m.selectedSess, cmd) + } +} + +func TestEventsTimeline_SessionChangeDropsTools(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + m.handleTimelineLoaded(timelineLoadedMsg{req: m.timeline.reqSeq, tl: sampleTimeline()}) + if m.timeline.tl == nil { + t.Fatalf("the timeline of a did not load") + } + openSession(t, m, "b") + if m.timeline.tl != nil || m.timeline.session != "b" { + t.Errorf("opening b kept the timeline of a: session %q", m.timeline.session) + } +} + +// Esc from a pane opened over the events pane starts the stopped tick again. +func TestEventsTimeline_EscFromUsageResumes(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + gen := m.timeline.tickGen + m.pane, m.usage.returnPane = paneUsage, paneEvents + if cmd := m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}); m.pane != paneEvents || cmd == nil || m.timeline.tickGen == gen { + t.Errorf("pane %v cmd %v tickGen %d (was %d)", m.pane, cmd, m.timeline.tickGen, gen) + } +} + +// The Agents overlay (A) returns from its own Esc arm, before the check above: it must also +// start the fetch again, or the tool rows stop updating. +func TestEventsTimeline_EscFromAgentsResumes(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + gen := m.timeline.tickGen + m.pane, m.previousPane = paneAgents, paneEvents + if cmd := m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}); m.pane != paneEvents || cmd == nil || m.timeline.tickGen == gen { + t.Errorf("pane %v cmd %v tickGen %d (was %d)", m.pane, cmd, m.timeline.tickGen, gen) + } +} + +// A cached-only session has no timeline on the server: opening one fetches nothing, but the +// timeline drops the previous session, so that session is not kept. +func TestEventsTimeline_CachedOnlySessionFetchesNothing(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + m.events["gone"] = []pipeline.SessionEvent{{SessionID: "gone", At: tlAt(1)}} + m.sessions = m.sessions[:1] + m.sessionsData = map[string]SessionMetadata{} + m.pane = paneSessions + m.rebuildSessionsTable() + if !slices.Contains(m.sessionRowIDs, "gone") { + t.Fatalf("cached-only session not listed: %v", m.sessionRowIDs) + } + if cmd := openSession(t, m, "gone"); cmd != nil || m.timeline.session != "gone" { + t.Errorf("cmd %v timeline session %q", cmd, m.timeline.session) + } +} + +// Esc back to the events pane of a cached-only session must not fetch: the 404 would retry on +// its backoff for as long as the pane is open. +func TestEventsTimeline_EscOnCachedOnlySessionFetchesNothing(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + m.events["gone"] = []pipeline.SessionEvent{{SessionID: "gone", At: tlAt(1)}} + m.sessions = m.sessions[:1] + m.sessionsData = map[string]SessionMetadata{} + openSession(t, m, "gone") + m.pane, m.usage.returnPane = paneUsage, paneEvents + if m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}); m.pane != paneEvents || m.timeline.loading { + t.Errorf("Esc from Usage: pane %v loading %v", m.pane, m.timeline.loading) + } + m.pane, m.previousPane = paneAgents, paneEvents + if m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}); m.pane != paneEvents || m.timeline.loading { + t.Errorf("Esc from Agents: pane %v loading %v", m.pane, m.timeline.loading) + } +} + +// The Timeline pane of a cached-only session fetches nothing and says why, and Esc from it +// back to the events pane fetches nothing too. +func TestEventsTimeline_TimelineOfCachedOnlySessionFetchesNothing(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + m.events["gone"] = []pipeline.SessionEvent{{SessionID: "gone", At: tlAt(1)}} + m.sessions = m.sessions[:1] + m.sessionsData = map[string]SessionMetadata{} + openSession(t, m, "gone") + if m.handleKey(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("T")}); m.pane != paneTimeline || m.timeline.loading { + t.Fatalf("T: pane %v loading %v", m.pane, m.timeline.loading) + } + if out := stripANSI(m.renderTimeline()); !strings.Contains(out, "no longer has this session") { + t.Errorf("the Timeline pane does not say why it is empty:\n%s", out) + } + if m.handleTimelineTick(timelineTickMsg{gen: m.timeline.tickGen}); m.timeline.loading { + t.Errorf("a tick on the Timeline pane of a cached-only session fetched") + } + if m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}); m.pane != paneEvents || m.timeline.loading { + t.Errorf("Esc from the Timeline: pane %v loading %v", m.pane, m.timeline.loading) + } +} + +// A timeline reply for the previous session, still in flight when a cached-only session opens, +// must not land on the new session, and the previous tick chain must stop. +func TestEventsTimeline_CachedOnlyDropsInFlightReply(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + inFlight, oldGen := m.timeline.reqSeq, m.timeline.tickGen + m.events["gone"] = []pipeline.SessionEvent{{SessionID: "gone", At: tlAt(1)}} + m.sessions = m.sessions[:1] + m.sessionsData = map[string]SessionMetadata{} + openSession(t, m, "gone") + m.handleTimelineLoaded(timelineLoadedMsg{req: inFlight, tl: sampleTimeline()}) + if m.timeline.tl != nil || m.timeline.loading { + t.Errorf("the reply for a landed on the cached-only session: tl %v loading %v", m.timeline.tl != nil, m.timeline.loading) + } + m.timeline.stale = true + if cmd := m.handleTimelineTick(timelineTickMsg{gen: oldGen}); cmd != nil { + t.Errorf("the tick chain of a kept running on the cached-only session") + } +} + +// A session list that omits the open session for one refresh (#870) must not end the tick +// chain: when the list holds the session again, a stream event still gets a fetch. +func TestEventsTimeline_ChainSurvivesAListBlip(t *testing.T) { + for _, pane := range []paneID{paneTimeline, paneEvents} { + m := eventsTimelineModel(t) + openSession(t, m, "a") + if pane == paneTimeline { + m.openTimeline("a") + } + m.timeline.loading = false + m.Update(sessionsLoadedMsg{}) + cmd := m.handleTimelineTick(timelineTickMsg{gen: m.timeline.tickGen}) + if cmd == nil { + t.Fatalf("pane %v: a tick during the blip scheduled no next tick", pane) + } + if m.timeline.loading { + t.Errorf("pane %v: a tick fetched a session the proxy does not list", pane) + } + m.Update(sessionsLoadedMsg{{ID: "a", UpdatedAt: tlAt(2)}}) + m.handleStreamEvent(apiclient.StreamEvent{Event: &pipeline.SessionEvent{SessionID: "a", At: tlAt(3)}}) + m.handleTimelineTick(timelineTickMsg{gen: m.timeline.tickGen}) + if !m.timeline.loading { + t.Errorf("pane %v: after the blip, a stream event got no fetch", pane) + } + } +} + +// A timeline that was never fetched, because the list omitted the session when the pane opened +// (#870), fetches on the first tick after the list holds the session again, with no stream event. +func TestEventsTimeline_FirstFetchAfterAListBlip(t *testing.T) { + for _, pane := range []paneID{paneTimeline, paneEvents} { + m := eventsTimelineModel(t) + m.selectedSess, m.pane = "a", paneEvents + m.Update(sessionsLoadedMsg{}) + if pane == paneTimeline { + m.openTimeline("a") + } else { + m.followEventsTimeline() + } + if m.timeline.loading { + t.Fatalf("pane %v: opening during the blip fetched a session the proxy does not list", pane) + } + m.handleTimelineTick(timelineTickMsg{gen: m.timeline.tickGen}) + if m.timeline.loading { + t.Errorf("pane %v: a tick during the blip fetched", pane) + } + m.Update(sessionsLoadedMsg{{ID: "a", UpdatedAt: tlAt(2)}}) + m.handleTimelineTick(timelineTickMsg{gen: m.timeline.tickGen}) + if !m.timeline.loading { + t.Errorf("pane %v: after the blip, a tick did not fetch the never-fetched timeline", pane) + } + } +} + +// A proxy with no timeline endpoint: a stream event does not refetch on every tick. +func TestEventsTimeline_UnsupportedStopsStaleFetch(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + m.handleTimelineLoaded(timelineLoadedMsg{req: m.timeline.reqSeq, err: apiclient.ErrNotFound}) + m.handleStreamEvent(apiclient.StreamEvent{Event: &pipeline.SessionEvent{SessionID: "a", At: tlAt(3)}}) + if cmd := m.handleTimelineTick(timelineTickMsg{gen: m.timeline.tickGen}); cmd == nil || m.timeline.loading { + t.Errorf("unsupported endpoint: tick cmd %v loading %v, want the chain without a fetch", cmd, m.timeline.loading) + } +} + +// Esc back to the events pane while the list omits the session (#870) starts the chain, so a +// stream event fetches once the list holds the session again. +func TestEventsTimeline_EscDuringAListBlipStartsTheChain(t *testing.T) { + m := eventsTimelineModel(t) + openSession(t, m, "a") + m.Update(sessionsLoadedMsg{}) + m.pane, m.usage.returnPane = paneUsage, paneEvents + m.timeline.loading = false + if cmd := m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}); cmd == nil || m.timeline.loading { + t.Fatalf("Esc during the blip: cmd %v loading %v, want the chain without a fetch", cmd, m.timeline.loading) + } + m.Update(sessionsLoadedMsg{{ID: "a", UpdatedAt: tlAt(2)}}) + m.handleStreamEvent(apiclient.StreamEvent{Event: &pipeline.SessionEvent{SessionID: "a", At: tlAt(3)}}) + if m.handleTimelineTick(timelineTickMsg{gen: m.timeline.tickGen}); !m.timeline.loading { + t.Errorf("after the blip, a stream event got no fetch") + } +} diff --git a/cmd/agentop/tui/events_tool_rows_test.go b/cmd/agentop/tui/events_tool_rows_test.go new file mode 100644 index 000000000..e92238b48 --- /dev/null +++ b/cmd/agentop/tui/events_tool_rows_test.go @@ -0,0 +1,402 @@ +package tui + +import ( + "slices" + "strings" + "testing" + "time" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/rossoctl/cortex/core/pipeline" + "github.com/rossoctl/cortex/core/session/timeline" +) + +// trExchange is exchange n of the session: a request at `at` seconds (seq 2n-1) and its +// response two seconds later (seq 2n), from subagent sub ("" for the main agent). +func trExchange(n int, at float64, sub string) []pipeline.SessionEvent { + id := "r" + string(rune('a'+n)) + return []pipeline.SessionEvent{ + {At: tlAt(at), Seq: uint64(2*n - 1), Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, + Host: "api.test", RequestID: id, Subagent: sub, Inference: &pipeline.InferenceExtension{Model: "claude-opus-5-5"}}, + {At: tlAt(at + 2), Seq: uint64(2 * n), Direction: pipeline.Outbound, Phase: pipeline.SessionResponse, + Host: "api.test", RequestID: id, Subagent: sub, StatusCode: 200, Duration: 2 * time.Second, + Inference: &pipeline.InferenceExtension{Model: "claude-opus-5-5"}}, + } +} + +// trTool is a tool that the response with seq asked for, from start to end seconds (end < 0: +// still running). +func trTool(seq uint64, id, name string, start, end float64, preview string) timeline.Tool { + t := timeline.Tool{Agent: timeline.MainAgent, ID: id, Name: name, Start: tlAt(start), Preview: preview, Seq: seq} + if end >= 0 { + t.End = tlPtr(end) + } + return t +} + +// toolRowsModel is the events pane on session "sess-1" with tl loaded for it, at 100s. +func toolRowsModel(t *testing.T, events []pipeline.SessionEvent, tl *timeline.Timeline) *model { + t.Helper() + m := fitModel(t, paneEvents, 200, 40, events) + m.now = func() time.Time { return tlAt(100) } + m.timeline = timelineState{session: m.selectedSess, cursor: -1, tl: tl, tools: indexTools(tl)} + m.rebuildEventsTable() + return m +} + +// colPos is the position of column id in the rows just built, -1 when not shown. +func colPos(m *model, id eventColumnID) int { + cols, _ := layoutColumns(m.eventColumns, m.eventColWidths, m.width) + return slices.IndexFunc(cols, func(c eventColumn) bool { return c.id == id }) +} + +// toolRowAt is the rendered row of the tool with id, and its index. +func toolRowAt(t *testing.T, m *model, id string) (int, []string) { + t.Helper() + i := slices.IndexFunc(m.visibleRows, func(r eventRow) bool { return r.tool != nil && r.tool.ID == id }) + if i < 0 { + t.Fatalf("no row for tool %q", id) + } + return i, m.eventsTbl.Rows()[i] +} + +func toolCell(m *model, row []string, id eventColumnID) string { + return strings.TrimSpace(row[colPos(m, id)]) +} + +func trailingOf(row []string) string { return row[len(row)-1] } + +func TestToolRows_OneTool(t *testing.T) { + events := slices.Concat(trExchange(1, 10, ""), trExchange(2, 30, "")) + m := toolRowsModel(t, events, &timeline.Timeline{Tools: []timeline.Tool{ + trTool(2, "t1", "Bash", 12, 16.21, "git -C /tmp/wt status"), + }}) + i, row := toolRowAt(t, m, "t1") + if m.visibleRows[i-1].resp == nil || m.visibleRows[i-1].resp.Seq != 2 { + t.Fatalf("the tool row is not right after its exchange: %v", m.visibleRows[i-1]) + } + for id, want := range map[eventColumnID]string{ + colIndex: "┗⚒", colDir: "", colTime: "12:00:12.00", colAction: "", colPlugin: "", + colMethod: "Bash", colStatus: "ok", colDuration: "4.21s", + } { + if got := toolCell(m, row, id); got != want { + t.Errorf("%s = %q, want %q", id, got, want) + } + } + if got := trailingOf(row); got != "git -C /tmp/wt status" { + t.Errorf("trailing = %q", got) + } + if want := colPos(m, colDuration) + 2; len(row) != want { + t.Errorf("cells = %d, want %d", len(row), want) + } +} + +func TestToolRows_Parallel(t *testing.T) { + m := toolRowsModel(t, trExchange(1, 10, ""), &timeline.Timeline{Tools: []timeline.Tool{ + trTool(2, "t1", "Bash", 12, 25.21, "a"), + trTool(2, "t2", "Read", 12, 25.21, "b"), + }}) + _, r1 := toolRowAt(t, m, "t1") + _, r2 := toolRowAt(t, m, "t2") + if toolCell(m, r1, colIndex) != "┣⚒" || toolCell(m, r2, colIndex) != "┗⚒" { + t.Errorf("# cells = %q, %q", toolCell(m, r1, colIndex), toolCell(m, r2, colIndex)) + } + for _, r := range [][]string{r1, r2} { + if d := toolCell(m, r, colDuration); !strings.HasPrefix(d, "∥") { + t.Errorf("DURATION = %q, want a ∥ prefix", d) + } + } +} + +func TestToolRows_States(t *testing.T) { + running := trTool(2, "run", "Bash", 12, -1, "") + failed := trTool(2, "err", "Bash", 12, 13, "") + failed.Error = true + lost := trTool(2, "lost", "Bash", 12, 14, "") + lost.Lost = true + end := trTool(2, "end", "StructuredOutput", 12, 15, "done") + end.Lost = true + m := toolRowsModel(t, trExchange(1, 10, ""), &timeline.Timeline{Tools: []timeline.Tool{running, failed, lost, end}}) + for _, c := range []struct{ id, status, duration, trailing string }{ + {"run", "running", "⋯ 1m28s", ""}, + {"err", "error", "1.00s", ""}, + {"lost", "lost", "", ""}, + {"end", "", "", "ends run · done"}, + } { + _, row := toolRowAt(t, m, c.id) + if got := toolCell(m, row, colStatus); got != c.status { + t.Errorf("%s: STATUS = %q, want %q", c.id, got, c.status) + } + if got := toolCell(m, row, colDuration); got != c.duration { + t.Errorf("%s: DURATION = %q, want %q", c.id, got, c.duration) + } + if got := trailingOf(row); got != c.trailing { + t.Errorf("%s: trailing = %q, want %q", c.id, got, c.trailing) + } + } +} + +func TestToolRows_RunAndBg(t *testing.T) { + m := toolRowsModel(t, trExchange(1, 10, ""), &timeline.Timeline{ + Tools: []timeline.Tool{ + trTool(2, "spawn", "Agent", 12, 13, "explore"), + trTool(2, "bash", "Bash", 12, 14, "go test"), + }, + Agents: []timeline.Agent{{ID: "a1", SpawnTool: "spawn", SpawnedAt: tlPtr(12), ReturnedAt: tlPtr(60)}}, + Jobs: []timeline.Job{{Agent: timeline.MainAgent, Start: tlAt(14), End: tlPtr(20)}}, + }) + if _, row := toolRowAt(t, m, "spawn"); !strings.HasPrefix(trailingOf(row), "run ") { + t.Errorf("Agent trailing = %q", trailingOf(row)) + } + if _, row := toolRowAt(t, m, "bash"); !strings.HasPrefix(trailingOf(row), "bg ") { + t.Errorf("Bash trailing = %q", trailingOf(row)) + } +} + +func TestToolRows_InExpandedBlock(t *testing.T) { + events := slices.Concat(trExchange(1, 10, ""), trExchange(2, 20, "a1"), trExchange(3, 30, "")) + tl := &timeline.Timeline{Tools: []timeline.Tool{trTool(4, "sub", "Read", 22, 23, "x")}} + tl.Tools[0].Agent = "a1" + m := toolRowsModel(t, events, tl) + if slices.ContainsFunc(m.visibleRows, func(r eventRow) bool { return r.tool != nil }) { + t.Fatalf("a collapsed block shows a tool row") + } + m.subagentOpen = map[string]bool{"a1": true} + m.rebuildEventsTable() + i, row := toolRowAt(t, m, "sub") + if r := m.visibleRows[i-1]; r.resp == nil || r.resp.Seq != 4 { + t.Errorf("the tool row is not right after its member exchange") + } + if got := toolCell(m, row, colDir); got != "│" { + t.Errorf("DIR = %q, want the block gutter", got) + } +} + +func TestToolRows_HideInactiveHidesTools(t *testing.T) { + events := slices.Concat(trExchange(1, 10, ""), trExchange(2, 20, "")) + events[2].Invocations = &pipeline.Invocations{Outbound: []pipeline.Invocation{{Plugin: "p", Action: pipeline.ActionAllow}}} + m := toolRowsModel(t, events, &timeline.Timeline{Tools: []timeline.Tool{ + trTool(2, "hidden", "Bash", 12, 13, ""), + trTool(4, "shown", "Bash", 22, 23, ""), + }}) + m.hideInactive = true + m.rebuildEventsTable() + toolRowAt(t, m, "shown") + if slices.ContainsFunc(m.visibleRows, func(r eventRow) bool { return r.tool != nil && r.tool.ID == "hidden" }) { + t.Errorf("the tool of a hidden exchange still shows") + } +} + +func TestToolRows_SortByDuration(t *testing.T) { + events := slices.Concat(trExchange(1, 10, ""), trExchange(2, 20, "")) + events[1].Duration = 4400 * time.Millisecond + m := toolRowsModel(t, events, &timeline.Timeline{Tools: []timeline.Tool{trTool(4, "long", "Bash", 62, -1, "")}}) + m.sortCol, m.sortDesc = colDuration, true + m.rebuildEventsTable() + i, row := toolRowAt(t, m, "long") + if i != 0 { + t.Errorf("the 38s tool is row %d, want 0 above the 4.4s exchange", i) + } + if got := toolCell(m, row, colIndex); got != "↳2" { + t.Errorf("# = %q, want ↳2", got) + } +} + +func TestToolRows_SortOtherColumnIsStable(t *testing.T) { + events := slices.Concat(trExchange(1, 10, ""), trExchange(2, 20, "")) + m := toolRowsModel(t, events, &timeline.Timeline{Tools: []timeline.Tool{ + trTool(2, "a", "Bash", 12, 13, ""), + trTool(4, "b", "Bash", 22, 23, ""), + }}) + m.sortCol = colCost + m.rebuildEventsTable() + var got []string + for _, r := range m.visibleRows { + switch { + case r.tool != nil: + got = append(got, r.tool.ID) + case r.resp != nil: + got = append(got, r.resp.RequestID) + } + } + if want := []string{"rb", "a", "rc", "b"}; !slices.Equal(got, want) { + t.Errorf("order = %v, want %v", got, want) + } +} + +func TestToolRows_SearchFindsPreview(t *testing.T) { + m := toolRowsModel(t, trExchange(1, 10, ""), &timeline.Timeline{Tools: []timeline.Tool{ + trTool(2, "t1", "Bash", 12, 13, "git -C /tmp/wt status"), + }}) + m.search = map[paneID]string{paneEvents: "git -C"} + m.rebuildEventsTable() + i, _ := toolRowAt(t, m, "t1") + if !slices.Contains(m.eventsTbl.Matches(), i) { + t.Errorf("matches = %v, want row %d", m.eventsTbl.Matches(), i) + } +} + +// With no column after DURATION and no METHOD shown, the preview and the tool name are not +// in any cell, so search must still find them in the row's dropped text. +func TestToolRows_SearchFindsHiddenText(t *testing.T) { + m := toolRowsModel(t, trExchange(1, 10, ""), &timeline.Timeline{Tools: []timeline.Tool{ + trTool(2, "t1", "Bash", 12, 13, "git -C /tmp/wt status"), + }}) + for _, id := range []eventColumnID{colBytes, colTokens, colCost, colHost, colMethod} { + m.eventColumns[id] = false + } + for _, q := range []string{"git -C", "Bash"} { + m.search = map[paneID]string{paneEvents: q} + m.rebuildEventsTable() + if p := colPos(m, colMethod); p >= 0 { + t.Fatalf("METHOD still shown at %d", p) + } + i, row := toolRowAt(t, m, "t1") + if strings.Contains(strings.Join(row, " "), q) { + t.Fatalf("query %q is in a shown cell %q; the test needs it hidden", q, row) + } + if !slices.Contains(m.eventsTbl.Matches(), i) { + t.Errorf("query %q: matches = %v, want row %d", q, m.eventsTbl.Matches(), i) + } + } +} + +func TestToolRows_EnterDoesNothing(t *testing.T) { + m := toolRowsModel(t, trExchange(1, 10, ""), &timeline.Timeline{Tools: []timeline.Tool{trTool(2, "t1", "Bash", 12, 13, "")}}) + i, _ := toolRowAt(t, m, "t1") + setCursorVisible(&m.eventsTbl, i) + if cmd := m.handleKey(tea.KeyMsg{Type: tea.KeyEnter}); cmd != nil { + t.Errorf("Enter on a tool row returned a cmd") + } + if m.pane != paneEvents || m.selectedEvent() != nil { + t.Errorf("pane %v, selected event %v", m.pane, m.selectedEvent()) + } +} + +func TestToolRows_CursorStaysOnTool(t *testing.T) { + events := slices.Concat(trExchange(1, 10, ""), trExchange(2, 20, "")) + m := toolRowsModel(t, events, &timeline.Timeline{Tools: []timeline.Tool{trTool(2, "t1", "Bash", 12, 13, "")}}) + i, _ := toolRowAt(t, m, "t1") + setCursorVisible(&m.eventsTbl, i) + m.selectedEventKey = m.selectedPinKey() + want := m.selectedEventKey + m.events[m.selectedSess] = append(m.events[m.selectedSess], trExchange(3, 30, "")...) + m.rebuildEventsTable() + if got := m.selectedPinKey(); got != want { + t.Fatalf("after a new event the cursor is on %v, want %v", got, want) + } + m.sortCol, m.sortDesc = colTime, true + m.rebuildEventsTable() + if got := m.selectedPinKey(); got != want { + t.Errorf("after a sort the cursor is on %v, want %v", got, want) + } +} + +func TestToolRows_TailFollowOnToolRow(t *testing.T) { + m := toolRowsModel(t, trExchange(1, 10, ""), &timeline.Timeline{Tools: []timeline.Tool{trTool(2, "t1", "Bash", 12, 13, "")}}) + setCursorVisible(&m.eventsTbl, len(m.eventsTbl.Rows())-1) + m.events[m.selectedSess] = append(m.events[m.selectedSess], trExchange(2, 20, "")...) + tl := &timeline.Timeline{Tools: []timeline.Tool{trTool(2, "t1", "Bash", 12, 13, ""), trTool(4, "t2", "Bash", 22, 23, "")}} + m.timeline.tl, m.timeline.tools = tl, indexTools(tl) + m.rebuildEventsTable() + if c, n := m.eventsTbl.Cursor(), len(m.eventsTbl.Rows()); c != n-1 { + t.Errorf("cursor %d, want the last row %d", c, n-1) + } + if m.selectedEvent() != nil { + t.Errorf("a tool row has an event") + } +} + +func TestToolRows_TimelineBeforeEvents(t *testing.T) { + m := toolRowsModel(t, nil, &timeline.Timeline{Tools: []timeline.Tool{trTool(2, "t1", "Bash", 12, 13, "")}}) + if len(m.visibleRows) != 0 { + t.Fatalf("rows with no events: %d", len(m.visibleRows)) + } + m.events[m.selectedSess] = trExchange(1, 10, "") + m.rebuildEventsTable() + toolRowAt(t, m, "t1") +} + +func TestToolRows_OrphanToolNotShown(t *testing.T) { + m := toolRowsModel(t, trExchange(1, 10, ""), &timeline.Timeline{Tools: []timeline.Tool{trTool(99, "t1", "Bash", 12, 13, "")}}) + if slices.ContainsFunc(m.visibleRows, func(r eventRow) bool { return r.tool != nil }) { + t.Errorf("a tool with no exchange shows") + } +} + +func TestToolRows_NarrowTerminalCellCount(t *testing.T) { + m := toolRowsModel(t, trExchange(1, 10, ""), &timeline.Timeline{Tools: []timeline.Tool{trTool(2, "t1", "Bash", 12, 13, "x")}}) + for _, w := range []int{40, 60, 80} { + m.width = w + m.rebuildEventsTable() + toolRowAt(t, m, "t1") + for i, row := range m.eventsTbl.Rows() { + if len(row) > len(m.eventsTbl.Columns()) { + t.Errorf("width %d row %d: %d cells, %d columns", w, i, len(row), len(m.eventsTbl.Columns())) + } + } + _ = m.eventsTbl.View() + } +} + +func TestToolRows_DetailSearchSkipsToolRows(t *testing.T) { + m := toolRowsModel(t, trExchange(1, 10, ""), &timeline.Timeline{Tools: []timeline.Tool{trTool(2, "t1", "Bash", 12, 13, "")}}) + ti, _ := toolRowAt(t, m, "t1") + m.detailSearch = &detailSearch{ + hits: map[eventKey]bool{keyOf(m.visibleRows[0].event): true}, + last: map[string]uint64{m.selectedSess: 1}, oldest: map[string]uint64{m.selectedSess: 1}, + newest: map[string]uint64{m.selectedSess: 0}, + } + _ = m.detailSearchStale() + for _, rows := range [][]int{m.detailMatchRows(), m.detailNewRows()} { + if slices.Contains(rows, ti) { + t.Errorf("rows %v hold the tool row %d", rows, ti) + } + } +} + +func TestToolRows_LinkedEventNoTools(t *testing.T) { + events := trExchange(1, 10, "") + events[0].SessionID, events[1].SessionID = "bucket", "bucket" + m := toolRowsModel(t, events, &timeline.Timeline{Tools: []timeline.Tool{trTool(2, "t1", "Bash", 12, 13, "")}}) + if slices.ContainsFunc(m.visibleRows, func(r eventRow) bool { return r.tool != nil }) { + t.Errorf("a linked exchange got the session's tool") + } +} + +// The model writes a tool's name and preview: a control rune in either must not reach the +// terminal, where an OSC 52 sequence would write the clipboard. +func TestToolRows_ControlRunesRemoved(t *testing.T) { + events := slices.Concat(trExchange(1, 10, ""), trExchange(2, 30, "")) + m := toolRowsModel(t, events, &timeline.Timeline{Tools: []timeline.Tool{ + trTool(2, "t1", "Bash\x1b[2J", 12, 16, "echo \x1b]52;c;aGk=\x07"), + }}) + _, row := toolRowAt(t, m, "t1") + for _, cell := range []string{toolCell(m, row, colMethod), trailingOf(row)} { + if strings.ContainsAny(cell, "\x1b\x07") { + t.Errorf("control rune in cell %q", cell) + } + } +} + +// Under a sort, a tool row of exchange 1000 or more shows its parent's number whole: the +// # column is 4 wide, so ↳ gives way to the number. +func TestToolRows_SortedParentNumberFits(t *testing.T) { + var events []pipeline.SessionEvent + for n := 1; n <= 1000; n++ { + events = append(events, trExchange(n, float64(n*3), "")...) + } + m := toolRowsModel(t, events, &timeline.Timeline{Tools: []timeline.Tool{ + trTool(2000, "big", "Bash", 3002, 3003, ""), + trTool(1998, "small", "Bash", 2999, 3000, ""), + }}) + m.sortCol, m.sortDesc = colDuration, true + m.rebuildEventsTable() + if _, row := toolRowAt(t, m, "big"); toolCell(m, row, colIndex) != "1000" { + t.Errorf("exchange 1000: # = %q, want 1000", toolCell(m, row, colIndex)) + } + if _, row := toolRowAt(t, m, "small"); toolCell(m, row, colIndex) != "↳999" { + t.Errorf("exchange 999: # = %q, want ↳999", toolCell(m, row, colIndex)) + } +} diff --git a/cmd/agentop/tui/help_overlay.go b/cmd/agentop/tui/help_overlay.go index 5dcdf3a8d..eef049acd 100644 --- a/cmd/agentop/tui/help_overlay.go +++ b/cmd/agentop/tui/help_overlay.go @@ -330,6 +330,7 @@ var paneKeys = map[paneID]keyGroup{ {"/ then tab", "search each row's details: ↵, n and N find the next match"}, {"c", "column picker (checkboxes + descriptions)"}, {"c then s", "sort by a column: desc → asc → chronological"}, + {"⚒ rows", "tool calls. DURATION, for a tool: from the reply to the next request of the same agent"}, {"↵ on ▲ row", "load the page before the oldest event shown"}, {"t", "back to the live tail (resumes updates)"}, {"esc / ← / h", "back to sessions"}, diff --git a/cmd/agentop/tui/help_overlay_test.go b/cmd/agentop/tui/help_overlay_test.go index fd50c81a5..35cf2f6b9 100644 --- a/cmd/agentop/tui/help_overlay_test.go +++ b/cmd/agentop/tui/help_overlay_test.go @@ -554,7 +554,7 @@ func TestHelpOverlayScrollKeys(t *testing.T) { // that no longer existed. Too small fails loudly; too large fails silently. const ( helpWideTerminal = 100 - helpNoScrollHeight = 119 + helpNoScrollHeight = 121 ) // With everything visible there must be no scroll affordance — it would be noise @@ -715,3 +715,20 @@ func TestPaneKeys_TheAgentsNoteSaysWhatTheScopeReaches(t *testing.T) { } } } + +// The events help and the DURATION column both say how a tool's time is measured. +func TestHelp_EventsExplainsToolTime(t *testing.T) { + const phrase = "for a tool: from the reply to the next request of the same agent" + var help []string + for _, b := range paneKeys[paneEvents].bindings { + help = append(help, b.desc) + } + if !strings.Contains(strings.Join(help, "\n"), phrase) { + t.Errorf("the events help does not contain %q", phrase) + } + for _, c := range eventColumns { + if c.id == colDuration && !strings.Contains(c.desc, phrase) { + t.Errorf("the DURATION desc %q does not contain %q", c.desc, phrase) + } + } +} diff --git a/cmd/agentop/tui/history_test.go b/cmd/agentop/tui/history_test.go index fe428c529..fdfa5f1df 100644 --- a/cmd/agentop/tui/history_test.go +++ b/cmd/agentop/tui/history_test.go @@ -113,7 +113,11 @@ func TestHistory_AnArchivedSessionIsListedAndOpens(t *testing.T) { if cmd == nil { t.Fatal("Enter on an archived row fetched nothing") } - m.Update(cmd()) + msg := cmd() + if b, ok := msg.(tea.BatchMsg); ok { + msg = b[0]() // the snapshot; the timeline fetch for the tool rows comes after it + } + m.Update(msg) if n := len(m.events["old"]); n != 4 { t.Fatalf("the archived timeline holds %d events, want 4", n) } diff --git a/cmd/agentop/tui/keys.go b/cmd/agentop/tui/keys.go index 975265178..3a0f8caf1 100644 --- a/cmd/agentop/tui/keys.go +++ b/cmd/agentop/tui/keys.go @@ -511,6 +511,7 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { // picker mode it backs out further to the Pods picker, tearing down PF + // SSE. It is the ONLY pane that does that — a key-opened surface returning // to its caller must never cost the connection. + from := m.pane switch m.pane { case panePluginDetail: // Return to whichever pane invoked the detail (Pipeline or Catalog). @@ -564,12 +565,14 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { m.pane = paneSessions } m.timeline.tickGen++ - // The timeline's session is a #1542 retention key (watchingSession), + // Back on the events or detail pane, the timeline follows that pane's session + // for its tool rows. That session is selectedSess, so it adds no retention. + if (m.pane == paneEvents || m.pane == paneDetail) && m.selectedSess != "" { + return m.followEventsTimeline() + } + // Else the timeline's session is a #1542 retention key (watchingSession), // so it must not outlive the pane: left set, it would keep streaming - // that session's events for the rest of the run. Cleared here rather - // than pane-checked in the predicate — the drill-down path - // (openEventsAtSeq) clears it too, the session staying watched - // through selectedSess. + // that session's events for the rest of the run. m.timeline.session = "" case paneUsage: // Return to whichever pane opened it, from usageState's own field — @@ -621,6 +624,11 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { m.backToPodsPane() } } + // Back on the events or detail pane from a pane opened over it: the timeline tick + // stopped on that pane, so start it again for the tool rows. + if from != paneEvents && from != paneDetail && m.timelineChainLive() { + return m.resumeTimeline() + } return nil case "enter", "right", "l": @@ -720,11 +728,14 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { if !live[id] { // Cached-only: there is no server-side session to snapshot, so the // fetch would 404 and flash an error over the very events this - // change preserved. + // change preserved. Its timeline would 404 too: set the session + // only, so the previous one is not kept, and fetch nothing. + m.setTimelineSession(id) return nil } + follow := m.followEventsTimeline() // Snapshot in case the stream hasn't yet delivered history. - return m.snapshotCmd(id) + return tea.Batch(m.snapshotCmd(id), follow) case paneDetail: // Enter is the only fold key, so → and l stay as they were: nothing. if msg.String() == "enter" { @@ -742,8 +753,8 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { return nil } er, ok := m.selectedEventRow() - if !ok { - return nil + if !ok || er.tool != nil { + return nil // a tool row opens nothing (#1776) } // Render from the summary immediately, then fill the message bodies in // when they arrive — the timeline no longer carries them. See @@ -1027,7 +1038,7 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { // The operator moved the cursor: an event the Timeline asked to land on no longer applies. m.revealSeq, m.revealSess = 0, "" m.eventsTbl, cmd = m.eventsTbl.Update(msg) - m.selectedEventKey = keyOf(m.selectedEvent()) + m.selectedEventKey = m.selectedPinKey() return cmd case paneDetail: m.detailCursorKey(msg.String()) @@ -1085,7 +1096,7 @@ func (m *model) goTop() { setCursorVisible(&m.sessionsTbl, 0) case paneEvents: setCursorVisible(&m.eventsTbl, 0) - m.selectedEventKey = keyOf(m.selectedEvent()) + m.selectedEventKey = m.selectedPinKey() // `g` is an explicit "take me to the oldest end", so it also becomes where // the next session opens — see EventSettings.OpenAtOldest. m.setOpenAtOldest(true) @@ -1104,7 +1115,7 @@ func (m *model) goBottom() { setCursorVisible(&m.sessionsTbl, len(m.sessionsTbl.Rows())-1) case paneEvents: setCursorVisible(&m.eventsTbl, len(m.eventsTbl.Rows())-1) - m.selectedEventKey = keyOf(m.selectedEvent()) + m.selectedEventKey = m.selectedPinKey() // The mirror of `g` above: back to the tail, and that is where the next // session opens. m.setOpenAtOldest(false) @@ -1141,7 +1152,7 @@ func (m *model) pageActivePane(msg tea.KeyMsg) tea.Cmd { switch m.pane { case paneEvents: page(&m.eventsTbl) - m.selectedEventKey = keyOf(m.selectedEvent()) + m.selectedEventKey = m.selectedPinKey() case paneSessions: page(&m.sessionsTbl) case panePipeline: diff --git a/cmd/agentop/tui/older_notice_test.go b/cmd/agentop/tui/older_notice_test.go index 018a76f71..5b94f92a7 100644 --- a/cmd/agentop/tui/older_notice_test.go +++ b/cmd/agentop/tui/older_notice_test.go @@ -237,7 +237,7 @@ func TestOlderRow_StaysFirstUnderASort(t *testing.T) { if desc { want = 510 } - if e := m.selectedEventOn(1); e == nil || e.Seq != want { + if e := m.selectedRowOn(1).event; e == nil || e.Seq != want { t.Errorf("desc=%v: row 1 is %+v, want Seq %d", desc, e, want) } } diff --git a/cmd/agentop/tui/search_surfaces.go b/cmd/agentop/tui/search_surfaces.go index 6be923102..4c93b6042 100644 --- a/cmd/agentop/tui/search_surfaces.go +++ b/cmd/agentop/tui/search_surfaces.go @@ -105,7 +105,7 @@ func (s eventsSurface) matchLines() []int { return s.m.eventsTbl.Matches() } func (s eventsSurface) spot() searchSpot { c := s.m.eventsTbl.Cursor() return searchSpot{ - row: c, event: keyOf(s.m.selectedEvent()), + row: c, event: s.m.selectedPinKey(), tail: s.m.sortCol == "" && c >= len(s.m.eventsTbl.Rows())-1, } } @@ -123,7 +123,7 @@ func (s eventsSurface) lineOf(sp searchSpot) int { // keeps the cursor on it — through an eviction or a re-sort — instead of on its row number. func (s eventsSurface) reveal(i int) { setCursorVisible(&s.m.eventsTbl, i) - s.m.selectedEventKey = keyOf(s.m.selectedEvent()) + s.m.selectedEventKey = s.m.selectedPinKey() } // restore puts a following cursor back on the newest row, so it goes on following. diff --git a/cmd/agentop/tui/stream_retention_test.go b/cmd/agentop/tui/stream_retention_test.go index 3d2438739..429579911 100644 --- a/cmd/agentop/tui/stream_retention_test.go +++ b/cmd/agentop/tui/stream_retention_test.go @@ -129,10 +129,10 @@ func TestStreamRetentionTimelineStopsAfterEsc(t *testing.T) { } // The Enter drill-down is an exit from the timeline too: openEventsAtSeq moves -// the pane to Events while the session stays watched through selectedSess, so -// timeline.session must be cleared there as Esc clears it — otherwise, once the -// user opens another session, the timeline's old session keeps streaming into -// m.events for the rest of the run. +// the pane to Events while the session stays watched through selectedSess. The +// timeline then follows selectedSess, so when the user opens another session the +// timeline's old session must not keep streaming into m.events for the rest of +// the run. func TestStreamRetentionTimelineStopsAfterDrillDown(t *testing.T) { m := newRetentionModel(t, "other", 0) m.pane = paneTimeline @@ -150,6 +150,7 @@ func TestStreamRetentionTimelineStopsAfterDrillDown(t *testing.T) { } // The user opens session C — the state Enter on Sessions leaves. m.selectedSess = "other" + m.followEventsTimeline() m.handleStreamEvent(apiclient.StreamEvent{Event: &pipeline.SessionEvent{ At: time.Now(), SessionID: "tl-sess", Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, }}) @@ -176,3 +177,31 @@ func TestPluginRunsCountUnwatchedStream(t *testing.T) { t.Errorf("un-watched session retained %d events", len(m.events["bg"])) } } + +// Erasing the session the events pane followed clears the timeline's #1542 retention key too, +// both for a per-session erase and for a clear of all sessions. +func TestStreamRetentionTimelineStopsAfterErase(t *testing.T) { + for _, tc := range []struct { + name string + erase func(m *model) + }{ + {"erase one", func(m *model) { m.forgetSessions([]string{"tl-sess"}) }}, + {"clear all", func(m *model) { m.resetAfterClear() }}, + } { + t.Run(tc.name, func(t *testing.T) { + m := newRetentionModel(t, "tl-sess", 0) + m.followEventsTimeline() + m.timeline.tl = sampleTimeline() + tc.erase(m) + if m.timeline.session != "" || m.timeline.tl != nil { + t.Fatalf("erase left timeline session %q (tl kept: %v)", m.timeline.session, m.timeline.tl != nil) + } + m.handleStreamEvent(apiclient.StreamEvent{Event: &pipeline.SessionEvent{ + At: time.Now(), SessionID: "tl-sess", Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, + }}) + if len(m.events["tl-sess"]) != 0 { + t.Errorf("erased session holds %d streamed events", len(m.events["tl-sess"])) + } + }) + } +} diff --git a/cmd/agentop/tui/table/table.go b/cmd/agentop/tui/table/table.go index 849e3e5e5..de825dc4b 100644 --- a/cmd/agentop/tui/table/table.go +++ b/cmd/agentop/tui/table/table.go @@ -570,10 +570,16 @@ func (m Model) headersView() string { // truncated first cell. No caller rendered a one-cell row before this (the pipeline // pane's "── (app) ──" divider carries a cell per column), so the convention changes // nothing that existed. +// +// A row of n cells, with 1 < n < len(cols), draws cells 0..n-2 in their columns and cell +// n-1 across all the remaining columns. That cell is as wide as the remaining columns +// together, plus the padding between them, so the line is as wide as a full row. The +// events pane's tool rows use it for their last cell. Rows of other shapes are unchanged. func (m *Model) renderRow(r int) string { if len(m.rows[r]) == 1 { return m.renderSectionRow(r) } + widths := m.cellWidths(len(m.rows[r])) texts := make([]string, len(m.rows[r])) var spans [][]find.Range if r < len(m.hits) && m.hits[r] { @@ -581,10 +587,10 @@ func (m *Model) renderRow(r int) string { } lit := false for i, value := range m.rows[r] { - if m.cols[i].Width <= 0 { + if widths[i] <= 0 { continue } - texts[i] = runewidth.Truncate(value, m.cols[i].Width, "…") + texts[i] = runewidth.Truncate(value, widths[i], "…") if spans != nil { spans[i] = m.cellSpans(r, i, texts[i]) lit = lit || len(spans[i]) > 0 @@ -602,22 +608,47 @@ func (m *Model) renderRow(r int) string { s := make([]string, 0, len(m.cols)) for i := range m.rows[r] { - if m.cols[i].Width <= 0 { + if widths[i] <= 0 { continue } var sp []find.Range if spans != nil { sp = spans[i] } - box := lipgloss.NewStyle().Width(m.cols[i].Width).MaxWidth(m.cols[i].Width).Inline(true).Inherit(row) + box := lipgloss.NewStyle().Width(widths[i]).MaxWidth(widths[i]).Inline(true).Inherit(row) // Filled to the column before it is painted, so the text and the padding after it are // one run in the row's style rather than a run and a pad lipgloss styles apart. - filled := runewidth.FillRight(texts[i], m.cols[i].Width) + filled := runewidth.FillRight(texts[i], widths[i]) s = append(s, m.styles.Cell.Inherit(row).Render(box.Render(paint(filled, sp, row, match)))) } return lipgloss.JoinHorizontal(lipgloss.Top, s...) } +// cellWidths gives the text width of each of a row's n cells. The cells of a full row get +// their column's width. For 1 < n < len(cols), the last cell gets the sum of the remaining +// shown columns' widths plus 2 columns of cell padding between each pair of them. +func (m *Model) cellWidths(n int) []int { + w := make([]int, n) + for i := range w { + w[i] = m.cols[i].Width + } + if n >= len(m.cols) { + return w + } + w[n-1] = 0 + k := 0 + for _, c := range m.cols[n-1:] { + if c.Width > 0 { + w[n-1] += c.Width + k++ + } + } + if k > 1 { + w[n-1] += 2 * (k - 1) + } + return w +} + // renderSectionRow draws a one-cell row across the whole table width, in the same // row/cursor/match styles a data row gets. The line is the sum of the column widths // — without each cell's padding, which a spanning line has no use for, so it sits a diff --git a/cmd/agentop/tui/table/table_test.go b/cmd/agentop/tui/table/table_test.go index b5ff4f04a..9476e86e7 100644 --- a/cmd/agentop/tui/table/table_test.go +++ b/cmd/agentop/tui/table/table_test.go @@ -10,6 +10,7 @@ import ( "testing" tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" "github.com/rossoctl/cortex/cmd/agentop/tui/table" ) @@ -238,3 +239,56 @@ func TestSectionRowSearches(t *testing.T) { t.Errorf("matches = %v, want [1]", got) } } + +// spanTable has columns 4, 6, 6, 6 wide. A full row is 30 columns: each cell has 1 column +// of padding at each side. +func spanTable(rows []table.Row) table.Model { + tb := table.New( + table.WithColumns([]table.Column{ + {Title: "w", Width: 4}, {Title: "x", Width: 6}, {Title: "y", Width: 6}, {Title: "z", Width: 6}, + }), + table.WithRows(rows), + table.WithFocused(true), + ) + tb.SetHeight(len(rows) + 1) + return tb +} + +const fullRowWidth = 30 + +// A row with fewer cells than columns (and more than one) puts its last cell across the +// remaining columns: 6 + 6 + 2 columns of padding between them = 14 columns of text. +func TestRenderRow_LastCellSpans(t *testing.T) { + tb := spanTable([]table.Row{{"a", "b", "long text that is twenty"}, {"c", "d", "e", "f"}}) + l := stripEsc(lineFor(t, tb, "long text")) + if !strings.Contains(l, "long text tha…") { + t.Errorf("the last cell does not span 14 columns: %q", l) + } + if got, full := lipgloss.Width(l), lipgloss.Width(stripEsc(lineFor(t, tb, "c"))); got != fullRowWidth || full != fullRowWidth { + t.Errorf("width = %d, full row = %d, want %d", got, full, fullRowWidth) + } +} + +func TestRenderRow_SpanTruncatesAtTableWidth(t *testing.T) { + tb := spanTable([]table.Row{{"a", "b", strings.Repeat("y", 80)}}) + l := strings.TrimRight(stripEsc(lineFor(t, tb, "yyy")), " ") + if !strings.HasSuffix(l, "…") { + t.Errorf("a long last cell does not end in an ellipsis: %q", l) + } + if w := lipgloss.Width(stripEsc(lineFor(t, tb, "yyy"))); w > fullRowWidth { + t.Errorf("width = %d, wider than a full row (%d)", w, fullRowWidth) + } +} + +func TestRenderRow_SpanKeepsCursorAndMatchStyles(t *testing.T) { + s := searchStyles(t) + tb := spanTable([]table.Row{{"a", "b", "spanned-x"}, {"c", "d", "the needle"}}) + tb.SetStyles(s) + if l := lineFor(t, tb, "spanned-x"); !strings.Contains(l, s.Selected.Render("spanned-x ")) { + t.Errorf("the cursor row's spanning cell is not in Selected across its 14 columns: %q", l) + } + tb.SetSearch("needle", nil) + if l := lineFor(t, tb, "the needle"); !strings.Contains(l, s.Match.Render("needle")) { + t.Errorf("a hit in the spanning cell is not drawn in Match: %q", l) + } +} diff --git a/cmd/agentop/tui/timeline_pane.go b/cmd/agentop/tui/timeline_pane.go index a442d491f..3ac8b8f29 100644 --- a/cmd/agentop/tui/timeline_pane.go +++ b/cmd/agentop/tui/timeline_pane.go @@ -77,8 +77,11 @@ type timelineState struct { // it shows "all" without changing it; see window. windowIdx int tl *timeline.Timeline - err error - loading bool + // tools is tl's tools by the seq of the response that asked for them, for the events + // table. It is set with tl, once per fetch. + tools map[uint64][]toolRow + err error + loading bool // stale is set when the stream delivers an event for this session; the next tick refetches. stale bool // fails counts consecutive failed fetches, and retryAt is when the next may go; see @@ -128,18 +131,55 @@ func (t *timelineState) window(now time.Time) int { func (m *model) openTimeline(session string) tea.Cmd { m.timeline.returnPane = m.pane m.pane = paneTimeline + m.setTimelineSession(session) + return m.resumeTimeline() +} + +// setTimelineSession makes session the timeline's session. A change drops what the previous +// session loaded, and makes its in-flight reply and its tick chain stale, so neither lands on +// the new session. +func (m *model) setTimelineSession(session string) { if m.timeline.session != session { - m.timeline.tl, m.timeline.err, m.timeline.cursor, m.timeline.scroll = nil, nil, -1, 0 + m.timeline.tl, m.timeline.tools, m.timeline.err, m.timeline.cursor, m.timeline.scroll = nil, nil, nil, -1, 0 m.timeline.fails, m.timeline.retryAt = 0, time.Time{} + m.timeline.reqSeq++ + m.timeline.tickGen++ + m.timeline.loading, m.timeline.stale = false, false } m.timeline.session = session +} + +// followEventsTimeline starts the timeline fetch chain for the session the events pane shows. +// The timeline session is then selectedSess, so it adds no #1542 retention. +func (m *model) followEventsTimeline() tea.Cmd { + if m.selectedSess == "" { + return nil + } + m.setTimelineSession(m.selectedSess) return m.resumeTimeline() } -// resumeTimeline starts one refresh chain and fetches now. +// timelineChainLive reports a pane that uses the timeline: the Timeline pane, or the events +// and detail panes while the timeline follows their session. +func (m *model) timelineChainLive() bool { + switch m.pane { + case paneTimeline: + return true + case paneEvents, paneDetail: + return m.selectedSess != "" && m.timeline.session == m.selectedSess + } + return false +} + +// resumeTimeline starts one refresh chain and fetches now if the proxy lists the session. func (m *model) resumeTimeline() tea.Cmd { m.timeline.tickGen++ - return tea.Batch(m.fetchTimeline(), timelineTick(m.timeline.tickGen)) + var fetch tea.Cmd + // cached-only: the proxy has no timeline for it, so a fetch only 404s + if m.sessionServed(m.timeline.session) { + fetch = m.fetchTimeline() + } + return tea.Batch(fetch, timelineTick(m.timeline.tickGen)) } func timelineTick(gen uint64) tea.Cmd { @@ -183,7 +223,11 @@ func (m *model) handleTimelineLoaded(msg timelineLoadedMsg) { return } m.timeline.tl, m.timeline.err, m.timeline.fetchedAt = msg.tl, nil, m.clock() + m.timeline.tools = indexTools(msg.tl) m.timeline.fails, m.timeline.retryAt = 0, time.Time{} + if m.pane == paneEvents && m.timeline.session == m.selectedSess { + m.rebuildEventsTable() + } } // sessionServed reports a session the proxy lists, so it can be snapshotted and has a timeline. @@ -197,7 +241,7 @@ func (m *model) sessionServed(id string) bool { } func (m *model) handleTimelineTick(msg timelineTickMsg) tea.Cmd { - if m.pane != paneTimeline || msg.gen != m.timeline.tickGen { + if !m.timelineChainLive() || msg.gen != m.timeline.tickGen { return nil } t := &m.timeline @@ -206,8 +250,12 @@ func (m *model) handleTimelineTick(msg timelineTickMsg) tea.Cmd { idle := !t.fetchedAt.IsZero() && now.Sub(t.fetchedAt) >= timelineIdleRefresh && m.timelineLive() // A failed fetch is retried on its backoff, and holds back the other triggers until then: a // stream event or a live session's idle refresh would otherwise retry a dead proxy every tick. - retry := t.err != nil && !errors.Is(t.err, errTimelineUnsupported) - if waiting := !t.retryAt.IsZero() && now.Before(t.retryAt); (t.stale || idle || retry) && !waiting && !t.loading { + unsupported := errors.Is(t.err, errTimelineUnsupported) + retry := t.err != nil && !unsupported + // The chain outlives a list refresh that omits the session (#870); only the fetch waits. + served := m.sessionServed(t.session) + pending := t.tl == nil && t.err == nil // never fetched: resumeTimeline skipped it while unlisted + if waiting := !t.retryAt.IsZero() && now.Before(t.retryAt); (t.stale || idle || retry || pending) && served && !unsupported && !waiting && !t.loading { fetch = m.fetchTimeline() } return tea.Batch(fetch, timelineTick(msg.gen)) @@ -485,10 +533,9 @@ func (m *model) openEventsAtSeq(session string, seq uint64) tea.Cmd { } m.selectedSess = session m.pane = paneEvents - // Leaving the timeline pane: its session is a #1542 retention key, so the - // drill-down drops it the same way Esc does — the session stays watched - // through selectedSess, which the lines above just set. - m.timeline.session = "" + // The timeline keeps its session for the events pane's tool rows. That session is + // selectedSess, which the lines above just set, so it adds no #1542 retention. + follow := m.followEventsTimeline() m.revealSeq, m.revealSess = seq, session m.rebuildEventsTable() revealed := m.revealPendingSeq() @@ -496,9 +543,9 @@ func (m *model) openEventsAtSeq(session string, seq uint64) tea.Cmd { if !revealed { m.revealSeq, m.revealSess = 0, "" } - return nil + return follow } - return m.snapshotCmd(session) + return tea.Batch(m.snapshotCmd(session), follow) } // revealPendingSeq moves the events cursor to revealSeq if that event is on screen in its own @@ -633,6 +680,9 @@ func (m *model) renderTimeline() string { } b.WriteString(styleMuted.Render(" loading… the last try failed, retrying"+wait+": "+t.err.Error()) + "\n") return b.String() + case t.tl == nil && !m.sessionServed(t.session): + b.WriteString(styleMuted.Render(" The proxy no longer has this session, so it has no timeline.") + "\n") + return b.String() case t.tl == nil: b.WriteString(styleMuted.Render(" loading…") + "\n") return b.String() diff --git a/cmd/agentop/tui/timeline_pane_test.go b/cmd/agentop/tui/timeline_pane_test.go index ea7b45fed..fdfa2a742 100644 --- a/cmd/agentop/tui/timeline_pane_test.go +++ b/cmd/agentop/tui/timeline_pane_test.go @@ -455,6 +455,7 @@ func TestTimelineKeys_EscReturnsAndArrowsStay(t *testing.T) { func TestTimelineOpen_FromEventsAndFromSessions(t *testing.T) { m := fitModel(t, paneEvents, 120, 40, nil) m.client = deadClient() + m.sessions = []session.SessionSummary{{ID: "sess-1"}} m.handleKey(keyRune('T')) if m.pane != paneTimeline || m.timeline.session != "sess-1" || m.timeline.returnPane != paneEvents || !m.timeline.loading { t.Fatalf("T from events: pane %v session %q return %v loading %v", m.pane, m.timeline.session, m.timeline.returnPane, m.timeline.loading) @@ -463,6 +464,7 @@ func TestTimelineOpen_FromEventsAndFromSessions(t *testing.T) { m = fitModel(t, paneSessions, 120, 40, nil) m.client = deadClient() m.sessionRowIDs = []string{"row-a", "row-b"} + m.sessions = []session.SessionSummary{{ID: "row-a"}, {ID: "row-b"}} m.sessionsTbl.SetRows([]table.Row{make(table.Row, len(m.sessionsTbl.Columns())), make(table.Row, len(m.sessionsTbl.Columns()))}) m.sessionsTbl.SetCursor(1) m.handleKey(keyRune('T')) diff --git a/cmd/agentop/tui/tool_rows.go b/cmd/agentop/tui/tool_rows.go new file mode 100644 index 000000000..4c54f1380 --- /dev/null +++ b/cmd/agentop/tui/tool_rows.go @@ -0,0 +1,173 @@ +package tui + +import ( + "strings" + "time" + + "github.com/rossoctl/cortex/core/session/timeline" +) + +// endsRunTools are the tools that end the run by design when they get no result. The +// source is core/session/timeline/agents.go. Do not add other names. +var endsRunTools = map[string]bool{"StructuredOutput": true, "SubagentHandback": true} + +// toolRow is one tool call of the timeline, with the figures its events-table row shows. +type toolRow struct { + timeline.Tool + Parallel bool // another tool of the same response has the same End + EndsRun bool // Lost and Name is StructuredOutput or SubagentHandback + RunFrom *time.Time // SpawnedAt of the agent whose SpawnTool is this tool's ID + RunTo *time.Time // its ReturnedAt; nil while the subagent runs + BgFrom *time.Time // Start of this Bash tool's job (fact 4) + BgTo *time.Time // the job's End; nil while it runs +} + +// jobKey finds a Bash tool's job from an older proxy, which sends no ToolUseID: its agent and +// start, which is the tool's end. Two calls whose results share one request share that time. +type jobKey struct { + agent string + start int64 +} + +// indexTools groups the tools of tl by the seq of the response that asked for them, in +// timeline order. It returns nil for a nil tl. +func indexTools(tl *timeline.Timeline) map[uint64][]toolRow { + if tl == nil { + return nil + } + spawned := make(map[string]timeline.Agent, len(tl.Agents)) + for _, a := range tl.Agents { + if a.SpawnTool != "" { + spawned[a.SpawnTool] = a + } + } + // Build ends a call at the request carrying its first result, clamped up to the call's + // Start, and starts the job at that request. So a job's call has End = max(job Start, Start). + // Build keeps only the first result of an ID, so with a reused ID (call_0) the first such + // call owns the job. The request can arrive before the asking response ends, so its Seq can + // be lower than the call's: Seq does not order them. + byTool := map[int]timeline.Job{} + legacy := map[jobKey]timeline.Job{} + for _, j := range tl.Jobs { + if j.ToolUseID == "" { + k := jobKey{j.Agent, j.Start.UnixNano()} + if _, dup := legacy[k]; !dup { + legacy[k] = j + } + continue + } + for i, t := range tl.Tools { + if t.Agent != j.Agent || t.ID != j.ToolUseID || t.Name != "Bash" || t.End == nil { + continue + } + end := j.Start + if end.Before(t.Start) { + end = t.Start + } + if _, dup := byTool[i]; t.End.Equal(end) && !dup { + byTool[i] = j + break + } + } + } + type endKey struct { + seq uint64 + end int64 + } + ends := map[endKey]int{} + for _, t := range tl.Tools { + if t.End != nil { + ends[endKey{t.Seq, t.End.UnixNano()}]++ + } + } + + idx := make(map[uint64][]toolRow) + for i, t := range tl.Tools { + r := toolRow{Tool: t, EndsRun: t.Lost && endsRunTools[t.Name]} + if t.End != nil { + r.Parallel = ends[endKey{t.Seq, t.End.UnixNano()}] > 1 + if t.Name == "Bash" { + j, ok := byTool[i] + if !ok { + j, ok = legacy[jobKey{t.Agent, t.End.UnixNano()}] + } + if ok { + r.BgFrom, r.BgTo = &j.Start, j.End + } + } + } + if a, ok := spawned[t.ID]; ok && a.SpawnedAt != nil { + r.RunFrom, r.RunTo = a.SpawnedAt, a.ReturnedAt + } + idx[t.Seq] = append(idx[t.Seq], r) + } + return idx +} + +// status is the STATUS cell (rule 7). +func (t toolRow) status() string { + switch { + case t.End == nil: + return "running" + case t.Error: + return "error" + case t.EndsRun: + return "" + case t.Lost: + return "lost" + } + return "ok" +} + +// duration is the DURATION sort value (rule 11). +func (t toolRow) duration(now time.Time) time.Duration { + switch { + case t.End == nil: + return now.Sub(t.Start) + case t.Lost: + return 0 + } + return t.End.Sub(t.Start) +} + +// durationCell is the DURATION cell (rule 8). +func (t toolRow) durationCell(now time.Time) string { + switch { + case t.End == nil: + return "⋯ " + formatDuration(now.Sub(t.Start)) + case t.Lost: + return "" + } + s := formatDuration(t.End.Sub(t.Start)) + if t.Parallel && s != "" { + s = "∥" + s + } + return s +} + +// trailing is the cell after DURATION (rule 10): run, bg, ends run and the preview, joined +// with " · ". +func (t toolRow) trailing(now time.Time) string { + parts := make([]string, 0, 4) + if t.RunFrom != nil { + parts = append(parts, "run "+span(*t.RunFrom, t.RunTo, now)) + } + if t.BgFrom != nil { + parts = append(parts, "bg "+span(*t.BgFrom, t.BgTo, now)) + } + if t.EndsRun { + parts = append(parts, "ends run") + } + if t.Preview != "" { + parts = append(parts, t.Preview) + } + return strings.Join(parts, " · ") +} + +// span formats to−from, or "⋯ " while to is nil. +func span(from time.Time, to *time.Time, now time.Time) string { + if to == nil { + return "⋯ " + formatDuration(now.Sub(from)) + } + return formatDuration(to.Sub(from)) +} diff --git a/cmd/agentop/tui/tool_rows_test.go b/cmd/agentop/tui/tool_rows_test.go new file mode 100644 index 000000000..5355c4e1c --- /dev/null +++ b/cmd/agentop/tui/tool_rows_test.go @@ -0,0 +1,290 @@ +package tui + +import ( + "strings" + "testing" + "time" + + "github.com/rossoctl/cortex/core/pipeline" + "github.com/rossoctl/cortex/core/session/timeline" +) + +// tlTool is a tool of agent "main" that response seq asked for at start. end < 0 means no end. +func tlTool(seq uint64, id, name string, start, end float64) timeline.Tool { + t := timeline.Tool{Agent: "main", ID: id, Name: name, Start: tlAt(start), Seq: seq, Preview: "p-" + id} + if end >= 0 { + t.End = tlPtr(end) + } + return t +} + +// onlyRow indexes tl and returns the one tool row with the given id. +func onlyRow(t *testing.T, tl *timeline.Timeline, id string) toolRow { + t.Helper() + for _, rows := range indexTools(tl) { + for _, r := range rows { + if r.ID == id { + return r + } + } + } + t.Fatalf("no tool row %q", id) + return toolRow{} +} + +func TestIndexTools_Nil(t *testing.T) { + if idx := indexTools(nil); idx != nil { + t.Fatalf("indexTools(nil) = %v, want nil", idx) + } +} + +func TestIndexTools_GroupsBySeqInOrder(t *testing.T) { + tl := &timeline.Timeline{Tools: []timeline.Tool{ + tlTool(5, "a", "Bash", 1, 2), + tlTool(5, "b", "Read", 1, 3), + tlTool(7, "c", "Grep", 4, 5), + }} + idx := indexTools(tl) + if len(idx[5]) != 2 || idx[5][0].Name != "Bash" || idx[5][1].Name != "Read" { + t.Fatalf("idx[5] = %+v, want Bash then Read", idx[5]) + } + if len(idx[7]) != 1 || idx[7][0].Name != "Grep" { + t.Fatalf("idx[7] = %+v, want Grep", idx[7]) + } +} + +func TestIndexTools_Parallel(t *testing.T) { + tl := &timeline.Timeline{Tools: []timeline.Tool{ + tlTool(5, "a", "Bash", 1, 9), + tlTool(5, "b", "Read", 1, 9), + tlTool(5, "c", "Grep", 1, 4), + tlTool(6, "d", "Bash", 10, 12), + tlTool(7, "e", "Bash", 13, -1), + tlTool(7, "f", "Read", 13, -1), + }} + want := map[string]bool{"a": true, "b": true, "c": false, "d": false, "e": false, "f": false} + for id, par := range want { + if got := onlyRow(t, tl, id).Parallel; got != par { + t.Errorf("tool %s Parallel = %v, want %v", id, got, par) + } + } +} + +func TestToolRow_DurationCell(t *testing.T) { + now := tlAt(100) + lost := tlTool(1, "l", "Bash", 1, 2) + lost.Lost = true + eor := tlTool(1, "s", "StructuredOutput", 1, 2) + eor.Lost = true + cases := []struct { + name string + row toolRow + want string + }{ + {"parallel", toolRow{Tool: tlTool(1, "a", "Bash", 1, 14.21), Parallel: true}, "∥13.21s"}, + {"short", toolRow{Tool: tlTool(1, "a", "Bash", 1, 1.3)}, "300ms"}, + {"running", toolRow{Tool: tlTool(1, "a", "Bash", 62, -1)}, "⋯ 38.00s"}, + {"lost", toolRow{Tool: lost}, ""}, + {"end of run", toolRow{Tool: eor, EndsRun: true}, ""}, + } + for _, c := range cases { + if got := c.row.durationCell(now); got != c.want { + t.Errorf("%s: durationCell = %q, want %q", c.name, got, c.want) + } + } +} + +func TestToolRow_Status(t *testing.T) { + errTool := tlTool(1, "e", "Bash", 1, 2) + errTool.Error = true + lost := tlTool(1, "l", "Bash", 1, 2) + lost.Lost = true + eor := tlTool(1, "s", "StructuredOutput", 1, 2) + eor.Lost = true + tl := &timeline.Timeline{Tools: []timeline.Tool{tlTool(1, "r", "Bash", 1, -1), errTool, lost, eor, tlTool(1, "o", "Bash", 1, 3)}} + want := map[string]string{"r": "running", "e": "error", "l": "lost", "s": "", "o": "ok"} + for id, st := range want { + if got := onlyRow(t, tl, id).status(); got != st { + t.Errorf("tool %s status = %q, want %q", id, got, st) + } + } +} + +func TestIndexTools_AgentRun(t *testing.T) { + tl := &timeline.Timeline{ + Tools: []timeline.Tool{tlTool(3, "t1", "Agent", 8, 9.7)}, + Agents: []timeline.Agent{{ID: "main"}, {ID: "sub", Parent: "main", SpawnTool: "t1", SpawnedAt: tlPtr(10), ReturnedAt: tlPtr(358)}}, + } + if got, want := onlyRow(t, tl, "t1").trailing(tlAt(400)), "run 5m48s · p-t1"; got != want { + t.Errorf("ended run: trailing = %q, want %q", got, want) + } + tl.Agents[1].ReturnedAt = nil + if got, want := onlyRow(t, tl, "t1").trailing(tlAt(40)), "run ⋯ 30.00s · p-t1"; got != want { + t.Errorf("running run: trailing = %q, want %q", got, want) + } +} + +func TestIndexTools_BgJob(t *testing.T) { + tl := &timeline.Timeline{ + Tools: []timeline.Tool{tlTool(4, "b", "Bash", 19, 20)}, + Jobs: []timeline.Job{{Agent: "main", ID: "j1", Start: tlAt(20), End: tlPtr(262)}}, + } + if got, want := onlyRow(t, tl, "b").trailing(tlAt(300)), "bg 4m02s · p-b"; got != want { + t.Errorf("ended job: trailing = %q, want %q", got, want) + } + tl.Jobs[0].End = nil + if got := onlyRow(t, tl, "b").trailing(tlAt(50)); !strings.HasPrefix(got, "bg ⋯ ") { + t.Errorf("running job: trailing = %q, want prefix %q", got, "bg ⋯ ") + } + + // Two parallel tools end where the job starts: only the Bash one owns it. + tl = &timeline.Timeline{ + Tools: []timeline.Tool{tlTool(4, "b", "Bash", 19, 20), tlTool(4, "r", "Read", 19, 20)}, + Jobs: []timeline.Job{{Agent: "main", ID: "j1", Start: tlAt(20), End: tlPtr(262)}}, + } + if got := onlyRow(t, tl, "b").trailing(tlAt(300)); !strings.HasPrefix(got, "bg ") { + t.Errorf("Bash tool: trailing = %q, want a bg part", got) + } + if got, want := onlyRow(t, tl, "r").trailing(tlAt(300)), "p-r"; got != want { + t.Errorf("Read tool: trailing = %q, want %q", got, want) + } + + // Two Bash calls of one response: their results share a request, so they end at one time. + // Only the call that started the job owns it, and each job goes to its own call. + tl = &timeline.Timeline{ + Tools: []timeline.Tool{tlTool(4, "b1", "Bash", 19, 20), tlTool(4, "b2", "Bash", 19, 20)}, + Jobs: []timeline.Job{{Agent: "main", ID: "j1", ToolUseID: "b2", Start: tlAt(20), End: tlPtr(262)}}, + } + if got, want := onlyRow(t, tl, "b1").trailing(tlAt(300)), "p-b1"; got != want { + t.Errorf("Bash call with no job: trailing = %q, want %q", got, want) + } + if got := onlyRow(t, tl, "b2").trailing(tlAt(300)); !strings.HasPrefix(got, "bg 4m02s") { + t.Errorf("Bash call with the job: trailing = %q, want a bg part", got) + } + tl.Jobs = append(tl.Jobs, timeline.Job{Agent: "main", ID: "j2", ToolUseID: "b1", Start: tlAt(20), End: tlPtr(80)}) + if got := onlyRow(t, tl, "b1").trailing(tlAt(300)); !strings.HasPrefix(got, "bg 1m00s") { + t.Errorf("second job: trailing = %q, want bg 1m00s", got) + } + + // A reused ID (call_0): the job goes to the call that started it, not to a later call + // with the same ID. + tl = &timeline.Timeline{ + Tools: []timeline.Tool{tlTool(4, "call_0", "Bash", 19, 20), tlTool(9, "call_0", "Bash", 40, 50)}, + Jobs: []timeline.Job{{Agent: "main", ID: "j1", ToolUseID: "call_0", Start: tlAt(20), End: tlPtr(262)}}, + } + idx := indexTools(tl) + if got := idx[4][0].trailing(tlAt(300)); !strings.HasPrefix(got, "bg 4m02s") { + t.Errorf("call that started the job: trailing = %q, want a bg part", got) + } + if got, want := idx[9][0].trailing(tlAt(300)), "p-call_0"; got != want { + t.Errorf("later call with the same ID: trailing = %q, want %q", got, want) + } + + // Build clamped the tool's End up to its Start: the result request (seq 3, 4.9s) came + // before the asking response (seq 4) ended at 5s. The job still goes to the call. + tl = &timeline.Timeline{ + Tools: []timeline.Tool{tlTool(4, "t1", "Bash", 5, 5)}, + Jobs: []timeline.Job{{Agent: "main", ID: "bk1", ToolUseID: "t1", Start: tlAt(4.9), End: tlPtr(10), Seq: 3}}, + } + if got := onlyRow(t, tl, "t1").trailing(tlAt(300)); !strings.HasPrefix(got, "bg ") { + t.Errorf("clamped End: trailing = %q, want a bg part", got) + } + + // A reused ID where only the later call starts a job: the earlier call gets none. + tl = &timeline.Timeline{ + Tools: []timeline.Tool{tlTool(4, "call_0", "Bash", 19, 20), tlTool(9, "call_0", "Bash", 40, 50)}, + Jobs: []timeline.Job{{Agent: "main", ID: "j1", ToolUseID: "call_0", Start: tlAt(50), End: tlPtr(60), Seq: 10}}, + } + idx = indexTools(tl) + if got, want := idx[4][0].trailing(tlAt(300)), "p-call_0"; got != want { + t.Errorf("earlier call, no job: trailing = %q, want %q", got, want) + } + if got := idx[9][0].trailing(tlAt(300)); !strings.HasPrefix(got, "bg 10.00s") { + t.Errorf("later call with the job: trailing = %q, want bg 10.00s", got) + } + + // A job of another agent matches nothing. + tl = &timeline.Timeline{ + Tools: []timeline.Tool{tlTool(4, "b1", "Bash", 19, 20), tlTool(4, "b2", "Bash", 19, 20)}, + Jobs: []timeline.Job{{Agent: "main", ID: "j1", ToolUseID: "b2", Start: tlAt(20), End: tlPtr(262)}}, + } + tl.Jobs[0].Agent = "sub" + if got, want := onlyRow(t, tl, "b2").trailing(tlAt(300)), "p-b2"; got != want { + t.Errorf("job of another agent: trailing = %q, want %q", got, want) + } +} + +func TestToolRow_EndsRunTrailing(t *testing.T) { + h := tlTool(2, "h", "SubagentHandback", 1, 1) + h.Lost, h.Preview = true, "x" + tl := &timeline.Timeline{Tools: []timeline.Tool{h}} + r := onlyRow(t, tl, "h") + if !r.EndsRun { + t.Fatal("lost SubagentHandback: EndsRun = false") + } + if got, want := r.trailing(tlAt(5)), "ends run · x"; got != want { + t.Errorf("trailing = %q, want %q", got, want) + } + r.Preview = "" + if got, want := r.trailing(tlAt(5)), "ends run"; got != want { + t.Errorf("empty preview: trailing = %q, want %q", got, want) + } + // Not lost: an ordinary tool, not the end of the run. + h.Lost = false + if onlyRow(t, &timeline.Timeline{Tools: []timeline.Tool{h}}, "h").EndsRun { + t.Error("SubagentHandback with a result: EndsRun = true") + } +} + +func TestToolRow_DurationSortValue(t *testing.T) { + now := tlAt(100) + lost := tlTool(1, "l", "Bash", 1, 2) + lost.Lost = true + cases := []struct { + name string + row toolRow + want time.Duration + }{ + {"running", toolRow{Tool: tlTool(1, "a", "Bash", 60, -1)}, 40 * time.Second}, + {"lost", toolRow{Tool: lost}, 0}, + {"end of run", toolRow{Tool: lost, EndsRun: true}, 0}, + {"ended", toolRow{Tool: tlTool(1, "a", "Bash", 1, 3.5)}, 2500 * time.Millisecond}, + } + for _, c := range cases { + if got := c.row.duration(now); got != c.want { + t.Errorf("%s: duration = %v, want %v", c.name, got, c.want) + } + } +} + +// Some local models send one tool ID (call_0) on every turn: the pin finds the row of its own +// response, not the first row with that ID. +func TestPinKey_ToolRowWithAReusedID(t *testing.T) { + a, b := tlTool(4, "call_0", "Bash", 1, 2), tlTool(9, "call_0", "Bash", 5, 6) + rows := []eventRow{{tool: &toolRow{Tool: a}}, {tool: &toolRow{Tool: b}}} + if got := findByKey(rows, pinKeyOf(rows[1])); got != 1 { + t.Errorf("findByKey = %d, want 1", got) + } +} + +// The clamp case in append order: the result request (at 8) is appended before the +// response that asked for the call (finished at 10), so it has the lower Seq. +func TestIndexTools_BgJobOfAClampedCallFromBuild(t *testing.T) { + tools := []pipeline.InferenceTool{{Name: "Bash"}} + h := []pipeline.InferenceMessage{{Role: "user", Content: "go"}} + ev := []pipeline.SessionEvent{ + {Seq: 1, At: tlAt(0), Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, RequestID: "r1", Host: "llm", + Inference: &pipeline.InferenceExtension{AgentRole: pipeline.AgentRoleMain, Tools: tools, Messages: h}}, + {Seq: 2, At: tlAt(8), Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, RequestID: "r2", Host: "llm", + Inference: &pipeline.InferenceExtension{AgentRole: pipeline.AgentRoleMain, Tools: tools, Messages: h, + ToolResults: []pipeline.InferenceToolResult{{ToolUseID: "t1", Content: "Command running in background with ID: bk1"}}}}, + {Seq: 3, At: tlAt(9), Direction: pipeline.Outbound, Phase: pipeline.SessionResponse, RequestID: "r2", Host: "llm", StatusCode: 200, Duration: time.Second, + Inference: &pipeline.InferenceExtension{FinishReason: "end_turn"}}, + {Seq: 4, At: tlAt(10), Direction: pipeline.Outbound, Phase: pipeline.SessionResponse, RequestID: "r1", Host: "llm", StatusCode: 200, Duration: 10 * time.Second, + Inference: &pipeline.InferenceExtension{FinishReason: "tool_use", ToolCalls: []pipeline.InferenceToolCall{{ID: "t1", Name: "Bash"}}}}, + } + tl := timeline.Build("s", ev, time.Time{}) + if got := onlyRow(t, tl, "t1").trailing(tlAt(300)); !strings.HasPrefix(got, "bg ") { + t.Errorf("clamped End from Build: trailing = %q, want a bg part", got) + } +} diff --git a/core/session/timeline/agents.go b/core/session/timeline/agents.go index 5d49748d9..eba6207d9 100644 --- a/core/session/timeline/agents.go +++ b/core/session/timeline/agents.go @@ -122,7 +122,7 @@ func buildAgents(t *Timeline, events []model.SessionEvent) { if m := backgroundRe.FindStringSubmatch(r.Content); m != nil { if _, dup := jobs[m[1]]; !dup { jobs[m[1]] = len(t.Jobs) - t.Jobs = append(t.Jobs, Job{Agent: c.agent, ID: m[1], Start: c.req.At, Seq: c.req.Seq}) + t.Jobs = append(t.Jobs, Job{Agent: c.agent, ID: m[1], ToolUseID: r.ToolUseID, Start: c.req.At, Seq: c.req.Seq}) } } } diff --git a/core/session/timeline/timeline.go b/core/session/timeline/timeline.go index 04e0eb6f2..a0cc2ebb3 100644 --- a/core/session/timeline/timeline.go +++ b/core/session/timeline/timeline.go @@ -109,11 +109,13 @@ type Tool struct { // running in background with ID: …") to the naming that id. Its tool call // ends at the acknowledgement, so without this the agent looks idle while the job runs. type Job struct { - Agent string `json:"agent"` - ID string `json:"id"` - Start time.Time `json:"start"` - End *time.Time `json:"end,omitempty"` - Seq uint64 `json:"seq"` + Agent string `json:"agent"` + ID string `json:"id"` + // ToolUseID is the Bash call whose result started the job. Empty from an older proxy. + ToolUseID string `json:"toolUseId,omitempty"` + Start time.Time `json:"start"` + End *time.Time `json:"end,omitempty"` + Seq uint64 `json:"seq"` } // Return is a subagent's result reaching another agent's context. diff --git a/core/session/timeline/timeline_test.go b/core/session/timeline/timeline_test.go index b58573feb..601adeb0b 100644 --- a/core/session/timeline/timeline_test.go +++ b/core/session/timeline/timeline_test.go @@ -744,8 +744,8 @@ func TestBackgroundJobRunsUntilItsNotification(t *testing.T) { r.turn("", 30, 1, h2, nil, "end_turn") r.turn("", 60, 1, append(h2, pipeline.InferenceMessage{Role: "user", Content: "\nbk42\ncompleted"}), nil, "end_turn") tl := Build("s", r.events, time.Time{}) - if len(tl.Jobs) != 1 || tl.Jobs[0].ID != "bk42" || !tl.Jobs[0].Start.Equal(at(2)) || tl.Jobs[0].End == nil || !tl.Jobs[0].End.Equal(at(60)) { - t.Fatalf("jobs = %+v, want bk42 from 2s to 60s", tl.Jobs) + if len(tl.Jobs) != 1 || tl.Jobs[0].ID != "bk42" || tl.Jobs[0].ToolUseID != "t1" || !tl.Jobs[0].Start.Equal(at(2)) || tl.Jobs[0].End == nil || !tl.Jobs[0].End.Equal(at(60)) { + t.Fatalf("jobs = %+v, want bk42 of t1 from 2s to 60s", tl.Jobs) } } diff --git a/docs/session-api.md b/docs/session-api.md index 4afadaf0f..f9c560780 100644 --- a/docs/session-api.md +++ b/docs/session-api.md @@ -185,7 +185,7 @@ the whole list. {"id": "a1bf9707ee0e71012", "parent": "main", "label": "Survey the code", "type": "Explore", "spawnTool": "toolu_…", "spawnedAt": "…", "returnedAt": "…"}], "turns": [{"agent": "main", "start": "…", "end": "…", "finish": "tool_use", "seq": 12}], - "jobs": [{"agent": "main", "id": "bk42", "start": "…", "end": "…", "seq": 40}], + "jobs": [{"agent": "main", "id": "bk42", "toolUseId": "toolu_…", "start": "…", "end": "…", "seq": 40}], "tools": [{"agent": "main", "id": "toolu_…", "name": "Bash", "start": "…", "end": "…", "preview": "go test ./...", "seq": 13}], "returns": [{"agent": "a1bf9707ee0e71012", "into": "main", "at": "…", "seq": 90}], diff --git a/docs/superpowers/plans/2026-11-10-agentop-tool-rows-1776.md b/docs/superpowers/plans/2026-11-10-agentop-tool-rows-1776.md new file mode 100644 index 000000000..fa45ba367 --- /dev/null +++ b/docs/superpowers/plans/2026-11-10-agentop-tool-rows-1776.md @@ -0,0 +1,275 @@ +# agentop Tool Rows Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Show each tool call as its own row in agentop's events table, under the exchange whose response asked for it, with a measured duration that the existing sort and search use. + +**Architecture:** The events pane runs the Timeline pane's existing fetch chain for the selected session. Each fetched `*timeline.Timeline` is indexed once into a map from response seq to tool rows. `rebuildEventsTable` adds the tool rows after their exchange row. A tool row has cells up to DURATION and one last cell that spans the rest of the table (a new `tui/table` rule). + +**Tech Stack:** Go 1.26.10, Bubble Tea, the repo's own `cmd/agentop/tui/table` package. + +**Spec:** issue rossoctl/cortex#1776, section "Spec (v1)" (rules 1-16, facts 1-8, tasks 1-6). Read it with `gh issue view 1776 -R rossoctl/cortex`. This plan refers to its rules by number ("rule 8"). + +## Global Constraints + +- No new API field and no change under `core/`. All data comes from `GET /v1/sessions/{id}/timeline` (`apiclient.Client.GetTimeline`). +- Not in v1: Enter on a tool row, totals per tool name, a "waiting for you" label. +- End-of-run tool names are exactly `StructuredOutput` and `SubagentHandback`. Do not add others. +- Tool rows come from the timeline's `tools[]`, never from events' `ToolCalls`/`ToolResults` (agentop loads `view=summary`, which strips them). +- Log nothing new. No new dependency. +- Code comments, commit messages and PR body in ASD-STE100 Simplified Technical English. Commits use `git commit -s` and end with `Assisted-By: Claude (Anthropic AI) `. No `Co-Authored-By`. +- Run `cmd/agentop` tests while you work with `scripts/dev/go-test-shards.sh -short cmd/agentop` (from the repo root). + +## Decisions made for this plan (with reasons) + +1. **One shared `timelineState`.** The events pane uses `m.timeline`, not a second state. Reason: the spec (task 1) says so, and a second chain doubles the fetches when the user opens `T` from the events pane. Setting `m.timeline.session = m.selectedSess` adds no #1542 retention: `watchingSession` already holds `selectedSess`. +2. **Chain runs on `paneEvents` and `paneDetail`** when `m.timeline.session == m.selectedSess`. Reason: Esc from detail returns to the table, and the rows must be current there. On any other pane the tick stops, as now. +3. **Join check.** A tool joins a row only when `exchangeResp(row) != nil`, `exchangeResp(row).Seq == tool.Seq` and `eventSession(m.selectedSess, resp) == m.selectedSess`. Reason: linked-bucket events carry seqs of another session (`linked.go`), and the timeline is for `selectedSess` only. +4. **`error` STATUS is plain text.** Reason: the events table has no per-cell color (see `newEventsTable`). "The error style that exchange rows use" is therefore no style. +5. **Tool row cells.** Walk the shown columns in order. Columns `#`, TIME, DIR, ACTION, PLUGIN, METHOD, STATUS, DURATION get the tool cell (rules 2-8). At the first shown column outside that set, append the trailing cell (rule 10) and stop. If no such column is shown, the trailing text is not drawn (search still sees it). Reason: these eight columns are first in `eventColumns` display order, so the cells stay parallel to `cols` when the picker or the fit drops a column. +6. **Sort key for "other columns".** `sortValue{numeric: k.numeric}`, where `k` is `sortCol.sortKey(ctxs[0])`. Reason: `sortValue.less` reads `numeric` from its receiver, so a non-numeric zero beside numeric keys gives an inconsistent order. Tool rows exist only when `ctxs` is not empty. +7. **Pin key.** Add `tool, toolAgent string` to `eventKey`. A new `pinKeyOf(er eventRow) eventKey` returns `{tool: id, toolAgent: agent}` for a tool row and `keyOf(er.event)` otherwise. Every `keyOf(m.selectedEvent())` and `keyOf(m.selectedEventOn(...))` caller changes to `pinKeyOf` of the same row. Reason: rule 14, and `keyOf(nil)` is the zero (unpinned) key. Do not name it `rowKey`: `events_columns.go` already declares `type rowKey struct` (the sort key), so that name does not compile. +8. **Index location.** The index is a field `tools map[uint64][]toolRow` on `timelineState`, set in `handleTimelineLoaded` on success and cleared with `tl` when the session changes. Reason: spec task 2, "once per fetched timeline". +9. **`now`** for `running`, `run ⋯` and `bg ⋯` is `m.clock()` read once per rebuild. Reason: the model already injects a clock for tests. +10. **One PR.** The estimate is about 1,400 counted lines, under the 2,000 limit. + +## Review Focus + +1. A timeline that arrives before the snapshot (no event rows yet): no panic, no tool rows; the rows show on the next rebuild. Test in Task 4 (`TestToolRows_TimelineBeforeEvents`). +2. A timeline from a previous session left in `m.timeline` after the user opens another session: no tool rows of the old session. Test in Task 1 (`TestEventsTimeline_SessionChangeDropsTools`). +3. A tool whose `Seq` matches no visible row (evicted or older page not loaded): it is not shown and nothing breaks. Test in Task 4 (`TestToolRows_OrphanToolNotShown`). +4. A narrow terminal that drops DURATION and the columns after it: tool rows still have no more cells than columns (the table panics on more). Test in Task 4 (`TestToolRows_NarrowTerminalCellCount`). +5. Detail search (`/` then tab) and `n`/`N` step over tool rows without a nil dereference. Test in Task 4 (`TestToolRows_DetailSearchSkipsToolRows`). + +--- + +### Task 1: Timeline data on the events pane + +**Depends on:** none + +**Files:** +- Modify: `cmd/agentop/tui/timeline_pane.go` (`timelineState`, `openTimeline`, `handleTimelineLoaded`, `handleTimelineTick`, `openEventsAtSeq`) +- Modify: `cmd/agentop/tui/keys.go` (Enter on a session near line 718; Esc from the timeline near line 565; Esc/back from detail) +- Modify: `cmd/agentop/tui/app.go` (stale marking near line 2414; `timelineLoadedMsg` case) +- Test: `cmd/agentop/tui/events_timeline_test.go` (new) + +**Interfaces:** +- Produces: `func (m *model) followEventsTimeline() tea.Cmd` — sets `m.timeline.session = m.selectedSess` (resetting `tl`, `err`, `fails`, `retryAt`, `cursor=-1`, `scroll=0` when the session changes) and returns `m.resumeTimeline()`. Returns nil when `m.selectedSess == ""`. +- Produces: `func (m *model) timelineChainLive() bool` — true when `m.pane == paneTimeline`, or `m.pane` is `paneEvents`/`paneDetail` and `m.timeline.session == m.selectedSess != ""`. +- Produces: `handleTimelineLoaded` calls `m.rebuildEventsTable()` after a successful load when `m.pane == paneEvents && m.timeline.session == m.selectedSess`. This task does not add the `tools` field; Task 4 adds it, fills it with `indexTools` (Task 2), and adds `m.timeline.tools = nil` to `followEventsTimeline`, so Task 1 and Task 2 stay independent. + +- [ ] **Step 1: Write the failing tests** in `events_timeline_test.go`, built like `TestTimelineRefresh` in `timeline_pane_test.go`: + - `TestEventsTimeline_ChainStartsOnEventsPane`: Enter on a session in the sessions pane returns a cmd; after it, `m.timeline.session == m.selectedSess` and a `timelineTickMsg{gen: m.timeline.tickGen}` handled on `paneEvents` returns a non-nil cmd (the next tick). + - `TestEventsTimeline_StreamMarksStale`: on `paneEvents`, a streamed event for `selectedSess` sets `m.timeline.stale`; an event of another session does not. + - `TestEventsTimeline_NotFoundGivesNoError`: a `timelineLoadedMsg{err: apiclient.ErrNotFound}` on the events pane leaves `m.timeline.tl == nil` and puts no flash or error in the events view (`m.View()` does not contain "timeline"). + - `TestEventsTimeline_TickStopsOffPane`: on `paneSessions`, `handleTimelineTick` returns nil. + - `TestEventsTimeline_EscFromTimelineResumes`: open `T` from the events pane, then Esc: pane is `paneEvents`, `m.timeline.session == m.selectedSess`, and a cmd is returned. + - `TestEventsTimeline_SessionChangeDropsTools`: with `tl` loaded for session "a", opening session "b" sets `m.timeline.tl == nil`. + +- [ ] **Step 2: Run them, expect FAIL.** `go test ./tui -run TestEventsTimeline -count=1` from `cmd/agentop`. + +- [ ] **Step 3: Implement.** + - `handleTimelineTick`: replace `m.pane != paneTimeline` with `!m.timelineChainLive()`. + - `app.go` stale marking: replace `m.pane == paneTimeline && m.timeline.session == e.SessionID` with `m.timelineChainLive() && m.timeline.session == e.SessionID`. + - Enter on a session (keys.go, after `m.rebuildEventsTable()`): batch `m.followEventsTimeline()` with the snapshot cmd it returns; also return it on the cached-only path. + - `openEventsAtSeq`: replace `m.timeline.session = ""` with a call to `followEventsTimeline()` and batch its cmd with the snapshot cmd. Update the comment: the session stays the same as `selectedSess`, so it adds no retention. + - Esc from the timeline (keys.go ~565): after the pane is restored, if it is `paneEvents` or `paneDetail` and `m.selectedSess != ""`, return `m.followEventsTimeline()` instead of clearing `m.timeline.session`; else keep today's clear. + - Back from detail to events (keys.go `case paneDetail: m.pane = paneEvents`): no change needed, the chain runs on both panes. + - `handleTimelineLoaded`: after success, rebuild the events table as in Interfaces. `timelineLoadedMsg` case in `app.go` keeps `harvestForTimeline`. + +- [ ] **Step 4: Run tests, expect PASS**, and run the existing `go test ./tui -run 'TestTimeline' -count=1` (the Timeline pane must not change). + +- [ ] **Step 5: Commit** `feat: Fetch the timeline while the events pane shows a session`. + +--- + +### Task 2: Tool index + +**Depends on:** none + +**Files:** +- Create: `cmd/agentop/tui/tool_rows.go` +- Test: `cmd/agentop/tui/tool_rows_test.go` + +**Interfaces:** +- Produces: + +```go +// toolRow is one tool call of the timeline, with the figures its events-table row shows. +type toolRow struct { + timeline.Tool + Parallel bool // another tool of the same response has the same End + EndsRun bool // Lost and Name is StructuredOutput or SubagentHandback + RunFrom *time.Time // SpawnedAt of the agent whose SpawnTool is this tool's ID + RunTo *time.Time // its ReturnedAt; nil while the subagent runs + BgFrom *time.Time // Start of this Bash tool's job (fact 4) + BgTo *time.Time // the job's End; nil while it runs +} + +func indexTools(tl *timeline.Timeline) map[uint64][]toolRow // key: Tool.Seq; order: tl.Tools order; nil for nil tl +func (t toolRow) status() string // rule 7: "running", "error", "lost", "ok" or "" +func (t toolRow) duration(now time.Time) time.Duration // rule 11 DURATION sort value +func (t toolRow) durationCell(now time.Time) string // rule 8 +func (t toolRow) trailing(now time.Time) string // rule 10, parts joined with " · " +``` + +- [ ] **Step 1: Write failing table tests** in `tool_rows_test.go` (reuse `tlAt`/`tlPtr` from `timeline_pane_test.go`): + - `TestIndexTools_GroupsBySeqInOrder`: tools with Seq 5,5,7 → `len(idx[5]) == 2`, names in input order. + - `TestIndexTools_Parallel`: two tools of Seq 5 with the same End → both `Parallel`; a third tool of Seq 5 with another End → not `Parallel`; a single tool → not `Parallel`; tools with nil End are never `Parallel`. + - `TestToolRow_DurationCell`: ended 13.21s parallel → `"∥13.21s"`; ended 300ms → `"300ms"`; running started 38s before `now` → `"⋯ 38s"` (format `"⋯ " + formatDuration(now-start)`, and `formatDuration(38s)` is `"38.00s"`, so expect `"⋯ 38.00s"`); lost → `""`; end-of-run → `""`. + - `TestToolRow_Status`: nil End → `"running"`; `Error` → `"error"`; `Lost` + `Bash` → `"lost"`; `Lost` + `StructuredOutput` → `""`; ended → `"ok"`. + - `TestIndexTools_AgentRun`: agent `{SpawnTool: "t1", SpawnedAt: tlPtr(10), ReturnedAt: tlPtr(358)}` → `trailing` starts `"run 5m48s · "` followed by the preview; with nil ReturnedAt and `now = tlAt(40)` → `"run ⋯ 30.00s · "`. + - `TestIndexTools_BgJob`: Bash tool ending at 20 for agent "main", job `{Agent: "main", Start: tlAt(20), End: tlPtr(262)}` → `"bg 4m02s · "`; job with nil End → `"bg ⋯ …"`. Two parallel tools ending at 20, one `Bash` and one `Read`: only the Bash one gets `bg`. A job of another agent matches nothing. + - `TestToolRow_EndsRunTrailing`: lost `SubagentHandback` with preview "x" → trailing `"ends run · x"`; empty preview → `"ends run"`. + - `TestToolRow_DurationSortValue`: running → `now-start`; lost and end-of-run → 0; ended → `End-Start`. + +- [ ] **Step 2: Run, expect FAIL** (`go test ./tui -run 'TestIndexTools|TestToolRow' -count=1`). + +- [ ] **Step 3: Implement `tool_rows.go`.** Build a `map[string]timeline.Agent` by `SpawnTool` and index jobs by `(Agent, Start.UnixNano())` once, then one pass over `tl.Tools`. Parallel: per Seq, count End values (`UnixNano`). End-of-run names in a package-level `map[string]bool` with a one-line comment that names `core/session/timeline/agents.go` as the source. Trailing parts order: run, bg, ends run, preview; skip empty parts. + +- [ ] **Step 4: Run, expect PASS.** + +- [ ] **Step 5: Commit** `feat: Index timeline tools by the response that asked for them`. + +--- + +### Task 3: Spanning last cell in `tui/table` + +**Depends on:** none + +**Files:** +- Modify: `cmd/agentop/tui/table/table.go` (`renderRow`, comment above it) +- Test: `cmd/agentop/tui/table/table_test.go` + +**Interfaces:** +- Produces: rule "a row with 1 < len(cells) < len(cols) renders cells 0..n-2 in their columns and cell n-1 across the remaining columns, width = sum of the remaining columns' `Width` plus `2*(k-1)` cell padding for the k columns it covers". Rows of other shapes render as today. + +- [ ] **Step 1: Check callers.** `grep -rn "table.Row{" cmd/agentop/tui --include=*.go | grep -v _test` and read each row builder that can produce fewer cells than columns. Expected: none outside the one-cell section rows. Write the result in the commit message body. If one exists, stop and report it in the task review: the rule would change that caller. + +- [ ] **Step 2: Write failing tests** in `table_test.go`: + - `TestRenderRow_LastCellSpans`: columns widths 4,6,6,6; row `{"a","b","long text that is twenty"}` → the rendered line (ANSI stripped) contains `"long text that is twenty"` up to the width that 2 columns plus padding give, and the line width equals a full 4-cell row's width. + - `TestRenderRow_SpanTruncatesAtTableWidth`: a trailing cell longer than the span ends with `"…"` and the line is not wider than a full row. + - `TestRenderRow_SpanKeepsCursorAndMatchStyles`: the cursor row's spanning cell carries the `Selected` style; a search hit inside the spanning cell is painted with `Match` (use the existing helpers in `search_test.go`). + +- [ ] **Step 3: Run, expect FAIL** (`go test ./tui/table -count=1`). + +- [ ] **Step 4: Implement** in `renderRow`: when `1 < len(row) < len(cols)`, compute the span width over the remaining columns with `Width > 0` and render the last cell in one box of that width; `cellSpans(r, n-1, text)` still indexes the search cell by its row position. Update the doc comment. + +- [ ] **Step 5: Run, expect PASS. Commit** `feat: Let a short table row's last cell span the remaining columns`. + +--- + +### Task 4: Tool rows in `rebuildEventsTable` + +**Depends on:** Task 1, Task 2, Task 3 + +**Files:** +- Modify: `cmd/agentop/tui/events_pane.go` (`eventKey`, `findByKey`, `eventRow`, `rebuildEventsTable`, `selectedEventOn` callers) +- Modify: `cmd/agentop/tui/events_columns.go` (tool cells and sort keys) +- Modify: `cmd/agentop/tui/timeline_pane.go` (`handleTimelineLoaded` sets `m.timeline.tools = indexTools(msg.tl)`; reset `tools` wherever `tl` is reset) +- Modify: `cmd/agentop/tui/keys.go` (Enter on a tool row; `keyOf(m.selectedEvent())` at ~1030, 1088, 1107, 1144) +- Modify: `cmd/agentop/tui/search_surfaces.go` (lines ~108, ~126), `cmd/agentop/tui/timeline_pane.go` `revealPendingSeq` (~532) +- Test: `cmd/agentop/tui/events_tool_rows_test.go` (new) + +**Interfaces:** +- Consumes: `toolRow`, `indexTools`, `status`, `durationCell`, `duration`, `trailing` (Task 2); `followEventsTimeline` and the rebuild on load (Task 1); the spanning cell (Task 3). +- Produces: `eventRow.tool *toolRow`, `eventRow.parent int` (the parent exchange's index in `ctxs`), `eventRow.last bool` (last tool of its exchange); `eventKey.tool`, `eventKey.toolAgent`; `func pinKeyOf(er eventRow) eventKey`; `func (m *model) selectedRowOn(row int) eventRow`; `func toolCells(cols []eventColumn, cc cellContext, t eventRow, sorted bool, now time.Time) table.Row` in `events_columns.go`. + +- [ ] **Step 1: Write failing tests** in `events_tool_rows_test.go`. Build a model like `rebuildBenchModel` (events with `Seq` set on responses) and set `m.timeline.session = "s"`, `m.timeline.tl`, `m.timeline.tools = indexTools(tl)`, and a fixed `m.clock`. Read rendered cells from `m.eventsTbl.Rows()`: + - `TestToolRows_OneTool`: row after exchange 12 has `#` `"┗⚒"`, empty DIR, TIME = tool start in `"15:04:05.00"`, METHOD `"Bash"`, STATUS `"ok"`, DURATION `"4.21s"`, last cell = preview; `len(row) == index of first column after DURATION + 1`. + - `TestToolRows_Parallel`: two tools → `"┣⚒"` then `"┗⚒"`, both DURATION start with `"∥"`. + - `TestToolRows_States`: running (`"running"`, `"⋯ …"`), error (`"error"`), lost (`"lost"`, empty DURATION), end-of-run (empty STATUS and DURATION, last cell starts `"ends run"`). + - `TestToolRows_RunAndBg`: Agent tool last cell starts `"run "`; Bash with job starts `"bg "`. + - `TestToolRows_InExpandedBlock`: subagent tools render right after the member exchange inside the expanded block, DIR cell `"│"`; collapsed block → no tool rows. + - `TestToolRows_HideInactiveHidesTools`: with `hideInactive` and an inactive exchange, its tools are gone too. + - `TestToolRows_SortByDuration`: `m.sortCol = colDuration`, `m.sortDesc = true`: a 38s running tool ranks above a 4.4s exchange; its `#` is `"↳14"` (the parent's `#`). + - `TestToolRows_SortOtherColumnIsStable`: sort by COST: tool rows keep their place after their exchange among ties (tie-break by `parent`). + - `TestToolRows_SearchFindsPreview`: `m.search[paneEvents] = "git -C"` → the tool row index is in `m.eventsTbl.Matches()`. + - `TestToolRows_EnterDoesNothing`: cursor on a tool row, Enter → pane stays `paneEvents`, cmd nil, `m.selectedEvent() == nil`. + - `TestToolRows_CursorStaysOnTool`: cursor on a tool row, then a new event arrives and a sort is toggled → cursor is on the same tool (`pinKeyOf` equal). + - `TestToolRows_TailFollowOnToolRow`: no sort, cursor on the last row, which is a tool row; a new exchange with a tool arrives → after the rebuild the cursor is on the new last row, and `m.selectedEvent() == nil` does not panic. (Yank needs no test: it runs only in the detail pane, and a tool row cannot open the detail pane, rule 13.) + - Review Focus tests: `TestToolRows_TimelineBeforeEvents`, `TestToolRows_OrphanToolNotShown`, `TestToolRows_NarrowTerminalCellCount` (width 80: every row has `len(row) <= len(m.eventsTbl.Columns())`), `TestToolRows_DetailSearchSkipsToolRows` (start a detail search with tool rows visible; `detailMatchRows`, `detailSearchStale`, `detailNewRows` do not panic and never return a tool row index). + - `TestToolRows_LinkedEventNoTools`: a response whose `SessionID` is another session with a matching `Seq` gets no tool rows (decision 3). + +- [ ] **Step 2: Run, expect FAIL** (`go test ./tui -run TestToolRows -count=1`). + +- [ ] **Step 3: Implement.** + - `eventRow`: add `tool *toolRow`, `parent int`, `last bool`. `rowHolds` is unchanged (tool rows have nil `event` and `resp`). + - `eventKey`/`findByKey`: a key with `tool != ""` matches the row whose `tool.ID == k.tool && tool.Agent == k.toolAgent`; other keys as today. `rowKey(er)` per decision 7. Replace the `keyOf(m.selectedEvent())`, `keyOf(m.selectedEventOn(prevRow))`, `keyOf(r.event)` pin writes listed in Files with `pinKeyOf` of the same row. Add `selectedRowOn(row int) eventRow` beside `selectedEventOn` (zero `eventRow{}` when `row` is out of range), and change `events_pane.go` ~446 to `pinKeyOf(m.selectedRowOn(prevRow))`. Change the keys.go and search_surfaces.go callers to `er, _ := m.selectedEventRow(); pinKeyOf(er)`. Leave the `keyOf(r.event)` calls in `search_details.go`: they key the detail-search hits by event, and tool rows are skipped there by `r.event == nil`. `revealPendingSeq` and `loadOlderFromRow` only ever land on event rows; leave them. + - `renderEventRow(j)` in `rebuildEventsTable`: after the exchange row, look up `m.timeline.tools[resp.Seq]` (decision 3; only when `m.timeline.session == m.selectedSess`), and append one row per tool to `rows`, `m.visibleRows` (an `eventRow{tool: &t, parent: j, last: i == n-1}`) and, when searching, `full` (cells: tool name for METHOD, the full trailing text for the last cell, `NoMatch` false). Return the number of tool rows appended. In the main loop, when `sortCol != nil`, append one key per tool row after the exchange's key: `rowKey{val: , exchange: j}` (the existing `rowKey` struct type; rule 11, decision 6). Member rows in blocks reach `renderEventRow` too, so rule 1 holds inside expanded blocks with no extra code. + - `toolCells` per decision 5 and rules 2-10: `#` is `"┣⚒"`/`"┗⚒"`, or `"↳" + ` when `sorted`; DIR is `"│"` when the parent row is a subagent member and `m.subagentBlocks != nil`; apply each column's `align`. The parent `#` is the parent ctx's `#` column cell (`blockIDs` then `ids`). + - Enter (keys.go ~744): after `selectedEventRow`, return nil when `er.tool != nil` (rule 13). + - `handleTimelineLoaded`: set `m.timeline.tools = indexTools(msg.tl)` with `tl`; set it to nil wherever `tl` is set to nil (`openTimeline`, `followEventsTimeline`). On error keep the old index (rule 15: no extra error; a 404 before any load leaves it nil). + +- [ ] **Step 4: Run, expect PASS.** Then `scripts/dev/go-test-shards.sh -short cmd/agentop` from the repo root: all pass. + +- [ ] **Step 5: Commit** `feat: List tool calls in the events table with their duration`. + +--- + +### Task 5: Help text + +**Depends on:** none + +**Files:** +- Modify: `cmd/agentop/tui/events_columns.go` (colDuration `desc`) +- Modify: `cmd/agentop/tui/help_overlay.go` (`paneEvents` bindings) +- Modify: `cmd/agentop/README.md` (Events section, after the sort paragraph near line 1110) +- Test: `cmd/agentop/tui/help_overlay_test.go` + +**Interfaces:** none. + +- [ ] **Step 1: Write failing test** `TestHelp_EventsExplainsToolTime`: the events help overlay text contains `"for a tool: from the reply to the next request of the same agent"`; the DURATION column's `desc` contains the same phrase. + +- [ ] **Step 2: Run, expect FAIL.** + +- [ ] **Step 3: Implement.** DURATION `desc`: `"how long the exchange took; for a tool: from the reply to the next request of the same agent"`. Help overlay: add binding `{"⚒ rows", "tool calls; for a tool: from the reply to the next request of the same agent"}`. If the binding is wider than the overlay's fit tests allow, shorten the key text, not the phrase. README: one short paragraph: tool rows, `┣⚒`/`┗⚒`/`↳`, and that the time is seen from outside: it includes the client's own delay and can miss a tool that started while the reply still streamed. + +- [ ] **Step 4: Run, expect PASS**, including `go test ./tui -run 'TestHelp|Fit' -count=1`. + +- [ ] **Step 5: Commit** `docs: Say how agentop measures a tool's duration`. + +--- + +### Task 6: Benchmark + +**Depends on:** Task 4 + +**Files:** +- Modify: `cmd/agentop/tui/events_rebuild_bench_test.go` + +**Interfaces:** consumes `indexTools` and the Task 4 rendering. + +- [ ] **Step 1:** In `rebuildBenchModel`, give each response `Seq: uint64(2*i+2)` (requests `2*i+1`) and build a `*timeline.Timeline` with one tool per response (95% of steps have one tool, fact table) and two parallel tools on every 20th response, each ended 1s after the response. Set `m.timeline.session = "s"`, `m.timeline.tl`, `m.timeline.tools = indexTools(tl)`. Add a `"sorted"` sub-benchmark with `m.sortCol = colDuration`. + +- [ ] **Step 2: Run** `go test ./tui -run '^$' -bench BenchmarkRebuildEventsTable -benchmem -count=6` on this branch. For the base figures, run from the worktree root: `git worktree add --detach /tmp/tool-rows-1776-base refs/base/tool-rows-1776`, then `(cd /tmp/tool-rows-1776-base/cmd/agentop && go test ./tui -run '^$' -bench BenchmarkRebuildEventsTable -benchmem -count=6)`, then `git worktree remove --force /tmp/tool-rows-1776-base`. The base has no `"sorted"` sub-benchmark; compare only the sub-benchmarks that both have. Put both tables in the commit message body so the PR body can quote them (use `benchstat` if installed; otherwise the raw lines). + +- [ ] **Step 3: Commit** `test: Add tool rows to the events-table rebuild benchmark`. + +--- + +## PRs + +| PR | Title | Parent | Tasks | estLines | +|---|---|---|---|---| +| P1 | `feat: List tool calls in agentop's events table with their duration` | "" | 1, 2, 3, 4, 5, 6 | 1400 | + +Estimate per task: T1 180, T2 320, T3 110, T4 700, T5 50, T6 40. + +The PR body reports the Task 6 benchmark against `main` and lists the Review Focus items. + +## Gate + +From the worktree root. The gofmt and vet steps were run once on the unchanged base (ce712778) during plan review: both pass, with no change to how they are called. + +```sh +(cd cmd/agentop && test -z "$(gofmt -l .)" && GOWORK=off go vet ./...) && scripts/dev/go-test-shards.sh -race cmd/agentop && (cd cmd/agentop && go test -race $(go list ./... | grep -v 'agentop$')) +``` + +## For a human after review + +Needs: all tasks merged into the PR branch (P1). + +1. Install the branch build of agentop only (`go build -o /tmp/agentop ./cmd/agentop`); do not run `make dev-install` (it restarts the shared proxy). +2. Run `/tmp/agentop` against the laptop service (`:47601`). Open 2 or 3 large live sessions. Check: tool rows under each exchange, `∥` on parallel tools, `running` that updates, Agent `run`, Bash `bg`, and sort by DURATION (`c`, then `s` on DURATION). +3. Watch the timeline fetch size on a large live session (up to ~470 KB per fetch, up to once a second). Decide if a smaller projection of the timeline endpoint is necessary. That decision is a new issue, not part of this PR.