fix(retry): treat provider usage-limit errors as terminal, not transient - #1174
fix(retry): treat provider usage-limit errors as terminal, not transient#1174samirhvbr wants to merge 2 commits into
Conversation
A plan/subscription usage-limit rejection (e.g. ChatGPT's 5-hour or weekly cap, HTTP 429 `usage_limit_reached`) was classified as an ordinary transient throttle and retried by both retry layers — the SDK ModelSettings retry (`DEFAULT_MODEL_RETRY`, up to 5x) and Strix's turn replay (`_is_transient_model_error` -> `_run_cycle`, up to 5x). Retrying cannot succeed until the window resets (often hours away); it only burns more of the same exhausted quota and spends minutes thrashing exponential backoff, flooding the log with `openai.agents: Error streaming response` before the run finally stops. Classify usage-limit errors as terminal so the run fails fast and lands on the existing resumable stop path (`run_strix_scan` catches the RateLimitError, logs the `strix --resume <run>` hint, and stops the root): - add `codex.is_usage_limit_error()` (text-marker predicate, mirroring `is_content_guardrail_error`); - exclude it from `_is_transient_model_error` (Strix replay layer); - exclude it from the SDK retry policy via a small wrapper (`_default_model_retry_policy`). Ordinary 429 throttles are unchanged and still retried. Adds regression tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Bump .fork-version to 1.5.3+shvia.2; FORK.md changelog + PR usestrix#1174. - The usage-limit terminal fast-fail (cherry-picked from the upstream PR branch) now lands in the fork's master, so `strix-run --auto` fails over promptly instead of after minutes of doomed retries. - Lower the default --auto threshold to 2 (more eager failover). - escopo: F3 marked done. Note: API keys were migrated out of ~/.bashrc into .env (gitignored). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BJDMregdMDmfDAYotXAHjh
Greptile SummaryThis PR adds text-based detection of provider usage-limit errors and excludes matching failures from both model retry layers so exhausted subscription windows fail quickly.
Confidence Score: 4/5The PR needs adjustment before merging because usage-limit failures from LiteLLM-routed providers can now fail the scan instead of producing the promised resumable stop. Retry suppression is provider-agnostic, while terminal rate-limit recovery is restricted to OpenAI's exception class, leaving supported non-OpenAI routes on the generic failure path. Files Needing Attention: strix/core/execution.py, strix/config/models.py, and strix/core/runner.py Important Files Changed
Prompt To Fix All With AI### Issue 1
strix/core/execution.py:135-136
**Non-OpenAI limits fail**
When a LiteLLM-routed provider raises a marker-bearing usage-limit error, this classification suppresses both retry layers, but `run_strix_scan` only handles `openai.RateLimitError` as resumable. The provider exception therefore reaches the generic failure path, marking the root failed and re-raising instead of showing the resume hint.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(retry): treat provider usage-limit e..." | Re-trigger Greptile |
| if codex.is_usage_limit_error(exc): | ||
| return False |
There was a problem hiding this comment.
When a LiteLLM-routed provider raises a marker-bearing usage-limit error, this classification suppresses both retry layers, but run_strix_scan only handles openai.RateLimitError as resumable. The provider exception therefore reaches the generic failure path, marking the root failed and re-raising instead of showing the resume hint.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/core/execution.py
Line: 135-136
Comment:
**Non-OpenAI limits fail**
When a LiteLLM-routed provider raises a marker-bearing usage-limit error, this classification suppresses both retry layers, but `run_strix_scan` only handles `openai.RateLimitError` as resumable. The provider exception therefore reaches the generic failure path, marking the root failed and re-raising instead of showing the resume hint.
**Knowledge Base Used:**
- [Scan execution lifecycle](https://app.greptile.com/strix-org-3/-/custom-context/knowledge-base/usestrix/strix/-/docs/scan-execution.md)
- [Configuration and provider settings](https://app.greptile.com/strix-org-3/-/custom-context/knowledge-base/usestrix/strix/-/docs/configuration-and-provider-settings.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Pull request overview
This PR updates Strix’s retry behavior to treat provider “usage-limit reached” (plan/subscription window exhausted) errors as terminal rather than transient, preventing repeated retries that cannot succeed until the provider’s window resets.
Changes:
- Add
codex.is_usage_limit_error(exc)predicate based on provider error text markers. - Exclude usage-limit errors from Strix’s execution replay retry (
execution._is_transient_model_error). - Exclude usage-limit errors from the SDK-level model retry policy by composing the existing retry policy with an additional “not usage limit” guard.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
strix/config/codex.py |
Adds is_usage_limit_error and marker definitions for detecting subscription usage-limit rejections. |
strix/core/execution.py |
Treats usage-limit errors as non-transient to stop Strix turn replay retries. |
strix/config/models.py |
Wraps the default SDK retry policy with an additional predicate to prevent retrying usage-limit errors. |
tests/test_model_retry.py |
Adds regression test ensuring usage-limit errors are not retried by the SDK retry policy. |
tests/test_execution_transient_retry.py |
Adds regression tests ensuring usage-limit errors are terminal in Strix replay logic and ordinary 429s remain transient. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Follow-up to the usage-limit terminal classification (review feedback): retry suppression is provider-agnostic (text-marker based), but run_strix_scan only mapped openai.RateLimitError to the resumable stop. A LiteLLM-routed provider that surfaces a usage-limit as a different exception type therefore reached the generic failure path (root marked failed, re-raised) instead of the promised resumable stop with the `strix --resume` hint. Route any error where codex.is_usage_limit_error(exc) is true to the same resumable stop as RateLimitError. Adds a regression test covering a non-OpenAI (LiteLLM-routed) usage-limit exception. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…trix#1174 review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BJDMregdMDmfDAYotXAHjh
Problem
A subscription usage-limit rejection — e.g. ChatGPT's 5-hour / weekly cap,
returned as HTTP 429 with
usage_limit_reached— is currently treated as anordinary transient throttle and retried by both retry layers:
ModelSettingsretry (DEFAULT_MODEL_RETRY, up to 5×), and_is_transient_model_error→_run_cycle, up to 5×).Neither can succeed: the window won't reset for (often) hours. So a run that hits
the cap spends minutes thrashing exponential backoff — flooding the log with
openai.agents: Error streaming response— and burns more of the same exhaustedquota before it finally stops.
Fix
Classify usage-limit errors as terminal so the run fails fast and lands on
the resumable stop path that already exists (
run_strix_scancatches theRateLimitError, logs thestrix --resume <run>hint, stops the root):codex.is_usage_limit_error(exc)— text-marker predicate, mirroring theexisting
is_content_guardrail_error._is_transient_model_error(Strix replay layer)._default_model_retry_policy.Ordinary 429 throttles are unchanged (still retried) — covered by the existing
test_rate_limit_is_retried. New regression tests assert usage-limit is terminaland that a plain 429 is not misclassified.
ruff format,ruff check, and the suite pass locally.🤖 Generated with Claude Code