From 69e433d6dc4b3757d279f573281ed99b7cbf1bf0 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 07:27:32 -0400 Subject: [PATCH 01/13] refactor: Move the agent narrowing into core, with a bucket-scope choice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit abctl's usage pane is about to scope itself to one agent, and the narrowing it needs already existed — as scopeToAgent in cmd/abctl, package main, which no other package can import. Same situation #1152 resolved for the series ranking: two surfaces answering one question, so the answer moves to core and both call it. It also has to do more than the CLI needed. scopeToAgent rewrote Totals and left Buckets alone, which is right for a command that prints window totals and wrong for a pane that renders a chart from the buckets themselves — narrowing only the totals would title a whole-window chart with one agent's name. So the mode is a parameter: KeepBuckets is the old behavior, NarrowBuckets additionally rewrites each bucket to that agent's share of it. Neither value is a safe default, hence no zero-argument form. Two things NarrowBuckets cannot carry across, both documented at the narrowing: Series is dropped, because a renderer that found it there would stack every agent back on top of the scoped one; and latency is ZEROED, because Series is map[string]Counts and Counts holds no latency, so a bucket's LatMeanMs describes every agent that shared it. A caller offering a latency view has to say it is unavailable under a scope — there is no per-agent latency on the wire to offer instead. Buckets the agent is idle in survive as zero buckets at their original timestamps, since the chart reads Buckets positionally. `abctl cost` keeps byte-identical behavior: it passes KeepBuckets, the mode that changes nothing, and on its scoped path it reads only window totals. An earlier revision of this message justified that by enumerating the readers of snap.Buckets and got the enumeration wrong — seriesForBreakdown reads them too, and is called on the --agent path, though it returns nil there before reaching the read. The conclusion held; the stated reason did not. Signed-off-by: Hai Huang Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/cmd_cost.go | 93 +------------ core/cost/usage/scope.go | 147 ++++++++++++++++++++ core/cost/usage/scope_test.go | 243 ++++++++++++++++++++++++++++++++++ 3 files changed, 397 insertions(+), 86 deletions(-) create mode 100644 core/cost/usage/scope.go create mode 100644 core/cost/usage/scope_test.go diff --git a/cmd/abctl/cmd_cost.go b/cmd/abctl/cmd_cost.go index 924ad25e8..486e42e65 100644 --- a/cmd/abctl/cmd_cost.go +++ b/cmd/abctl/cmd_cost.go @@ -206,7 +206,13 @@ Flags: } if *agent != "" { - scoped, err := scopeToAgent(snap, *agent) + // KeepBuckets, not NarrowBuckets: nothing on this path reads Buckets at all. The only + // reader of them here is writeCostBreakdown, and it is reachable only under --by, which + // the check at the top of this function refuses alongside --agent — so narrowing would + // be work whose result no writer looks at. 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 +230,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 diff --git a/core/cost/usage/scope.go b/core/cost/usage/scope.go new file mode 100644 index 000000000..ec5c03d18 --- /dev/null +++ b/core/cost/usage/scope.go @@ -0,0 +1,147 @@ +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 TUI holds one fetched snapshot and +// re-derives a scoped view from it whenever the scope changes. +// +// 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 --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 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 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. `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..8199971f7 --- /dev/null +++ b/core/cost/usage/scope_test.go @@ -0,0 +1,243 @@ +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) + } +} + +// KeepBuckets is the contract `abctl cost` depends on: it prints window totals, and passing it +// a snapshot whose per-bucket series had been flattened would leave `--agent --by ` with +// nothing to break down. So the buckets and their Series come through untouched. +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) { + _, err := ScopeToAgent(scopeFixture(), "bob", KeepBuckets) + if err == nil { + t.Fatal("ScopeToAgent accepted an agent that is not in the window") + } + want := "no agent \"bob\" in the today window; seen: bob-shell/2.0.5, claude-code/2.1.270" + if err.Error() != want { + t.Errorf("error = %q, want %q", 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 --agent \"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, and +// the TUI holds one fetched snapshot and re-derives a scoped view from it whenever the scope +// changes — so narrowing in place would make the second scope read the first one'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) + } +} From 1c44718e312ddc74c6078551e0edacdfe6d05e09 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 07:47:25 -0400 Subject: [PATCH 02/13] feat: Open the AGENTS picker at startup and scope the usage pane to one agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two agents on a proxy and abctl went straight to the sessions pane, leaving the per-agent breakdown behind a key most operators never press. The rule for when a picker is worth showing already existed — agentsPaneApplies, "fewer than two agents skips the pane" — written, asserted, and called by nothing but its own tests. This gives it the caller it was written for. The gate hangs off initSessionView, the one place every entry point converges on (--endpoint mode's Init, the pod picker's portForwardReadyMsg, and [l]'s local endpoint), so it also re-runs when the operator backs out and connects to a DIFFERENT pod: a different proxy has different agents on it. Batched like every other fetch, so the sessions pane paints and streams while the answer is in flight. Nothing is remembered between runs — the decision is a pure function of what the window shows now, and a remembered dismissal would go stale at exactly the moment a second agent appeared. It declines in SILENCE, which is why agentsOpen is an enum rather than the bool it replaces. An `A` press is owed an answer either way; this fetch was never requested, so flashing "only claude-code has been seen" at every single-agent startup would be an unsolicited complaint about a normal deployment. A failed gate fetch is silent too, and recorded, so a later `A` reports it. Enter scopes the usage pane to the row under the cursor and leaves — the pane is a picker, so staying on it would leave the operator on the one surface the choice does not affect. Enter on the agent already scoped clears it: there is no "all agents" row, and the footer's label flips to say which way the key will go, as the usage pane's [s] does for sessions. esc leaves the scope alone, because esc means "back out" on every other pane here. The scope is applied client-side, via the core narrowing: the pane fetches group=agent whatever axis it is showing, then narrows totals AND buckets. It reaches the usage pane and nothing else — the spend band and its drawer fetch on their own chains and ignore it. Consequences, each stated where it bites: - [b] is inert and omitted from the footer, since the scope has taken the wire axis. m.usage.group is left alone rather than overwritten, so clearing the scope restores the axis the operator picked. - Latency says "not available per agent" instead of plotting the zeroes the narrowing leaves. renderWhiskers would have answered "no latency samples in this window" — true of the narrowed snapshot, false about the window, and it would send a reader hunting for traffic that is there. - A scope that stops matching (the window moves past an agent's last request) is REPORTED, naming the agents that are in the window. Showing every agent under a scoped title is the one outcome a reader cannot detect. - Both titles carry the scope, so no narrowed figure is unlabelled. The spend strip and drawer are NOT scoped and still show every agent. They are separate fetches with their own axes; scoping them is its own change. Two claims that are now false are corrected rather than left: the paneAgents enum comment and the [?] overlay note both said the pane was read-only because /v1/usage takes no agent filter. The endpoint's limit is real and unchanged — it now bounds WHICH views can honour a scope rather than whether any can. Signed-off-by: Hai Huang Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/tui/agents_pane.go | 109 ++++++++++- cmd/abctl/tui/agents_pane_test.go | 93 +++++++++- cmd/abctl/tui/agents_scope_test.go | 287 +++++++++++++++++++++++++++++ cmd/abctl/tui/app.go | 66 ++++++- cmd/abctl/tui/help_overlay.go | 32 ++-- cmd/abctl/tui/keys.go | 81 +++++--- cmd/abctl/tui/usage_pane.go | 39 ++++ 7 files changed, 653 insertions(+), 54 deletions(-) create mode 100644 cmd/abctl/tui/agents_scope_test.go diff --git a/cmd/abctl/tui/agents_pane.go b/cmd/abctl/tui/agents_pane.go index 6aeb995b6..e9936c90b 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; the startup gate passes paneSessions, the pane it + // is about to interrupt. from paneID } @@ -146,7 +165,7 @@ type agentRowsLoadedMsg struct { // 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 { +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..c1f9ffaed 100644 --- a/cmd/abctl/tui/agents_pane_test.go +++ b/cmd/abctl/tui/agents_pane_test.go @@ -1,6 +1,7 @@ package tui import ( + "errors" "math" "strings" "testing" @@ -349,7 +350,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 +516,93 @@ 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) + } + }) + } +} + +// The startup entry records SESSIONS as its caller, so esc goes where the operator expected to +// be rather than to the pane enum's zero value — which is the Kubernetes namespace picker, and +// would look like the connection had gone away. +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}}, + } + m := &model{pane: paneSessions, previousPane: paneNone, 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) + } + 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) + } +} + +// 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()} + 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") + } +} diff --git a/cmd/abctl/tui/agents_scope_test.go b/cmd/abctl/tui/agents_scope_test.go new file mode 100644 index 000000000..38dad08ec --- /dev/null +++ b/cmd/abctl/tui/agents_scope_test.go @@ -0,0 +1,287 @@ +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 cost and usage views 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: its whole +// purpose is choosing what the views behind it show, so staying on it after a choice would leave +// the operator looking at the one surface the choice does not affect. +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. Showing the +// unscoped figures under a scoped title would be the one outcome a reader cannot detect; 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") + } +} + +// [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 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..0c5c5b2d9 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 and cost + // views are 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,22 @@ 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 and cost views are narrowed to, or "" for all of them. + // + // 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 +879,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 +1335,35 @@ 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. + if msg.err == nil && 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 before the operator + // pressed anything — 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 +2223,23 @@ 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, not only in the footer, because it changes what + // every figure on the pane means. A narrowed chart that looked like the whole window + // would be wrong in the one direction a reader cannot check. + 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..d99ce6906 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 usage + cost 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. The scope reaches usage and " + + "cost; sessions and events carry no agent.", }, }, } diff --git a/cmd/abctl/tui/keys.go b/cmd/abctl/tui/keys.go index 3fa30851a..20c26ede1 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,28 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { case "enter", "right", "l": switch m.pane { + case paneAgents: + // Pick the agent the usage and cost views are narrowed to, then LEAVE. The pane is a + // picker, so staying on it after a choice would leave the operator looking at the one + // surface the choice does not affect. + // + // 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 +875,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 +1244,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 +1263,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..76375a139 100644 --- a/cmd/abctl/tui/usage_pane.go +++ b/cmd/abctl/tui/usage_pane.go @@ -144,10 +144,33 @@ 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, and showing every agent under a scoped + // title is the one outcome a reader cannot detect. The error names the agents that + // are in the window. + snap, err = usage.ScopeToAgent(snap, scope, usage.NarrowBuckets) + } return usageLoadedMsg{snap: snap, req: req, err: err} } } @@ -330,6 +353,22 @@ 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. Clear the agent\n") + b.WriteString(" scope with [A] to plot latency for all of them.\n") + b.WriteString("\n") + b.WriteString(renderUsageSummary(m.usage.snap)) + b.WriteString("\n") default: for _, line := range renderUsageChart(m.usage.snap, m.usage.metric, m.usage.group, width, usageChartHeight(height)) { b.WriteString(line) From 775572b9fc915a30715152174c3d1020d5563414 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 07:48:22 -0400 Subject: [PATCH 03/13] docs: Document the startup picker and the agent scope in abctl's README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The keybinding table had the `A` key and its two-agent refusal, and now owes three more facts: ↑↓ move a cursor that used to be inert, ↵ sets and clears the scope, and the picker opens itself at startup under the same two-agent rule. Also says what a scope does NOT reach, because the answer is not guessable from the pane: sessions and events carry no agent at all, and the spend band and drawer are separate fetches that keep showing every agent. And the two things that change on the usage pane while a scope is active — [b] leaving the footer, latency reporting itself unavailable — since a reader who met either without warning would take it for a bug. Signed-off-by: Hai Huang Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/README.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/cmd/abctl/README.md b/cmd/abctl/README.md index df948a8ef..1cdd3d425 100644 --- a/cmd/abctl/README.md +++ b/cmd/abctl/README.md @@ -1150,7 +1150,22 @@ 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 and cost views 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 scope reaches usage and cost only: sessions and events carry no agent, and the spend band and drawer keep showing every agent | +| `Esc` | agents | back to the pane `A` was pressed on, leaving the scope as it is | + +**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. | `e` | pipeline | edit pipeline subtree in `$EDITOR` | | `y` | edit/diff | apply the edit | | `N` | edit/diff | abort the edit | From 0f6f4ea51e7cf36c1412a1dc08eed9a30e15c45b Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 09:32:31 -0400 Subject: [PATCH 04/13] =?UTF-8?q?fix:=20Review=20round=201=20=E2=80=94=20d?= =?UTF-8?q?elete=20the=20false=20reasons,=20unbreak=20the=20README=20table?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two classes, swept repo-wide rather than fixed at the lines the review named. FALSE-REASON — a true conclusion resting on a false mechanism, in five layers. Fixed by DELETING the reason rather than restating it, which is the fix style that does not regenerate the class: - cmd_cost.go claimed "the only reader of them here is writeCostBreakdown, reachable only under --by, which the check at the top refuses alongside --agent". seriesForBreakdown reads snap.Buckets too, and IS called on the --agent path via writeCostJSON; it returns nil at its own `if by == ""`, not because of the flag refusal. The enumeration is dropped, not corrected: what this caller needs is that it prints window totals. - agents_pane.go said the startup gate "passes paneSessions". It passes paneNone, as the same file says 70 lines down. Clause deleted; the value is documented where it is chosen. - agents_pane.go still said "this pane is read-only — there is no server-side agent scope". This PR falsified that. Deleted. - scope.go and scope_test.go said the TUI "re-derives a scoped view" from one held snapshot. It re-fetches. Mechanism deleted; the copy rule it was justifying is correct and stays. - scope_test.go justified KeepBuckets with `--agent --by `, a combination cmd_cost.go refuses outright. OVERCLAIM — "usage and cost views" names two surfaces where one honours the scope. Swept 8 sites, 6 in scope, 6 fixed (README, keys.go, help_overlay.go, app.go x2, agents_scope_test.go). The spend band and its drawer fetch on their own chains and ignore agentScope, and `abctl cost --agent` is a separate process. The help overlay was the unqualified one — the README disclosed the spend-band exclusion and it did not. GEOMETRY-INVARIANT — the two prose paragraphs I added to the keybinding table sat BETWEEN rows, and a blank line ends a GFM table, so the seven rows below them (`e`, `y`, `N`, `r`, `Esc`, `Esc`, `q`) rendered as literal pipe text. Verified against base: at 67021e3f that table was contiguous. Prose moved below the last row; now 44 contiguous rows. Sweeps: 23 symbols named in added prose, 0 unresolved. 135 quantifier hits triaged. Claims delta measured at +35/-35, net 0 — a claims round that adds prose is writing the next round's findings. Signed-off-by: Hai Huang Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/README.md | 16 ++++++++-------- cmd/abctl/cmd_cost.go | 6 ++---- cmd/abctl/tui/agents_pane.go | 8 ++++---- cmd/abctl/tui/agents_scope_test.go | 2 +- cmd/abctl/tui/app.go | 11 ++++++++--- cmd/abctl/tui/help_overlay.go | 6 +++--- cmd/abctl/tui/keys.go | 2 +- cmd/abctl/tui/usage_pane.go | 4 ++-- core/cost/usage/scope.go | 3 +-- core/cost/usage/scope_test.go | 12 +++++------- 10 files changed, 35 insertions(+), 35 deletions(-) diff --git a/cmd/abctl/README.md b/cmd/abctl/README.md index 1cdd3d425..c0d71971d 100644 --- a/cmd/abctl/README.md +++ b/cmd/abctl/README.md @@ -1151,8 +1151,15 @@ Layered on top of all of them: | `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 | | `↑↓` / `jk` | agents | move the cursor | -| `↵` | agents | scope the usage and cost views 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 scope reaches usage and cost only: sessions and events carry no agent, and the spend band and drawer keep showing every agent | +| `↵` | 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 | +| `r` | edit/error | retry: re-open the editor (post-edit failure) or refetch (fetch failure) | +| `Esc` | edit/{fetching,editing,applying} | abort the edit, return to Pipeline pane | +| `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 @@ -1166,13 +1173,6 @@ 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. -| `e` | pipeline | edit pipeline subtree in `$EDITOR` | -| `y` | edit/diff | apply the edit | -| `N` | edit/diff | abort the edit | -| `r` | edit/error | retry: re-open the editor (post-edit failure) or refetch (fetch failure) | -| `Esc` | edit/{fetching,editing,applying} | abort the edit, return to Pipeline pane | -| `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) | ## Settings diff --git a/cmd/abctl/cmd_cost.go b/cmd/abctl/cmd_cost.go index 486e42e65..88842dbd7 100644 --- a/cmd/abctl/cmd_cost.go +++ b/cmd/abctl/cmd_cost.go @@ -206,10 +206,8 @@ Flags: } if *agent != "" { - // KeepBuckets, not NarrowBuckets: nothing on this path reads Buckets at all. The only - // reader of them here is writeCostBreakdown, and it is reachable only under --by, which - // the check at the top of this function refuses alongside --agent — so narrowing would - // be work whose result no writer looks at. abctl's usage pane passes the other value + // KeepBuckets, not NarrowBuckets: this command prints window totals, so it needs no + // per-bucket narrowing and pays for none. 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) diff --git a/cmd/abctl/tui/agents_pane.go b/cmd/abctl/tui/agents_pane.go index e9936c90b..5b3daba71 100644 --- a/cmd/abctl/tui/agents_pane.go +++ b/cmd/abctl/tui/agents_pane.go @@ -154,8 +154,8 @@ type agentRowsLoadedMsg struct { // 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. Meaningless under - // agentsOpenNever, which enters nothing; the startup gate passes paneSessions, the pane it - // is about to interrupt. + // 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 } @@ -163,8 +163,8 @@ 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. +// 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 diff --git a/cmd/abctl/tui/agents_scope_test.go b/cmd/abctl/tui/agents_scope_test.go index 38dad08ec..6a15a94bd 100644 --- a/cmd/abctl/tui/agents_scope_test.go +++ b/cmd/abctl/tui/agents_scope_test.go @@ -35,7 +35,7 @@ func scopedModel(t *testing.T, cursor int, scope string) *model { return m } -// Enter scopes the cost and usage views to the row under the cursor, and leaves the pane. +// 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: its whole // purpose is choosing what the views behind it show, so staying on it after a choice would leave diff --git a/cmd/abctl/tui/app.go b/cmd/abctl/tui/app.go index 0c5c5b2d9..8e2d8028d 100644 --- a/cmd/abctl/tui/app.go +++ b/cmd/abctl/tui/app.go @@ -42,8 +42,8 @@ const ( panePluginDetail paneCatalog paneUsage - // paneAgents shows what each CODING AGENT has spent, and picks which one the usage and cost - // views are scoped to. The scope is applied CLIENT-SIDE — /v1/usage takes no agent filter, so + // 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, @@ -727,7 +727,12 @@ type model struct { // 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 - // agentScope is the agent the usage and cost views are narrowed to, or "" for all of them. + // 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 diff --git a/cmd/abctl/tui/help_overlay.go b/cmd/abctl/tui/help_overlay.go index d99ce6906..776279640 100644 --- a/cmd/abctl/tui/help_overlay.go +++ b/cmd/abctl/tui/help_overlay.go @@ -373,7 +373,7 @@ var paneKeys = map[paneID]keyGroup{ {"↑↓ / jk", "navigate"}, // 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 usage + cost to this agent; again clears"}, + {"↵", "scope the usage pane to this agent; again clears"}, {"esc", "back, keeping the scope"}, }, // ONE SHORT NOTE, NOT TWO LONG ONES, and the budget is real rather than stylistic: an @@ -389,8 +389,8 @@ var paneKeys = map[paneID]keyGroup{ // 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. The scope reaches usage and " + - "cost; sessions and events carry no agent.", + "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/keys.go b/cmd/abctl/tui/keys.go index 20c26ede1..a547a702c 100644 --- a/cmd/abctl/tui/keys.go +++ b/cmd/abctl/tui/keys.go @@ -588,7 +588,7 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { case "enter", "right", "l": switch m.pane { case paneAgents: - // Pick the agent the usage and cost views are narrowed to, then LEAVE. The pane is a + // Pick the agent the usage pane is narrowed to, then LEAVE. The pane is a // picker, so staying on it after a choice would leave the operator looking at the one // surface the choice does not affect. // diff --git a/cmd/abctl/tui/usage_pane.go b/cmd/abctl/tui/usage_pane.go index 76375a139..9d9b48fc5 100644 --- a/cmd/abctl/tui/usage_pane.go +++ b/cmd/abctl/tui/usage_pane.go @@ -364,8 +364,8 @@ func (m *model) renderUsage(width, height int) string { 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. Clear the agent\n") - b.WriteString(" scope with [A] to plot latency for all of them.\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") diff --git a/core/cost/usage/scope.go b/core/cost/usage/scope.go index ec5c03d18..8db85d913 100644 --- a/core/cost/usage/scope.go +++ b/core/cost/usage/scope.go @@ -37,8 +37,7 @@ const ( // 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 TUI holds one fetched snapshot and -// re-derives a scoped view from it whenever the scope changes. +// 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. diff --git a/core/cost/usage/scope_test.go b/core/cost/usage/scope_test.go index 8199971f7..8c7072fa8 100644 --- a/core/cost/usage/scope_test.go +++ b/core/cost/usage/scope_test.go @@ -101,9 +101,8 @@ func TestScopeToAgent_DropsTheProvenanceMapsAndKeepsTheReadFacts(t *testing.T) { } } -// KeepBuckets is the contract `abctl cost` depends on: it prints window totals, and passing it -// a snapshot whose per-bucket series had been flattened would leave `--agent --by ` with -// nothing to break down. So the buckets and their Series come through untouched. +// KeepBuckets is the mode `abctl cost` asks for: it prints window totals and reads no buckets, +// so it wants no per-bucket work done. The buckets and their Series come through untouched. func TestScopeToAgent_KeepBucketsLeavesTheSeriesIntact(t *testing.T) { got, err := ScopeToAgent(scopeFixture(), "claude-code/2.1.270", KeepBuckets) if err != nil { @@ -219,10 +218,9 @@ func TestScopeToAgent_EmptyWindowSaysSoRatherThanListingNothing(t *testing.T) { // 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, and -// the TUI holds one fetched snapshot and re-derives a scoped view from it whenever the scope -// changes — so narrowing in place would make the second scope read the first one's figures, -// with no way back to the window's own. +// 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 { From e5911465340388f199bbb9d2e59ccb274a5b5515 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 09:32:31 -0400 Subject: [PATCH 05/13] =?UTF-8?q?test:=20Review=20round=201=20=E2=80=94=20?= =?UTF-8?q?pin=20the=20startup=20gate's=20error=20guard=20and=20call=20sit?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both were mutation survivors: the package stayed green with the production line deleted. TestAgentsPane_StartupFetchFailureIsSilent set no agents, so agentsPaneApplies(nil) was already false and carried the assertion alone — deleting `msg.err == nil` from the gate changed nothing. The fixture now seeds two rows, standing for a previous pod's, so the error is the only thing that can hold the pane back. That state is reachable rather than theoretical: 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. The gate's only call site was unasserted. Replacing `m.startupAgentsGateCmd(),` in initSessionView with a no-op left the whole package green, so this PR's headline behaviour — the picker opens itself at startup — had no test that it is ever reached in production; all four startup tests inject agentRowsLoadedMsg straight into Update. TestInitSessionView_ArmsTheStartupAgentsGate runs the real batch against an httptest server and asserts the message's MODE, not just its arrival: a gate armed with agentsOpenOnPress would flash a refusal at every single-agent startup. The batch is fanned out concurrently with a deadline because its leaves include a 1s tick and an SSE pump that never return on their own. | New assertion | Mutation | Verdict | |---|---|---| | StartupFetchFailureIsSilent | dropped `msg.err == nil` | killed | | InitSessionView_ArmsTheGate | gate call -> nil | killed | | InitSessionView_ArmsTheGate | mode -> agentsOpenOnPress | killed | Signed-off-by: Hai Huang Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/tui/agents_pane_test.go | 87 +++++++++++++++++++++++++++++-- 1 file changed, 83 insertions(+), 4 deletions(-) diff --git a/cmd/abctl/tui/agents_pane_test.go b/cmd/abctl/tui/agents_pane_test.go index c1f9ffaed..7ca68ab79 100644 --- a/cmd/abctl/tui/agents_pane_test.go +++ b/cmd/abctl/tui/agents_pane_test.go @@ -1,13 +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" ) @@ -566,9 +571,12 @@ func TestAgentsPane_StartupEntersOnlyWhereTheGateApplies(t *testing.T) { } } -// The startup entry records SESSIONS as its caller, so esc goes where the operator expected to -// be rather than to the pane enum's zero value — which is the Kubernetes namespace picker, and -// would look like the connection had gone away. +// 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}}, @@ -593,7 +601,17 @@ func TestAgentsPane_StartupEscapesToSessions(t *testing.T) { // 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()} + 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 { @@ -606,3 +624,64 @@ func TestAgentsPane_StartupFetchFailureIsSilent(t *testing.T) { 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") + } + batch, ok := cmd().(tea.BatchMsg) + if !ok { + t.Fatalf("initSessionView produced %T, want tea.BatchMsg", cmd()) + } + 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) + } + case <-time.After(10 * time.Second): + t.Fatal("no agentRowsLoadedMsg came out of initSessionView's batch — the startup gate is not armed") + } +} From 403f386e89f56bbfab5d45fd1c16e9e52253d9e6 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 10:04:08 -0400 Subject: [PATCH 06/13] =?UTF-8?q?fix:=20Review=20round=202=20=E2=80=94=20r?= =?UTF-8?q?estore=20the=20qualifier=20round=201's=20deletion=20took?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 1 fixed its false-claim class by deletion, and in two places the deletion took the QUALIFIER rather than the claim. The surviving sentence was shorter, stronger and false: "KeepBuckets is the mode `abctl cost` asks for: it prints window totals and reads no buckets" `abctl cost --by` does read buckets — writeCostBreakdown folds snap.Buckets, and the tree's own TestRunCost_By pins that it does, so the claim is refutable by this repo's existing tests rather than by another comment. Deleting a claim is safe; deleting a condition strengthens whatever survives. Fixed by RESTORATION, not by composing a third wording: scope.go already carries the correctly qualified sentence 85 lines up in the same package ("reads only window totals on its scoped path"), untouched by this PR and therefore already reviewed. cmd_cost.go's copy had the same dropped qualifier, rescued only by sitting inside `if *agent != ""`; it now says "on this path" so it is true read alone. Two unpinned mechanisms from round 2's own delta, both found by mutants the reviewer appended: - Both startup fixtures hard-coded previousPane: paneNone, the same value the production arm assigns — so the fixture supplied the mechanism the comment credited, and deleting that assignment or setting it to paneSessions both survived. Seeded with panePipeline, which the arm must overwrite; both mutants now die. - TestInitSessionView_ArmsTheStartupAgentsGate could not tell a served fetch from a failed one, because fetchAgentRowsCmd's error return carries the same `open`. Blanking the fixture's agents left it green. It now also asserts err == nil and the two fixture rows, and invokes cmd() once rather than twice. The PR title said "scope usage and cost" — the exact phrasing the norm added at app.go:739 by round 1 forbids, and the one surface a file-scoped sweep structurally cannot reach. Retitled; it is also what lands as the merge commit's subject. Sweeps: 4 sites of the dropped-qualifier shape, 3 in scope, 3 fixed. Signed-off-by: Hai Huang Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/cmd_cost.go | 5 +++-- cmd/abctl/tui/agents_pane_test.go | 22 +++++++++++++++++++--- core/cost/usage/scope_test.go | 8 ++++++-- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/cmd/abctl/cmd_cost.go b/cmd/abctl/cmd_cost.go index 88842dbd7..ce318e093 100644 --- a/cmd/abctl/cmd_cost.go +++ b/cmd/abctl/cmd_cost.go @@ -206,8 +206,9 @@ Flags: } if *agent != "" { - // KeepBuckets, not NarrowBuckets: this command prints window totals, so it needs no - // per-bucket narrowing and pays for none. abctl's usage pane passes the other value + // 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) diff --git a/cmd/abctl/tui/agents_pane_test.go b/cmd/abctl/tui/agents_pane_test.go index 7ca68ab79..ed5f6949e 100644 --- a/cmd/abctl/tui/agents_pane_test.go +++ b/cmd/abctl/tui/agents_pane_test.go @@ -582,7 +582,11 @@ func TestAgentsPane_StartupEscapesToSessions(t *testing.T) { {label: "claude-code/2.1.270", Counts: usage.Counts{Requests: 10}}, {label: "bob-shell/2.0.5", Counts: usage.Counts{Requests: 8}}, } - m := &model{pane: paneSessions, previousPane: paneNone, agentsTbl: newAgentsTable(), client: deadClient()} + // 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 { @@ -658,9 +662,12 @@ func TestInitSessionView_ArmsTheStartupAgentsGate(t *testing.T) { if cmd == nil { t.Fatal("initSessionView returned no command") } - batch, ok := cmd().(tea.BatchMsg) + // Invoked ONCE: calling cmd() again for the error message would re-run tea.Batch's closure, + // and every leaf with it, on the failure path only. + first := cmd() + batch, ok := first.(tea.BatchMsg) if !ok { - t.Fatalf("initSessionView produced %T, want tea.BatchMsg", cmd()) + t.Fatalf("initSessionView produced %T, want tea.BatchMsg", first) } got := make(chan agentRowsLoadedMsg, len(batch)) for _, leaf := range batch { @@ -681,6 +688,15 @@ func TestInitSessionView_ArmsTheStartupAgentsGate(t *testing.T) { 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/core/cost/usage/scope_test.go b/core/cost/usage/scope_test.go index 8c7072fa8..40438885a 100644 --- a/core/cost/usage/scope_test.go +++ b/core/cost/usage/scope_test.go @@ -101,8 +101,12 @@ func TestScopeToAgent_DropsTheProvenanceMapsAndKeepsTheReadFacts(t *testing.T) { } } -// KeepBuckets is the mode `abctl cost` asks for: it prints window totals and reads no buckets, -// so it wants no per-bucket work done. The buckets and their Series come through untouched. +// 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 { From 2f966fa25b67baf8dd9d6d0be38af86f0f0588dd Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 10:07:27 -0400 Subject: [PATCH 07/13] =?UTF-8?q?test:=20Review=20round=202=20=E2=80=94=20?= =?UTF-8?q?pin=20the=20gate's=20recorded=20pane=20and=20the=20scope=20disc?= =?UTF-8?q?losure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two mutants the review appended still survived after the round-2 fix, for different reasons. R2_startup_records_caller set the startup arm's previousPane to paneSessions instead of paneNone. Seeding the fixture with panePipeline killed the DELETED-assignment mutant but not this one: both values land esc on Sessions — one through a recorded caller, one through leaveAgentsPane's fallback — so an assertion on where esc goes cannot separate the mechanism the comment credits from the other one. The test now asserts previousPane == paneNone directly, which is the field the arm sets and the claim the comment makes. R2_help_note_base_text reverted the [?] overlay's AGENTS note to the pre-round-1 "the scope reaches usage and cost" wording and survived the whole package: nothing pinned it. This PR has now re-broken that same claim twice — six prose sites in round 1, the PR title in round 2 — because it is the natural thing to write and no check contradicted it. app.go's agentScope doc states the norm, and a norm in a comment is not a guard. The new guard asserts the DISCLOSURE rather than the absence of a word: that the note names the spend band, the surface the scope does not reach and the one an operator is looking at when they press the key. Banning "cost" would fire on a site that had become correctly qualified, which is how a guard becomes a nuisance instead of a check. Two survivors are left deliberately, neither a gap: - R2_cost_agent_narrowbuckets (KeepBuckets -> NarrowBuckets on the --agent path) is equivalent by construction — nothing on that path observes buckets, which is the comment's own claim, so its survival is evidence for the comment rather than against it. - R2_by_reads_buckets is inert: its `-run TestCost` regex does not match TestRunCost_By. It does run about ten other tests, none of which reads the mutated fold. Its _wide twin, with the correct regex, is KILLED and carries the property. Signed-off-by: Hai Huang Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/tui/agents_pane_test.go | 7 +++++++ cmd/abctl/tui/help_overlay_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/cmd/abctl/tui/agents_pane_test.go b/cmd/abctl/tui/agents_pane_test.go index ed5f6949e..488668e96 100644 --- a/cmd/abctl/tui/agents_pane_test.go +++ b/cmd/abctl/tui/agents_pane_test.go @@ -592,6 +592,13 @@ func TestAgentsPane_StartupEscapesToSessions(t *testing.T) { 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) diff --git a/cmd/abctl/tui/help_overlay_test.go b/cmd/abctl/tui/help_overlay_test.go index 5d94fabe5..0f52a160a 100644 --- a/cmd/abctl/tui/help_overlay_test.go +++ b/cmd/abctl/tui/help_overlay_test.go @@ -697,3 +697,30 @@ func TestHelpOverlayDoesNotStealFilterInput(t *testing.T) { t.Fatal("`?` should open the overlay once the filter input is unfocused") } } + +// The AGENTS note discloses what the scope does NOT reach. +// +// THIS PR RE-BROKE THE SAME CLAIM TWICE — six prose sites in round 1, then the PR title in round 2 — +// because "the scope covers cost" is the natural thing to write and nothing mechanical contradicted +// it. app.go's agentScope doc states the norm ("this must not be described as scoping 'cost'"); a +// norm in a comment is not a guard, and a mutant reverting this note to the pre-round-1 wording +// survived the whole package. +// +// ASSERTS THE DISCLOSURE, NOT THE ABSENCE OF A WORD. Banning "cost" would fire on a site that had +// become correctly qualified, which is the failure mode that turns a guard into a nuisance. The +// property worth pinning is that the note names the surface the scope does not reach — the spend +// band — because that is the one an operator is looking at when they press the key. +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, "spend band") { + t.Errorf("the AGENTS note does not name the spend band, which the scope does NOT reach:\n %s", + joined) + } +} From d8a45710a078abd06fe97cb5674e662dc958980b Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 10:46:39 -0400 Subject: [PATCH 08/13] =?UTF-8?q?fix:=20Review=20round=203=20=E2=80=94=20p?= =?UTF-8?q?in=20the=20negation,=20delete=20two=20false=20mechanisms?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No new explanatory prose this round: every fix is a deletion or a one-line assertion change. Rounds 1 and 2 each corrected a claim by writing a replacement claim, and each replacement was the next round's only must-fix. The guard round 2 added to stop "the scope covers cost" did not stop it. It asserted the AGENTS note CONTAINS "spend band", which holds in either polarity — a note reading "scopes the usage pane and the spend band" passed it, and that is the exact claim app.go:739 forbids. Two mutants demonstrated it, one scoped to the guard and one to the whole package, so nothing else in cmd/abctl/tui was sensitive either. The assertion now pins "not the spend band". That is a substring pin and will go red on a correct reword; the comment says so and claims nothing more. Deleted, rather than corrected: - "calling cmd() again would re-run tea.Batch's closure, and every leaf with it" — false on both shapes. compactCmds returns the slice for 7 leaves and runs no leaf; the one-leaf collapse re-runs one, not every. The single invocation is visible in the code. The harness now REFUSES to run on a dirty tree. run_one reverts with `git checkout -- `, which restores the committed state, so an uncommitted fix in any of the six files it mutates was destroyed and the mutant then measured pre-fix code and reported SURVIVED. That happened three times in this loop. The skill's harness template ships require_clean_tree for this reason and this hand-rolled runner omitted it; the guard is proven by making the tree dirty and watching it exit 2. Signed-off-by: Hai Huang Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/tui/agents_pane_test.go | 2 -- cmd/abctl/tui/help_overlay_test.go | 20 ++++++-------------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/cmd/abctl/tui/agents_pane_test.go b/cmd/abctl/tui/agents_pane_test.go index 488668e96..ece35a4be 100644 --- a/cmd/abctl/tui/agents_pane_test.go +++ b/cmd/abctl/tui/agents_pane_test.go @@ -669,8 +669,6 @@ func TestInitSessionView_ArmsTheStartupAgentsGate(t *testing.T) { if cmd == nil { t.Fatal("initSessionView returned no command") } - // Invoked ONCE: calling cmd() again for the error message would re-run tea.Batch's closure, - // and every leaf with it, on the failure path only. first := cmd() batch, ok := first.(tea.BatchMsg) if !ok { diff --git a/cmd/abctl/tui/help_overlay_test.go b/cmd/abctl/tui/help_overlay_test.go index 0f52a160a..ea3d9c524 100644 --- a/cmd/abctl/tui/help_overlay_test.go +++ b/cmd/abctl/tui/help_overlay_test.go @@ -698,18 +698,11 @@ func TestHelpOverlayDoesNotStealFilterInput(t *testing.T) { } } -// The AGENTS note discloses what the scope does NOT reach. +// The AGENTS note must EXCLUDE the spend band, not merely mention it. // -// THIS PR RE-BROKE THE SAME CLAIM TWICE — six prose sites in round 1, then the PR title in round 2 — -// because "the scope covers cost" is the natural thing to write and nothing mechanical contradicted -// it. app.go's agentScope doc states the norm ("this must not be described as scoping 'cost'"); a -// norm in a comment is not a guard, and a mutant reverting this note to the pre-round-1 wording -// survived the whole package. -// -// ASSERTS THE DISCLOSURE, NOT THE ABSENCE OF A WORD. Banning "cost" would fire on a site that had -// become correctly qualified, which is the failure mode that turns a guard into a nuisance. The -// property worth pinning is that the note names the surface the scope does not reach — the spend -// band — because that is the one an operator is looking at when they press the key. +// 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 { @@ -719,8 +712,7 @@ func TestPaneKeys_TheAgentsNoteSaysWhatTheScopeDoesNotReach(t *testing.T) { if joined == "" { t.Fatal("the AGENTS group carries no notes; the disclosure has nowhere to live") } - if !strings.Contains(joined, "spend band") { - t.Errorf("the AGENTS note does not name the spend band, which the scope does NOT reach:\n %s", - joined) + if !strings.Contains(joined, "not the spend band") { + t.Errorf("the AGENTS note does not EXCLUDE the spend band:\n %s", joined) } } From 92bb95226833f935c67509697be7f12a8cd9f523 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 13:53:11 -0400 Subject: [PATCH 09/13] =?UTF-8?q?fix:=20Review=20round=204=20=E2=80=94=20d?= =?UTF-8?q?isclose=20the=20residual=20the=20scoped=20usage=20pane=20leaves?= =?UTF-8?q?=20out?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 4 (mrsabath, CHANGES_REQUESTED): one must-fix and one suggestion, both about usage.Snapshot's two whole-window residual fields. CLASS: a surface that renders a scoped money figure without saying what that figure leaves out. UngroupedCostMicros and UngroupedAvoidedMicros are whole-window residuals — the part of a total that no series entry carries — and they survive ScopeToAgent, so under a scope they describe traffic belonging to no agent inside a snapshot whose Totals describe one. Swept every consumer of ScopeToAgent's result and every money figure each one renders: two callers, cmd_cost.go's --agent path and the usage pane. One gap fixed, one deferred. - tui.costUngroupedRow states the residual beside the pane's COST cell, mirroring writeCostSummary's --agent note, and is called from BOTH branches that render the summary — the default one and the latency one. The pane is the surface that gained a scope in this PR and had nothing to say about this. - scope.go's field-by-field audit now accounts for both fields, and for why they are KEPT rather than dropped: abctl cost reads UngroupedCostMicros off the snapshot this function returns, so nilling them here would retract a disclosure already being made. - The "every" sentence no longer claims the two overshoot fields are the only ones it has to account for. The count is dropped rather than corrected, since accounting for these two makes it four. MEASURED, because the comment was going to claim the opposite: nilling UngroupedCostMicros in ScopeToAgent fails two cmd/abctl tests (TestRunCost_AgentDisclosesCostNoAgentCarries and TestRunCost_JSONScopedToAnAgentNarrowsProvenanceAndNamesTheAgent), while nilling UngroupedAvoidedMicros failed nothing at all. Every guard either field had lived a module away in the consumer, which is why the survival is now pinned in core by TestScopeToAgent_KeepsTheWindowResidualsForTheCallerToDisclose. The helper's name also makes an existing citation resolve: cmd_cost_test.go has referred to tui.costUngroupedRow as "the pane's form of it" while it existed nowhere in the tree. Pre-existing and outside this diff; the new helper is named to match rather than repaired separately. DEFERRED, not fixed here: abctl cost renders an avoided-cost figure and discloses no UngroupedAvoidedMicros beside it. That gap is pre-existing — this PR only moved scopeToAgent into core, leaving the --agent path's output untouched — and cmd_cost.go's own comment already records it as a gap rather than a consequence. The usage pane renders no avoided figure at all, so it has no counterpart gap. See the PR body. Signed-off-by: Hai Huang Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/tui/agents_scope_test.go | 97 ++++++++++++++++++++++++++++++ cmd/abctl/tui/usage_pane.go | 46 ++++++++++++++ core/cost/usage/scope.go | 27 ++++++++- core/cost/usage/scope_test.go | 37 ++++++++++++ 4 files changed, 205 insertions(+), 2 deletions(-) diff --git a/cmd/abctl/tui/agents_scope_test.go b/cmd/abctl/tui/agents_scope_test.go index 6a15a94bd..94c3c0fb2 100644 --- a/cmd/abctl/tui/agents_scope_test.go +++ b/cmd/abctl/tui/agents_scope_test.go @@ -254,6 +254,103 @@ func TestUsagePane_LatencyUnderAScopeSaysItIsUnavailable(t *testing.T) { } } +// 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) + } + }) + } +} + +// 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") diff --git a/cmd/abctl/tui/usage_pane.go b/cmd/abctl/tui/usage_pane.go index 9d9b48fc5..71695f3d5 100644 --- a/cmd/abctl/tui/usage_pane.go +++ b/cmd/abctl/tui/usage_pane.go @@ -369,6 +369,9 @@ func (m *model) renderUsage(width, height int) string { 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) @@ -377,6 +380,7 @@ func (m *model) renderUsage(width, height int) string { b.WriteString("\n") b.WriteString(renderUsageSummary(m.usage.snap)) b.WriteString("\n") + b.WriteString(costUngroupedRow(m.usage.snap, m.agentScope)) if !m.usage.lastFetch.IsZero() { b.WriteString(fmt.Sprintf("\n updated %s ago (every %s)\n", time.Since(m.usage.lastFetch).Truncate(time.Second), usagePollInterval)) @@ -389,3 +393,45 @@ 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, "+ + "so per-agent figures do not sum to the window total\n", + formatUSDTotalMicros(micros)) +} diff --git a/core/cost/usage/scope.go b/core/cost/usage/scope.go index 8db85d913..81de68aec 100644 --- a/core/cost/usage/scope.go +++ b/core/cost/usage/scope.go @@ -88,13 +88,36 @@ func ScopeToAgent(snap *Snapshot, agent string, buckets BucketScope) (*Snapshot, // 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 + // 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 diff --git a/core/cost/usage/scope_test.go b/core/cost/usage/scope_test.go index 40438885a..f325f8e69 100644 --- a/core/cost/usage/scope_test.go +++ b/core/cost/usage/scope_test.go @@ -101,6 +101,43 @@ func TestScopeToAgent_DropsTheProvenanceMapsAndKeepsTheReadFacts(t *testing.T) { } } +// 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. // From cd4bbc533f5cefb4ccc158ae6146ccb6c376bc34 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 14:42:26 -0400 Subject: [PATCH 10/13] =?UTF-8?q?fix:=20Review=20round=205=20=E2=80=94=20m?= =?UTF-8?q?ake=20the=20scoped=20note=20cost=20the=20usage=20pane=20no=20ro?= =?UTF-8?q?w?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Class GEOMETRY-INVARIANT. Round 4 added the scoped residual note as a new row after the summary. usagePaneChromeRows does not count it, and the chrome test renders unscoped, so it could not see it: a scoped pane with a residual came out one row past the terminal at 80x26 and 120x26. Fixed by subtraction rather than by budget arithmetic: - The note now takes the blank row above the summary instead of adding a row, so the scoped pane spends exactly what the unscoped one does. Subtracting the note from the chart's budget was tried first and changed nothing at these sizes: the chart grows in steps (13 rows, or 14 with its caption), and at these sizes a body of bodyHeight rows already overflows the terminal by one. - The pane's note drops its trailing clause ("so per-agent figures do not sum to the window total"). At 107 columns it wrapped at 80 and stole a second row. abctl cost keeps the full sentence; stdout has no width budget. Swept: every row renderUsage writes conditionally, in both branches that render the summary. 2 sites, fixed 1. The latency branch appends the note too, and measures 16 rows of an 80x24 terminal, so it has no budget to break. New guard: TestUsagePane_TheScopedNoteFitsWhereTheUnscopedPaneDoes (assertFits on a scoped pane with a residual at 80x26 and 120x26). | Mutation | Verdict | |---|---| | the note adds a row again (round 4's shape) | killed | | the deleted clause restored, wrapping at 80 | killed | | the note never rendered (fixture-reach check) | killed, on its Fatalf | Two round-4 mutants lost their targets to this move and are retargeted in the harness; the full prior suite shows no new survivor. Signed-off-by: Hai Huang Assisted-By: Claude (Anthropic AI) --- cmd/abctl/tui/agents_scope_test.go | 19 +++++++++++++++++++ cmd/abctl/tui/usage_pane.go | 12 ++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/cmd/abctl/tui/agents_scope_test.go b/cmd/abctl/tui/agents_scope_test.go index 94c3c0fb2..cd9f577a5 100644 --- a/cmd/abctl/tui/agents_scope_test.go +++ b/cmd/abctl/tui/agents_scope_test.go @@ -299,6 +299,25 @@ func TestUsagePane_ScopedSummaryDisclosesTheCostNoAgentCarries(t *testing.T) { } } +// 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. // diff --git a/cmd/abctl/tui/usage_pane.go b/cmd/abctl/tui/usage_pane.go index 71695f3d5..b2eb51c7b 100644 --- a/cmd/abctl/tui/usage_pane.go +++ b/cmd/abctl/tui/usage_pane.go @@ -377,10 +377,15 @@ func (m *model) renderUsage(width, height int) string { 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") - b.WriteString(costUngroupedRow(m.usage.snap, m.agentScope)) if !m.usage.lastFetch.IsZero() { b.WriteString(fmt.Sprintf("\n updated %s ago (every %s)\n", time.Since(m.usage.lastFetch).Truncate(time.Second), usagePollInterval)) @@ -431,7 +436,6 @@ func costUngroupedRow(snap *usage.Snapshot, scope string) string { if micros == 0 || negativeCost(micros) { return "" } - return fmt.Sprintf(" note %s of this window is attributed to no agent, "+ - "so per-agent figures do not sum to the window total\n", + return fmt.Sprintf(" note %s of this window is attributed to no agent\n", formatUSDTotalMicros(micros)) } From 5d9ed4d976a75971289a8f39f21ae2cda69a120f Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 17:09:41 -0400 Subject: [PATCH 11/13] =?UTF-8?q?fix:=20Review=20round=207=20=E2=80=94=20p?= =?UTF-8?q?in=20the=20sort,=20drop=20the=20scoped=20header's=20breakdown,?= =?UTF-8?q?=20gate=20on=20Sessions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 7 was the whole-diff confirmation pass. Its two must-fix were round-1 misses rather than repairs of repairs: both lines blame to the feature commit or to code older than the PR. UNFAILABLE-ASSERTION (probabilistic). The only guard on the sorted "seen:" list made one ScopeToAgent call over a two-label map, so with sort.Strings(known) deleted the map walk still came out sorted in 21 of 200 runs. The test now repeats the call 64 times. Swept: every sort and every range over a map that the PR adds in production code; both are this site. SELF-CONSISTENCY. Under an agent scope the usage header still printed "by " from the persisted setting while the chart was ungrouped, breaking the rule the comment above it states. The breakdown arm now requires no scope, so the header falls through to "ungrouped", a string TestUsageScopeMax_CoversTheWidestHeader already lists. Swept: every read of m.usage.group outside tests; the header's was the only one wrong under a scope. GUARD-REACHABILITY. The startup gate entered AGENTS whatever pane its reply found, although its comment says it interrupts the sessions view: an operator who had moved inside the fetch's latency was pulled into the picker, and esc then landed on Sessions. The gate now also requires m.pane == paneSessions. Every initSessionView caller puts the model on Sessions before the fetch goes out, so the condition only declines for an operator who has left. FALSE-REASON, fixed by deletion. Swept every "the one a reader cannot " reason the PR adds, reading wrapped lines rather than grepping them, and deleted the false ones: - usage_pane.go, agents_scope_test.go: "showing every agent under a scoped title is the one outcome a reader cannot detect" - keys.go, agents_scope_test.go: "the one surface the choice does not affect" (Sessions, where a startup pick lands, is another) - app.go: "the one direction a reader cannot check", which claims the opposite direction to scope.go's, and "not only in the footer": the usage footer does not show the agent scope The first two are deleted from the PR body as well. The library's empty-window message drops "--agent", a CLI flag the TUI was printing to operators who never typed it. Left open as design calls, listed in the PR body's deferred section: an idle scoped agent gets an error screen rather than an empty chart, and a startup pick lands on Sessions rather than Usage. | New assertion | Mutation | Verdict | |---|---|---| | the repeated unknown-agent call | sort.Strings deleted, -count=200 | killed 200/200 | | TestUsageHeader_ClaimsNoBreakdownWhileScoped | breakdown arm ignores the scope | killed | | same test, fixture reach | breakdown arm never fires | killed, on its Fatalf | | TestAgentsPane_StartupLeavesAnOperatorWhoHasMoved | gate ignores the pane | killed | | the existing startup tests (inverse) | gate fires only from the usage pane | killed | | TestScopeToAgent_EmptyWindowSaysSoRatherThanListingNothing | "--agent" restored | killed | G1_err_conjunct, tui_startup_gate_always and tui_startup_gate_errignored lost their target line to the gate edit; each has a retargeted twin, killed. Otherwise the full prior suite matches round 7's run, except scope_sort, which now dies. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/tui/agents_pane_test.go | 19 +++++++++++++++++++ cmd/abctl/tui/agents_scope_test.go | 26 +++++++++++++++++++------- cmd/abctl/tui/app.go | 17 +++++++++-------- cmd/abctl/tui/keys.go | 4 +--- cmd/abctl/tui/usage_pane.go | 9 +++++---- core/cost/usage/scope.go | 2 +- core/cost/usage/scope_test.go | 18 +++++++++++------- 7 files changed, 65 insertions(+), 30 deletions(-) diff --git a/cmd/abctl/tui/agents_pane_test.go b/cmd/abctl/tui/agents_pane_test.go index ece35a4be..da6505f12 100644 --- a/cmd/abctl/tui/agents_pane_test.go +++ b/cmd/abctl/tui/agents_pane_test.go @@ -605,6 +605,25 @@ func TestAgentsPane_StartupEscapesToSessions(t *testing.T) { } } +// 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}}, + } + for _, start := range []paneID{paneUsage, paneEvents, paneAgents} { + 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) + } + } +} + // 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 diff --git a/cmd/abctl/tui/agents_scope_test.go b/cmd/abctl/tui/agents_scope_test.go index cd9f577a5..32143f098 100644 --- a/cmd/abctl/tui/agents_scope_test.go +++ b/cmd/abctl/tui/agents_scope_test.go @@ -37,9 +37,7 @@ func scopedModel(t *testing.T, cursor int, scope string) *model { // 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: its whole -// purpose is choosing what the views behind it show, so staying on it after a choice would leave -// the operator looking at the one surface the choice does not affect. +// 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}) @@ -172,10 +170,9 @@ func TestFetchUsage_UnscopedAsksForThePanesOwnAxis(t *testing.T) { // 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. Showing the -// unscoped figures under a scoped title would be the one outcome a reader cannot detect; the -// error names the agents that ARE in the window, which is what the operator needs in order to -// pick a different one. +// 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)) @@ -220,6 +217,21 @@ func TestUsageFooter_OmitsTheBreakdownKeyWhileScoped(t *testing.T) { } } +// 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) diff --git a/cmd/abctl/tui/app.go b/cmd/abctl/tui/app.go index 8e2d8028d..1ab0faecb 100644 --- a/cmd/abctl/tui/app.go +++ b/cmd/abctl/tui/app.go @@ -1357,12 +1357,14 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // 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. - if msg.err == nil && agentsPaneApplies(m.agents) { + // + // 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 before the operator - // pressed anything — 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 + // 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 @@ -2228,9 +2230,8 @@ 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, not only in the footer, because it changes what - // every figure on the pane means. A narrowed chart that looked like the whole window - // would be wrong in the one direction a reader cannot check. + // 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) } diff --git a/cmd/abctl/tui/keys.go b/cmd/abctl/tui/keys.go index a547a702c..ce7a5e223 100644 --- a/cmd/abctl/tui/keys.go +++ b/cmd/abctl/tui/keys.go @@ -588,9 +588,7 @@ 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, so staying on it after a choice would leave the operator looking at the one - // surface the choice does not affect. + // 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 diff --git a/cmd/abctl/tui/usage_pane.go b/cmd/abctl/tui/usage_pane.go index b2eb51c7b..520c4548a 100644 --- a/cmd/abctl/tui/usage_pane.go +++ b/cmd/abctl/tui/usage_pane.go @@ -166,9 +166,7 @@ func (m *model) fetchUsage() tea.Cmd { // 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, and showing every agent under a scoped - // title is the one outcome a reader cannot detect. The error names the agents that - // are in the window. + // window moves past an agent's last request. snap, err = usage.ScopeToAgent(snap, scope, usage.NarrowBuckets) } return usageLoadedMsg{snap: snap, req: req, err: err} @@ -330,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", diff --git a/core/cost/usage/scope.go b/core/cost/usage/scope.go index 81de68aec..07f35c97b 100644 --- a/core/cost/usage/scope.go +++ b/core/cost/usage/scope.go @@ -57,7 +57,7 @@ func ScopeToAgent(snap *Snapshot, agent string, buckets BucketScope) (*Snapshot, // 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", + 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", diff --git a/core/cost/usage/scope_test.go b/core/cost/usage/scope_test.go index f325f8e69..4bf0ad6cf 100644 --- a/core/cost/usage/scope_test.go +++ b/core/cost/usage/scope_test.go @@ -234,13 +234,17 @@ func TestScopeToAgent_NarrowBucketsZeroesLatencyItCannotAttribute(t *testing.T) // 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) { - _, err := ScopeToAgent(scopeFixture(), "bob", KeepBuckets) - if err == nil { - t.Fatal("ScopeToAgent accepted an agent that is not in the window") - } want := "no agent \"bob\" in the today window; seen: bob-shell/2.0.5, claude-code/2.1.270" - if err.Error() != want { - t.Errorf("error = %q, want %q", err, want) + // 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) + } } } @@ -251,7 +255,7 @@ func TestScopeToAgent_EmptyWindowSaysSoRatherThanListingNothing(t *testing.T) { if err == nil { t.Fatal("ScopeToAgent accepted an agent against an empty window") } - want := "no agent traffic in the today window, so --agent \"bob\" matches nothing" + want := "no agent traffic in the today window, so \"bob\" matches nothing" if err.Error() != want { t.Errorf("error = %q, want %q", err, want) } From ad24b46d690c407cb3e579aa8f76ec185effc7ff Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 17:47:06 -0400 Subject: [PATCH 12/13] =?UTF-8?q?test:=20Review=20round=208=20=E2=80=94=20?= =?UTF-8?q?make=20the=20gate=20test=20cover=20every=20pane,=20not=20three?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Class UNFAILABLE-ASSERTION, the "counts known names" variant, in round 7's own test. TestAgentsPane_StartupLeavesAnOperatorWhoHasMoved says the gate interrupts Sessions and nothing else, and checked three panes it named. Detail was not among them: a gate widened to admit Detail, or one excluding exactly the three tested panes, passed the whole tui package. The loop now walks every paneID from 0 to lastPaneID except Sessions, the form spend_drawer_test.go already uses, and asserts how many panes it visited so the skip cannot hide an empty loop. Swept: the tests round 7 added; this is the only one that iterates a set of panes. Also re-wraps the gate comment round 7 left at 116 columns. Its words are unchanged. | Mutation | Verdict | |---|---| | gate also admits Detail (R8a, survived at f1aca5e7) | killed | | gate excludes only the three panes the old test named (R8b, survived) | killed | | the test's loop visits nothing (R8c) | killed, on the visit count | The full prior suite matches round 8's run. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/tui/agents_pane_test.go | 10 +++++++++- cmd/abctl/tui/app.go | 6 +++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/cmd/abctl/tui/agents_pane_test.go b/cmd/abctl/tui/agents_pane_test.go index da6505f12..c5ee76ead 100644 --- a/cmd/abctl/tui/agents_pane_test.go +++ b/cmd/abctl/tui/agents_pane_test.go @@ -613,7 +613,12 @@ func TestAgentsPane_StartupLeavesAnOperatorWhoHasMoved(t *testing.T) { {label: "claude-code/2.1.270", Counts: usage.Counts{Requests: 10}}, {label: "bob-shell/2.0.5", Counts: usage.Counts{Requests: 8}}, } - for _, start := range []paneID{paneUsage, paneEvents, paneAgents} { + 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) @@ -622,6 +627,9 @@ func TestAgentsPane_StartupLeavesAnOperatorWhoHasMoved(t *testing.T) { 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. diff --git a/cmd/abctl/tui/app.go b/cmd/abctl/tui/app.go index 1ab0faecb..bf029ef73 100644 --- a/cmd/abctl/tui/app.go +++ b/cmd/abctl/tui/app.go @@ -1364,9 +1364,9 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // 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. + // 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() From 1f7dc022330310d80ebfaf1003484a1c67cb204b Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 29 Sep 2026 19:19:56 -0400 Subject: [PATCH 13/13] docs: Point main's Currencies references at usage.ScopeToAgent Rebasing onto main brought three comments from the billing-unit work that cite scopeToAgent for why Currencies is carried over under an --agent scope. This branch moved that function into core as usage.ScopeToAgent, and the rebase carried main's explanation there with it, so the references follow. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/abctl/cmd_cost.go | 4 ++-- cmd/abctl/cmd_cost_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/abctl/cmd_cost.go b/cmd/abctl/cmd_cost.go index ce318e093..c0c36a12c 100644 --- a/cmd/abctl/cmd_cost.go +++ b/cmd/abctl/cmd_cost.go @@ -295,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 @@ -637,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.