From 928f053610b0da647cba21d19794cf5d91adc169 Mon Sep 17 00:00:00 2001 From: ian-at-strix Date: Wed, 7 Oct 2026 21:35:33 -0400 Subject: [PATCH] feat(llm): retry 4xx model errors (#1486) * feat(llm): log the upstream provider behind OpenRouter LiteLLM drops OpenRouter's top-level `provider` from stream chunks (including error chunks) and the `metadata.provider_name` of non-2xx replies, so the request log could not say which provider served or failed an attempt. Record both, plus OpenRouter's `metadata.error_type`, per attempt and emit them as `upstream=` / `upstream_error=` on the llm_request line. * feat(llm): retry 4xx model errors * test: let the blocked-provider test run out of 4xx retries * refactor: retry any 4xx or 5xx status * fix: don't retry 401, 402 or 403 * fix: don't retry 404 --- strix/core/execution.py | 4 +--- tests/test_execution_transient_retry.py | 14 ++++++-------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/strix/core/execution.py b/strix/core/execution.py index 508eccfe..5997e3db 100644 --- a/strix/core/execution.py +++ b/strix/core/execution.py @@ -184,9 +184,7 @@ def _is_transient_model_error(exc: BaseException) -> bool: return True code = _model_error_status_code(exc) if code is not None: - import litellm - - return bool(litellm._should_retry(code)) + return code >= 400 and code not in (401, 402, 403, 404) return isinstance(exc, APIError) diff --git a/tests/test_execution_transient_retry.py b/tests/test_execution_transient_retry.py index 57b26db3..f0276932 100644 --- a/tests/test_execution_transient_retry.py +++ b/tests/test_execution_transient_retry.py @@ -79,12 +79,13 @@ def test_content_guardrail_is_not_retried() -> None: assert execution._is_transient_model_error(guardrail) is False -def test_client_errors_are_not_transient() -> None: +def test_client_errors_are_transient() -> None: bad_request = BadRequestError( "bad", response=httpx.Response(400, request=_request()), body=None ) - assert execution._is_transient_model_error(bad_request) is False - assert execution._is_transient_model_error(_status_error(404)) is False + assert execution._is_transient_model_error(bad_request) is True + for status in (401, 402, 403, 404): + assert execution._is_transient_model_error(_status_error(status)) is False assert execution._is_transient_model_error(ValueError("nope")) is False @@ -167,11 +168,8 @@ async def test_run_cycle_gives_up_after_max_retries( async def test_run_cycle_does_not_retry_permanent_error( monkeypatch: pytest.MonkeyPatch, ) -> None: - bad_request = BadRequestError( - "bad", response=httpx.Response(400, request=_request()), body=None - ) - streams = [_FakeStream(exc=bad_request), _FakeStream()] - with pytest.raises(BadRequestError): + streams = [_FakeStream(exc=ValueError("nope")), _FakeStream()] + with pytest.raises(ValueError, match="nope"): await _run_once(monkeypatch, streams)