fix(models/anthropic): suppress Pydantic serialization warnings during serialization#2995
fix(models/anthropic): suppress Pydantic serialization warnings during serialization#2995lizradway wants to merge 2 commits into
Conversation
…g streaming Suppress PydanticSerializationUnexpectedValue warnings emitted by model_dump() when the Anthropic SDK (>=0.84.0) returns ParsedTextBlock objects in streaming responses. This replaces the previous per-event-type workarounds with a single warnings filter scoped to the model_dump() call. Fixes strands-agents#1865
Adds test_stream_all_events_no_pydantic_warnings which verifies that PydanticSerializationUnexpectedValue warnings are suppressed for all event types (message_start, content_block_start, content_block_delta, content_block_stop, message_stop) during streaming.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| pydantic_warnings = [w for w in caught_warnings if "PydanticSerializationUnexpectedValue" in str(w.message)] | ||
| assert len(pydantic_warnings) == 0, f"Unexpected Pydantic warnings: {pydantic_warnings}" | ||
|
|
||
| assert {"messageStart": {"role": "assistant"}} in events |
There was a problem hiding this comment.
Issue: These five assert {...} in events checks assert event-by-event. Since this test fully controls the mock model_dump() return values, the expected event sequence is known and deterministic, so the in checks silently miss ordering, missing events, or unexpected extra events (including the trailing metadata event). Per TESTING.md: "When you control the shape, assert the whole object — not field-by-field."
Suggestion: Build the expected list and assert in a single equality, e.g. tru_events = events / exp_events = [ ... ] then assert tru_events == exp_events. This documents the full expected stream and catches regressions the per-event membership checks would miss.
| else: | ||
| with warnings.catch_warnings(): | ||
| warnings.filterwarnings("ignore", message=".*PydanticSerializationUnexpectedValue.*") | ||
| yield self.format_chunk(event.model_dump()) |
There was a problem hiding this comment.
Issue: The yield happens inside the with warnings.catch_warnings() block. catch_warnings() saves and mutates the global warnings filter state and only restores it on __exit__. Because a generator suspends at yield with the context manager still on the stack, the modified global filter stays active the entire time the consumer processes the chunk — and across any await/task switch that happens in between. With two streams interleaving, the nested save/restore of the global state can leak/restore the wrong filter (catch_warnings is explicitly documented as not thread/concurrency-safe). Only model_dump() actually needs suppression.
Suggestion: Compute the dump inside the context and yield outside it:
with warnings.catch_warnings():
warnings.filterwarnings("ignore", message=".*PydanticSerializationUnexpectedValue.*")
chunk = event.model_dump()
yield self.format_chunk(chunk)|
Assessment: Comment Clean, focused bug fix that collapses the per-event-type workarounds into a single scoped suppression — the message-regex scope is appropriately narrow. One correctness/robustness concern around where the Review themes
Nice simplification overall — removing the special-cased event handling makes |
Description
When using the Anthropic SDK >= 0.84.0, every streaming response triggers multiple
PydanticSerializationUnexpectedValuewarnings on stderr becausemodel_dump()encountersParsedTextBlockobjects that don't match the discriminated union schema.The previous fix (from #1746) worked around this by manually building dicts for
message_stopandcontent_block_stopevents, but the same warnings still fire formessage_start,content_block_start, andcontent_block_delta.This PR replaces those per-event-type workarounds with a single
warnings.filterwarnings("ignore", message=".*PydanticSerializationUnexpectedValue.*")scoped to themodel_dump()call. The serialization still produces correct data — it just emits noise that we suppress.Related Issues
Fixes #1865
Documentation PR
N/A
Type of Change
Bug fix
Testing
Existing regression tests (
test_stream_message_stop_no_pydantic_warnings,test_stream_content_block_stop_no_pydantic_warnings) already verify that Pydantic serialization warnings do not leak during streaming. Both pass with this change.Added new regression test as well.
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.