fix(anthropic): stop double-counting output tokens on streamed messages - #4377
Conversation
message_delta.usage.output_tokens is the running total for the whole message, and message_start.usage.output_tokens is a partial count already included in it. _process_response_item added the two, so every streamed Anthropic span reported inflated output tokens, inflated total tokens and inflated cost downstream. No exception was raised. The vendor SDK assigns rather than adds (anthropic/lib/streaming/_messages.py:506), as does this repo's own Bedrock streaming wrapper. Replaying the existing test_anthropic_message_streaming_legacy cassette gives 174 output tokens against a ground truth of 171. The existing test asserted input_tokens and the input+output==total identity but never the output value itself, so the bug passed CI. Added that assertion.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAnthropic streaming usage handling now treats ChangesAnthropic streaming usage
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Gentle ping: CodeRabbit, GitGuardian, and the CLA are all green, and the branch is current as of yesterday. The test workflows are still waiting on the fork workflow approval click; would a maintainer be able to approve the run so CI can report on this change? |
Problem
_process_response_itemtreatsmessage_delta.usage.output_tokensas an increment and adds it to the count already stored frommessage_start:Anthropic sends
message_delta.usage.output_tokensas the running total for the whole message. The value inmessage_startis a partial count that is already contained in it. Adding them inflates output tokens, and therefore total tokens and any cost computed downstream, on every streamed Anthropic span. Nothing raises, so the wrong number just propagates.The vendor SDK assigns rather than adds, in
anthropic/lib/streaming/_messages.py:This repo's own Bedrock streaming wrapper also overwrites rather than accumulates (
bedrock/streaming_wrapper.py,usage.update(...)).Reproduction
Replaying the SSE events from this repo's existing cassette,
tests/cassettes/test_messages/test_anthropic_message_streaming_legacy.yaml, through both accumulators:A 1.75% overcount on this recording. The gap is whatever
message_startreported, so it grows with prompt-cache and tool-use shapes where that initial count is larger.Fix
Overwrite with the delta's value instead of adding, and leave the stored count alone when the delta omits
output_tokens.Test
test_anthropic_message_streaming_legacyalready assertedinput_tokens == 17and theinput + output == totalidentity, but never asserted the output value itself, so the bug passed CI. This PR adds that one assertion. It fails withassert 174 == 171onmainand passes with the fix.Summary by CodeRabbit