fix(models): status-less stream faults classify for every provider family - #55
Merged
Conversation
… family
Two halves of one truth path, either alone leaving the bug alive:
- streaming.py: the Responses `error` event is flat (code/message on the
event itself); it was read with Anthropic's nesting, which returned {}
and replaced the provider's verdict with a synthetic {type: unknown}.
Normalize the flat grammar into the {code, message} shape the
response.failed arm already yields.
- exceptions.py: the status-less in-stream classification arm was gated
to Anthropic. It now hoists message/type/code extraction above a
provider dispatch (the provider's real words survive for every
provider, mapped or not) and classifies the openai/azure family by its
code vocabulary; bedrock joins the anthropic branch it already shares
on the status-code path.
Before: an Azure mid-stream server fault or rate limit — HTTP 200, no
status code — fell past every arm as UNKNOWN/non-retryable with the
literal message 'API Error', so a caller's retry ladder saw a terminal
verdict for the most retryable faults a provider emits. Three production
turns died that way on one transient Azure wobble.
Deliberately unchanged, pinned by tests: the synthetic max_retries and
incomplete_stream markers stay UNKNOWN/non-retryable, and unmapped codes
default to non-retryable — flipping any of these is a policy decision,
not a classification repair.
…e-less failed marker Coverage-audit hardening: a retryable verdict must carry a positive retry_after (a ladder reading None degenerates to a hot loop); a payload whose code and type map differently proves code wins; and the bare response.failed marker the classifier keys on is now produced by an accumulator test, pinning the wire-to-classifier join.
…pe; drop an unreachable fallback The incomplete_stream marker is emitted as a type, not a code — the pin row now feeds the shape the adapter actually produces. The flat error arm's nested-error fallback was unreachable (the flat grammar is the only one this event carries) and is gone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A mid-stream SSE fault arrives under HTTP 200 — no status code — and the classification tree gated everything on
elif status_code:, with the only status-less arm restricted to Anthropic. An OpenAI/Azure in-stream fault therefore kept the defaults:UNKNOWN,is_retryable=False, and the literal message "API Error". A caller's retry ladder sees a terminal verdict for the most retryable faults a provider emits (an in-stream rate limit included).Two halves, either alone leaving the bug alive (proven by driving each half separately):
adapters/streaming.py— the Responseserrorevent is flat (code/messageon the event itself, per the generatedResponseErrorEventtype); it was read with Anthropic's nested shape, which returned{}and replaced the provider's verdict with a synthetic{"type": "unknown"}. It now normalizes the flat grammar into the same{code, message}shape theresponse.failedarm already yields.agents/exceptions.py— the status-less arm now hoists message/type/code extraction above a provider dispatch (the provider's real words survive for every provider, mapped or not) and classifies the openai/azure family by its code vocabulary (server_error/bareresponse.failed→ SERVICE_UNAVAILABLE retryable;rate_limit_exceeded/rate_limit_error→ RATE_LIMIT retryable;insufficient_quota→ INSUFFICIENT_CREDITS non-retryable).bedrockjoins the anthropic branch — the same share the status-code arm already makes; without it every Bedrock in-stream fault is unclassified-terminal.Deliberately unchanged, pinned by tests
The synthetic
max_retriesandincomplete_streammarkers stay UNKNOWN/non-retryable, and unmapped codes default to non-retryable. Flipping any of these is a policy decision, tracked separately.Notes for review
exception=and a status-less{"error": ...}dict would take this arm. No call site does both today (audited: anthropic.py, anthropic_oauth.py, openai.py are the only status-less-dict callers; google/openrouter/local pass response objects; openai_oauth stamps a synthetic 500).arun_streamingend-to-end over the fake session (classification + retryable + the provider's words), and the harmonize classification table parameterized across provider families.tests/models/test_adapter_harmonize.py: every stream outcome maps to either a valid HarmonizedResponse or a typed, classified ModelAPIError.