Skip to content

fix(platform): keep zero-cost fallback models after a credit-exhausted credential (#1454)#1937

Open
larryro wants to merge 1 commit into
mainfrom
tale/xs70hwjzkz4qc2wm75ggaggd3h897zwf
Open

fix(platform): keep zero-cost fallback models after a credit-exhausted credential (#1454)#1937
larryro wants to merge 1 commit into
mainfrom
tale/xs70hwjzkz4qc2wm75ggaggd3h897zwf

Conversation

@larryro

@larryro larryro commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Problem

Closes #1454.

Chat requests fail with "The AI provider's credit limit was exceeded" and no successful response is returned, even when fallback models are configured.

In the common single-credential setup (one OpenRouter key, several models on it), the adaptive fallback loop treats an out-of-funds (credit_exhausted / HTTP 402) failure as a property of the whole credential: it retires credentialScopeKey(provider+key) and skips every remaining model that shares that key. That is correct for paid models — but it also skips zero-cost models (OpenRouter :free variants, or models priced at 0) that draw no credits and would have answered. The turn ends with a failed message and nothing tried.

Fix

Make credit retirement spare zero-cost models, while leaving auth / unreachable retirement untouched.

services/platform/convex/providers/failure_scope.ts:

  • Split the credit scope into its own namespace creditScopeKey() (distinct from the auth credentialScopeKey()) so the dead-set records why a credential died.
  • retiredScopeKey('credit_exhausted', …) now retires the credit scope; auth_error still retires the auth credential; provider_unreachable still retires the endpoint.
  • New isFreeModel() helper: a model is free when its id carries the OpenRouter :free suffix or both token prices are explicitly 0. Unconfigured (undefined) pricing is not treated as free (conservative).
  • isModelScopeRetired() skips a model on a credit-dead credential only when it is not free. Auth-dead and endpoint-dead resources still skip every model (a bad key / down host kills free models too).

The resolved ResolvedModelData already carries modelId / inputCentsPerMillion / outputCentsPerMillion, so no call-site changes were needed.

Tests

failure_scope.test.ts extended: paid sibling still skipped after credit death; :free and zero-priced siblings now attempted; free models still skipped on auth / endpoint death; isFreeModel truth table.

  • vitest --project server providers + agent_chat suites: 230 passed
  • tsc --noEmit: clean
  • oxlint --type-aware: 0 warnings / 0 errors

Summary by CodeRabbit

  • Bug Fixes
    • Improved model fallback strategy to better distinguish between credential authentication failures and credit exhaustion, allowing free models to remain available for selection even when paid options are unavailable due to credit limits.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change addresses credit-exhaustion failures by splitting how model fallback retirement is handled. ScopeModelData gains optional modelId and per-million token pricing fields. isFreeModel detects zero-cost models via an OpenRouter :free model ID suffix or explicit zero input/output pricing. A new creditScopeKey function introduces a credit:-prefixed namespace separate from credentialScopeKey. retiredScopeKey now routes auth_error to the credential scope and credit_exhausted to the credit scope. isModelScopeRetired is updated so that credential and endpoint scope deaths retire all models, but credit-scope deaths retire only paid models—free models remain eligible. Tests and an inline comment are updated accordingly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description provides a clear problem statement, detailed explanation of the fix, affected files, and test results. However, the PR description does not include the required checklist items from the template. Add the template checklist with status for each item (bun run check, lint:sast, translations, docs, README updates) or mark as N/A with reasons.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: keeping zero-cost fallback models available after a credential has exhausted credits, which is the core fix for issue #1454.
Linked Issues check ✅ Passed The PR directly addresses issue #1454 by implementing the fallback mechanism to attempt zero-cost models even after credential credit exhaustion, fully meeting the expected behavior requirements.
Out of Scope Changes check ✅ Passed All changes are scoped to the failure_scope module and its tests, directly supporting the credit exhaustion fallback fix. No unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tale/xs70hwjzkz4qc2wm75ggaggd3h897zwf

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@services/platform/convex/providers/failure_scope.ts`:
- Around line 52-54: The condition checking for free models uses
includes(':free') which performs substring matching rather than suffix matching.
This causes false positives where model IDs like vendor/model:freedom would be
incorrectly classified as free. Replace the includes(':free') method call with
endsWith(':free') to ensure only model IDs that actually end with the :free
suffix are detected, preventing misclassification of similarly named models and
avoiding incorrect credit-scope retirement bypass.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1f7bfc7c-9e31-4631-a6c7-d8b118d9d794

📥 Commits

Reviewing files that changed from the base of the PR and between b418075 and dc0122b.

📒 Files selected for processing (3)
  • services/platform/convex/lib/agent_chat/internal_actions.ts
  • services/platform/convex/providers/failure_scope.test.ts
  • services/platform/convex/providers/failure_scope.ts

Comment on lines +52 to +54
typeof data.modelId === 'string' &&
data.modelId.toLowerCase().includes(':free')
) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use suffix matching for :free detection.

Line 53 uses includes(':free'), but this behavior is defined as a :free suffix check. A model ID like vendor/model:freedom would be misclassified as free and incorrectly bypass credit-scope retirement.

Proposed fix
 export function isFreeModel(data: ScopeModelData): boolean {
   if (
     typeof data.modelId === 'string' &&
-    data.modelId.toLowerCase().includes(':free')
+    data.modelId.toLowerCase().endsWith(':free')
   ) {
     return true;
   }
   return data.inputCentsPerMillion === 0 && data.outputCentsPerMillion === 0;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
typeof data.modelId === 'string' &&
data.modelId.toLowerCase().includes(':free')
) {
export function isFreeModel(data: ScopeModelData): boolean {
if (
typeof data.modelId === 'string' &&
data.modelId.toLowerCase().endsWith(':free')
) {
return true;
}
return data.inputCentsPerMillion === 0 && data.outputCentsPerMillion === 0;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@services/platform/convex/providers/failure_scope.ts` around lines 52 - 54,
The condition checking for free models uses includes(':free') which performs
substring matching rather than suffix matching. This causes false positives
where model IDs like vendor/model:freedom would be incorrectly classified as
free. Replace the includes(':free') method call with endsWith(':free') to ensure
only model IDs that actually end with the :free suffix are detected, preventing
misclassification of similarly named models and avoiding incorrect credit-scope
retirement bypass.

@larryro

larryro commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

Desk Review — #1454 fix (keep zero-cost fallback models after a credit-exhausted credential)

Verdict: READY TO MERGE.

I reviewed the whole implementation (not just the diff), traced both consumers, ran the project's tests, and confirmed CI.

What the fix does, and why it's correct

credit_exhausted previously retired the entire credentialScopeKey, so isModelScopeRetired skipped every sibling model on the same key — including zero-cost ones that draw no credits. That's the root cause of "fallbacks attempted, none succeed." The fix splits credential death into two namespaces — cred: (auth) and credit: (funds) — via the new creditScopeKey, and makes credit retirement conditional: isModelScopeRetired still skips paid siblings but spares free ones (isFreeModel: OpenRouter :free suffix, or explicit 0/0 token pricing). Auth and endpoint deaths still retire free models too (checked first, unconditionally).

Verified:

  • retiredScopeKey auth path is byte-identical to before; only the credit path moved to a new prefix. No code reads the raw cred:/credit: prefixes — both consumers (agent_chat/internal_actions.ts and workflow_engine/.../execute_llm_node.ts) interact only through the helpers, so the split is transparent and consistent across both.
  • ResolvedModelData.modelId is always present and pricing flows from definition.cost?.{input,output}CentsPerMillion, so isFreeModel reads real data. Strict === 0 means unconfigured pricing is correctly not treated as free → paid-but-unpriced models stay retired (safe direction).
  • 402 / credit-phrase errors classify to credit_exhausted (lib/error_classification.ts) ahead of the auth branch — the exact path this fixes.
  • Loop is bounded: each model index is attempted at most once; a free model that itself 402s retires only the credit: scope and doesn't wrongly block other frees or un-block paid siblings.

Tests / CI

  • bunx vitest --run --project server: 501 files, 6135 tests, all pass (no regressions).
  • failure_scope.test.ts: 22/22 pass, covering paid-still-skipped, :free/zero-priced spared, free-still-skipped on auth & endpoint death, and isFreeModel truth table.
  • CI on the PR: 46 pass, 5 skipping (conditional jobs), 0 fail, 0 pending — green.

Scope note (non-blocking)

fallbackModels is purely operator-configured (supportedModels.slice(1)); there's no hardcoded free default. So this fix rescues chat when a zero-cost fallback is configured. A workspace whose only models are paid on the one exhausted key will still fail — but that's correct behavior (no model can run when the funds are gone and everything costs money), not a bug. The fix correctly targets the actual defect: wrongly skipping viable zero-cost siblings.

Optional polish (nits — not blocking)

  1. creditScopeKey vs credentialScopeKey differ by one substring and produce near-identical strings; a typo would silently misroute. Consider fundsScopeKey/authScopeKey for harder-to-confuse names.
  2. isFreeModel uses .includes(':free'); .endsWith(':free') matches the OpenRouter suffix convention more precisely.
  3. Add unit assertions for one-side-zero pricing ({input:0, output:250} → not free) and mixed-case :FREE to lock in the .toLowerCase() path.

None of these affect correctness. Approving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Chat requests fail with "AI provider's credit limit exceeded" – no successful response returned

1 participant