Do not double-count Anthropic streaming output tokens#558
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses token-usage accounting in the Anthropic streaming path to avoid double-counting output tokens by treating message_delta usage as cumulative (authoritative) rather than incremental. It also introduces a separate change to functool to substitute a zero-value object when a typed tool handler returns a typed-nil pointer output.
Changes:
- Fix Anthropic streaming usage accumulation by overwriting (not summing) output token counts from
message_delta. - Add a regression test ensuring streamed Anthropic usage reports the final cumulative output token count.
- Add
functoolsupport (and tests) for typed-nil pointer outputs by substituting a non-nil zero value for schema validation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| provider/anthropicprovider/agent.go | Overwrites output-derived usage from message_delta to avoid double-counting cumulative output tokens. |
| provider/anthropicprovider/agent_test.go | Adds a streaming regression test asserting correct output/total token counts. |
| tool/functool/func.go | Adds typed-nil pointer output substitution to avoid null failing output schema validation. |
| tool/functool/func_test.go | Adds a test verifying typed-nil pointer output returns a non-nil zero-value pointer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // When Out is a pointer type, a handler may legitimately return a typed-nil | ||
| // pointer to mean "no result". A typed nil marshals to JSON null, which fails | ||
| // validation against the (non-nullable, object-rooted) output schema derived | ||
| // from the pointed-to type. Substitute the zero value of the element type in | ||
| // that case, mirroring the MCP go-sdk's HandlerFor behavior. |
There was a problem hiding this comment.
| if hasPointerOut { | ||
| elemZero = reflect.New(outType.Elem()).Interface().(Out) | ||
| } |
There was a problem hiding this comment.
Real bug, thank you — a named pointer output type (type P *T) would panic because reflect.New(outType.Elem()) yields an unnamed *T. Fixed in #557 by converting to the output type before the assertion (reflect.New(outType.Elem()).Convert(outType).Interface().(Out)), with a named-pointer regression test. This PR no longer touches functool.
The streaming path accumulated usage by summing message_start and message_delta with usage.Add. But Anthropic reports the final cumulative output token count on message_delta, while message_start carries only a small placeholder output count (typically 1). Summing them over-counted output tokens (and the derived total) by the placeholder amount on every streamed response. Overwrite the output-derived counts from message_delta instead of adding them, matching the SDK accumulator (which sets, not adds, OutputTokens on message_delta) and mirroring the equivalent Gemini streaming-usage fix.
e21a9dd to
68f4fd8
Compare
The streaming path accumulates usage by summing every event:
But Anthropic splits streaming usage across those two events:
message_startcarries the final input/cache counts but only a small placeholder output count (typicallyoutput_tokens: 1), whilemessage_deltacarries the final cumulative output count. The SDK’s own accumulator reflects this — formessage_deltait setsacc.Usage.OutputTokens = event.Usage.OutputTokensrather than adding.Summing them double-counts the
message_startplaceholder, so every streamed response over-reports output tokens (and the derived total) by that amount. Withmessage_startoutput_tokens: 1andmessage_deltaoutput_tokens: 5, the emitted usage isoutput=6, total=16instead ofoutput=5, total=15.This is the same cumulative-vs-incremental trap as the merged Gemini streaming-usage fix (#509).
Fix
Overwrite the output-derived counts from
message_deltainstead of adding them:Input and cache counts still come from
message_start. This is also correct if the stream ever sends multiplemessage_deltaevents (each carries the cumulative count).Test
TestStreamingUsage_DoesNotDoubleCountOutputTokensstreamsmessage_startoutput_tokens: 1thenmessage_deltaoutput_tokens: 5and asserts the final usage isoutput=5, total=15. It fails on the old code (6/16) and passes with the fix.