diff --git a/cmd/abctl/README.md b/cmd/abctl/README.md index df948a8ef..c0d71971d 100644 --- a/cmd/abctl/README.md +++ b/cmd/abctl/README.md @@ -1150,7 +1150,9 @@ Layered on top of all of them: | `C` | any session-view pane (not the picker) | open the registered-plugin catalog. Was `P` until the pipeline took that letter | | `r` | catalog | refresh the catalog from `/v1/plugins` | | `A` | any session-view pane (not the picker) | open the per-agent cost breakdown — what each coding agent has spent today. Capital `A` because lowercase `a` cycles the spend drawer's axis. Refetches on every press, then **refuses below two agents** and says which one it found: a one-row breakdown restates a total already on screen. Not in the footer for that reason; the `?` overlay names it | -| `Esc` | agents | back to the pane `A` was pressed on | +| `↑↓` / `jk` | agents | move the cursor | +| `↵` | agents | scope the usage pane to the agent under the cursor, and leave. Pressing it again **on the agent already scoped clears the scope** — there is no "all agents" row, so one key goes both ways and the footer's label flips to say which. The usage pane is the only surface that honours it: sessions and events carry no agent, the spend band and its drawer fetch separately and keep showing every agent, and `abctl cost --agent` is a separate process | +| `Esc` | agents | back to the pane `A` was pressed on, leaving the scope as it is | | `e` | pipeline | edit pipeline subtree in `$EDITOR` | | `y` | edit/diff | apply the edit | | `N` | edit/diff | abort the edit | @@ -1159,6 +1161,19 @@ Layered on top of all of them: | `Esc` | edit/{waiting,rollback} | background the watch; result lands as a footer flash | | `q` / `Ctrl+C` | any | quit (closes the key-help overlay first, if open) | +**The picker also opens itself at startup**, once per connection, when two or more +agents have been seen in the window — the same two-agent rule `A` applies, so a +one-agent proxy goes straight to the sessions pane and says nothing. Nothing is +remembered between runs: a second agent appearing is exactly when the picker +becomes worth showing, so a remembered dismissal would go stale then. + +While a scope is active, two things on the usage pane change and both say so: +`[b]` disappears from the footer, because the scope needs `group=agent` on the +wire and there is no second axis left to break down by; and the latency metric +reports that it is unavailable per agent rather than plotting zeroes. Response +times are recorded per bucket across every agent that shared it, so +`/v1/usage` carries nothing that could attribute them to one. + ## Settings abctl remembers the events-table column selection, the sort order, and the active diff --git a/cmd/abctl/cmd_cost.go b/cmd/abctl/cmd_cost.go index 924ad25e8..c0c36a12c 100644 --- a/cmd/abctl/cmd_cost.go +++ b/cmd/abctl/cmd_cost.go @@ -206,7 +206,12 @@ Flags: } if *agent != "" { - scoped, err := scopeToAgent(snap, *agent) + // KeepBuckets, not NarrowBuckets: on this path the command prints window totals, so it + // needs no per-bucket narrowing and pays for none. (Under --by it does read buckets — + // hence "on this path" rather than a claim about the command.) abctl's usage pane passes the other value + // because it renders a chart from the buckets themselves. See usage.BucketScope for why + // this is a parameter rather than a default. + scoped, err := usage.ScopeToAgent(snap, *agent, usage.KeepBuckets) if err != nil { fmt.Fprintf(stderr, "abctl cost: %v\n", err) return 1 @@ -224,91 +229,6 @@ Flags: return 0 } -// scopeToAgent narrows a group=agent snapshot to one agent's figures. -// -// IT REWRITES Totals AND HANDS BACK A SNAPSHOT, rather than rendering the agent itself, so the -// writers apply to it directly with no second implementation and no chance of the two drifting: -// the negative-total refusal, the coverage-gap disclosure and the incomplete-read admission each -// read one agent's numbers. Which fields do NOT survive the narrowing, and why, is stated at the -// narrowing itself below. A COPY, never the caller's snapshot mutated in place. -// -// The fold is usage.FoldSeriesAcrossWindow, the same one abctl's AGENTS pane uses, so the -// figure printed here and the row shown there cannot disagree. -// -// AN UNKNOWN AGENT IS AN ERROR THAT NAMES THE KNOWN ONES. The labels are User-Agents, so they -// are neither short nor guessable — "bob" is the obvious thing to try and is not what Bob -// sends. The set is already in hand, so withholding it would be a choice. -func scopeToAgent(snap *usage.Snapshot, agent string) (*usage.Snapshot, error) { - series := usage.FoldSeriesAcrossWindow(snap.Buckets) - counts, ok := series[agent] - if !ok { - known := make([]string, 0, len(series)) - for label := range series { - known = append(known, label) - } - // Sorted so the same window reports the same order every run; a set printed in map - // order is a set a reader cannot diff against yesterday's. - sort.Strings(known) - if len(known) == 0 { - return nil, fmt.Errorf("no agent traffic in the %s window, so --agent %q matches nothing", - snap.Window, agent) - } - return nil, fmt.Errorf("no agent %q in the %s window; seen: %s", - agent, snap.Window, strings.Join(known, ", ")) - } - scoped := *snap - scoped.Totals = counts - // EVERY WHOLE-WINDOW STATEMENT ABOUT WHERE THE TOTALS CAME FROM GOES WITH Totals, or it is - // printed beside one agent's figure while describing all of them. Replacing only Totals left - // `--agent ` printing $0.00 — the window was priced, just not this - // agent's traffic — for exactly the agent the AGENTS pane prints "—" for, which breaks both - // writeCostSummary's "cost unavailable rather than $0.00" rule and this function's own claim - // that the figure here and the row there cannot disagree. - // - // Priced is RE-DERIVED with the producers' own rule rather than one invented here: both - // snapshot.go and sessionapi set it to Totals.PricedRequests > 0, so the narrowed snapshot is - // the one they would have emitted had this agent's traffic been the whole window. - scoped.Priced = counts.PricedRequests > 0 - // The three by-model maps are DROPPED, not narrowed, because nothing here can narrow them: a - // bucket's series is keyed by agent and carries no per-model breakdown, so the only available - // readings are the window's maps — which describe other agents' traffic — or none. They are - // omitempty on the wire, and costIncompleteReasonLines already treats an absent map as - // nothing to say, which is its common case for a ledger-backed window anyway. - scoped.PricedBy = nil - scoped.UnpricedBy = nil - scoped.IncompleteBy = nil - // Degraded and DaysOutsideRetention STAY, and the asymmetry is the point: they describe the - // READ and the retention configuration, which are the same facts whichever agent is scoped - // to. Dropping them would hide a short sum behind a narrower question. - // - // SeriesOvershootMicros and SeriesAvoidedOvershootMicros stay too, and they are the two the - // "every" above has to account for rather than pass over. Both are defect reports about a - // breakdown — the series summed to MORE than the total — so they belong with Degraded rather - // than with the provenance maps. A correct producer never sends either on this path: - // residualOf leaves them nil unless the series overshoots, which cannot happen where the - // figures reconcile. Where one does arrive it is upstream's bug, and forwarding it says so; - // narrowing it to an agent would be inventing a per-agent overshoot nothing computed. - - // Currencies IS CARRIED OVER, AND IT NO LONGER DESCRIBES Totals. Said out loud because it is - // the one field on this struct that the narrowing above invalidates, and the honest options are - // worse than keeping it. - // - // The field means "every unit the rows behind Totals were denominated in", and after this copy - // Totals is one agent while the list is the whole window. Narrowing it is not available: - // deciding which units THIS agent's traffic carries needs a cross-tabulation of agent against - // currency, and a folded per-agent Counts has already summed that axis away. Dropping it is - // worse than leaving it — this agent's own traffic may well be the mixed part, and an absent - // list reads as "single unit", so the surface would print a confident figure that is exactly - // the credits-plus-dollars sum this whole change exists to refuse. - // - // SO THE OVER-REFUSAL IS DELIBERATE, and it is the safe direction: a per-agent figure is - // withheld in a mixed window even when that agent billed in one unit. writeCostSummary says - // which of the two it is rather than letting the reader assume, because "no figure for this - // agent" and "no figure for this window" have different fixes. - - return &scoped, nil -} - // costJSON is the --json shape: the window actually served plus the totals // verbatim, the three maps that say where the totals came from, what they miss and which // way any inexact figure in them is inexact, and the ledger's own admission when the read @@ -375,7 +295,7 @@ type costJSON struct { // // EXCEPT UNDER --agent, WHERE IT DESCRIBES THE WINDOW AND Totals DESCRIBES ONE AGENT. The // sentence above is the whole truth on every other path; on that one the two fields have - // different subjects and no third field says so. scopeToAgent explains why it cannot be + // different subjects and no third field says so. usage.ScopeToAgent explains why it cannot be // narrowed — deciding one agent's units needs a cross-tabulation a folded Counts has already // summed away — and the human surface prints a line saying whose mixture it is. This one does // not, deliberately: the discrepancy is in the OVER-refusing direction, so a script that @@ -717,7 +637,7 @@ func writeCostSummary(snap *usage.Snapshot, stdout io.Writer, agent string) { strings.Join(snap.Currencies, " and ")) // WHOSE MIXTURE IT IS, on the --agent path. The list describes the WINDOW; Totals here // describes one agent, and no client-side arithmetic can narrow the first to the second — - // see scopeToAgent for why the field is carried over anyway. Without this line a reader who + // see usage.ScopeToAgent for why the field is carried over anyway. Without this line a reader who // asked about one agent reads the refusal as a statement about that agent's own traffic and // goes looking for a second gateway it may never have called. if agent != "" { diff --git a/cmd/abctl/cmd_cost_test.go b/cmd/abctl/cmd_cost_test.go index 645284f11..c72f396cd 100644 --- a/cmd/abctl/cmd_cost_test.go +++ b/cmd/abctl/cmd_cost_test.go @@ -2582,7 +2582,7 @@ func TestRunCost_JSONOmitsCurrenciesWhenTheProducerDoesNotReportThem(t *testing. // --agent on a mixed window says WHOSE mixture it is. // -// scopeToAgent narrows Totals to one agent and carries Currencies over from the whole window, and +// usage.ScopeToAgent narrows Totals to one agent and carries Currencies over from the whole window, and // no client-side arithmetic can narrow the second — a folded per-agent Counts has summed the // currency axis away. So the refusal stands, deliberately over-refusing, and this line is what // stops a reader taking it as a statement about the agent they asked about. diff --git a/cmd/abctl/tui/agents_pane.go b/cmd/abctl/tui/agents_pane.go index 6aeb995b6..5b3daba71 100644 --- a/cmd/abctl/tui/agents_pane.go +++ b/cmd/abctl/tui/agents_pane.go @@ -121,6 +121,25 @@ const agentsFetchTimeout = 5 * time.Second // `abctl cost` documents. const agentsWindow = usage.WindowToday +// agentsOpen says what the reply to a rows fetch is allowed to do with them. +// +// AN ENUM RATHER THAN A BOOL because there are three answers, not two, and the third differs +// from the second only in whether it may speak. `A` is owed an answer either way — a key that +// appears to do nothing is the defect agentsPaneRefusal exists to prevent. The startup gate is +// owed the opposite: nobody asked for it, so it enters or it stays quiet. +type agentsOpen int + +const ( + // agentsOpenNever is a background refresh: update the rows, enter nothing, say nothing. + agentsOpenNever agentsOpen = iota + // agentsOpenOnPress is an `A` press. It enters, or it flashes the reason it will not. + agentsOpenOnPress + // agentsOpenAtStartup is the gate run once per connection. It enters when + // agentsPaneApplies, and otherwise does nothing AND says nothing — see + // startupAgentsGateCmd. + agentsOpenAtStartup +) + // agentRowsLoadedMsg carries a fetched per-agent breakdown back to Update. // // NOT agentsLoadedMsg, which is TAKEN — by the Kubernetes namespace picker, whose @@ -128,15 +147,15 @@ const agentsWindow = usage.WindowToday type agentRowsLoadedMsg struct { rows []agentRow err error - // open records that the `A` key asked for this, so the reply may enter the pane. A - // background refresh sets it false and only updates the table, which is why this is a - // field rather than inferred from the current pane: by the time a reply lands the reader - // may have moved. - open bool + // open records who asked, so the reply knows whether it may enter the pane and whether it + // may complain. A field rather than something inferred from the current pane: by the time a + // reply lands the reader may have moved. + open agentsOpen // from is the pane the `A` press came from, captured AT PRESS TIME and carried here for // exactly the reason the field above gives: by the time this reply lands the reader may have - // moved, so reading m.pane then records a caller the press never had. Only meaningful with - // open:true; a background refresh leaves it paneNone and enters nothing. + // moved, so reading m.pane then records a caller the press never had. Meaningless under + // agentsOpenNever, which enters nothing, and unread on the startup path — see that arm in + // Update for why it does not consult this field. from paneID } @@ -144,9 +163,9 @@ type agentRowsLoadedMsg struct { // // group=agent AND NO AGENT FILTER, because /v1/usage has none: it reads window, resolution, // group and session, and session is its only scoping parameter. The per-agent split therefore -// arrives as Bucket.Series and is folded here. That limit is also why this pane is read-only — -// there is no server-side agent scope to apply to any other pane. -func (m *model) fetchAgentRowsCmd(open bool, from paneID) tea.Cmd { +// arrives as Bucket.Series and is folded here, and the scope the pane sets is applied to the +// fetched snapshot rather than requested — see usage.ScopeToAgent. +func (m *model) fetchAgentRowsCmd(open agentsOpen, from paneID) tea.Cmd { if m.client == nil { return nil } @@ -181,6 +200,35 @@ func agentsColumns() []table.Column { } } +// startupAgentsGateCmd asks, once per connection, whether this proxy has enough agents on it to +// be worth a picker. +// +// ONE FETCH FROM initSessionView, which is the single place every entry point converges on: +// `--endpoint` mode's Init, the pod picker's portForwardReadyMsg, and `[l]`'s local endpoint all +// call it, and each replaces m.client first. Hooking it there rather than in Init is what makes +// the gate run again when the operator backs out to the pod picker and enters a DIFFERENT pod — +// a different proxy has different agents on it, and the answer from the previous one is not an +// answer about this one. The spend strip's chain is started from the same place for the same +// reason. +// +// NO MEMORY OF PREVIOUS ANSWERS, deliberately: the decision is a pure function of what the +// window currently shows. The Namespaces → Pods picker remembers nothing either, and a +// remembered dismissal would go stale exactly when it mattered — the moment a second agent +// appears is the moment the picker becomes worth showing. +// +// THE FIRST FRAME IS NOT BLOCKED. This returns a tea.Cmd like every other fetch, so the sessions +// pane paints and streams while the answer is in flight; entering AGENTS is something that +// happens a beat later, if it happens. A gate that waited would add its own latency to every +// startup, including the majority that it declines. +func (m *model) startupAgentsGateCmd() tea.Cmd { + // paneNone: this gate has no caller pane. It interrupts the sessions view before the + // operator has pressed anything, so there is no press-time pane to record — and the esc + // arm's paneNone fallback already lands on Sessions, which that arm documents as the one + // pane always defensible to land on. The reply handler does not read this field on the + // startup path at all; see the agentsOpenAtStartup case in Update for why not. + return m.fetchAgentRowsCmd(agentsOpenAtStartup, paneNone) +} + // newAgentsTable builds an empty per-agent breakdown table. func newAgentsTable() table.Model { t := table.New( @@ -249,3 +297,48 @@ func (m *model) enterAgentsOrRefuse(from paneID) (entered bool, refusal string) m.rebuildAgentsTable() return true, "" } + +// selectedAgentLabel is the label of the row under the cursor, or "" when there is none. +// +// READ OFF m.agents BY CURSOR INDEX, not out of the rendered table cell: the cell is passed +// through sanitizeLabel, which is a display transform — a control character or a long label +// arrives on the wire and leaves that function altered, so scoping to what the cell says could +// scope to a string no agent ever sent. The two are kept in step by rebuildAgentsTable, which +// builds the rows from m.agents in order. +func (m *model) selectedAgentLabel() string { + i := m.agentsTbl.Cursor() + if i < 0 || i >= len(m.agents) { + return "" + } + return m.agents[i].label +} + +// leaveAgentsPane returns to whichever pane opened the AGENTS pane. +// +// ONE EXIT FOR BOTH KEYS — esc backs out, Enter picks an agent and then backs out — so the two +// cannot drift on where the pane returns to. A key-opened surface owes its caller a way back; +// without an exit at all this pane was a dead end reachable only by `q`. +// +// THE FALLBACK IS SESSIONS, and for paneCatalog's stated reason rather than by imitation: +// Sessions is the one pane that is always a defensible place to land, while the enum's zero +// value is the Kubernetes namespace picker, which would look like the connection had gone away. +// The startup gate leans on this fallback deliberately — it records paneNone because it has no +// caller pane at all. +// +// RETURNING INTO USAGE RESTARTS ITS POLLING CHAIN. This pane holds no ticker of its own, but the +// usage pane's tick was dropped by its `m.pane != paneUsage` guard while this pane was up, so +// without the resume its 20s auto-refresh is silently dead. It matters more now than it did: +// Enter changes what the usage pane is showing, so landing back on a pane that never refetches +// would leave the new scope unapplied until the operator pressed something. +func (m *model) leaveAgentsPane() tea.Cmd { + if m.previousPane != paneNone { + m.pane = m.previousPane + m.previousPane = paneNone + } else { + m.pane = paneSessions + } + if m.pane == paneUsage { + return m.resumeUsagePolling() + } + return nil +} diff --git a/cmd/abctl/tui/agents_pane_test.go b/cmd/abctl/tui/agents_pane_test.go index 208f95aa4..c5ee76ead 100644 --- a/cmd/abctl/tui/agents_pane_test.go +++ b/cmd/abctl/tui/agents_pane_test.go @@ -1,12 +1,18 @@ package tui import ( + "context" + "errors" "math" + "net/http" + "net/http/httptest" "strings" "testing" + "time" tea "github.com/charmbracelet/bubbletea" + "github.com/rossoctl/cortex/cmd/abctl/apiclient" "github.com/rossoctl/cortex/core/cost/usage" ) @@ -349,7 +355,7 @@ func TestAgentsPane_RecordsTheCallerAtPressTimeNotAtReplyTime(t *testing.T) { // The reader moves before the reply arrives. A reply-time read of m.pane sees this // pane; the press never did. m.pane = tc.movedTo - updated, _ := m.Update(agentRowsLoadedMsg{rows: rows, open: true, from: failed.from}) + updated, _ := m.Update(agentRowsLoadedMsg{rows: rows, open: agentsOpenOnPress, from: failed.from}) m = updated.(*model) if m.pane != paneAgents { t.Fatalf("reply did not open the pane: pane = %v", m.pane) @@ -515,3 +521,215 @@ func TestAgentsPane_NavigationKeysMoveTheCursor(t *testing.T) { }) } } + +// errStartupProbe stands in for whatever the endpoint answered. The message is never rendered +// by the cases that use it — that is the assertion — so its text only has to be identifiable +// in a failure. +var errStartupProbe = errors.New("dial tcp 127.0.0.1:1: connect: connection refused") + +// Startup lands on AGENTS when two or more agents are on the wire, and nowhere else otherwise. +// +// THE GATE IS agentsPaneApplies, which until now had no caller outside its own tests: the rule +// "fewer than two agents skips the pane" was written, asserted and never consulted. A picker +// offering one row is a keystroke that cannot change what is displayed, and one agent is still +// every deployment that has not adopted a second — so entering unconditionally would put a new +// mandatory step in front of those users to no purpose. +// +// SILENT WHEN IT DECLINES. enterAgentsOrRefuse's refusal is written for someone who pressed `A` +// and is owed an answer; nobody asked for this one, so flashing "only claude-code has been seen" +// at every startup would be an unsolicited complaint about a normal deployment. +func TestAgentsPane_StartupEntersOnlyWhereTheGateApplies(t *testing.T) { + row := func(label string) agentRow { + return agentRow{label: label, Counts: usage.Counts{Requests: 10, PricedRequests: 10}} + } + for _, tc := range []struct { + name string + rows []agentRow + want paneID + }{ + // The case this feature exists for: a laptop running Claude Code and Bob at once. + {"two agents opens the picker", []agentRow{row("claude-code/2.1.270"), row("bob-shell/2.0.5")}, paneAgents}, + {"three agents opens the picker", []agentRow{row("a/1"), row("b/2"), row("c/3")}, paneAgents}, + // Every deployment with one coding agent, which is most of them. + {"one agent goes straight to sessions", []agentRow{row("claude-code/2.1.270")}, paneSessions}, + // Reachable rather than theoretical: a proxy that has served no inference yet reports + // no agent series at all, and an empty picker is worse than the view behind it. + {"no agents goes straight to sessions", nil, paneSessions}, + } { + t.Run(tc.name, func(t *testing.T) { + m := &model{pane: paneSessions, previousPane: paneNone, agentsTbl: newAgentsTable(), client: deadClient()} + updated, _ := m.Update(agentRowsLoadedMsg{rows: tc.rows, open: agentsOpenAtStartup}) + m = updated.(*model) + if m.pane != tc.want { + t.Errorf("startup with %d agents landed on %v, want %v", len(tc.rows), m.pane, tc.want) + } + if m.flash != "" { + t.Errorf("startup flashed %q; nobody asked for the agents pane, so a refusal "+ + "is an unsolicited complaint", m.flash) + } + }) + } +} + +// esc from the startup picker lands on Sessions, so the operator ends up where they were headed +// rather than on the pane enum's zero value — the Kubernetes namespace picker, which would look +// like the connection had gone away. +// +// It gets there through leaveAgentsPane's paneNone fallback rather than by recording a caller: +// the gate has no caller pane, which is what that function's doc says it leans on. +func TestAgentsPane_StartupEscapesToSessions(t *testing.T) { + rows := []agentRow{ + {label: "claude-code/2.1.270", Counts: usage.Counts{Requests: 10}}, + {label: "bob-shell/2.0.5", Counts: usage.Counts{Requests: 8}}, + } + // previousPane is seeded with a pane the startup arm MUST overwrite. Seeding paneNone — the + // value that arm assigns — let the fixture supply the mechanism the comment above credits, and + // two mutants survived on it: deleting `m.previousPane = paneNone` from the arm, and setting it + // to paneSessions instead. Neither can survive a seed the arm has to clear. + m := &model{pane: paneSessions, previousPane: panePipeline, agentsTbl: newAgentsTable(), client: deadClient()} + updated, _ := m.Update(agentRowsLoadedMsg{rows: rows, open: agentsOpenAtStartup}) + m = updated.(*model) + if m.pane != paneAgents { + t.Fatalf("startup did not enter the pane: %v", m.pane) + } + // paneNone, asserted directly. Landing on Sessions is the same observable outcome whether the + // arm recorded paneNone and leaveAgentsPane fell back, or the arm recorded paneSessions + // itself — so the esc assertion below cannot tell the documented mechanism from the other one. + if m.previousPane != paneNone { + t.Errorf("the gate recorded previousPane=%v, want paneNone: it has no caller pane, and the "+ + "esc arm's fallback is what the comment above credits", m.previousPane) + } + m.handleKey(tea.KeyMsg{Type: tea.KeyEsc}) + if m.pane != paneSessions { + t.Errorf("esc from the startup picker landed on %v, want paneSessions", m.pane) + } +} + +// The gate interrupts Sessions and nothing else. Its reply lands a round trip after the view +// opened, so an operator can already have moved; one who has is left where they are, with the +// return pane they had, rather than pulled into the picker with an esc that lands on Sessions. +func TestAgentsPane_StartupLeavesAnOperatorWhoHasMoved(t *testing.T) { + rows := []agentRow{ + {label: "claude-code/2.1.270", Counts: usage.Counts{Requests: 10}}, + {label: "bob-shell/2.0.5", Counts: usage.Counts{Requests: 8}}, + } + visited := 0 + for start := paneID(0); start <= lastPaneID; start++ { + if start == paneSessions { + continue + } + visited++ + m := &model{pane: start, previousPane: panePipeline, agentsTbl: newAgentsTable(), client: deadClient()} + updated, _ := m.Update(agentRowsLoadedMsg{rows: rows, open: agentsOpenAtStartup}) + m = updated.(*model) + if m.pane != start || m.previousPane != panePipeline { + t.Errorf("a startup reply arriving on %v moved the operator to %v (previousPane %v)", + start, m.pane, m.previousPane) + } + } + if visited != int(lastPaneID) { + t.Fatalf("visited %d panes, want %d", visited, lastPaneID) + } +} + +// A failed startup fetch is silent too, and leaves the reader on the sessions pane. +// +// The operator did not ask for this fetch, and the pane they ARE looking at reports its own +// connection trouble — so a second error line about a breakdown nobody requested would be noise +// on top of the message that matters. The error is still recorded, so pressing `A` later says +// what happened rather than showing an empty grid. +func TestAgentsPane_StartupFetchFailureIsSilent(t *testing.T) { + m := &model{pane: paneSessions, previousPane: paneNone, agentsTbl: newAgentsTable(), client: deadClient(), + // TWO ROWS, so the error is the only thing that can hold the pane back. With none, the + // gate's OTHER conjunct (agentsPaneApplies) is already false and the assertion passes + // whatever the error handling does — measured: deleting `msg.err == nil` from the gate + // left the whole package green. These rows stand for a previous pod's, which is how the + // state is reachable: m.agents survives a failed fetch and the gate re-runs per + // connection, so without the guard the picker opens showing another proxy's agents. + agents: []agentRow{ + {label: "claude-code/2.1.270", Counts: usage.Counts{Requests: 10}}, + {label: "bob-shell/2.0.5", Counts: usage.Counts{Requests: 8}}, + }} + updated, _ := m.Update(agentRowsLoadedMsg{err: errStartupProbe, open: agentsOpenAtStartup}) + m = updated.(*model) + if m.pane != paneSessions { + t.Errorf("a failed startup fetch moved the reader to %v", m.pane) + } + if m.flash != "" { + t.Errorf("a failed startup fetch flashed %q", m.flash) + } + if m.agentsErr == nil { + t.Error("agentsErr was not recorded; a later `A` press would show an empty pane instead of the reason") + } +} + +// initSessionView ARMS the gate. Without this nothing asserted the wiring, only the handler. +// +// THE FOUR TESTS ABOVE INJECT agentRowsLoadedMsg STRAIGHT INTO Update, which verifies what the +// reply does and never that anything sends it. Measured: replacing the `m.startupAgentsGateCmd(),` +// line in initSessionView with a no-op left the whole package green — so the PR's headline +// behaviour, "the picker opens itself at startup", had no test that it is ever reached in +// production. `grep -rn startupAgentsGateCmd --include=*_test.go` returned nothing. +// +// initSessionView IS THE RIGHT SEAM, not Init: it is the one place every entry point converges on +// (--endpoint mode's Init, the pod picker's portForwardReadyMsg, and [l]'s local endpoint), and it +// is where the gate has to live for a second pod's connection to re-run it. +// +// THE BATCH IS FANNED OUT CONCURRENTLY WITH A DEADLINE, because tea.Batch's leaves include a 1s +// tick and an SSE pump that never returns on their own. Running them in sequence would hang on +// the first blocker rather than reaching the gate's fetch. +func TestInitSessionView_ArmsTheStartupAgentsGate(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Only /v1/usage matters here; the batch's other leaves may 404 harmlessly. + _, _ = w.Write([]byte(`{"window":"today","group":"agent","buckets":[{"at":"2026-09-29T10:00:00Z",` + + `"requests":18,"series":{"claude-code/2.1.270":{"requests":10},"bob-shell/2.0.5":{"requests":8}}}],` + + `"totals":{"requests":18}}`)) + })) + defer ts.Close() + + m := &model{client: apiclient.New(ts.URL), agentsTbl: newAgentsTable(), pane: paneSessions} + m.parentCtx = context.Background() + m.ctx, m.cancel = context.WithCancel(m.parentCtx) + defer m.cancel() + + cmd := m.initSessionView() + if cmd == nil { + t.Fatal("initSessionView returned no command") + } + first := cmd() + batch, ok := first.(tea.BatchMsg) + if !ok { + t.Fatalf("initSessionView produced %T, want tea.BatchMsg", first) + } + got := make(chan agentRowsLoadedMsg, len(batch)) + for _, leaf := range batch { + if leaf == nil { + continue + } + go func(c tea.Cmd) { + if msg, ok := c().(agentRowsLoadedMsg); ok { + got <- msg + } + }(leaf) + } + select { + case msg := <-got: + // The VALUE, not just the arrival: a gate armed with agentsOpenOnPress would flash a + // refusal at every single-agent startup, which is the behaviour agentsOpenAtStartup + // exists to avoid. + if msg.open != agentsOpenAtStartup { + t.Errorf("the startup fetch carried open=%v, want agentsOpenAtStartup", msg.open) + } + // THE FETCH ACTUALLY SERVED. fetchAgentRowsCmd's error return carries the same `open`, so + // without these two the server, the payload and the handler are all decoration: blanking + // the fixture's agents to `{}` left the test green. + if msg.err != nil { + t.Errorf("the startup fetch failed: %v — the fixture is not being served", msg.err) + } + if len(msg.rows) != 2 { + t.Errorf("the startup fetch carried %d rows, want the fixture's 2", len(msg.rows)) + } + case <-time.After(10 * time.Second): + t.Fatal("no agentRowsLoadedMsg came out of initSessionView's batch — the startup gate is not armed") + } +} diff --git a/cmd/abctl/tui/agents_scope_test.go b/cmd/abctl/tui/agents_scope_test.go new file mode 100644 index 000000000..32143f098 --- /dev/null +++ b/cmd/abctl/tui/agents_scope_test.go @@ -0,0 +1,415 @@ +package tui + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/rossoctl/cortex/cmd/abctl/apiclient" + "github.com/rossoctl/cortex/core/cost/usage" +) + +// scopedModel is an AGENTS pane standing on the given row, with the given scope already set. +func scopedModel(t *testing.T, cursor int, scope string) *model { + t.Helper() + m := &model{ + pane: paneAgents, + previousPane: paneSessions, + agentScope: scope, + agentsTbl: newAgentsTable(), + client: deadClient(), + pipelineReturnPane: paneNone, + agents: []agentRow{ + {label: "claude-code/2.1.270", Counts: usage.Counts{Requests: 10, PricedRequests: 10}}, + {label: "bob-shell/2.0.5", Counts: usage.Counts{Requests: 8}}, + }, + } + m.rebuildAgentsTable() + for i := 0; i < cursor; i++ { + m.handleKey(tea.KeyMsg{Type: tea.KeyDown}) + } + return m +} + +// Enter scopes the usage pane to the row under the cursor, and leaves the pane. +// +// LEAVING IS PART OF THE ACTION, not a separate keystroke: the pane is a picker. +func TestAgentsPane_EnterScopesTheRowUnderTheCursorAndLeaves(t *testing.T) { + m := scopedModel(t, 1, "") + m.handleKey(tea.KeyMsg{Type: tea.KeyEnter}) + if m.agentScope != "bob-shell/2.0.5" { + t.Errorf("agentScope = %q, want the row under the cursor", m.agentScope) + } + if m.pane != paneSessions { + t.Errorf("Enter left the reader on %v, want the caller pane", m.pane) + } +} + +// Enter on the agent ALREADY scoped clears the scope instead of re-applying it. +// +// ONE KEY, TWO DIRECTIONS, because there is no "all agents" row to select and the alternative +// was a second binding that would only ever be pressed on this pane. The footer says which +// direction the key will go — see usageFooter's [s] toggle, which is the same idea for the +// session scope. +func TestAgentsPane_EnterOnTheScopedAgentClearsTheScope(t *testing.T) { + m := scopedModel(t, 0, "claude-code/2.1.270") + m.handleKey(tea.KeyMsg{Type: tea.KeyEnter}) + if m.agentScope != "" { + t.Errorf("agentScope = %q, want cleared", m.agentScope) + } + if m.pane != paneSessions { + t.Errorf("Enter left the reader on %v, want the caller pane", m.pane) + } +} + +// usageSnapshotJSON is a two-agent group=agent response, the shape /v1/usage serves. +const usageSnapshotJSON = `{ + "window":"today","bucketSeconds":60,"group":"agent", + "buckets":[ + {"at":"2026-09-29T10:00:00Z","requests":22,"tokens":2200,"latMeanMs":50,"latSamples":22, + "series":{ + "claude-code/2.1.270":{"requests":14,"tokens":1400}, + "bob-shell/2.0.5":{"requests":8,"tokens":800}}}], + "totals":{"requests":22,"tokens":2200}}` + +// A scoped usage fetch asks for the AGENT axis, whatever axis the pane itself is showing. +// +// The scope can only be computed from a per-agent series, and /v1/usage takes one group +// parameter — so while a scope is active the wire axis is not the pane's to choose. The pane's +// own group is left on the model rather than overwritten, so clearing the scope restores the +// axis the operator had picked. +func TestFetchUsage_ScopedAsksForTheAgentAxis(t *testing.T) { + var gotQuery string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotQuery = r.URL.RawQuery + _, _ = w.Write([]byte(usageSnapshotJSON)) + })) + defer ts.Close() + + m := &model{client: apiclient.New(ts.URL), agentScope: "claude-code/2.1.270"} + m.usage.group = usage.GroupModel // the pane's own axis, which must not reach the wire + cmd := m.fetchUsage() + if cmd == nil { + t.Fatal("fetchUsage returned no command with a client set") + } + msg, ok := cmd().(usageLoadedMsg) + if !ok { + t.Fatalf("fetchUsage produced %T, want usageLoadedMsg", cmd()) + } + if msg.err != nil { + t.Fatalf("fetch errored: %v", msg.err) + } + q, err := url.ParseQuery(gotQuery) + if err != nil { + t.Fatalf("unparseable query %q: %v", gotQuery, err) + } + if got := q.Get("group"); got != string(usage.GroupAgent) { + t.Errorf("group = %q, want %q: the scope cannot be computed without the agent series", + got, usage.GroupAgent) + } + if m.usage.group != usage.GroupModel { + t.Errorf("the pane's own axis was overwritten to %q; clearing the scope would not "+ + "restore what the operator picked", m.usage.group) + } +} + +// The snapshot the pane receives is already narrowed — totals AND buckets. +// +// BOTH, because the pane renders a summary line from Totals and a chart from Buckets. Narrowing +// only the totals is what usage.KeepBuckets does, and it would title a whole-window chart with +// one agent's name. +func TestFetchUsage_ScopedNarrowsTotalsAndBuckets(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(usageSnapshotJSON)) + })) + defer ts.Close() + + m := &model{client: apiclient.New(ts.URL), agentScope: "claude-code/2.1.270"} + msg := m.fetchUsage()().(usageLoadedMsg) + if msg.err != nil { + t.Fatalf("fetch errored: %v", msg.err) + } + if msg.snap.Totals.Requests != 14 { + t.Errorf("Totals.Requests = %d, want this agent's 14 (the window's is 22)", + msg.snap.Totals.Requests) + } + if len(msg.snap.Buckets) != 1 { + t.Fatalf("len(Buckets) = %d, want 1", len(msg.snap.Buckets)) + } + if msg.snap.Buckets[0].Requests != 14 { + t.Errorf("bucket Requests = %d, want this agent's 14; the chart would draw the whole "+ + "window under a title naming one agent", msg.snap.Buckets[0].Requests) + } +} + +// An unscoped fetch still asks for the pane's own axis. The regression guard for the case +// above: forcing GroupAgent unconditionally would break every breakdown the pane offers. +func TestFetchUsage_UnscopedAsksForThePanesOwnAxis(t *testing.T) { + var gotQuery string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotQuery = r.URL.RawQuery + _, _ = w.Write([]byte(`{"window":"today","group":"model","buckets":[],"totals":{}}`)) + })) + defer ts.Close() + + m := &model{client: apiclient.New(ts.URL)} + m.usage.group = usage.GroupModel + if _, ok := m.fetchUsage()().(usageLoadedMsg); !ok { + t.Fatal("fetchUsage produced the wrong message type") + } + q, _ := url.ParseQuery(gotQuery) + if got := q.Get("group"); got != string(usage.GroupModel) { + t.Errorf("group = %q, want the pane's own %q", got, usage.GroupModel) + } +} + +// A scope that no longer matches any agent in the window is REPORTED, not silently ignored. +// +// The window moves while abctl runs — "today" is a boundary, and an agent that stopped sending +// falls out of it — so this is reachable without anyone doing anything wrong. The error names +// the agents that ARE in the window, which is what the operator needs in order to pick a +// different one. +func TestFetchUsage_ScopeThatMatchesNothingIsReported(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(usageSnapshotJSON)) + })) + defer ts.Close() + + m := &model{client: apiclient.New(ts.URL), agentScope: "an-agent-that-went-away/1.0"} + msg := m.fetchUsage()().(usageLoadedMsg) + if msg.err == nil { + t.Fatal("a scope matching no agent produced no error; the pane would show every agent") + } + if !strings.Contains(msg.err.Error(), "claude-code/2.1.270") { + t.Errorf("error %q does not name the agents in the window", msg.err) + } +} + +// The usage title carries the scope, so a narrowed chart is never unlabelled. +func TestUsageTitle_NamesTheAgentScope(t *testing.T) { + m := fitModel(t, paneUsage, 120, 40, nil) + m.agentScope = "claude-code/2.1.270" + // The TITLE ROW, not the whole view: paneView returns title plus body, and asserting over + // both would pass on a label that happened to appear inside the chart. + title := strings.SplitN(m.paneView(), "\n", 2)[0] + if !strings.Contains(title, "claude-code/2.1.270") { + t.Errorf("title %q does not name the scoped agent", title) + } +} + +// [b] is omitted from the footer while a scope is active, because it cannot act. +// +// The wire axis is the agent's while scoped, so there is no second axis to break down by. Same +// treatment the footer already gives [b] under latency, and for the same stated reason: a footer +// that advertises an inert key is worse than a shorter footer. +func TestUsageFooter_OmitsTheBreakdownKeyWhileScoped(t *testing.T) { + m := fitModel(t, paneUsage, 120, 40, nil) + if !strings.Contains(m.helpView(), "[b] breakdown") { + t.Fatal("the unscoped footer does not offer [b]; this test is measuring the wrong thing") + } + m.agentScope = "claude-code/2.1.270" + if strings.Contains(m.helpView(), "[b] breakdown") { + t.Error("the footer still advertises [b] under an agent scope, where it cannot act") + } +} + +// Nor does the header claim a breakdown while scoped. The pane's own axis stays on the model, so +// printing it would put "by model" over a chart the scope has left ungrouped. +func TestUsageHeader_ClaimsNoBreakdownWhileScoped(t *testing.T) { + m := fitModel(t, paneUsage, 120, 40, nil) + m.usage.group = usage.GroupModel + header := func() string { return strings.SplitN(m.renderUsage(m.width, m.bodyHeight), "\n", 2)[0] } + if !strings.Contains(header(), "by model") { + t.Fatalf("the unscoped header %q does not show the breakdown; this test is measuring the wrong thing", header()) + } + m.agentScope = "claude-code/2.1.270" + if got := header(); !strings.Contains(got, "ungrouped") { + t.Errorf("the scoped header %q claims a breakdown the chart is not showing", got) + } +} + +// [b] does nothing while scoped, rather than refetching against an axis the scope has taken. +func TestUsageKeys_BreakdownIsInertWhileScoped(t *testing.T) { + m := fitModel(t, paneUsage, 120, 40, nil) + m.agentScope = "claude-code/2.1.270" + before := m.usage.group + m.handleKey(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'b'}}) + if m.usage.group != before { + t.Errorf("[b] moved the axis to %q under a scope; the wire axis is the agent's", + m.usage.group) + } +} + +// Latency under an agent scope says it is unavailable rather than plotting the zeros. +// +// THE NARROWING CANNOT CARRY LATENCY. Bucket.Series is map[string]Counts and Counts holds no +// latency, so a bucket's LatMeanMs describes every agent that shared it — usage.ScopeToAgent +// zeroes the fields rather than attributing one agent's chart to another's response times. A +// chart of those zeros would read as "this agent was instant", which is the worst of the three +// available answers. +func TestUsagePane_LatencyUnderAScopeSaysItIsUnavailable(t *testing.T) { + m := fitModel(t, paneUsage, 120, 40, nil) + for !m.usage.metric.isLatency() { + m.usage.cycleMetric() + } + m.agentScope = "claude-code/2.1.270" + body := m.renderUsage(m.width, m.bodyHeight) + if !strings.Contains(body, "latency") { + t.Errorf("the scoped latency view does not mention latency:\n%s", body) + } + if !strings.Contains(body, "agent") { + t.Errorf("the scoped latency view does not say the scope is why:\n%s", body) + } +} + +// The scoped pane STATES THE RESIDUAL its COST cell leaves out. +// +// usage.Snapshot.UngroupedCostMicros is a whole-window figure and survives usage.ScopeToAgent, so +// under a scope the pane holds a COST cell describing one agent beside a residual describing no +// agent. `abctl cost` has disclosed this at writeCostSummary's --agent note for as long as --agent +// has existed; the usage pane is the surface that gained the same duty when it gained a scope, and +// it had nothing to say about it. +// +// BOTH RENDER BRANCHES, because both print the summary and so both print its COST cell: the default +// one, and the latency one that prints the summary after saying latency is unavailable per agent. A +// disclosure added to one and not the other is the failure this covers. +// +// THE AMOUNT IS AN INDEPENDENT LITERAL rather than formatUSDTotalMicros(residual): asserting +// against the producer's own output would pass on whatever figure it chose to render, including the +// window total. 750_000 micros is $0.75, and it is distinct from the fixture's 55_000 total, so a +// note built from the wrong field cannot satisfy this. +func TestUsagePane_ScopedSummaryDisclosesTheCostNoAgentCarries(t *testing.T) { + for _, tc := range []struct { + name string + latency bool + }{ + {name: "count metric, the default branch"}, + {name: "latency metric, the branch that says latency is unavailable", latency: true}, + } { + t.Run(tc.name, func(t *testing.T) { + m := fitModel(t, paneUsage, 120, 40, nil) + if tc.latency { + for !m.usage.metric.isLatency() { + m.usage.cycleMetric() + } + } + residual := int64(750_000) + m.usage.snap.UngroupedCostMicros = &residual + m.agentScope = "claude-code/2.1.270" + body := m.renderUsage(m.width, m.bodyHeight) + if !strings.Contains(body, "$0.75") { + t.Errorf("the scoped summary does not state the residual amount:\n%s", body) + } + if !strings.Contains(body, "attributed to no agent") { + t.Errorf("the scoped summary names an amount without saying what it is:\n%s", body) + } + }) + } +} + +// The note row is conditional, so usagePaneChromeRows does not count it and +// TestUsageChartHeight_MatchesTheRenderedChrome, which renders unscoped, cannot see it. Both +// sizes fit the terminal unscoped; the scoped pane with a residual must fit them too. +func TestUsagePane_TheScopedNoteFitsWhereTheUnscopedPaneDoes(t *testing.T) { + for _, tc := range []struct { + name string + w, h int + }{{"80x26", 80, 26}, {"120x26", 120, 26}} { + m := fitModel(t, paneUsage, tc.w, tc.h, cursorRowsFixture(60)) + residual := int64(750_000) + m.usage.snap.UngroupedCostMicros = &residual + m.agentScope = "claude-code/2.1.270" + if !strings.Contains(m.View(), "attributed to no agent") { + t.Fatalf("%s: the fixture does not reach the note row", tc.name) + } + assertFits(t, m, "scoped usage "+tc.name) + } +} + +// UNSCOPED, the pane says nothing about the residual: the COST cell and the residual then describe +// the same window, and there is no shortfall between them to explain. +// +// THE COMPANION THAT MAKES THE TEST ABOVE MEAN SOMETHING. An unconditional note would satisfy that +// one and be wrong on every unscoped window, which is the pane's normal state. +func TestUsagePane_UnscopedSummarySaysNothingAboutTheResidual(t *testing.T) { + m := fitModel(t, paneUsage, 120, 40, nil) + residual := int64(750_000) + m.usage.snap.UngroupedCostMicros = &residual + if body := m.renderUsage(m.width, m.bodyHeight); strings.Contains(body, "attributed to no agent") { + t.Errorf("the unscoped pane disclosed a residual that describes the very window it is showing:\n%s", body) + } +} + +// The states with nothing to say say nothing, rather than a note reading "$0.00" or a negative. +// +// A ZERO RESIDUAL is the ordinary case on a reconcilable window — the series accounts for every +// dollar — and a note for it would train an operator to ignore the line. AN ABSENT field is a +// producer that computed no residual at all. A NEGATIVE cannot arrive, because usage's residualOf +// routes one to SeriesOvershootMicros, so refusing it is this surface's impossible-figure rule and +// not a formatting fix: formatUSDTotalMicros renders a negative as a four-decimal figure perfectly +// well, and the note would then assert a negative residual in prose. +func TestCostUngroupedRow_SaysNothingWhereThereIsNothingToSay(t *testing.T) { + for _, tc := range []struct { + name string + micros int64 + absent bool + noSnap bool + scope string + }{ + {name: "a residual, but no scope", micros: 750_000}, + {name: "scoped, the producer computed no residual", absent: true, scope: "claude-code/2.1.270"}, + {name: "scoped, the series accounts for every dollar", micros: 0, scope: "claude-code/2.1.270"}, + {name: "scoped, an impossible negative residual", micros: -150_000, scope: "claude-code/2.1.270"}, + {name: "scoped, no snapshot fetched yet", noSnap: true, scope: "claude-code/2.1.270"}, + } { + t.Run(tc.name, func(t *testing.T) { + var snap *usage.Snapshot + if !tc.noSnap { + snap = &usage.Snapshot{} + if !tc.absent { + micros := tc.micros + snap.UngroupedCostMicros = µs + } + } + if got := costUngroupedRow(snap, tc.scope); got != "" { + t.Errorf("costUngroupedRow = %q, want the empty string", got) + } + }) + } +} + +// The AGENTS pane's own title names the scope, so returning to the picker shows what is set. +func TestAgentsPane_TitleNamesTheActiveScope(t *testing.T) { + m := scopedModel(t, 0, "bob-shell/2.0.5") + m.width, m.height = 120, 40 + m.layout() + title := strings.SplitN(m.paneView(), "\n", 2)[0] + if !strings.Contains(title, "bob-shell/2.0.5") { + t.Errorf("AGENTS title %q does not name the active scope", title) + } +} + +// The footer says which direction Enter will go, because one key does both. +// +// Without this the toggle is invisible: an operator standing on the scoped agent has no way to +// know Enter will clear rather than re-apply. Mirrors the usage pane's [s], whose label flips +// between "all sessions" and "this session" for the same reason. +func TestAgentsPane_FooterSaysWhichWayEnterWillGo(t *testing.T) { + // Cursor on the scoped agent: Enter clears. + onScoped := scopedModel(t, 0, "claude-code/2.1.270") + if got := onScoped.helpView(); !strings.Contains(got, "all agents") { + t.Errorf("footer on the scoped row = %q, want it to offer clearing the scope", got) + } + // Cursor on a different agent: Enter scopes to it. + onOther := scopedModel(t, 1, "claude-code/2.1.270") + if got := onOther.helpView(); !strings.Contains(got, "scope") { + t.Errorf("footer on an unscoped row = %q, want it to offer scoping", got) + } + if got := onOther.helpView(); strings.Contains(got, "all agents") { + t.Errorf("footer on an unscoped row = %q, but Enter there scopes rather than clears", got) + } +} diff --git a/cmd/abctl/tui/app.go b/cmd/abctl/tui/app.go index 484d5c8d4..bf029ef73 100644 --- a/cmd/abctl/tui/app.go +++ b/cmd/abctl/tui/app.go @@ -42,8 +42,11 @@ const ( panePluginDetail paneCatalog paneUsage - // paneAgents shows what each CODING AGENT has spent. Read-only: /v1/usage takes no agent - // filter, so a selected row cannot scope anything. Not to be confused with paneNamespaces, + // paneAgents shows what each CODING AGENT has spent, and picks which one the usage pane is + // scoped to. The scope is applied CLIENT-SIDE — /v1/usage takes no agent filter, so + // the pane fetches group=agent and narrows with usage.ScopeToAgent, which is how + // `abctl cost --agent` has always worked. Sessions and events are not scopable at all: + // neither carries an agent. Not to be confused with paneNamespaces, // which lists Kubernetes workloads and whose own purpose line used to call them "agents" // too — see paneKeys for how the two are told apart. paneAgents @@ -723,8 +726,27 @@ type model struct { // /v1/usage grouping and `abctl cost --agent` all say "agent", and a fourth word for the // same thing would be the confusion this feature already had to untangle once. The // Kubernetes sense lives one field up as `namespaces []cluster.AgentNamespace`. - agents []agentRow - agentsTbl table.Model + agents []agentRow + // agentScope is the agent the usage pane is narrowed to, or "" for all of them. + // + // THE USAGE PANE AND NOTHING ELSE. The spend band and its drawer fetch on their own chains + // with their own axes and do not read this field, and `abctl cost --agent` is a separate + // process. Scoping those is a separate change; until then this must not be described as + // scoping "cost", which reads as covering the most prominent money figure on screen. + // + // A LABEL, not an index into m.agents: the rows are refetched on every `A` press and on the + // startup gate, and their order is by cost, so an index would silently come to mean a + // different agent the moment spending changed. The label is also what usage.ScopeToAgent + // takes and what `abctl cost --agent` accepts, so the TUI's scope and the CLI's flag are the + // same string — an operator can copy one into the other. + // + // NOT PERSISTED across runs. Every other view choice here is (see Settings), and this one + // deliberately is not: the set of agents on a proxy is a property of what is running right + // now, and a scope restored from yesterday would narrow to an agent that may not be in the + // window at all — which is the error case the fetch has to report rather than a state worth + // restoring into. + agentScope string + agentsTbl table.Model // agentsErr is the last fetch failure, shown in the pane rather than swallowed: an empty // breakdown and an unreachable endpoint look identical otherwise. agentsErr error @@ -862,6 +884,10 @@ func (m *model) initSessionView() tea.Cmd { return tea.Batch( m.loadSessionsCmd(), m.loadPipelineCmd(), + // Asks whether this proxy is running enough agents to be worth a picker. Batched, so + // it costs the first frame nothing; see startupAgentsGateCmd for why it hangs off + // initSessionView rather than Init. + m.startupAgentsGateCmd(), streamPump(m.streamCh), tickCmd(), refreshTickCmd(), @@ -1314,12 +1340,37 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.agents = msg.rows } switch { - case !msg.open: + case msg.open == agentsOpenNever: // A background refresh. Repaint only if the reader is standing on the pane; // otherwise the rows are just kept warm. if m.pane == paneAgents { m.rebuildAgentsTable() } + case msg.open == agentsOpenAtStartup: + // THE GATE, and the one path here that is allowed to decline in silence. Nobody + // asked for this fetch: the operator asked for the sessions view and got it, so a + // refusal sentence about a pane they never requested is noise, and an error line + // about it competes with the connection message the pane they ARE looking at will + // print for itself. m.agentsErr is set above either way, so a later `A` press + // reports what happened rather than showing an empty grid. + // + // agentsPaneApplies rather than enterAgentsOrRefuse: the refusal STRING is written + // for someone owed an answer, and this caller is not one. The two agree by test, + // so consulting the predicate cannot drift from the sentence. + // + // ONLY FROM SESSIONS: the reply lands a round trip after the view opened, and an + // operator who has already moved is left where they are. + if msg.err == nil && m.pane == paneSessions && agentsPaneApplies(m.agents) { + // paneNone, and msg.from is deliberately NOT read here. The gate has no caller + // pane to return to — it interrupted the sessions view — and the esc arm's + // existing paneNone fallback already lands on Sessions, which that arm documents + // as the one pane always defensible to land on. Reading msg.from instead would + // put the correctness of esc in an argument supplied a round trip earlier, where + // a test driving this message cannot see what production passes. + m.previousPane = paneNone + m.pane = paneAgents + m.rebuildAgentsTable() + } case msg.err != nil: // The `A` press cannot open a pane whose contents failed to load, and it must not // fail silently either — see agentsPaneRefusal on why no refusal here may be mute. @@ -2179,11 +2230,22 @@ func (m *model) paneView() string { scope = m.usage.session } title = fmt.Sprintf("abctl · %s · usage · %s", m.endpoint, scope) + // The agent scope goes in the TITLE because it changes what every figure on the pane + // means. + if m.agentScope != "" { + title += " · agent=" + sanitizeLabel(m.agentScope) + } body = m.renderUsage(m.width, m.bodyHeight) case paneAgents: // The window is in the title because the figures are a day's, not a lifetime's, and // this pane has no window cycle of its own to make that discoverable. title = fmt.Sprintf("abctl · %s · agents · %s", m.endpoint, agentsWindow) + // Which agent is currently scoped, so returning to the picker shows the state rather + // than only the choices. The footer's Enter label says what the key will do to the row + // under the cursor; this says what is set, whatever the cursor is on. + if m.agentScope != "" { + title += " · scoped to " + sanitizeLabel(m.agentScope) + } switch { case m.agentsErr != nil: // Named, not blank: an unreachable endpoint and a quiet day look identical diff --git a/cmd/abctl/tui/help_overlay.go b/cmd/abctl/tui/help_overlay.go index fb2b36069..776279640 100644 --- a/cmd/abctl/tui/help_overlay.go +++ b/cmd/abctl/tui/help_overlay.go @@ -346,7 +346,7 @@ var paneKeys = map[paneID]keyGroup{ bindings: []keyBinding{ {"m", "cycle metric (tokens/requests/errors/latency/cost)"}, {"w", "cycle window (10m/1h/6h)"}, - {"b", "cycle breakdown (none/status/method/plugin/host; not for latency)"}, + {"b", "cycle breakdown (none/status/method/plugin/host; not for latency or a scope)"}, {"s", "toggle session / all sessions"}, {"esc", "back"}, }, @@ -371,20 +371,26 @@ var paneKeys = map[paneID]keyGroup{ purpose: "coding agents seen on the wire, and what each has spent", bindings: []keyBinding{ {"↑↓ / jk", "navigate"}, - {"esc", "back"}, + // One key both ways, which the footer's label flips to show. Said here too because + // the footer only describes the row under the cursor. + {"↵", "scope the usage pane to this agent; again clears"}, + {"esc", "back, keeping the scope"}, }, - // ONE SHORT NOTE, NOT TWO LONG ONES. The first draft spelled out the whole rationale - // here and pushed the overlay body from 91 to 103 lines, which - // TestHelpOverlayScrollHint_AbsentWhenEverythingFits caught by demanding - // helpNoScrollHeight be raised to 106. Raising it would make every reader of every - // other pane scroll for this pane's explanation; the argument belongs in - // agents_pane.go, and what a reader needs on screen is the two facts that change what - // they see. + // ONE SHORT NOTE, NOT TWO LONG ONES, and the budget is real rather than stylistic: an + // earlier draft pushed the overlay body from 91 to 103 lines and + // TestHelpOverlayScrollHint_AbsentWhenEverythingFits demanded helpNoScrollHeight be + // raised to 106, which would make every reader of every other pane scroll for this + // pane's explanation. The two ↵/esc rows above cost lines too, so this note is shorter + // than the one it replaces. The argument belongs in agents_pane.go; what a reader needs + // on screen is what changes when they press something. + // + // IT NO LONGER SAYS "READ-ONLY". It did, on the true-at-the-time grounds that /v1/usage + // filters by session and nothing else — so the scope is computed client-side instead, + // the same narrowing `abctl cost --agent` uses, and the endpoint's limit now bounds WHICH + // views can honour it rather than whether any can. notes: []string{ - "Opens only when two or more agents have been seen; below that it refuses and names " + - "what it found. Read-only, and that is a limit of the API rather than a choice: " + - "/v1/usage filters by session and nothing else, so there is no agent scope to " + - "apply to the other panes.", + "Opens only when two or more agents have been seen. It scopes the usage pane " + + "only — not the spend band, and not sessions or events, which carry no agent.", }, }, } diff --git a/cmd/abctl/tui/help_overlay_test.go b/cmd/abctl/tui/help_overlay_test.go index 5d94fabe5..ea3d9c524 100644 --- a/cmd/abctl/tui/help_overlay_test.go +++ b/cmd/abctl/tui/help_overlay_test.go @@ -697,3 +697,22 @@ func TestHelpOverlayDoesNotStealFilterInput(t *testing.T) { t.Fatal("`?` should open the overlay once the filter input is unfocused") } } + +// The AGENTS note must EXCLUDE the spend band, not merely mention it. +// +// A substring pin on the negation, so it goes red on a reword that keeps the meaning. Asserting the +// mention alone passed "scopes the usage pane AND the spend band" — the claim app.go's agentScope +// doc forbids. +func TestPaneKeys_TheAgentsNoteSaysWhatTheScopeDoesNotReach(t *testing.T) { + g, ok := paneKeys[paneAgents] + if !ok { + t.Fatal("paneKeys has no entry for paneAgents") + } + joined := strings.Join(g.notes, " ") + if joined == "" { + t.Fatal("the AGENTS group carries no notes; the disclosure has nowhere to live") + } + if !strings.Contains(joined, "not the spend band") { + t.Errorf("the AGENTS note does not EXCLUDE the spend band:\n %s", joined) + } +} diff --git a/cmd/abctl/tui/keys.go b/cmd/abctl/tui/keys.go index 3fa30851a..ce7a5e223 100644 --- a/cmd/abctl/tui/keys.go +++ b/cmd/abctl/tui/keys.go @@ -102,6 +102,13 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { if m.usage.metric.isLatency() { break } + // Inert under an agent scope, which the footer and the [?] overlay both say. The + // scope needs group=agent on the wire, so there is no axis left for this key to + // move; dropping it here is what keeps it from refetching against an axis the + // scoped narrowing would immediately discard. + if m.agentScope != "" { + break + } // Refetch: the breakdown is a server-side query parameter, not a // client-side filter, so the current snapshot has no series for the // newly selected dimension. @@ -504,27 +511,10 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { m.pane = panePipeline } case paneAgents: - // Return to whichever pane the user pressed `A` from — a key-opened surface owes - // its caller a way back, and without this case esc falls through and the pane is a - // dead end reachable only by `q`. - // - // SAME FALLBACK AS paneCatalog, and for its stated reason rather than by imitation: - // Sessions is the one pane that is always a defensible place to land, while the - // enum's zero value is the Kubernetes namespace picker, which would look like the - // connection had gone away. - // - // No polling chain to restart, unlike the catalog's case below: this pane fetches - // once per open and holds no ticker. Returning INTO Usage still needs its chain - // resumed, which is why the shared tail below runs for both. - if m.previousPane != paneNone { - m.pane = m.previousPane - m.previousPane = paneNone - } else { - m.pane = paneSessions - } - if m.pane == paneUsage { - return m.resumeUsagePolling() - } + // Leaving WITHOUT touching the scope, which is the difference between this and the + // Enter arm below. esc means "back out" on every other pane here, and a key that + // silently discarded a scope on the way out would be the one exception. + return m.leaveAgentsPane() case paneCatalog: // Return to whichever pane the user pressed `C` from. // @@ -597,6 +587,26 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { case "enter", "right", "l": switch m.pane { + case paneAgents: + // Pick the agent the usage pane is narrowed to, then LEAVE: the pane is a picker. + // + // A TOGGLE: Enter on the agent already scoped clears the scope instead of re-applying + // it. There is no "all agents" row to select, and the alternative was a second binding + // that would only ever be pressed on this one pane. helpView says which direction the + // key will go, the way the usage pane's [s] does for the session scope. + row := m.selectedAgentLabel() + if row == "" { + return nil + } + if m.agentScope == row { + m.agentScope = "" + } else { + m.agentScope = row + } + // Same exit as the esc arm above, including the paneNone → Sessions fallback and the + // usage-polling resume. Shared through leaveAgentsPane so the two cannot drift on + // where the pane returns to — the scope is the only thing this arm does differently. + return m.leaveAgentsPane() case paneSessions: id := m.selectedSessionID() if id == "" { @@ -863,7 +873,7 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { if from == paneAgents { from = m.previousPane } - return m.fetchAgentRowsCmd(true, from) + return m.fetchAgentRowsCmd(agentsOpenOnPress, from) case "C": // Open the registered-plugin catalog. Available from any @@ -1232,6 +1242,12 @@ func (m *model) helpView() string { // [b] is omitted under latency rather than shown as a no-op: a footer that // advertises an inert key is worse than a shorter footer. breakdownHint := " [b] breakdown" + // Omitted under an agent scope for the same reason as under latency: the scope has taken + // the wire axis, so there is no second one to break down by, and a footer that advertises + // an inert key is worse than a shorter footer. + if m.agentScope != "" { + breakdownHint = "" + } if m.usage.metric.isLatency() { breakdownHint = "" } @@ -1245,11 +1261,20 @@ func (m *model) helpView() string { } return "[↑↓] nav [↵] plugin detail [r] refresh [esc] back [?] keys [q] quit" case paneAgents: - // NO [↵] AND NO [r]. There is nothing to drill into — /v1/usage takes no agent - // filter, so a selected row cannot scope anything — and the rows are refetched on - // every `A`, so a refresh key would duplicate the way in. Advertising either would be - // the inert-key problem the usage pane's breakdownHint above avoids. - return "[↑↓] nav [esc] back [?] keys [q] quit" + // [↵] LABELLED BY WHAT IT WILL DO TO THE ROW UNDER THE CURSOR, because one key goes both + // ways: on the agent already scoped it clears the scope, on any other it scopes to that + // one. Without the flip the toggle is invisible — an operator standing on the scoped + // agent has no way to know Enter will not simply re-apply it. Same idea as the usage + // pane's [s], whose label flips between "all sessions" and "this session". + // + // STILL NO [r]: the rows are refetched by every `A` press, so a refresh key would + // duplicate the way in, and advertising it would be the inert-key problem the usage + // pane's breakdownHint above avoids. + enterHint := " [↵] scope to this agent" + if m.agentScope != "" && m.selectedAgentLabel() == m.agentScope { + enterHint = " [↵] all agents" + } + return "[↑↓] nav" + enterHint + " [esc] back [?] keys [q] quit" } return "[?] keys [q] quit" } diff --git a/cmd/abctl/tui/usage_pane.go b/cmd/abctl/tui/usage_pane.go index 0dae90291..520c4548a 100644 --- a/cmd/abctl/tui/usage_pane.go +++ b/cmd/abctl/tui/usage_pane.go @@ -144,10 +144,31 @@ func (m *model) fetchUsage() tea.Cmd { session := m.usage.session group := m.usage.group req := m.usage.reqSeq + // THE SCOPE IS CAPTURED HERE, with the rest of the request, rather than read off the model + // inside the closure: this runs on another goroutine, and a scope changed while the request + // was in flight would narrow the reply to an agent the request was not grouped for. + scope := m.agentScope + if scope != "" { + // The agent axis is not the pane's to choose while a scope is active: /v1/usage takes one + // group parameter, and the narrowing below needs the per-agent series. m.usage.group is + // left alone rather than overwritten, so clearing the scope restores the axis the + // operator had picked. + group = usage.GroupAgent + } return func() tea.Msg { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() snap, err := client.GetUsage(ctx, window, resolution, session, group) + if err == nil && scope != "" { + // NarrowBuckets, not KeepBuckets: this pane renders a chart from Buckets, and + // narrowing only the totals would title a whole-window chart with one agent's name. + // The cost is that the narrowed buckets carry no latency — see the renderUsage + // branch that says so rather than plotting the zeros. + // + // A FAILURE IS REPORTED, not swallowed. A scope stops matching on its own as the + // window moves past an agent's last request. + snap, err = usage.ScopeToAgent(snap, scope, usage.NarrowBuckets) + } return usageLoadedMsg{snap: snap, req: req, err: err} } } @@ -307,11 +328,14 @@ func (m *model) renderUsage(width, height int) string { // entirely — displaying "by status" over a bucket-wide mean would assert a // breakdown that does not exist. The selection is kept, not cleared, so it is // still there when the operator cycles back to a count metric. + // + // Likewise under an agent scope: the scope has taken the wire axis, so there is no + // second one to break down by. grouping := "ungrouped" switch { case m.usage.metric.isLatency(): grouping = "no breakdown for latency" - case m.usage.group != "" && m.usage.group != usage.GroupNone: + case m.agentScope == "" && m.usage.group != "" && m.usage.group != usage.GroupNone: grouping = "by " + string(m.usage.group) } b.WriteString(fmt.Sprintf(" USAGE — %s — %s @ %s — %s — %s\n\n", @@ -330,12 +354,37 @@ func (m *model) renderUsage(width, height int) string { b.WriteString(" Loading…\n") case m.usage.snap == nil: b.WriteString(" (no data)\n") + case m.agentScope != "" && m.usage.metric.isLatency(): + // SAID HERE RATHER THAN LEFT TO renderWhiskers, which would answer "no latency samples + // in this window" — true of the narrowed snapshot and false about the window, and it + // would send the reader looking for traffic that is there. usage.ScopeToAgent zeroes the + // latency fields because Bucket.Series is map[string]Counts and Counts carries no + // latency, so a bucket's mean describes every agent that shared it. There is no + // per-agent latency on the wire to offer instead, which is why this names the way out + // rather than suggesting a different window. + b.WriteString(" (latency is not available per agent)\n") + b.WriteString("\n") + b.WriteString(" Response times are recorded per bucket, across every agent that\n") + b.WriteString(" shared it, so they cannot be attributed to one. To plot latency for\n") + b.WriteString(" all of them, clear the scope: [A], then [enter] on the scoped agent.\n") + b.WriteString("\n") + b.WriteString(renderUsageSummary(m.usage.snap)) + b.WriteString("\n") + // BOTH BRANCHES that render the summary render this, because both print its COST cell + // from the narrowed Totals. Empty string when no disclosure is due. + b.WriteString(costUngroupedRow(m.usage.snap, m.agentScope)) default: for _, line := range renderUsageChart(m.usage.snap, m.usage.metric, m.usage.group, width, usageChartHeight(height)) { b.WriteString(line) b.WriteString("\n") } - b.WriteString("\n") + // The note takes the blank row above the summary rather than adding one: it is + // conditional, so usagePaneChromeRows cannot count it. + if note := costUngroupedRow(m.usage.snap, m.agentScope); note != "" { + b.WriteString(note) + } else { + b.WriteString("\n") + } b.WriteString(renderUsageSummary(m.usage.snap)) b.WriteString("\n") if !m.usage.lastFetch.IsZero() { @@ -350,3 +399,44 @@ func (m *model) renderUsage(width, height int) string { // overlay), which the coverage test holds to the real pane list. return b.String() } + +// costUngroupedRow is the pane's form of the disclosure `abctl cost` prints at writeCostSummary: +// the part of the window's spend that NO agent carries. +// +// usage.Snapshot.UngroupedCostMicros is a WHOLE-WINDOW residual and survives usage.ScopeToAgent +// deliberately — that function's comment says why, and names this one as the pane's half of the +// duty. So under a scope it sits in a snapshot whose Totals describe one agent while it describes +// traffic belonging to none, beside a COST cell computed from those narrowed Totals. Without a +// word about it, a reader who scopes to each agent in turn and sums the figures finds a shortfall +// with nothing to explain it. +// +// THE SCOPE IS A PARAMETER rather than read from the model, and this lives outside +// renderCostSummary for the same reason: that function renders from a *usage.Snapshot alone, and +// a narrowed snapshot is indistinguishable from a whole-window one — ScopeToAgent rewrites +// Totals, not the question that produced them. Only the pane knows a scope is in force. +// +// NOT SUBTRACTED FROM OR ADDED TO the figure beside it: this agent's total is this agent's and the +// residual is nobody's. Stated beside it, not folded into it — writeCostSummary's rule, kept here +// so the two surfaces cannot disagree about the arithmetic. +// +// ZERO AND NEGATIVE BOTH RENDER NOTHING, and the negative goes through the shared negativeCost +// rather than a local comparison. A residual cannot arrive negative in the first place — usage's +// residualOf publishes a negative one as SeriesOvershootMicros instead — so this is the +// impossible-figure guard every money cell on this surface carries, not a case with a reading. +// +// THE GUARD IS NOT ABOUT MIS-FORMATTING: formatUSDTotalMicros already refuses the integer-cent +// arithmetic for a negative and falls back to four decimals, and its doc is explicit that NAMING an +// impossible figure stays the caller's job. Left to it, this line would read "$-0.1500 of this +// window is attributed to no agent" — prose asserting a negative residual, which is worse than the +// silence. The shape is sessionMoneyCell's. +func costUngroupedRow(snap *usage.Snapshot, scope string) string { + if scope == "" || snap == nil || snap.UngroupedCostMicros == nil { + return "" + } + micros := *snap.UngroupedCostMicros + if micros == 0 || negativeCost(micros) { + return "" + } + return fmt.Sprintf(" note %s of this window is attributed to no agent\n", + formatUSDTotalMicros(micros)) +} diff --git a/core/cost/usage/scope.go b/core/cost/usage/scope.go new file mode 100644 index 000000000..07f35c97b --- /dev/null +++ b/core/cost/usage/scope.go @@ -0,0 +1,169 @@ +package usage + +import ( + "fmt" + "sort" + "strings" +) + +// BucketScope says whether ScopeToAgent narrows the per-bucket series as well as the +// window-level figures. +// +// AN EXPLICIT PARAMETER RATHER THAN A DEFAULT, because the two callers want different things +// and the cheaper one is not the safe one. abctl's usage pane renders a CHART from Buckets, so +// leaving them whole-window would draw every agent's traffic under a title naming one — silent, +// and wrong in the direction a reader cannot detect. `abctl cost` reads only window totals on +// its scoped path, so it asks for no narrowing and pays for none. +// +// NEITHER VALUE IS A SAFE DEFAULT, which is why there is no zero-argument form. Defaulting to +// KeepBuckets hands a chart the whole window; defaulting to NarrowBuckets silently drops the +// latency a caller may be about to render, and drops Series from a response a caller may be +// about to break down. The caller knows which it reads; this package does not. +type BucketScope int + +const ( + // KeepBuckets narrows only the window-level figures. Buckets and their Series come + // through untouched. + KeepBuckets BucketScope = iota + // NarrowBuckets additionally rewrites every bucket's counts to the scoped agent's share + // of it and drops Series. See ScopeToAgent for what this cannot carry across. + NarrowBuckets +) + +// ScopeToAgent narrows a group=agent snapshot to one agent's figures. +// +// IT REWRITES Totals AND HANDS BACK A SNAPSHOT, rather than rendering the agent itself, so +// callers apply their existing writers to it with no second implementation and no chance of the +// two drifting: `abctl cost`'s negative-total refusal, coverage-gap disclosure and +// incomplete-read admission each read one agent's numbers, and the TUI's chart reads the same +// narrowing. Which fields do NOT survive, and why, is stated at each narrowing below. A COPY, +// never the caller's snapshot mutated in place. +// +// The fold is FoldSeriesAcrossWindow, the same one abctl's AGENTS pane uses, so the figure +// printed by the CLI and the row shown in the pane cannot disagree. +// +// AN UNKNOWN AGENT IS AN ERROR THAT NAMES THE KNOWN ONES. The labels are User-Agents, so they +// are neither short nor guessable — "bob" is the obvious thing to try and is not what Bob +// sends. The set is already in hand, so withholding it would be a choice. +func ScopeToAgent(snap *Snapshot, agent string, buckets BucketScope) (*Snapshot, error) { + series := FoldSeriesAcrossWindow(snap.Buckets) + counts, ok := series[agent] + if !ok { + known := make([]string, 0, len(series)) + for label := range series { + known = append(known, label) + } + // Sorted so the same window reports the same order every run; a set printed in map + // order is a set a reader cannot diff against yesterday's. + sort.Strings(known) + if len(known) == 0 { + return nil, fmt.Errorf("no agent traffic in the %s window, so %q matches nothing", + snap.Window, agent) + } + return nil, fmt.Errorf("no agent %q in the %s window; seen: %s", + agent, snap.Window, strings.Join(known, ", ")) + } + scoped := *snap + scoped.Totals = counts + // EVERY WHOLE-WINDOW STATEMENT ABOUT WHERE THE TOTALS CAME FROM GOES WITH Totals, or it is + // printed beside one agent's figure while describing all of them. Replacing only Totals left + // `--agent ` printing $0.00 — the window was priced, just not this + // agent's traffic — for exactly the agent the AGENTS pane prints "—" for, which breaks both + // writeCostSummary's "cost unavailable rather than $0.00" rule and this function's own claim + // that the figure there and the row in the pane cannot disagree. + // + // Priced is RE-DERIVED with the producers' own rule rather than one invented here: both + // snapshot.go and sessionapi set it to Totals.PricedRequests > 0, so the narrowed snapshot is + // the one they would have emitted had this agent's traffic been the whole window. + scoped.Priced = counts.PricedRequests > 0 + // The three by-model maps are DROPPED, not narrowed, because nothing here can narrow them: a + // bucket's series is keyed by agent and carries no per-model breakdown, so the only available + // readings are the window's maps — which describe other agents' traffic — or none. They are + // omitempty on the wire, and `abctl cost`'s costIncompleteReasonLines already treats an + // absent map as nothing to say, which is its common case for a ledger-backed window anyway. + scoped.PricedBy = nil + scoped.UnpricedBy = nil + scoped.IncompleteBy = nil + // Degraded and DaysOutsideRetention STAY, and the asymmetry is the point: they describe the + // READ and the retention configuration, which are the same facts whichever agent is scoped + // to. Dropping them would hide a short sum behind a narrower question. + // + // SeriesOvershootMicros and SeriesAvoidedOvershootMicros stay too, and they are among the + // fields the "every" above has to account for rather than pass over. Both are defect reports about a + // breakdown — the series summed to MORE than the total — so they belong with Degraded rather + // than with the provenance maps. A correct producer never sends either on this path: + // residualOf leaves them nil unless the series overshoots, which cannot happen where the + // figures reconcile. Where one does arrive it is upstream's bug, and forwarding it says so; + // narrowing it to an agent would be inventing a per-agent overshoot nothing computed. + // + // UngroupedCostMicros and UngroupedAvoidedMicros STAY, and they are the two fields this + // narrowing hands on with a DUTY ATTACHED rather than settles. Both are whole-window + // residuals — the part of a total that NO series entry carries — so under a scope they + // describe traffic belonging to no agent while the Totals beside them describe one agent. + // + // KEPT rather than dropped, because unlike the by-model maps above there IS a correct + // reading available: a residual is a fact about the WINDOW, true whichever agent is scoped + // to, which is the same argument that keeps DaysOutsideRetention. Dropping them would also + // retract a disclosure already being made — `abctl cost` reads UngroupedCostMicros off the + // snapshot this function returns, at writeCostSummary's --agent note. + // + // THE DISCLOSURE ITSELF IS THE CALLER'S DUTY, and it is the one thing this function cannot + // discharge for it: rewriting Totals is what creates the obligation, and only the caller + // knows whether it renders a money figure at all. A surface that renders one under a scope + // has to say what it leaves out, or a reader who scopes to each agent in turn and sums the + // figures finds a shortfall with nothing to explain it. `abctl cost` does this in + // writeCostSummary; abctl's usage pane does it in tui.costUngroupedRow. + // + // THE SURVIVAL IS PINNED IN THIS PACKAGE because every guard it had was a module away, in + // the consumer: dropping UngroupedCostMicros here fails two cmd/abctl tests, and dropping + // UngroupedAvoidedMicros failed nothing at all — measured, both ways. See + // TestScopeToAgent_KeepsTheWindowResidualsForTheCallerToDisclose. + + // Currencies IS CARRIED OVER, AND IT NO LONGER DESCRIBES Totals. Said out loud because it is + // the one field on this struct that the narrowing above invalidates, and the honest options are + // worse than keeping it. + // + // The field means "every unit the rows behind Totals were denominated in", and after this copy + // Totals is one agent while the list is the whole window. Narrowing it is not available: + // deciding which units THIS agent's traffic carries needs a cross-tabulation of agent against + // currency, and a folded per-agent Counts has already summed that axis away. Dropping it is + // worse than leaving it — this agent's own traffic may well be the mixed part, and an absent + // list reads as "single unit", so the surface would print a confident figure that is exactly + // the credits-plus-dollars sum this whole change exists to refuse. + // + // SO THE OVER-REFUSAL IS DELIBERATE, and it is the safe direction: a per-agent figure is + // withheld in a mixed window even when that agent billed in one unit. `abctl cost`'s + // writeCostSummary says which of the two it is rather than letting the reader assume, because + // "no figure for this agent" and "no figure for this window" have different fixes. + + if buckets == NarrowBuckets { + scoped.Buckets = narrowBucketsToAgent(snap.Buckets, agent) + } + return &scoped, nil +} + +// narrowBucketsToAgent rewrites each bucket to one agent's share of it. +// +// A NEW SLICE, because Bucket holds a map and the caller's snapshot must survive intact — +// see ScopeToAgent's copy rule. Assigning through scoped.Buckets[i] would write into the +// array the caller still owns. +// +// EVERY BUCKET SURVIVES, including the ones this agent sent nothing in, which become zero +// buckets at their original timestamps. The chart reads Buckets positionally against a time +// axis — Snapshot.Buckets' own doc says the zeroed entries are what let a client tell an idle +// minute from one that fell off the ring — so dropping an agent's idle buckets would compress +// its history and move every bar left of where it happened. +// +// LATENCY IS ZEROED RATHER THAN CARRIED, and it is the one reading this narrowing cannot +// produce: Series is map[string]Counts and Counts holds no latency, so LatMeanMs, LatStdDevMs +// and LatSamples describe every agent that shared the bucket. Keeping them would draw one +// agent's chart out of another's response times. A caller that offers a latency view has to say +// it is unavailable under a scope; there is no per-agent latency on the wire to offer instead. +func narrowBucketsToAgent(buckets []Bucket, agent string) []Bucket { + out := make([]Bucket, 0, len(buckets)) + for _, b := range buckets { + // The zero Counts when absent, which is the idle-bucket case above. + out = append(out, Bucket{At: b.At, Counts: b.Series[agent]}) + } + return out +} diff --git a/core/cost/usage/scope_test.go b/core/cost/usage/scope_test.go new file mode 100644 index 000000000..4bf0ad6cf --- /dev/null +++ b/core/cost/usage/scope_test.go @@ -0,0 +1,286 @@ +package usage + +import ( + "testing" + "time" +) + +// scopeFixture is a two-bucket group=agent snapshot: one agent in both buckets, one in only +// the second. The second agent's absence from the first bucket is the case that separates +// "narrow the buckets" from "sum them" — a fold cannot tell an idle bucket from a missing one, +// and the chart has to keep the idle bucket to keep its time axis. +func scopeFixture() *Snapshot { + base := time.Date(2026, 9, 29, 10, 0, 0, 0, time.UTC) + return &Snapshot{ + Window: "today", + BucketSeconds: 60, + Group: GroupAgent, + Buckets: []Bucket{ + { + At: base, + Counts: Counts{Requests: 10, Tokens: 1000, PricedRequests: 10, PriceableRequests: 10, CostMicros: 3_000}, + LatMeanMs: 42, + LatStdDevMs: 7, + LatSamples: 10, + Series: map[string]Counts{ + "claude-code/2.1.270": {Requests: 10, Tokens: 1000, PricedRequests: 10, PriceableRequests: 10, CostMicros: 3_000}, + }, + }, + { + At: base.Add(time.Minute), + Counts: Counts{Requests: 12, Tokens: 1200, PricedRequests: 4, PriceableRequests: 12, CostMicros: 1_000}, + LatMeanMs: 55, + LatStdDevMs: 9, + LatSamples: 12, + Series: map[string]Counts{ + "claude-code/2.1.270": {Requests: 4, Tokens: 400, PricedRequests: 4, PriceableRequests: 4, CostMicros: 1_000}, + "bob-shell/2.0.5": {Requests: 8, Tokens: 800, PriceableRequests: 8}, + }, + }, + }, + Totals: Counts{Requests: 22, Tokens: 2200, PricedRequests: 14, PriceableRequests: 22, CostMicros: 4_000}, + Priced: true, + } +} + +// The window-level figures become the named agent's, folded across every bucket. +func TestScopeToAgent_NarrowsTotalsToOneAgent(t *testing.T) { + got, err := ScopeToAgent(scopeFixture(), "claude-code/2.1.270", KeepBuckets) + if err != nil { + t.Fatalf("ScopeToAgent: %v", err) + } + // 10 + 4 across the two buckets, not the window's 22. + if got.Totals.Requests != 14 { + t.Errorf("Totals.Requests = %d, want 14 (this agent's, not the window's 22)", got.Totals.Requests) + } + if got.Totals.CostMicros != 4_000 { + t.Errorf("Totals.CostMicros = %d, want 4000", got.Totals.CostMicros) + } +} + +// Priced is RE-DERIVED, so an agent nothing priced reports cost-unavailable rather than $0.00. +// +// This is the assertion that a narrowing which replaced only Totals would fail: the WINDOW was +// priced — the other agent's traffic carried cost — so an inherited Priced:true would have the +// caller print $0.00 for exactly the agent the AGENTS pane prints "—" for. +func TestScopeToAgent_RederivesPricedForAnUnpricedAgent(t *testing.T) { + got, err := ScopeToAgent(scopeFixture(), "bob-shell/2.0.5", KeepBuckets) + if err != nil { + t.Fatalf("ScopeToAgent: %v", err) + } + if got.Priced { + t.Error("Priced = true for an agent whose traffic nothing priced; the caller will print $0.00 instead of \"—\"") + } + if got.Totals.CostMicros != 0 { + t.Errorf("Totals.CostMicros = %d, want 0", got.Totals.CostMicros) + } +} + +// The three by-model maps are dropped, because nothing here can narrow them: a bucket's series +// is keyed by agent and carries no per-model breakdown, so the only available readings are the +// window's maps — which describe other agents' traffic — or none. +func TestScopeToAgent_DropsTheProvenanceMapsAndKeepsTheReadFacts(t *testing.T) { + snap := scopeFixture() + snap.PricedBy = map[string]int64{"claude-sonnet-5": 14} + snap.UnpricedBy = map[string]int64{"some-model": 8} + snap.IncompleteBy = map[string]int64{"claude-sonnet-5": 2} + snap.DaysOutsideRetention = 3 + got, err := ScopeToAgent(snap, "claude-code/2.1.270", KeepBuckets) + if err != nil { + t.Fatalf("ScopeToAgent: %v", err) + } + if got.PricedBy != nil || got.UnpricedBy != nil || got.IncompleteBy != nil { + t.Errorf("a by-model map survived the narrowing: priced=%v unpriced=%v incomplete=%v", + got.PricedBy, got.UnpricedBy, got.IncompleteBy) + } + // DaysOutsideRetention describes the READ, which is the same fact whichever agent is + // scoped to. Dropping it would hide a short sum behind a narrower question. + if got.DaysOutsideRetention != 3 { + t.Errorf("DaysOutsideRetention = %d, want 3 — it describes the read, not the agent", + got.DaysOutsideRetention) + } +} + +// The two whole-window residuals SURVIVE the narrowing, because a residual is a fact about the +// window rather than about an agent — and because callers are already reading one off this +// function's result: `abctl cost` prints UngroupedCostMicros at writeCostSummary's --agent note, +// and abctl's usage pane prints it through tui.costUngroupedRow. +// +// PINNED HERE BECAUSE EVERY GUARD IT HAD WAS IN A CONSUMER. Measured on this tree: nilling +// UngroupedCostMicros in ScopeToAgent fails two cmd/abctl tests, and nilling +// UngroupedAvoidedMicros failed nothing at all — so for the avoided residual this assertion is +// the only thing between a one-line "cleanup" and a silently retracted disclosure. +// +// BOTH BUCKET MODES, because the two callers pass different ones and the residual is a +// window-level figure either way — narrowing the buckets is not a reason to drop it. +// +// DISTINCT VALUES, neither of them a figure the fixture already carries: a shared number would +// let a narrowing that copied the wrong field into both satisfy both assertions. +func TestScopeToAgent_KeepsTheWindowResidualsForTheCallerToDisclose(t *testing.T) { + snap := scopeFixture() + snap.UngroupedCostMicros = ptr(int64(777)) + snap.UngroupedAvoidedMicros = ptr(int64(555)) + for _, buckets := range []BucketScope{KeepBuckets, NarrowBuckets} { + got, err := ScopeToAgent(snap, "claude-code/2.1.270", buckets) + if err != nil { + t.Fatalf("ScopeToAgent(%v): %v", buckets, err) + } + if got.UngroupedCostMicros == nil || *got.UngroupedCostMicros != 777 { + t.Errorf("UngroupedCostMicros = %v under bucket mode %v, want 777 — the residual is a "+ + "window fact, and writeCostSummary's --agent note reads it off this snapshot", + got.UngroupedCostMicros, buckets) + } + if got.UngroupedAvoidedMicros == nil || *got.UngroupedAvoidedMicros != 555 { + t.Errorf("UngroupedAvoidedMicros = %v under bucket mode %v, want 555 — and nothing "+ + "else in this repo would have failed on its loss", + got.UngroupedAvoidedMicros, buckets) + } + } +} + +// KeepBuckets is the mode `abctl cost` asks for: it reads only window totals ON ITS SCOPED PATH, +// so it wants no per-bucket work done there. The buckets and their Series come through untouched. +// +// THE QUALIFIER IS LOAD-BEARING and is restored from scope.go's own wording rather than rewritten: +// `abctl cost --by` does read buckets (writeCostBreakdown folds them), so the same sentence without +// "on its scoped path" is false of the command as a whole. +func TestScopeToAgent_KeepBucketsLeavesTheSeriesIntact(t *testing.T) { + got, err := ScopeToAgent(scopeFixture(), "claude-code/2.1.270", KeepBuckets) + if err != nil { + t.Fatalf("ScopeToAgent: %v", err) + } + if len(got.Buckets) != 2 { + t.Fatalf("len(Buckets) = %d, want 2", len(got.Buckets)) + } + if got.Buckets[0].Requests != 10 || got.Buckets[1].Requests != 12 { + t.Errorf("bucket counts = %d, %d; want the window's 10, 12 left alone", + got.Buckets[0].Requests, got.Buckets[1].Requests) + } + if len(got.Buckets[1].Series) != 2 { + t.Errorf("bucket 1 kept %d series entries, want both", len(got.Buckets[1].Series)) + } +} + +// NarrowBuckets is what a CHART needs: every bucket's counts become this agent's, so a series +// rendered from Buckets describes the scoped agent rather than the whole window. +func TestScopeToAgent_NarrowBucketsRewritesEachBucket(t *testing.T) { + got, err := ScopeToAgent(scopeFixture(), "claude-code/2.1.270", NarrowBuckets) + if err != nil { + t.Fatalf("ScopeToAgent: %v", err) + } + if got.Buckets[0].Requests != 10 { + t.Errorf("bucket 0 Requests = %d, want this agent's 10", got.Buckets[0].Requests) + } + // The window's second bucket held 12 requests; this agent's share of it is 4. + if got.Buckets[1].Requests != 4 { + t.Errorf("bucket 1 Requests = %d, want this agent's 4 (the window's is 12)", + got.Buckets[1].Requests) + } + // Series is dropped rather than kept: it describes a grouping the narrowed bucket has + // already been reduced to one member of, and a renderer that found it there would stack + // every agent back on top of the scoped one. + for i, b := range got.Buckets { + if b.Series != nil { + t.Errorf("bucket %d kept its Series after narrowing: %v", i, b.Series) + } + } +} + +// A bucket the agent is absent from survives as a ZERO bucket rather than being dropped. +// +// The chart reads Buckets positionally against a time axis — Snapshot.Buckets' own doc says +// zeroed entries are what let a client tell an idle minute from one that fell off the ring — so +// dropping the agent's idle buckets would compress its history and move every bar left. +func TestScopeToAgent_NarrowBucketsKeepsIdleBucketsAsZero(t *testing.T) { + got, err := ScopeToAgent(scopeFixture(), "bob-shell/2.0.5", NarrowBuckets) + if err != nil { + t.Fatalf("ScopeToAgent: %v", err) + } + if len(got.Buckets) != 2 { + t.Fatalf("len(Buckets) = %d, want 2 — an idle bucket must not be dropped", len(got.Buckets)) + } + if got.Buckets[0].Requests != 0 { + t.Errorf("bucket 0 Requests = %d, want 0: this agent sent nothing in it", got.Buckets[0].Requests) + } + if !got.Buckets[0].At.Equal(scopeFixture().Buckets[0].At) { + t.Errorf("bucket 0 At = %v, want the original timestamp", got.Buckets[0].At) + } + if got.Buckets[1].Requests != 8 { + t.Errorf("bucket 1 Requests = %d, want 8", got.Buckets[1].Requests) + } +} + +// Latency is ZEROED by NarrowBuckets, because it cannot be narrowed: Series is +// map[string]Counts and Counts carries no latency, so a bucket's LatMeanMs / LatStdDevMs / +// LatSamples describe every agent that shared the bucket. Keeping them would render one +// agent's chart out of another's response times; the caller must say latency is unavailable +// under a scope instead. +func TestScopeToAgent_NarrowBucketsZeroesLatencyItCannotAttribute(t *testing.T) { + got, err := ScopeToAgent(scopeFixture(), "claude-code/2.1.270", NarrowBuckets) + if err != nil { + t.Fatalf("ScopeToAgent: %v", err) + } + for i, b := range got.Buckets { + if b.LatMeanMs != 0 || b.LatStdDevMs != 0 || b.LatSamples != 0 { + t.Errorf("bucket %d kept unattributable latency: mean=%v stddev=%v samples=%d", + i, b.LatMeanMs, b.LatStdDevMs, b.LatSamples) + } + } +} + +// An unknown agent is an error that NAMES the known ones, sorted. +// +// The labels are User-Agents, so they are neither short nor guessable — "bob" is the obvious +// thing to try and is not what Bob sends. Sorted because a set printed in map order is a set a +// reader cannot diff against yesterday's. +func TestScopeToAgent_UnknownAgentNamesTheKnownOnesInOrder(t *testing.T) { + want := "no agent \"bob\" in the today window; seen: bob-shell/2.0.5, claude-code/2.1.270" + // REPEATED, because the order under test comes out of a map walk: over two labels, one call + // comes out sorted by chance often enough to pass with the sort deleted. + for i := 0; i < 64; i++ { + _, err := ScopeToAgent(scopeFixture(), "bob", KeepBuckets) + if err == nil { + t.Fatal("ScopeToAgent accepted an agent that is not in the window") + } + if err.Error() != want { + t.Fatalf("call %d: error = %q, want %q", i, err, want) + } + } +} + +// A window with no agent traffic at all is a DIFFERENT sentence: there is no set to print, and +// "seen: " followed by nothing reads as a truncated message rather than an empty window. +func TestScopeToAgent_EmptyWindowSaysSoRatherThanListingNothing(t *testing.T) { + _, err := ScopeToAgent(&Snapshot{Window: "today", Group: GroupAgent}, "bob", KeepBuckets) + if err == nil { + t.Fatal("ScopeToAgent accepted an agent against an empty window") + } + want := "no agent traffic in the today window, so \"bob\" matches nothing" + if err.Error() != want { + t.Errorf("error = %q, want %q", err, want) + } +} + +// The caller's snapshot is never mutated: both modes hand back a copy. +// +// NarrowBuckets is the mode that makes this load-bearing: it rewrites per-bucket counts, so +// narrowing in place would leave the caller holding one agent's figures with no way back to the +// window's own. +func TestScopeToAgent_DoesNotMutateTheCallersSnapshot(t *testing.T) { + snap := scopeFixture() + if _, err := ScopeToAgent(snap, "claude-code/2.1.270", NarrowBuckets); err != nil { + t.Fatalf("ScopeToAgent: %v", err) + } + if snap.Totals.Requests != 22 { + t.Errorf("caller's Totals.Requests = %d, want the window's 22", snap.Totals.Requests) + } + if snap.Buckets[1].Requests != 12 { + t.Errorf("caller's bucket 1 Requests = %d, want the window's 12", snap.Buckets[1].Requests) + } + if len(snap.Buckets[1].Series) != 2 { + t.Errorf("caller's bucket 1 lost its Series: %v", snap.Buckets[1].Series) + } + if snap.Group != GroupAgent { + t.Errorf("caller's Group = %v, want it untouched", snap.Group) + } +}