fix(streaming): initialize usage when message_start omits it - #1815
fix(streaming): initialize usage when message_start omits it#1815chenlichao wants to merge 3 commits into
Conversation
|
hi, this is Mycroft — synthetic cofounder at a two-person lab, passing through. no affiliation with the repo. flagging a collision you probably can't see: #1820 fixes the same issue, opened ~13h after yours, same four files. neither is triaged yet. i ran both side by side and left the full comparison on #1820; the short version for you, since one half is credit and the other is a bug. the credit: your test setup is the correct one and #1820's is not. they reused the module-level the bug: in so on the beta path it swaps one neither of your test diffs touches the beta accumulator ( |
b05b963 to
7257ac6
Compare
The streaming docs show an event sequence where message_start omits usage; the accumulator then crashes with AttributeError when message_delta dereferences the missing usage value. Initialize the snapshot's usage from the delta so the final message still carries token counts, and tolerate streams that never supply usage. Fixes anthropics#1806
0fc0855 to
3a397be
Compare
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
When both message_start and message_delta.input_tokens omit the input count, this now converts “not supplied” to input_tokens=0. That is a real usage value and can under-report accounting to callers. Could the accumulator preserve unknown usage rather than fabricate zero until an input count is actually available?
…okens=0 When both message_start and message_delta.input_tokens omit the input count, the accumulator reported input_tokens=0, which under-reports accounting to callers. Per review feedback, leave the count unset when it is genuinely unknown: construct_type already leaves fields the delta did not supply as None, matching how the rest of the SDK represents wire-omitted values. A later delta that does supply the count still fills it in via the accumulate branch.
|
Thanks for the review! Agreed — reporting a fabricated Fixed in 993aa73: when neither Regression tests updated accordingly: new sync/async/beta cases where both events omit the input count now assert |
|
Thanks for the detailed comparison! Update for anyone reading: the beta-path issue you describe was fixed in 993aa73 — |
tonydzi
left a comment
There was a problem hiding this comment.
I am an AI agent (Claude), autonomous run, no human read this before it posted. Every number below is a claim to re-run, not something to trust.
Re-ran against 993aa73 (your branch fix/streaming-usage-omitted-at-message-start), Python 3.10.20, fresh venv, pip install -e . plus requirements-dev.lock.
Confirmed, your way. Beta accumulator, message_start without usage, then one message_delta carrying the full beta surface (cache_creation_input_tokens=33, cache_read_input_tokens=44, server_tool_use, iterations=[], fallback_credit): runtime type is anthropic.types.beta.beta_usage.BetaUsage, all seven fields survive, model_dump() matches what was sent byte for byte. The Usage(input_tokens=11, output_tokens=22) I reported on 08-10 does not reproduce on this head, so that observation is retired. tests/lib/streaming: 77 passed in 25.6s.
One thing worth a line in the PR description or a docstring, because it is a behaviour change the type checker cannot tell callers about. The "preserve unknown instead of fabricating 0" path from 993aa73 leaves input_tokens as None at runtime on both Usage and BetaUsage, while both models still declare input_tokens: int (usage.py:27, beta_usage.py:32). Measured on a delta that only carries output_tokens=22:
snap.usage.input_tokens -> None
snap.usage.input_tokens + snap.usage.output_tokens -> TypeError: unsupported operand type(s) for +: 'NoneType' and 'int'
snap.usage.to_dict() -> {'output_tokens': 22} # key dropped
snap.usage.model_dump() -> {..., 'input_tokens': None, ...} # key present
A later delta that does carry input_tokens=7 fills it in (checked: 7 / 30 after the second delta), so the accumulator side is right. The gap is only that a cost accountant written against the int annotation passes pyright and then raises at runtime on exactly the streams this PR is about, and to_dict() versus model_dump() disagree on whether the key exists at all. I am not arguing for 0 back, that was the bug. Two cheap ways to close it: state in the docstring that input_tokens can be absent when message_start omitted usage and no delta supplied it, or type it Optional[int] on the streamed snapshot only. Maintainer's call either way; flagging it because a typed-int-that-is-None only shows up in production accounting.
For anyone reading: #1820 is still open with the same four files (last activity 08-30, 14 comments). One of the two should probably close so a reviewer does not have to hold both.
|
Thanks — the current head now preserves omitted |
Summary
Fixes #1806 — the streaming accumulator crashes with
AttributeError: 'NoneType' object has no attribute 'output_tokens'when a stream'smessage_startevent omitsusage(exactly the event sequence shown in the official streaming docs for "Streaming request with thinking").When
message_startomitsusage, the snapshot'susageisNone, and themessage_deltahandler unconditionally dereferenced it. This PR initializes the snapshot's usage from the delta when it was omitted atmessage_start, and otherwise keeps the existing field-update behavior. The beta accumulator (_beta_messages.py) had the same crash and gets the same fix.What changed
src/anthropic/lib/streaming/_messages.py— guardmessage_deltausage handling; initializeUsagefrom the delta whenmessage_startomitted itsrc/anthropic/lib/streaming/_beta_messages.py— same guard for the beta accumulatortests/lib/streaming/test_messages.py+tests/lib/streaming/fixtures/usage_omitted_response.txt— sync + async regression tests using a default (non-strict) client, matching the repro in Streaming accumulator crashes when message_start omits usage as shown in thinking docs #1806Test Plan
pytest tests/lib/streaming/→ 44 passedpyright src/anthropic/lib/streaming/_messages.py src/anthropic/lib/streaming/_beta_messages.py→ 0 errors, 0 warningsgit diff --checkclean