From d914db0dd21bb9d88bc327342c3a7dfd724f460d Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 29 Jul 2026 17:16:31 -0400 Subject: [PATCH] fix(agent-usage): clear Thufir pass-2 findings (aggregation, partial bar copy, e2e isolation) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three remaining pass-2 findings fixed: 1. sumKnownBucketTotals aggregation precedence (Thufir's explicit ruling). One unknown report-bearing bucket no longer erases the known subtotal. Track anyWithValue separately from sawAny; unknown buckets set partial=true but do not touch sumValue. Return unknown/null ONLY when no report-bearing bucket contributed any numeric value. Added three new tests: exact+unknown (preserves exact subtotal, partial=true), approximate+unknown (preserves approx subtotal, partial=true), all-unknown (returns unknown/null). 2. approx-partial daily bar trailing copy. trailingText for approx-partial now renders '≈N*' (asterisk suffix) instead of '≈N', giving sighted users a distinct partial signal beyond opacity alone. T2 e2e test extended with an incomplete-i/o bucket and an assertion on '≈1K*' to verify the distinguishable copy. 3. e2e guard surface isolation. Header and focused-total assertions in the T1b regression guard now target dedicated testids (agent-usage-header-value and agent-usage-focused-total-value) so each surface fails independently rather than passing via a descendant bar's ≈ text. Added testId prop to UsageStat (optional, threading to the value

only), wired from ApproxTokenStat. Added data-testid='agent-usage-header-value' to the header value span in AgentUsageSection. MINOR: fixed stale caveat-gate comment at agent-usage.spec.ts that still attributed the sentence to hasUnknownUsage=true instead of direct i/o incompleteness. Claim reconciliation: the stated 21/21 Playwright count was wrong — the file defines 17 tests and I ran 17/17. The prior run was against this file at this head. Gates: 3571 unit tests pass, tsc --noEmit clean, 17/17 agent-usage Playwright specs pass under the smoke project. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../agent-usage/lib/agentUsage.test.mjs | 39 ++++++++++-- .../features/agent-usage/lib/agentUsage.ts | 32 +++++----- .../agent-usage/ui/AgentUsageDailyBars.tsx | 8 ++- .../agent-usage/ui/AgentUsageFocusedView.tsx | 16 ++++- .../agent-usage/ui/AgentUsageSection.tsx | 5 +- desktop/tests/e2e/agent-usage.spec.ts | 61 ++++++++++++++++--- 6 files changed, 126 insertions(+), 35 deletions(-) diff --git a/desktop/src/features/agent-usage/lib/agentUsage.test.mjs b/desktop/src/features/agent-usage/lib/agentUsage.test.mjs index 33e953b69..83f619c96 100644 --- a/desktop/src/features/agent-usage/lib/agentUsage.test.mjs +++ b/desktop/src/features/agent-usage/lib/agentUsage.test.mjs @@ -630,17 +630,20 @@ test("sumKnownBucketTotals marks partial true when any bucket has an incomplete assert.equal(result.partial, true); }); -test("sumKnownBucketTotals returns unknown when any report-bearing bucket has no display value", () => { +test("sumKnownBucketTotals preserves known exact subtotal when a sibling bucket is unknown (partial=true)", () => { + // Thufir's explicit ruling: one unknown report-bearing bucket must NOT erase the known subtotal. + // The result surfaces the labeled lower bound rather than hiding measured data. const result = sumKnownBucketTotals([ bucket({ usage: reportedUsage({ totalTokens: usageField({ value: "100" }) }), reportCount: 1, }), - // report-bearing bucket with no total and no i/o — display is unknown + // report-bearing bucket with no total and no i/o — display is unknown, but does NOT erase sum bucket({ usage: reportedUsage(), reportCount: 1, hasUnknownUsage: true }), ]); - assert.equal(result.kind, "unknown"); - assert.equal(result.value, null); + assert.equal(result.kind, "exact"); + assert.equal(result.value, 100n); + assert.equal(result.partial, true); // unknown sibling sets partial }); test("sumKnownBucketTotals returns approximate from i/o sum when all bucket totals are null but i/o is known", () => { @@ -718,6 +721,34 @@ test("sumKnownBucketTotals returns approximate when mixed exact and approximate assert.equal(result.partial, false); }); +test("sumKnownBucketTotals preserves known approximate subtotal when a sibling bucket is unknown (partial=true)", () => { + // Parallel to the exact+unknown case: approximate data from one bucket must not be erased. + const result = sumKnownBucketTotals([ + bucket({ + usage: reportedUsage({ + inputTokens: usageField({ value: "400" }), + outputTokens: usageField({ value: "100" }), + }), + reportCount: 1, + }), + // report-bearing bucket with no display value — sets partial, does NOT erase sum + bucket({ usage: reportedUsage(), reportCount: 1, hasUnknownUsage: true }), + ]); + assert.equal(result.kind, "approximate"); + assert.equal(result.value, 500n); + assert.equal(result.partial, true); // unknown sibling sets partial +}); + +test("sumKnownBucketTotals returns unknown when all report-bearing buckets have no display value", () => { + const result = sumKnownBucketTotals([ + bucket({ usage: reportedUsage(), reportCount: 1, hasUnknownUsage: true }), + bucket({ usage: reportedUsage(), reportCount: 1, hasUnknownUsage: true }), + ]); + assert.equal(result.kind, "unknown"); + assert.equal(result.value, null); + assert.equal(result.partial, false); +}); + // ── deriveUsageIngressTrailing ──────────────────────────────────────────────── function baseSeries(overrides = {}) { diff --git a/desktop/src/features/agent-usage/lib/agentUsage.ts b/desktop/src/features/agent-usage/lib/agentUsage.ts index 0ff9bbc5d..b21322fcb 100644 --- a/desktop/src/features/agent-usage/lib/agentUsage.ts +++ b/desktop/src/features/agent-usage/lib/agentUsage.ts @@ -371,18 +371,17 @@ export function deriveUsageIngressTrailing(series: AgentUsageSeries): string { * provenance-bearing `DisplayTotal` for the overview/focused-view header. * * Aggregation rules: - * - `exact`: every report-bearing bucket has an exact display total. - * - `approximate`: at least one bucket is approximate (sum of all - * exact+approximate display values); a report-bearing bucket that is - * approximate or has no total does NOT force unknown if i/o is available. - * - `unknown`: any report-bearing bucket has no display value at all (neither - * exact nor approximate i/o available). + * - `exact`: every report-bearing bucket contributed an exact display total. + * - `approximate`: at least one bucket contributed an approximate value; + * `partial` is the union of contributing buckets' `DisplayTotal.partial`. + * Unknown-bucket peers set `partial = true` but do NOT erase the known sum — + * the result surfaces a labeled lower bound rather than hiding measured data. + * - `unknown`: NO report-bearing bucket has any display value at all. * - Empty window (no report-bearing buckets): `{ kind: "unknown", value: null, partial: false }`. * - * `partial` reflects the i/o and total completeness of the contributing - * buckets (union of each bucket's `DisplayTotal.partial`) — it does NOT fire - * from total absence alone. An approximate aggregate with complete i/o carries - * `partial: false` even though no genuine provider total was emitted. + * `partial` reflects i/o and total completeness of contributing buckets. + * An approximate aggregate with complete i/o and no exact totals carries + * `partial: false` — total absence alone does NOT trigger partial. * * The returned value is a *display* value only — never stored or wired. */ @@ -391,8 +390,8 @@ export function sumKnownBucketTotals( ): DisplayTotal { let sumValue = 0n; let sawAny = false; // any report-bearing bucket processed - let anyApprox = false; // at least one approximate bucket - let anyUnknown = false; // at least one report-bearing bucket with no display value + let anyApprox = false; // at least one approximate bucket contributed a value + let anyWithValue = false; // at least one bucket contributed a numeric value let partial = false; for (const bucket of buckets) { @@ -401,11 +400,13 @@ export function sumKnownBucketTotals( const dt = deriveDisplayTotal(bucket.usage); if (dt.kind === "exact" || dt.kind === "approximate") { sumValue += dt.value; + anyWithValue = true; if (dt.partial) partial = true; if (dt.kind === "approximate") anyApprox = true; } else { - // Report-bearing bucket with no display value → aggregate is unknown. - anyUnknown = true; + // Report-bearing bucket with no display value — sets partial but does NOT + // erase the sum already accumulated from sibling buckets. + partial = true; } } @@ -413,7 +414,8 @@ export function sumKnownBucketTotals( // Truly empty window — no report-bearing buckets at all. return { kind: "unknown", value: null, partial: false }; } - if (anyUnknown) { + if (!anyWithValue) { + // Report-bearing buckets exist but none had any display value. return { kind: "unknown", value: null, partial: false }; } if (anyApprox) { diff --git a/desktop/src/features/agent-usage/ui/AgentUsageDailyBars.tsx b/desktop/src/features/agent-usage/ui/AgentUsageDailyBars.tsx index 7e52ef67f..ff1bdd0c9 100644 --- a/desktop/src/features/agent-usage/ui/AgentUsageDailyBars.tsx +++ b/desktop/src/features/agent-usage/ui/AgentUsageDailyBars.tsx @@ -156,9 +156,11 @@ function DailyBar({ ? "—" : kind === "partial" ? `≥${formatTokenCountCompact(knownTokens ?? 0n)}` - : kind === "approx" || kind === "approx-partial" - ? `≈${formatTokenCountCompact(knownTokens ?? 0n)}` - : formatTokenCountCompact(knownTokens ?? 0n); + : kind === "approx-partial" + ? `≈${formatTokenCountCompact(knownTokens ?? 0n)}*` + : kind === "approx" + ? `≈${formatTokenCountCompact(knownTokens ?? 0n)}` + : formatTokenCountCompact(knownTokens ?? 0n); return (

+ ); } @@ -369,15 +374,22 @@ function UsageStat({ display, isPartial, label, + testId, }: { display: string | null; isPartial: boolean; label: string; + testId?: string; }) { return (

{label}

-

{display ?? "—"}

+

+ {display ?? "—"} +

{isPartial ? Partial : null}
); diff --git a/desktop/src/features/agent-usage/ui/AgentUsageSection.tsx b/desktop/src/features/agent-usage/ui/AgentUsageSection.tsx index 077b83796..61882febb 100644 --- a/desktop/src/features/agent-usage/ui/AgentUsageSection.tsx +++ b/desktop/src/features/agent-usage/ui/AgentUsageSection.tsx @@ -174,7 +174,10 @@ function AgentUsageCard({

Daily usage

- + {overallTotal.kind === "exact" ? `${formatTokenCountCompact(overallTotal.value)} tokens` : overallTotal.kind === "approximate" diff --git a/desktop/tests/e2e/agent-usage.spec.ts b/desktop/tests/e2e/agent-usage.spec.ts index de68501d2..45a3e1896 100644 --- a/desktop/tests/e2e/agent-usage.spec.ts +++ b/desktop/tests/e2e/agent-usage.spec.ts @@ -826,9 +826,9 @@ test("focused view shows daily bars, coverage dates, and a partial explanation w await expect(coverage).toBeVisible(); await expect(coverage).toContainText("reported turn"); - // The partial explanation must appear when usage is known-incomplete. - // The seed has hasUnknownUsage=true AND invalidReportCount=1, so both - // per-condition caveat sentences must appear independently. + // Both caveat sentences must appear when the gate conditions are met. + // The seed has inputTokens.incomplete=true (fires unknown-intervals sentence) + // AND invalidReportCount=1 (fires invalid-reports sentence). await expect( page.getByTestId("agent-usage-focused-unknown-intervals-caveat"), ).toBeVisible(); @@ -994,9 +994,12 @@ test("overview row, header, daily bar, and focused total all render ≈ when tot await expect(row).not.toContainText("No usage reported"); // Section header must render ≈ total (sumKnownBucketTotals → approximate). - const overallBars = page.getByTestId("agent-usage-overall-bars"); - await expect(overallBars).toBeVisible(); - await expect(overallBars).toContainText("≈"); + // Assert the dedicated value node directly so the header fails independently + // even if a child daily bar still shows ≈. + const headerValue = page.getByTestId("agent-usage-header-value"); + await expect(headerValue).toBeVisible(); + await expect(headerValue).toContainText("≈"); + await expect(headerValue).not.toContainText("No usage reported"); // Daily bar must render with a bar (not a hatched unknown baseline) and // show ≈ trailing label. @@ -1005,13 +1008,15 @@ test("overview row, header, daily bar, and focused total all render ≈ when tot await expect(dailyBars).toContainText("≈"); // Focused total must render ≈ approximate stat. + // Assert the dedicated value node directly so the stat fails independently + // even if a focused daily bar still shows ≈. await row.click(); await expect(page.getByTestId("user-profile-panel")).toBeVisible(); await expect(page.getByTestId("agent-usage-focused-view")).toBeVisible(); - const focusedTotals = page.getByTestId("agent-usage-focused-totals"); - await expect(focusedTotals).toBeVisible(); - await expect(focusedTotals).toContainText("≈"); - await expect(focusedTotals).not.toContainText("No usage reported"); + const focusedTotalValue = page.getByTestId("agent-usage-focused-total-value"); + await expect(focusedTotalValue).toBeVisible(); + await expect(focusedTotalValue).toContainText("≈"); + await expect(focusedTotalValue).not.toHaveText("—"); }); // ── T2: I/O-incomplete partial behavioral coverage ──────────────────────────── @@ -1037,6 +1042,21 @@ test("overview row shows Partial badge and ingress shows partial marker when I/O series: mockUsageSeries({ agents: [ mockAgentUsage(agentPubkey, { + buckets: [ + { + start: 1_700_000_000, + end: 1_700_086_400, + hasUnknownUsage: false, + reportCount: 1, + usage: { + // null total, incomplete input — approx-partial bar kind + estimatedCostUsd: costField(null), + inputTokens: usageField("800", true), // incomplete + outputTokens: usageField("200", false), + totalTokens: usageField(null), + }, + }, + ], usage: { // null total, incomplete I/O — per-field Partial must surface estimatedCostUsd: costField(null), @@ -1046,6 +1066,21 @@ test("overview row shows Partial badge and ingress shows partial marker when I/O }, }), ], + buckets: [ + { + start: 1_700_000_000, + end: 1_700_086_400, + hasUnknownUsage: false, + reportCount: 1, + usage: { + // null total, incomplete input — approx-partial aggregate + estimatedCostUsd: costField(null), + inputTokens: usageField("800", true), // incomplete + outputTokens: usageField("200", false), + totalTokens: usageField(null), + }, + }, + ], }), }, ); @@ -1059,6 +1094,12 @@ test("overview row shows Partial badge and ingress shows partial marker when I/O // Row must show the ≈ approximate total (both i/o present, null total → approx display). await expect(row).toContainText("≈ 1K"); + // Daily bar for an approx-partial bucket must use ≈N* trailing text + // (distinct from plain ≈N so a sighted user sees the partial signal). + const dailyBars = page.getByTestId("agent-usage-daily-bars"); + await expect(dailyBars).toBeVisible(); + await expect(dailyBars).toContainText("≈1K*"); + // Open the profile panel to reach the Info tab for ingress verification. await row.click(); await expect(page.getByTestId("user-profile-panel")).toBeVisible();