Repository navigation
fix(combo): scope Claude model failures to the model, not the account - #12340
Conversation
A priority combo whose steps are five models on one Claude OAuth connection stops at step 1. hasPerModelQuota() returns false for the claude provider, so markAccountUnavailable() records a model-specific 404 or 5xx against the connection row, and getPersistedConnectionCooldownSkipReason() then skips every sibling step before dispatch. Verified with a combo whose first step names a model that cannot exist: the 404 lands and step 2 is never tried, while the same combo works as soon as step 2 points at a different account. A Claude OAuth connection multiplexes Fable 5, Opus 5/4.8/4.7/4.6, Sonnet and Haiku behind one credential, which is the multiplexing hasPerModelQuota already describes. Its quota is a separate question: a 429 on a Max subscription is account-wide, and shouldMarkAccountExhaustedFrom429 pins that. So this adds hasPerModelFailureScope() for the non-quota statuses and leaves 429 alone. Combo exhaustion gets the same treatment for 404, which names one model the account cannot serve rather than a bad connection. The empty-content 502 was already exempt through isEmptyContentFailure. Closes cheaper-inference#12334
…-per-model-failure-scope
…caps Move hasPerModelFailureScope into a leaf so frozen accountFallback.ts and auth.ts stay at their file-size caps after cheaper-inference#12334. Register combo-claude-per-model-scope.test.ts in Stryker tap.testFiles so the mutation-test-coverage gate sees the covering unit test. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
|
Would this allow harnesses (like CC) to see that fable is 429ing, and automatically change to Opus, like it normally does in CC (without a proxy)? |
|
@cryptiklemur Not for a 429 in this PR. OmniRoute still treats Claude 429s as account-wide, so another model on the same OAuth connection remains on cooldown. This fixes model-specific failures such as a 404, allowing the combo to try the next model on that connection. Distinguishing a Fable-only limit from a shared account limit would need a separate change. |
|
Good fix — a Claude OAuth connection multiplexing Fable/Opus/Sonnet/Haiku |
|
Thanks @Kizuno18 — merging via the release merge-train. Validated in local merge-train (.claude/worktrees/merge-train-20260918-111718-suite.log) on the devbox @ train tip 7bb373fba5e241964c0ffb17bd03a804700839bb, boarded with 55 sibling PRs: typecheck:core, file-size, complexity, cognitive-complexity, changelog-integrity green; 747/747 changed-area node:test cases + 476/476 vitest green (fast parity mode — the full suite ran today on the release tip via the base-red train and runs again on the 3b train). Merged --admin per merge-gates §7. |
6d0dc5a
into
cheaper-inference:release/v3.8.51
…cheaper-inference#12340) * fix(combo): scope Claude model failures to the model, not the account A priority combo whose steps are five models on one Claude OAuth connection stops at step 1. hasPerModelQuota() returns false for the claude provider, so markAccountUnavailable() records a model-specific 404 or 5xx against the connection row, and getPersistedConnectionCooldownSkipReason() then skips every sibling step before dispatch. Verified with a combo whose first step names a model that cannot exist: the 404 lands and step 2 is never tried, while the same combo works as soon as step 2 points at a different account. A Claude OAuth connection multiplexes Fable 5, Opus 5/4.8/4.7/4.6, Sonnet and Haiku behind one credential, which is the multiplexing hasPerModelQuota already describes. Its quota is a separate question: a 429 on a Max subscription is account-wide, and shouldMarkAccountExhaustedFrom429 pins that. So this adds hasPerModelFailureScope() for the non-quota statuses and leaves 429 alone. Combo exhaustion gets the same treatment for 404, which names one model the account cannot serve rather than a bad connection. The empty-content 502 was already exempt through isEmptyContentFailure. Closes cheaper-inference#12334 * fix(combo): extract Claude per-model failure scope to keep file-size caps Move hasPerModelFailureScope into a leaf so frozen accountFallback.ts and auth.ts stay at their file-size caps after cheaper-inference#12334. Register combo-claude-per-model-scope.test.ts in Stryker tap.testFiles so the mutation-test-coverage gate sees the covering unit test. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Summary
A priority combo whose steps are five different models on one Claude OAuth connection stops at step 1: whatever kills the first step is recorded against the connection, and the pre-dispatch cooldown gate then skips every later step because they share that connection. The drop order is silently a single-model route.
hasPerModelQuota()returns false forclaude, so inmarkAccountUnavailable()the model-lockout branch never runs and the failure lands on the connection row instead.getPersistedConnectionCooldownSkipReason()takes it from there.To keep the model-specific bug in #12334 out of the picture, I reproduced it with a first step naming a model that cannot exist. The 404 lands and step 2 is never dispatched; point step 2 at a different account and the same combo works:
A Claude OAuth connection multiplexes Fable 5, Opus 5/4.8/4.7/4.6, Sonnet and Haiku behind one credential — the multiplexing the doc comment on
hasPerModelQuotaalready describes. Its quota is a different question, though: a 429 on a Max subscription is account-wide, andshouldMarkAccountExhaustedFrom429("claude", …)pins that inaccount-fallback-service.test.ts. AddingclaudetohasPerModelQuotabreaks that test, and rightly so.So this splits the two: a new
hasPerModelFailureScope()covers the non-quota statuses (404, 5xx, a stream that ended with no content),auth.tsuses it only whenstatus !== 429, and quota scope stays exactly as it was.targetExhaustiongets the same treatment for 404, which names one model the account cannot serve rather than a bad connection — the empty-content 502 was already exempt throughisEmptyContentFailure.Related Issues
Validation
tests/unit/combo-claude-per-model-scope.test.tsplus every existing test touchinghasPerModelQuotaortargetExhaustion— 17 files, 226 passing, 0 failingnpx eslinton the four touched files: same 3 pre-existingno-unused-varserrors inaccountFallback.tsbefore and after (line 2413 shifts to 2437), no new onesrelease/v3.8.51Full unit suite on this branch: 47 failures. On the same checkout with the patch stashed: 46. The one extra is
Vietnamese locale has no internal missing markers or empty fallbacks, which fails on a clean tree when run on its own:so it is suite variance, not this change.
Tests Added Or Updated
tests/unit/combo-claude-per-model-scope.test.ts(new) — mirrorscombo-provider-cooldown-sibling.test.ts: failure scope forclaude, quota scope unchanged, the connection-level override still winning, and the two sides of the pre-dispatch gate (a real cooldown still protects siblings; no cooldown lets step 2 dispatch).Coverage Notes
hasPerModelFailureScope()is covered by the new file. Theauth.tsandtargetExhaustion.tscall sites are covered indirectly by the existing lockout tests for gemini, github, nvidia, ollama, openrouter, vertex and agentrouter, which all still pass — they pin that nothing outsideclaudechanged scope.Reviewer Notes
The risk worth looking at is the 429 split. I deliberately left quota scope alone so a Max rate limit keeps retiring the account, since dropping to another model there would just burn a second upstream call. If you would rather have per-model quota for
claudetoo, that is a one-line change tohasPerModelQuotaplus an update to theshouldMarkAccountExhaustedFrom429expectation — happy to do it either way.Reproduced against
diegosouzapw/omniroute:main@sha256:88996a83and the previoussha256:611c555b, on a singleclaude_maxOAuth connection.