fix(ai): roll up RUN_FINISHED usage across iterations onto root span#961
fix(ai): roll up RUN_FINISHED usage across iterations onto root span#961feiiiiii5 wants to merge 2 commits into
Conversation
The chat engine overwrote `finishedEvent` per iteration and reset it to `null` in `beginIteration()`, so the terminal `onFinish` hook received only the last iteration's `RUN_FINISHED.usage`. The documented contract (docs/advanced/otel.md) is that the root `chat` span rolls up `gen_ai.usage.input_tokens` / `gen_ai.usage.output_tokens` across all iterations — and any middleware reading `FinishInfo.usage` saw the same truncated total. Add a separate `accumulatedUsage` field on `TextEngine` that survives `beginIteration()`'s per-iteration reset and accumulates every usage-bearing `RUN_FINISHED` chunk via the new shared `accumulateTokenUsage` helper. The helper sums core counts plus every optional numeric field (cost, cache/reasoning breakdowns, upstream cost split, duration-based billing, units billed); provider-shaped `providerUsageDetails` is preserved as latest-wins, matching how `onUsage` consumers see the most recent per-iteration bag. Per-iteration span attributes and the `gen_ai.client.token.usage` histogram are unchanged — they still carry each iteration's incremental values. The roll-up lives on the root span only. Closes TanStack#916
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change accumulates ChangesUsage rollup
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TextEngine
participant accumulateTokenUsage
participant MiddlewareOnFinish
participant RootChatSpan
TextEngine->>accumulateTokenUsage: accumulate each RUN_FINISHED usage
accumulateTokenUsage-->>TextEngine: return rolled-up TokenUsage
TextEngine->>MiddlewareOnFinish: invoke onFinish with FinishInfo.usage
MiddlewareOnFinish->>RootChatSpan: set rolled-up usage attributes
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/advanced/middleware.md (1)
461-469: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winStale
FinishInfo.usagetable entry contradicts the updated bullet above.Line 468 still describes
usageas "the agent loop's lastRUN_FINISHED.usage" — the exact pre-fix behavior this PR corrects (per the new bullet at Line 436, which says usage is now "rolled up across all agent-loop iterations"). This table row wasn't updated and will mislead readers about the current contract.📝 Proposed fix
-| `usage` | `{ promptTokens; completionTokens; totalTokens } \| undefined` | **Optional.** The agent loop's last `RUN_FINISHED.usage`. **Does not include finalization tokens** — use `onUsage` to observe those. Always guard with `if (info.usage)` or `info.usage?.`. | +| `usage` | `TokenUsage \| undefined` | **Optional.** Usage rolled up across all agent-loop iterations' `RUN_FINISHED` chunks. **Does not include finalization tokens** — use `onUsage` to observe those. Always guard with `if (info.usage)` or `info.usage?.`. |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/advanced/middleware.md` around lines 461 - 469, Update the FinishInfo.usage row in the onFinish documentation table to describe usage as rolled up across all agent-loop iterations, matching the updated contract and nearby bullet. Preserve the optionality and clarification that finalization tokens are excluded and should be observed via onUsage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/advanced/middleware.md`:
- Around line 461-469: Update the FinishInfo.usage row in the onFinish
documentation table to describe usage as rolled up across all agent-loop
iterations, matching the updated contract and nearby bullet. Preserve the
optionality and clarification that finalization tokens are excluded and should
be observed via onUsage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9986b9e8-1247-4a2f-8153-b1bd75f375b9
📒 Files selected for processing (7)
.changeset/ai-otel-root-span-usage-rollup.mddocs/advanced/middleware.mddocs/advanced/otel.mdpackages/ai/src/activities/chat/index.tspackages/ai/src/utilities/usage.tspackages/ai/tests/middleware.test.tspackages/ai/tests/middlewares/otel.test.ts
…llup contract (TanStack#961) CodeRabbit flagged the FinishInfo.usage table row at line 468 as contradicting the bullet at line 436, which this PR updated to describe usage as "rolled up across all agent-loop iterations' RUN_FINISHED chunks." The table row still said "the agent loop's last RUN_FINISHED.usage" — the pre-fix wording — and so misled readers about the current contract. Align the row with the bullet: replace the inline shape with the exported `TokenUsage` type and describe the value as rolled up across all agent-loop iterations' `RUN_FINISHED` chunks.
Closes #916.
Summary
The root
chatspan was carrying only the last iteration'sgen_ai.usage.input_tokens/gen_ai.usage.output_tokensinstead of the documented cross-iteration roll-up. Perdocs/advanced/otel.md:Root cause
TextEngine.handleRunFinishedEvent(chunk)overwrotethis.finishedEvent = chunkeach iteration, andbeginIteration()resetthis.finishedEvent = null. The terminal hook then passedusage: this.finishedEvent?.usagetorunOnFinish— i.e. only the final iteration'sRUN_FINISHED.usage. Any multi-iteration run (e.g. tool call → final answer) under-reported the root span and any middleware readingFinishInfo.usagesaw the same truncated total.Adapters can't compensate: each iteration is a fresh
chatStream()call on a stateless adapter, so aRUN_FINISHEDchunk can only ever carry that one API call's usage.Per-iteration data was correct —
onUsagefired with each iteration's own (incremental) usage, iteration spans got the right values, and thegen_ai.client.token.usagehistogram summed correctly. Only the root span's roll-up (andFinishInfo.usagefor any middleware relying on it) under-reported multi-iteration runs.Fix
Add a separate
accumulatedUsagefield onTextEnginethat:beginIteration()'s per-iteration reset (reset only at run start, via field initialization —TextEngineis instantiated perchat()call)RUN_FINISHEDchunk via the new sharedaccumulateTokenUsagehelper (the same summationonUsageconsumers already do)onFinishreceives, withfinishedEvent?.usageas a fallback only when noRUN_FINISHEDchunk carried usage at all (preserves priorundefinedbehavior for adapters that never report usage)accumulateTokenUsagehelperSums core counts (
promptTokens,completionTokens,totalTokens) plus every optional numeric field:cost,durationSeconds,unitsBilledpromptTokensDetails(cachedTokens, cacheWriteTokens, audioTokens, videoTokens, imageTokens, textTokens, documentTokens)completionTokensDetails(reasoningTokens, audioTokens, videoTokens, imageTokens, textTokens, documentTokens)costDetails(upstreamCost, upstreamInputCost, upstreamOutputCost)When only one side reports a field, that value is preserved (so a single iteration's cost still surfaces on the roll-up).
providerUsageDetailsis provider-shaped and not generally summable, so the latest value wins — matching howonUsageconsumers see the most recent per-iteration bag.What's unchanged
otelMiddleware.onChunk'sRUN_FINISHEDhandler still setsusageAttributes(chunk.usage)on the iteration span (each iteration's incremental values)gen_ai.client.token.usagehistogram —otelMiddleware.onUsagestill records per-iteration tokens; the roll-up lives on the root span onlyonUsagecallback shape — still fires per-iteration with that iteration's incremental usage, not the roll-upinfo.finishReason— still the last iteration's finishReasonTest plan
Red→green verification (both tests failed with
700 !== 1200before the fix, pass after):tests/middleware.test.ts— "rolls up usage across iterations in FinishInfo (root span contract)": two-iteration scripted adapter (500/50 → 700/80), assertsinfo.usage.promptTokens === 1200,completionTokens === 130,totalTokens === 1330, and thatonUsagestill fires per-iteration with incremental valuestests/middleware.test.ts— "rolls up cost and detail breakdowns across iterations": assertspromptTokensDetails.cachedTokens,completionTokensDetails.reasoningTokens,cost, andcostDetails.upstreamCost/upstreamInputCost/upstreamOutputCostall sum correctlytests/middlewares/otel.test.ts— "rolls up usage across iterations onto the root span (issue otelMiddleware root chat span carries last-iteration usage, not the documented cross-iteration roll-up #916)": asserts root span attributes (gen_ai.usage.input_tokens === 1200), per-iteration span attributes (500 and 700), and histogram records (4 records summing to 1200/130)Full test suite
Plus
tsc --noEmitclean.Repro verified
The reporter's repro (two-iteration scripted adapter, 500/50 → 700/80) produced
700 80on the root span before this fix. With this fix, the root span now carries1200 130, matching the documented contract.Summary by CodeRabbit
Bug Fixes
onFinishusage across all iterations.Documentation
onFinishFinishInfo.usagecontract (rolled-up agent-loop usage,undefinedwhen absent) and how it relates toonUsage.Tests
onFinishusage rollups and OTel duration/usage histogram alignment across iterations.