fix: normalize Anthropic usage for finish events so totals reflect th…#973
Conversation
…e last step in multi-step runs
🦋 Changeset detectedLatest commit: fcf4bff The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment has been minimized.
This comment has been minimized.
📝 Walkthrough🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Deploying voltagent with
|
| Latest commit: |
fcf4bff
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://905ca9e6.voltagent.pages.dev |
| Branch Preview URL: | https://fix-anthropic-usage.voltagent.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/agent/agent.ts (1)
1405-1416: LLM span finalization still captures pre-normalized usage.The span is finalized with
providerUsagebeforeusageForFinishis applied; the later finalize call is a no-op due to theendedguard. This means normalized usage won’t reach the span whentotalUsagediffers orusageis absent.🛠️ Suggested fix
- finalizeLLMSpan(SpanStatusCode.OK, { - usage: providerUsage, - finishReason: finalResult.finishReason, - }); + finalizeLLMSpan(SpanStatusCode.OK, { + usage: usageForFinish ?? providerUsage, + finishReason: finalResult.finishReason, + }); ... - finalizeLLMSpan(SpanStatusCode.OK, { - usage: usageForFinish, - finishReason: finalResult.finishReason, - });Also applies to: 1554-1556
🤖 Fix all issues with AI agents
In `@packages/core/src/utils/usage-normalizer.ts`:
- Around line 49-71: In normalizeFinishUsageStream, add a fallback check for
providerMetadata on parts with type "finish": when iterating, if part.type ===
"finish" and useLastStepUsage is false, call
shouldUseLastStepUsage(part.providerMetadata, lastStepUsage) and, if it returns
true, set useLastStepUsage = true and (if available) set lastStepUsage =
part.usage; then proceed with the existing replacement logic that yields {
...part, totalUsage: lastStepUsage } when useLastStepUsage and lastStepUsage are
present. This ensures providerMetadata attached to the final "finish" part
triggers normalization just like metadata on "finish-step" parts.
🧹 Nitpick comments (3)
packages/core/src/utils/usage-normalizer.spec.ts (3)
19-98: Good test coverage forresolveFinishUsage.The tests cover the key scenarios well. Consider adding an explicit test for the
undefinedreturn case when bothusageandtotalUsageare missing:🔧 Optional: Add test for undefined return case
it("returns undefined when both usage and totalUsage are missing", () => { expect(resolveFinishUsage({})).toBeUndefined(); expect(resolveFinishUsage({ providerMetadata: { anthropic: {} } })).toBeUndefined(); });
101-125: Test correctly validates the override behavior.The test verifies the primary use case. Optionally, consider adding assertions to verify that non-finish parts (like the text part) pass through unchanged:
const normalized = await collectStream(normalizeFinishUsageStream(toAsync(parts))); + expect(normalized[0]).toEqual({ type: "text", text: "hello" }); expect(normalized[2].totalUsage).toEqual(lastStepUsage);
174-192: Good edge case coverage.This test correctly validates that the normalizer doesn't introduce
totalUsagewhen it wasn't present.Consider adding a test for multi-step scenarios where multiple
finish-stepevents occur, to verify that the last step's usage is used (based on the implementation which always updateslastStepUsage):🔧 Optional: Add multi-step scenario test
it("uses the last finish-step usage in multi-step streams", async () => { const firstStepUsage: LanguageModelUsage = { inputTokens: 5, outputTokens: 3, totalTokens: 8, }; const lastStepUsage: LanguageModelUsage = { inputTokens: 10, outputTokens: 5, totalTokens: 15, }; const totalUsage: LanguageModelUsage = { inputTokens: 40, outputTokens: 20, totalTokens: 60, }; const parts = [ { type: "finish-step", usage: firstStepUsage, providerMetadata: { anthropic: { model: "claude" } }, }, { type: "finish-step", usage: lastStepUsage, providerMetadata: { anthropic: { model: "claude" } }, }, { type: "finish", finishReason: "stop", totalUsage }, ]; const normalized = await collectStream(normalizeFinishUsageStream(toAsync(parts))); expect(normalized[2].totalUsage).toEqual(lastStepUsage); });
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/src/utils/usage-normalizer.ts">
<violation number="1" location="packages/core/src/utils/usage-normalizer.ts:64">
P2: The finish-event normalization check ignores `part.usage`, so if no `finish-step` was emitted and Anthropic cache fields exist only on the finish usage, `shouldUseLastStepUsage` never triggers and totals remain unnormalized. Use the finish event usage when deciding.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| if (part.type === "finish" && !useLastStepUsage) { | ||
| if (shouldUseLastStepUsage(part.providerMetadata, lastStepUsage)) { |
There was a problem hiding this comment.
P2: The finish-event normalization check ignores part.usage, so if no finish-step was emitted and Anthropic cache fields exist only on the finish usage, shouldUseLastStepUsage never triggers and totals remain unnormalized. Use the finish event usage when deciding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/utils/usage-normalizer.ts, line 64:
<comment>The finish-event normalization check ignores `part.usage`, so if no `finish-step` was emitted and Anthropic cache fields exist only on the finish usage, `shouldUseLastStepUsage` never triggers and totals remain unnormalized. Use the finish event usage when deciding.</comment>
<file context>
@@ -60,6 +60,15 @@ export async function* normalizeFinishUsageStream<T extends StreamPartWithUsage>
}
+ if (part.type === "finish" && !useLastStepUsage) {
+ if (shouldUseLastStepUsage(part.providerMetadata, lastStepUsage)) {
+ useLastStepUsage = true;
+ if (part.usage) {
</file context>
| if (shouldUseLastStepUsage(part.providerMetadata, lastStepUsage)) { | |
| if (shouldUseLastStepUsage(part.providerMetadata, part.usage ?? lastStepUsage)) { |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/core/src/utils/usage-normalizer.ts`:
- Around line 63-69: The finish-part cache detection misses cache fields when
only the current finish part's usage contains them; update the call to
shouldUseLastStepUsage so it checks the current part's usage first by passing
part.usage ?? lastStepUsage (instead of lastStepUsage alone) when called in the
if (part.type === "finish" && !useLastStepUsage) block—this ensures
shouldUseLastStepUsage(part.providerMetadata, part.usage ?? lastStepUsage) will
detect cache_* fields present on the finish part and correctly set
useLastStepUsage/lastStepUsage.
| if (part.type === "finish" && !useLastStepUsage) { | ||
| if (shouldUseLastStepUsage(part.providerMetadata, lastStepUsage)) { | ||
| useLastStepUsage = true; | ||
| if (part.usage) { | ||
| lastStepUsage = part.usage; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the usage-normalizer.ts file
find . -name "usage-normalizer.ts" -type fRepository: VoltAgent/voltagent
Length of output: 108
🏁 Script executed:
# Read the file to understand the context
cat -n packages/core/src/utils/usage-normalizer.tsRepository: VoltAgent/voltagent
Length of output: 2861
🏁 Script executed:
# Search for shouldUseLastStepUsage function definition
rg "shouldUseLastStepUsage" -B 2 -A 10Repository: VoltAgent/voltagent
Length of output: 3416
🏁 Script executed:
# Search for cache_ field usage patterns
rg "cache_" -B 2 -A 2 | head -50Repository: VoltAgent/voltagent
Length of output: 1965
Consider cache_ detection on finish-part usage.*
If cache fields are present only on the "finish" part's usage, shouldUseLastStepUsage never sees them because the function is called with lastStepUsage (from prior steps) rather than the current part's usage. When a finish part arrives without prior finish-step parts and contains cache fields in its own usage, the detection fails and normalization doesn't trigger. Passing part.usage ?? lastStepUsage ensures the check evaluates the current part's usage first.
🛠️ Proposed adjustment
- if (part.type === "finish" && !useLastStepUsage) {
- if (shouldUseLastStepUsage(part.providerMetadata, lastStepUsage)) {
- useLastStepUsage = true;
- if (part.usage) {
- lastStepUsage = part.usage;
- }
- }
- }
+ if (part.type === "finish" && !useLastStepUsage) {
+ const candidateUsage = part.usage ?? lastStepUsage;
+ if (shouldUseLastStepUsage(part.providerMetadata, candidateUsage)) {
+ useLastStepUsage = true;
+ if (part.usage) {
+ lastStepUsage = part.usage;
+ }
+ }
+ }🤖 Prompt for AI Agents
In `@packages/core/src/utils/usage-normalizer.ts` around lines 63 - 69, The
finish-part cache detection misses cache fields when only the current finish
part's usage contains them; update the call to shouldUseLastStepUsage so it
checks the current part's usage first by passing part.usage ?? lastStepUsage
(instead of lastStepUsage alone) when called in the if (part.type === "finish"
&& !useLastStepUsage) block—this ensures
shouldUseLastStepUsage(part.providerMetadata, part.usage ?? lastStepUsage) will
detect cache_* fields present on the finish part and correctly set
useLastStepUsage/lastStepUsage.
1 participant
…e last step in multi-step runs
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
What is the new behavior?
fixes (issue)
Notes for reviewers
Summary by cubic
Normalize usage for Anthropic runs so finish events report token totals from the last step in multi-step runs. This fixes inflated totals in streaming and non-streaming flows.
Written for commit fcf4bff. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.