Skip to content

fix: use fallback chain when connected-providers cache missing#1314

Open
evsong wants to merge 1 commit into
code-yeongyu:devfrom
evsong:fix/category-model-ignored
Open

fix: use fallback chain when connected-providers cache missing#1314
evsong wants to merge 1 commit into
code-yeongyu:devfrom
evsong:fix/category-model-ignored

Conversation

@evsong
Copy link
Copy Markdown

@evsong evsong commented Jan 31, 2026

Summary

  • Fix category model configurations being ignored when connected-providers.json cache doesn't exist
  • Use first fallback chain entry instead of skipping to systemDefaultModel

Problem

When connected-providers.json cache is missing (first run or cache cleared), resolveModelWithFallback() was skipping the entire fallback chain and falling through to systemDefaultModel. This caused all category-specific model configurations (quick, visual-engineering, ultrabrain, etc.) to be ignored.

Related Issues: #1264, #1295

Solution

When cache is null, use the first entry in the fallback chain instead of skipping:

if (connectedSet === null) {
  const firstEntry = fallbackChain[0]
  if (firstEntry && firstEntry.providers.length > 0) {
    const model = `${firstEntry.providers[0]}/${firstEntry.model}`
    return { model, source: "provider-fallback", variant: firstEntry.variant }
  }
}

Testing

  • Updated 2 existing tests to reflect new behavior
  • All 46 tests pass

Summary by cubic

Fixes category-specific model selection when connected-providers.json is missing by using the first fallback chain entry instead of the system default. Ensures categories like quick, visual-engineering, and ultrabrain use their intended models on first run or after cache clear.

  • Bug Fixes
    • When no connected-providers cache exists, resolveModelWithFallback now picks the first fallback entry (preserving variant) and marks source as provider-fallback.
    • Updated tests to reflect the new behavior.

Written for commit 7ef706f. Summary will update on new commits.

When connected-providers.json cache doesn't exist (first run or cache cleared),
the model resolver was skipping the entire fallback chain and falling through
to systemDefaultModel. This caused all category-specific model configurations
to be ignored.

Fix: Use the first entry in fallback chain instead of skipping to systemDefault.
This ensures category models (quick, visual-engineering, etc.) work correctly
even without the cache.

Fixes code-yeongyu#1264
@github-actions
Copy link
Copy Markdown
Contributor

Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement (CLA).

To sign the CLA, please comment on this PR with:

I have read the CLA Document and I hereby sign the CLA

This is a one-time requirement. Once signed, all your future contributions will be automatically accepted.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@code-yeongyu
Copy link
Copy Markdown
Owner

[sisyphus-bot]

Thanks for this fix, @evsong!

Code Review Summary:

This correctly addresses the bug where category-specific model configurations were being ignored when connected-providers.json cache doesn't exist.

The Fix:

  • When connectedSet === null (no cache), instead of skipping the fallback chain entirely, the code now uses the first fallback entry
  • This preserves the variant field (e.g., "max" for claude-opus) which is critical for category-specific behavior
  • The model is correctly returned as {provider}/{model} with source "provider-fallback"

Test Updates:

  • Tests correctly updated to expect the first fallback entry instead of undefined/system-default

Before Merge:

  1. ⚠️ CLA signature required — Please comment: I have read the CLA Document and I hereby sign the CLA
  2. Human review pending — Needs maintainer approval

Once CLA is signed and approved, this is safe to merge.

Related: #1264, #1295

@code-yeongyu
Copy link
Copy Markdown
Owner

[sisyphus-bot]

PR sweep first-pass triage on dev.

  • Author: evsong
  • Title: fix: use fallback chain when connected-providers cache missing
  • Closing issues: none detected
  • Review decision: none yet
  • Mergeable state: CONFLICTING (rebase needed)
  • CI status: failing (cla)

Needs rebase + review. Please rebase onto current dev first; then a review can land cleanly.

Assigning code-yeongyu so the maintainer can prioritize a focused review of this PR. If the linked issue above has been triaged, the verdict there is the authoritative direction for this change.

@code-yeongyu code-yeongyu self-assigned this May 16, 2026
@code-yeongyu
Copy link
Copy Markdown
Owner

[sisyphus-bot] Hi evsong. 🙏 Thanks for the connected-providers cache-missing fallback fix.

Picking this back up from the 5/16 triage. The PR is small (37/24 across 2 files) and shows CONFLICTING against current dev. The connected-providers cache layer has been touched substantially on dev (especially around the recent free-model fallback work in #3821), so the diff doesn't apply cleanly anymore.

The fix (when readProviderModelsCache() returns null, walk the model-requirements fallback chain instead of failing) is still valid in concept. Could you rebase against current dev? Once it's clean I'll come back for a focused review. If after rebase the fallback chain you're invoking has changed shape (the chain matrix has expanded since January), the rebase may need light re-derivation.

Sorry for the long pause; really appreciate the careful cache-missing handling.

@code-yeongyu code-yeongyu force-pushed the dev branch 2 times, most recently from eb25d29 to 2bfad49 Compare May 23, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage:bug Confirmed bug with repro steps

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants