Skip to content

Commit df1eef6

Browse files
committed
fix(desktop,storage): keep the usage rings honest about what they sum
Second review round, all under the ring math rather than its shape. The token split clamped cacheRead to input, but the ledger keeps the provider's cached share unclamped when no prompt total was reported, so `cacheRead > input` is a normal aggregate — 200k cached tokens with zero prompt became a zero segment. The split now reads cacheRead as reported; `uncachedInput` already floors the other way, and the total remains the sum of the drawn segments. The time split decided presence through `totalDurationMs ?? 0`, which made a host that measured zero model time drop its row and its call count while the tool row survived on count. `totalDurationMs` is now a required summary key — the epoch bump already refuses peers that predate it, so the optional decoder path and its test were unreachable — and the split decides presence on the reported fields before filtering. The ring label says "Recorded Time": the figures are sums of per-call durations, not wall clock, and parallel or nested calls overlap. `toolUsage` stays optional for a data reason instead of a version one: tool rows predate connection attribution, so a `connectionSlug`-scoped query cannot answer the tool ledger honestly. The Host omits the split for that query rather than persist a slug that would only cover new rows and let history silently vanish under the filter. Every tool read now shares one row filter, so tool buckets honour the same Session, provider, and model filters the summary applies — they previously ignored Session identity. And the ts range moves into the invocation query: the ledger has no retention, and the unbounded decode ran on every summary refresh; the `(ts DESC, id)` index answers it. Generated-by: ZCode
1 parent 62a1acb commit df1eef6

15 files changed

Lines changed: 245 additions & 68 deletions

apps/desktop/src/main/__tests__/runtime-host-usage-ipc-main.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ test("settings usage stats use the canonical model-call total and load every act
5656
cacheHitRequests: 10,
5757
cacheCreateRequests: 5,
5858
errorRequests: 2,
59+
totalDurationMs: 0,
5960
},
6061
provenance: provenance(),
6162
} satisfies UsageQueryResult;
@@ -176,6 +177,7 @@ test("settings usage stats reject a non-advancing activity page", async () => {
176177
cacheHitRequests: 0,
177178
cacheCreateRequests: 0,
178179
errorRequests: 0,
180+
totalDurationMs: 0,
179181
},
180182
provenance: provenance(),
181183
} satisfies UsageQueryResult;
@@ -243,6 +245,7 @@ test("settings usage stats degrade instead of erroring when logs disagree with t
243245
cacheHitRequests: 0,
244246
cacheCreateRequests: 0,
245247
errorRequests: 0,
248+
totalDurationMs: 0,
246249
},
247250
provenance: provenance(),
248251
} satisfies UsageQueryResult;
@@ -316,6 +319,7 @@ test("settings usage stats group the provider breakdown by connection", async ()
316319
cacheHitRequests: 0,
317320
cacheCreateRequests: 0,
318321
errorRequests: 0,
322+
totalDurationMs: 0,
319323
},
320324
provenance: provenance(),
321325
} satisfies UsageQueryResult;
@@ -394,6 +398,7 @@ test("settings usage stats truncate the activity log at the cap instead of error
394398
cacheHitRequests: 0,
395399
cacheCreateRequests: 0,
396400
errorRequests: 0,
401+
totalDurationMs: 0,
397402
},
398403
provenance: provenance(),
399404
} satisfies UsageQueryResult;
@@ -467,6 +472,7 @@ test("settings usage stats name each row from the Host-resolved session title",
467472
cacheHitRequests: 0,
468473
cacheCreateRequests: 0,
469474
errorRequests: 0,
475+
totalDurationMs: 0,
470476
},
471477
provenance: provenance(),
472478
} satisfies UsageQueryResult;

apps/desktop/src/main/__tests__/session-inspector-panel-model.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ test('does not render legacy zero cost as a known free Session', () => {
5050
cacheHitRequests: 0,
5151
cacheCreateRequests: 0,
5252
errorRequests: 0,
53+
totalDurationMs: 0,
5354
provenance: {
5455
coverage: {
5556
attempts: 0,
@@ -85,6 +86,7 @@ test('reports incomplete provenance as unavailable regardless of recorded reques
8586
cacheHitRequests: 0,
8687
cacheCreateRequests: 0,
8788
errorRequests: 0,
89+
totalDurationMs: 0,
8890
provenance: {
8991
coverage: {
9092
attempts: 0,
@@ -121,6 +123,7 @@ test('does not estimate a cache-hit ratio from partial usage', () => {
121123
cacheHitRequests: 1,
122124
cacheCreateRequests: 0,
123125
errorRequests: 0,
126+
totalDurationMs: 0,
124127
provenance: {
125128
coverage: {
126129
attempts: 1,

apps/desktop/src/main/__tests__/session-inspector-usage-stats.test.ts

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ function usageSummary(overrides: Partial<UsageSummaryV2> = {}): UsageSummaryV2 {
4444
cacheHitRequests: 2,
4545
cacheCreateRequests: 0,
4646
errorRequests: 0,
47+
totalDurationMs: 0,
4748
...overrides,
4849
};
4950
}
@@ -183,15 +184,61 @@ test('splits recorded time between model calls and tool executions', () => {
183184
assert.equal(durationUsage?.totalDurationMs, 1_873_000 + 78_000);
184185
});
185186

186-
test('a host from before duration reporting shows no time split rather than zeros', () => {
187-
const { durationUsage } = deriveInspectorOverviewModel(undefined, usageSummary());
187+
test('keeps a cache-read share the provider reported without a prompt total', () => {
188+
// Core leaves the ledger's cacheRead unclamped when no prompt total was
189+
// reported, so cacheRead > input is a normal aggregate there — clamping it
190+
// to input would erase exactly the sessions the split exists to describe.
191+
const { tokenUsage } = deriveInspectorOverviewModel(
192+
undefined,
193+
usageSummary({
194+
totalTokens: {
195+
input: 0,
196+
output: 100,
197+
cacheMiss: 60_000,
198+
cacheRead: 200_000,
199+
cacheWrite: 0,
200+
reasoning: 0,
201+
total: 260_100,
202+
},
203+
}),
204+
);
205+
206+
assert.deepEqual(
207+
tokenUsage?.segments.map((segment) => [segment.kind, segment.tokens]),
208+
[
209+
['cacheRead', 200_000],
210+
['cacheMiss', 60_000],
211+
['output', 100],
212+
],
213+
);
214+
assert.equal(tokenUsage?.total, 260_100);
215+
});
216+
217+
test('a host that measured zero model time keeps its row and its call count', () => {
218+
// Presence follows the reported field, not a zero-derived default: zero is
219+
// a measurement, and the row carries the count the model totals show too.
220+
const { durationUsage } = deriveInspectorOverviewModel(
221+
undefined,
222+
usageSummary({ totalDurationMs: 0 }),
223+
);
224+
assert.deepEqual(
225+
durationUsage?.segments.map((segment) => [segment.kind, segment.count, segment.durationMs]),
226+
[['model', 3, 0]],
227+
);
228+
});
229+
230+
test('a row with neither a clock nor a count is dropped', () => {
231+
const { durationUsage } = deriveInspectorOverviewModel(
232+
undefined,
233+
usageSummary({ totalRequests: 0, totalDurationMs: 0 }),
234+
);
188235
assert.equal(durationUsage, undefined);
189236
});
190237

191238
test('a tool row without a recorded duration still reports its count', () => {
192239
const { durationUsage } = deriveInspectorOverviewModel(
193240
undefined,
194-
usageSummary({ totalRequests: 2, toolUsage: { requests: 4, durationMs: 0 } }),
241+
usageSummary({ totalRequests: 0, totalDurationMs: 0, toolUsage: { requests: 4, durationMs: 0 } }),
195242
);
196243

197244
assert.deepEqual(

apps/desktop/src/main/__tests__/use-session-trace.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ function usageSummary(
7979
cacheHitRequests: 0,
8080
cacheCreateRequests: 0,
8181
errorRequests: 0,
82+
totalDurationMs: 0,
8283
provenance: {
8384
coverage: {
8485
attempts: totalRequests,

apps/desktop/src/renderer/features/workbar/tools/inspector/session-inspector-overview-model.ts

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,9 @@ export interface InspectorOverviewModel {
149149
tokenUsage?: InspectorTokenUsage;
150150
/**
151151
* Where the session's recorded time went, model calls against tool
152-
* executions. Absent when the connected Runtime Host predates duration
153-
* reporting or nothing was recorded — an unknown split is not a zero split.
152+
* executions. Absent when the query could not answer either ledger — a
153+
* connection-scoped query omits the tool side, and a session with no
154+
* recorded time has no split to draw.
154155
*/
155156
durationUsage?: InspectorDurationUsage;
156157
}
@@ -181,7 +182,11 @@ export interface InspectorDurationUsageSegment {
181182
}
182183

183184
export interface InspectorDurationUsage {
184-
/** model + tool recorded time — the active time this panel can account for. */
185+
/**
186+
* model + tool recorded time. A sum of per-call durations, not wall-clock:
187+
* parallel settlements and nested calls overlap, so this can exceed the
188+
* session's elapsed time.
189+
*/
185190
totalDurationMs: number;
186191
segments: readonly InspectorDurationUsageSegment[];
187192
}
@@ -256,17 +261,19 @@ function usageCacheHitRate(usage: SessionUsageSummary | undefined): number | und
256261
* from the ledger: several providers report the cached share only, and a
257262
* ledger zero there would understate the uncached input the session paid for.
258263
* The ledger's own `cacheMiss` is kept as a floor for records that reported
259-
* the miss but no prompt total. Output is the metered figure as billed —
260-
* providers that itemize reasoning include it in it, which is what the
261-
* panel's row label says.
264+
* the miss but no prompt total. `cacheRead` is taken as reported — providers
265+
* that itemize the cached share without a prompt total leave it larger than
266+
* `input`, and clamping it here would erase exactly the sessions the split
267+
* exists to describe. Output is the metered figure as billed — providers that
268+
* itemize reasoning include it in it, which is what the panel's row label says.
262269
*/
263270
function usageTokenSplit(usage: SessionUsageSummary | undefined): InspectorTokenUsage | undefined {
264271
if (!usage) return undefined;
265272
const { input, output, cacheRead, cacheMiss } = usage.totalTokens;
266273
if (input + output <= 0) return undefined;
267274
const uncachedInput = Math.max(input - cacheRead, cacheMiss, 0);
268275
const segments = [
269-
{ kind: 'cacheRead' as const, tokens: Math.min(cacheRead, input) },
276+
{ kind: 'cacheRead' as const, tokens: cacheRead },
270277
{ kind: 'cacheMiss' as const, tokens: uncachedInput },
271278
{ kind: 'output' as const, tokens: output },
272279
].filter((segment) => segment.tokens > 0);
@@ -281,9 +288,11 @@ function usageTokenSplit(usage: SessionUsageSummary | undefined): InspectorToken
281288

282289
/**
283290
* Model-call time against tool-execution time, both session-wide from their
284-
* own ledgers. Tool rows without a recorded duration contribute a request to
285-
* the count and nothing to the clock; a split nobody measured is not a split
286-
* worth drawing.
291+
* own ledgers. Presence follows the reported fields, not a zero default: a
292+
* host that measured zero model time keeps its row — the call count is real —
293+
* and `toolUsage` is simply absent when the query could not be scoped. A row
294+
* with neither a clock nor a count is dropped; a split nobody measured is not
295+
* a split worth drawing.
287296
*/
288297
function usageDurationSplit(
289298
usage: SessionUsageSummary | undefined,
@@ -294,13 +303,19 @@ function usageDurationSplit(
294303
{
295304
kind: 'model' as const,
296305
count: usage.totalRequests,
297-
durationMs: usage.totalDurationMs ?? 0,
306+
durationMs: usage.totalDurationMs,
298307
},
299-
{ kind: 'tool' as const, count: usage.toolUsage?.requests ?? 0, durationMs: usage.toolUsage?.durationMs ?? 0 },
308+
...(usage.toolUsage
309+
? [
310+
{
311+
kind: 'tool' as const,
312+
count: usage.toolUsage.requests,
313+
durationMs: usage.toolUsage.durationMs,
314+
},
315+
]
316+
: []),
300317
] satisfies InspectorDurationUsageSegment[]
301-
).filter(
302-
(segment) => segment.durationMs > 0 || (segment.kind === 'tool' && segment.count > 0),
303-
);
318+
).filter((segment) => segment.durationMs > 0 || segment.count > 0);
304319
if (segments.length === 0) return undefined;
305320
return {
306321
totalDurationMs: segments.reduce((carry, segment) => carry + segment.durationMs, 0),

apps/desktop/src/renderer/locales/conversation-copy.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,7 @@ const COPY = {
611611
},
612612
durationUsage: {
613613
title: '耗时统计',
614-
center: '活跃时间',
614+
center: '记录时长',
615615
segment: {
616616
model: (count) => `LLM 调用 × ${count}`,
617617
tool: (count) => `工具执行 × ${count}`,
@@ -867,7 +867,7 @@ const COPY = {
867867
},
868868
durationUsage: {
869869
title: 'Time breakdown',
870-
center: 'Active Time',
870+
center: 'Recorded Time',
871871
segment: {
872872
model: (count) => `LLM Calls × ${count}`,
873873
tool: (count) => `Tool Runs × ${count}`,

packages/core/src/__tests__/usage-ledger-merge.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ function legacySummary(overrides: Partial<UsageSummaryV2> = {}): UsageSummaryV2
8181
cacheHitRequests: 0,
8282
cacheCreateRequests: 0,
8383
errorRequests: 1,
84+
totalDurationMs: 0,
8485
...overrides,
8586
};
8687
}
@@ -157,9 +158,8 @@ describe('usage ledger merge', () => {
157158
);
158159
assert.equal(merged.totalDurationMs, 1_200);
159160

160-
// A legacy store from before the field existed contributes nothing to the
161-
// sum rather than an unknown that would take the canonical half down with
162-
// it — the projection always measures the attempts it counts.
161+
// The projection always measures the attempts it counts; a legacy store
162+
// with no recorded time simply contributes a zero to the sum.
163163
const canonicalOnly = mergeUsageSummary(
164164
legacySummary(),
165165
{

packages/core/src/usage-ledger-merge.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,9 @@ export function mergeUsageSummary(
158158
range: projected.range,
159159
totalRequests: legacy.totalRequests + projected.totalRequests,
160160
totalCostUsd: legacy.totalCostUsd + projected.totalCostUsd,
161-
// The projection always measures the attempts it counted; a legacy store
162-
// from before the field simply contributes nothing to the sum.
163-
totalDurationMs: (legacy.totalDurationMs ?? 0) + projected.totalDurationMs,
161+
// The projection always measures the attempts it counted, and every legacy
162+
// summary carries the same field.
163+
totalDurationMs: legacy.totalDurationMs + projected.totalDurationMs,
164164
totalTokens: {
165165
input: legacy.totalTokens.input + projected.totalTokens.input,
166166
output: legacy.totalTokens.output + projected.totalTokens.output,

packages/core/src/usage-stats/types.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,18 @@ export interface UsageSummaryV2 {
6666
/**
6767
* Recorded model-call time, summed over the same rows as `totalTokens`.
6868
*
69-
* Optional because it postdates the summary's first consumers: a Runtime Host
70-
* that has not been upgraded omits it rather than reporting a zero it never
71-
* measured. Absent means unknown, not free of latency.
69+
* Every summary writes it, and the protocol epoch refuses peers that predate
70+
* it at the handshake, so a total without a time basis cannot cross the wire.
7271
*/
73-
totalDurationMs?: number;
72+
totalDurationMs: number;
7473
/**
7574
* Recorded tool executions behind the same query, from the tool-invocation
7675
* ledger — the model-call ledger does not describe them, so there is no
7776
* canonical source to merge and this comes from the store alone.
77+
*
78+
* Optional because tool rows that predate connection attribution cannot
79+
* answer a `connectionSlug` filter honestly: the Host omits the split for
80+
* that query rather than drawing a ring from rows it cannot scope.
7881
*/
7982
toolUsage?: { requests: number; durationMs: number };
8083
}

packages/runtime-host/src/__tests__/usage-pricing-protocol.test.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -330,10 +330,9 @@ describe('Usage/Pricing protocol', () => {
330330
}
331331
});
332332

333-
test('a summary carries duration totals as optional figures, not required zeros', () => {
334-
// Newer hosts report recorded call time and tool-execution totals; a host
335-
// from before them still encodes, because "absent" means unknown rather
336-
// than a measured zero.
333+
test('a summary always carries its recorded time; only the tool split is optional', () => {
334+
// The epoch bump makes the duration basis a handshake requirement, so a
335+
// summary without one is not an older host — it is a malformed frame.
337336
assert.doesNotThrow(() =>
338337
usageResponse({
339338
kind: 'summary',
@@ -345,6 +344,22 @@ describe('Usage/Pricing protocol', () => {
345344
provenance: validProvenance(),
346345
}),
347346
);
347+
assert.doesNotThrow(() =>
348+
usageResponse({
349+
kind: 'summary',
350+
summary: validSummary(),
351+
provenance: validProvenance(),
352+
}),
353+
);
354+
assert.throws(
355+
() =>
356+
usageResponse({
357+
kind: 'summary',
358+
summary: { ...validSummary(), totalDurationMs: undefined },
359+
provenance: validProvenance(),
360+
}),
361+
invalidFrame,
362+
);
348363
assert.throws(
349364
() =>
350365
usageResponse({
@@ -778,6 +793,7 @@ function validSummary() {
778793
cacheHitRequests: 0,
779794
cacheCreateRequests: 0,
780795
errorRequests: 0,
796+
totalDurationMs: 1_200,
781797
};
782798
}
783799

0 commit comments

Comments
 (0)