fix(provider): retry list_models on transient API failures - #90
Open
Sam Schillace (ramparte) wants to merge 1 commit into
Open
fix(provider): retry list_models on transient API failures#90Sam Schillace (ramparte) wants to merge 1 commit into
Sam Schillace (ramparte) wants to merge 1 commit into
Conversation
list_models() was the only network call in this module not covered by the shared retry_with_backoff()/_retry_config machinery. A single transient failure (5xx, Cloudflare challenge, connection/timeout error) killed it outright, causing the routing-matrix resolver's glob model pattern resolution to silently degrade delegation to the session's default model. Wrap the client.models.list() call in retry_with_backoff(), mirroring the exact error-translation and retry call shape already used by complete() (see _do_complete/_on_retry at __init__.py:3150-3298 and 3458): 5xx and Cloudflare challenges retry, 401/403/404 raise immediately as non-retryable kernel errors. Connection/timeout errors fall through to the same generic catch-all _do_complete() uses. AnthropicOverloadedError (529) is a subclass of AnthropicAPIStatusError and is retried via the existing status >= 500 branch -- no new classification introduced. SDK-level retries remain disabled (max_retries=0 on the AsyncAnthropic client) -- this module continues to own all retry policy via retry-after headers. Testing: - New tests/test_list_models_retry.py: first-try success (single call, unchanged result), recovery after one transient 500, exhaustion after persistent 500s, and immediate raise on non-retryable 401. - Full suite: 612 -> 616 passed (4 new tests), no regressions. - ruff/pyright (via uvx): identical issue set before/after (21 ruff errors, 14 pyright errors -- all pre-existing, only line-shifted by the inserted code), zero new findings. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Sam Schillace (ramparte)
requested review from
Brian Krabach (bkrabach) and
Salil Das (sadlilas)
August 12, 2026 21:25
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.
Problem
list_models()is the only network call in this module not protected by the module's own retry machinery. A single transient failure from/v1/modelskills it outright.The client is deliberately constructed with SDK retries off —
AsyncAnthropic(..., max_retries=0), per the comment "We handle retries ourselves (SDK max_retries=0) to properly honor retry-after headers".self._retry_configexists andcomplete()uses it.list_models()did not:Why it matters. The routing-matrix resolver resolves glob model patterns by calling
list_models()— theanthropicrouting matrix maps every role throughclaude-opus-*,claude-sonnet-*,claude-haiku-*. When the call fails,resolve_model_rolereturns[]and never raises, so delegation silently falls back to the session's default model instead of the intended one. The user gets a different model with no error and no log line pointing at the cause.This was observed in production on the OpenAI side twice within two hours — one HTTP 500
server_error, one Cloudflare 522 — each producingmodel_role 'fast' resolved to no candidates. Both were pure blips; a direct request immediately afterward returned 200 in ~0.5s. This module has the identical exposure against Anthropic's endpoint. Companion fix for the sibling provider: microsoft/amplifier-module-provider-openai#61.What this does
Wraps the
models.list()call in the module's existingretry_with_backoffwithself._retry_config, mirroring howcomplete()wires it (guarded inner function +_on_retryhook). Uses the baseself._retry_configrather than_build_retry_config(...), since that helper exists only to shrink the retry budget during model-fallback-on-overload, which does not apply to a parameterless GET.Exception translation is copied from
_do_complete, not reinvented:AnthropicRateLimitError->KernelRateLimitError(retryable, via the existing_parse_rate_limit_info)AnthropicAuthenticationError->KernelAuthenticationError(401, non-retryable)AnthropicAPIStatusError-> 403 Cloudflare-challenge detection via the existing_is_cloudflare_challenge, elseKernelAccessDeniedError; 404 ->KernelNotFoundError; >=500 ->KernelProviderUnavailableError(retryable); otherwise non-retryableKernelLLMErrorexcept Exception-> retryableKernelLLMError, matching this module's existing convention for connection/timeout errors (unlike the OpenAI sibling, this module does not explicitly importAPIConnectionError/APITimeoutErrorand relies on this catch-all)AnthropicOverloadedError(529) needs no separate branch — verified via__mro__that it subclassesAnthropicAPIStatusError, so it is already caught and retried by the>=500branch.max_retries=0on theAsyncAnthropicclient is not changed — it is intentional so the module can honorretry-afterheaders itself.All existing
list_models()behavior is preserved exactly: thefilteredparameter,_detect_family()grouping, and newest-per-family selection. Only the transport call is wrapped. The method still raises on persistent failure, so existing callers are unaffected — it just raises after exhausting retries. Docstring updated to say so.Scope
Deliberately minimal. No caching added — the routing resolver already caches a successful result for the session's lifetime, so provider-level caching would be redundant and would widen the blast radius.
Testing
612 passing at base (
68434cd) -> 616 after. No existing test modified. Four new tests intests/test_list_models_retry.py:asyncio.sleepnever awaited, result unchanged (guards against behavior drift)ruff: 21 findings before and after, diff of the two lists is empty. pyright: 14 before and after, identical messages, line-shifted only.