Skip to content

fix(catalog): skip static PROVIDER_MODELS when synced models exist #2625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diegosouzapw merged 1 commit into from
May 23, 2026
Merged

fix(catalog): skip static PROVIDER_MODELS when synced models exist #2625

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions src/app/api/v1/models/catalog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { getAllMusicModels } from "@omniroute/open-sse/config/musicRegistry";
import { REGISTRY } from "@omniroute/open-sse/config/providerRegistry";
import { CODEX_NATIVE_UNPREFIXED_MODELS } from "@omniroute/open-sse/services/model";
import { resolveNestedComboTargets } from "@omniroute/open-sse/services/combo";
import { getAllSyncedAvailableModels } from "@/lib/db/models";
import { getAllSyncedAvailableModels, type SyncedAvailableModel } from "@/lib/db/models";
import { getCompatibleFallbackModels } from "@/lib/providers/managedAvailableModels";
import { hasEligibleConnectionForModel } from "@/domain/connectionModelRules";
import {
Expand Down Expand Up @@ -596,6 +596,22 @@ export async function getUnifiedModelsResponse(
});
}

// Resolve synced available models (from auto-sync) — used to skip static
// PROVIDER_MODELS entries for providers that have a live, API-fresh list.
let syncedModelsByProvider: Record<string, SyncedAvailableModel[]> = {};
try {
syncedModelsByProvider = await getAllSyncedAvailableModels();
} catch (e) {
// DB unavailable — log and fall through; static models remain as defaults.
console.log("[catalog] Could not fetch synced available models:", e);
}
Comment on lines +602 to +607

@gemini-code-assist gemini-code-assist Bot May 23, 2026

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.

medium

The DB call is hoisted correctly, but it's currently redundant because it's called again at line 698. Additionally, this catch block silently swallows errors. It's better to fetch the data once and log any failures for observability.

Suggested change
try {
syncedModelsByProvider = await getAllSyncedAvailableModels();
} catch {
// DB unavailable — fall through, static models remain as defaults.
}
try {
syncedModelsByProvider = await getAllSyncedAvailableModels();
} catch (e) {
console.error("[catalog] Error fetching synced provider models:", e);
}

const providersWithSyncedModels = new Set(
Object.keys(syncedModelsByProvider).filter((pid) => {
const models = syncedModelsByProvider[pid];
return Array.isArray(models) && models.length > 0;
})
);

// Add provider models (chat)
for (const [alias, providerModels] of Object.entries(PROVIDER_MODELS)) {
const providerId = aliasToProviderId[alias] || alias;
Expand All @@ -609,6 +625,10 @@ export async function getUnifiedModelsResponse(
continue;
}

// Skip static models for providers that have synced available models
// (auto-sync provides the authoritative, up-to-date list from the API).
if (providersWithSyncedModels.has(canonicalProviderId)) continue;
Comment on lines +628 to +630

@gemini-code-assist gemini-code-assist Bot May 23, 2026

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.

medium

This change modifies production logic in src/ but does not include any tests. The Repository Style Guide (Rule 9) requires including tests when changing production code. Furthermore, the PR description mentions bypassing Husky hooks, which is a violation of Rule 7. Please add tests to verify that static models are correctly skipped when synced models are present and ensure the PR passes all automated checks.

References
  1. Always include tests when changing production code (src/, open-sse/, electron/, bin/). (link)
  2. Never bypass Husky hooks (--no-verify, --no-gpg-sign) without explicit operator approval. (link)


for (const model of providerModels) {
if (!providerSupportsModel(canonicalProviderId, model.id)) continue;
const aliasId = `${alias}/${model.id}`;
Expand Down Expand Up @@ -676,7 +696,8 @@ export async function getUnifiedModelsResponse(
}

try {
const syncedModelsByProvider = await getAllSyncedAvailableModels();
// Data already loaded above into syncedModelsByProvider; the try block
// here protects the for-loop / model processing from unexpected errors.
for (const [providerId, syncedModels] of Object.entries(syncedModelsByProvider)) {
if (!Array.isArray(syncedModels) || syncedModels.length === 0) continue;
if (blockedProviders.has(providerId)) continue;
Expand Down