fix(runtime-fallback): make fallback provider selection provider-agnostic (fixes #2303)#2695
Conversation
|
I have read the CLA Document and I hereby sign the CLA |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92c59ea55f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const preferredProvider = state.providerID.toLowerCase() | ||
| return connectedSet.has(preferredProvider) |
There was a problem hiding this comment.
This makes getNextFallback treat an entry as reachable whenever the current provider is still connected, even if the entry only names a provider-specific model. I checked src/shared/model-requirements.ts and several built-in chains contain single-provider models such as kimi-for-coding/k2p5, zai-coding-plan/glm-4.6v, and github-copilot/grok-code-fast-1, while src/shared/provider-model-id-transform.ts only rewrites Google/Copilot aliases. In the common case where anthropic is still connected but kimi-for-coding is not, this path now selects anthropic/k2p5, which will fail with a model-not-found error instead of skipping to the next valid fallback.
Useful? React with 👍 / 👎.
| if (entry.providers.some((provider) => connectedSet.has(provider.toLowerCase()))) { | ||
| return true | ||
| } | ||
| return preferredProvider ? connectedSet.has(preferredProvider) : false |
There was a problem hiding this comment.
The same reachability relaxation in tryFallbackRetry now keeps entries whose listed providers are disconnected as long as the task's current provider is connected. That is unsafe for provider-specific fallbacks in src/shared/model-requirements.ts like kimi-for-coding/k2p5 or zai-coding-plan/glm-4.6v: selectFallbackProvider will reuse the current provider, and src/shared/provider-model-id-transform.ts has no mapping for those aliases, so the background retry gets queued with an impossible model such as anthropic/k2p5 and burns an attempt before any real fallback can run.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 4 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Fixes fallback reachability logic by allowing the current connected provider to be used even if entry providers are listed as disconnected. Includes regression tests.
MoerAI
force-pushed
the
fix/provider-agnostic-fallback
branch
from
92c59ea to
0e610a7
Compare
|
I have read the CLA Document and I hereby sign the CLA |
2 participants
Summary
Problem
Runtime/model fallback provider selection was nominally provider-agnostic in
selectFallbackProvider, but upstream reachability gating could skip fallback entries before selection happened. If an entry's provider list was disconnected while the current provider remained connected, fallback progression stopped early instead of reusing the connected provider.Fix
Updated reachability checks in both model fallback hook and background fallback retry path to treat a fallback entry as reachable when either (a) any entry provider is connected or (b) the current/preferred provider is connected. Also normalized connectivity checks to lowercase to avoid case-sensitive mismatches.
Changes
src/hooks/model-fallback/hook.tssrc/hooks/model-fallback/hook.test.tssrc/features/background-agent/fallback-retry-handler.tssrc/features/background-agent/fallback-retry-handler.test.tsFixes #2303
Summary by cubic
Make fallback selection provider-agnostic and case-insensitive, continuing via the connected preferred provider when listed fallback providers are disconnected. Fixes #2303.
Written for commit 0e610a7. Summary will update on new commits.