credential_pool: scope model-attributable 429 cooldowns to (credential, model) - #61663
credential_pool: scope model-attributable 429 cooldowns to (credential, model)#61663darkyy92 wants to merge 3 commits into
Conversation
…l, model) A 429 from a model-specific quota bucket (e.g. the separate Fable/Mythos quota on Anthropic subscriptions) used to mark the whole credential exhausted, adopting that bucket's reset window — parking the credential for hours or days and blocking models whose shared unified budget was still free. With every pool entry tripped the same way, resolution fails pool-wide and every Anthropic model errors out (issue NousResearch#61451). - Attribution: a 429 is treated as model-scoped only when the response's anthropic-ratelimit-unified-*-utilization headers show every unified budget still clearly below its ceiling. Missing or unparseable headers keep the existing credential-wide behavior. - mark_exhausted_and_rotate(model=, model_scoped=) cools only the (credential, model) pair — in-memory, no PooledCredential schema change — and rotation/selection skips entries only for that model. - Wired up in recover_with_credential_pool (main loop) and _recover_provider_pool (auxiliary client); billing/auth paths are untouched. Fixes NousResearch#61451 Claude-Session: https://claude.ai/code/session_013akwLFDAY6WqFwZUN7Zw5e
Competing fix for #61451 with open PR #61481. This PR (5 files, focused) uses an in-memory |
|
Thanks for the focused conservative implementation. The premise is confirmed on current main: Suggested changes
Automated hermes-sweeper review. |
|
The conservative Anthropic-only attribution direction is substantially safer than blanket model scoping, but this is not merge-ready yet. The in-memory cooldown is attached to one Please keep ambiguous/missing-header 429s and every 401/402/403 credential-wide, then make the attributed cooldown survive pool reloads and thread the model through every selection/recovery seam. With those changes plus a real auxiliary recovery regression, this remains salvageable. |
- Share attributed Anthropic model cooldowns across pool instances with profile isolation and locking - Normalize model keys and thread models through selection and recovery paths - Add primary and auxiliary regressions while keeping ambiguous and non-429 failures credential-wide
|
Addressed the latest review in Changes:
Verification:
I intentionally did not persist model cooldowns across process restarts: the conservative design avoids an auth-store schema/migration change; a restart may cause one re-probe, while the requested pool reload/client eviction lifecycle is preserved. |
|
Closing after re-checking the premise against Anthropic's documented rate-limit model and the available subscription-header evidence. The implementation is focused and deliberately conservative, but its key inference is not established: low Changing credential selection on that heuristic could reuse a credential whose applicable shared limit is genuinely exhausted. The in-memory cooldown also disappears when auxiliary recovery reloads the pool, so the patch would not consistently enforce its own decision. Thank you for the careful fail-closed defaults, header parsing, and credential-pool tests. If Anthropic supplies a stable explicit exhausted-limiter identity in captured 429 responses, a narrowly signal-gated fix can be reconsidered with that evidence. |
Fixes #61451.
Problem
A 429 from a model-specific quota bucket (e.g. the separate Fable/Mythos-class quota on Anthropic subscriptions) marks the whole credential
STATUS_EXHAUSTEDand adopts that bucket'sreset_at— which can be days out. The credential is then parked for every model, even ones whose shared unified budget is still free. Once every pool entry trips the same way, token resolution fails pool-wide and all Anthropic models error out.Approach (conservative variant from the issue discussion)
No
PooledCredentialschema change; default behavior is unchanged.anthropic-ratelimit-unified-{5h,7d}-utilizationheaders show every reported unified budget clearly below its ceiling (< 0.98). If a unified budget is at its ceiling, or the headers are missing/unparseable, the existing credential-wide cooldown applies. This mirrors the header-based genuine-vs-upstream distinction already used innous_rate_guard.mark_exhausted_and_rotate(model=…, model_scoped=True)records an in-memory(credential_id, model) → reset_atcooldown instead of calling_mark_exhausted._available_entries/_select_unlockedaccept an optionalmodeland skip entries only for that model, so rotation still moves the rate-limited model to the next credential while the entry stays available for everything else. In-memory is deliberate: losing the map on restart costs at most one extra 429 before it's re-learned.recover_with_credential_pool(main loop; utilizations are captured intoerror_contextbyextract_api_error_context) and_recover_provider_pool(auxiliary client, from the exception's response headers). Billing (402) and auth (401/403) paths are untouched.Tests
tests/agent/test_credential_pool_model_scoped_429.py: model-scoped cooldown blocks only that model and rotates correctly, not persisted to disk, expires, returnsNonewhen all entries are cooled for the model, credential-wide 429 unchanged, header/context heuristic parsing (fractions, percent-style, missing, garbage), andextract_api_error_contextheader capture. Existing routing-test stubs updated for the new kwargs. Pool-related suites pass locally; two pre-existing failures on my machine (test_persist_preserves_concurrent_disk_only_entry,test_remove_index_does_not_resurrect_via_disk_merge) fail identically on clean main.https://claude.ai/code/session_013akwLFDAY6WqFwZUN7Zw5e