From 6b5590e8f45f6cb2d65009d1a178ace8128e173c Mon Sep 17 00:00:00 2001 From: Hernan Javier Ardila Sanchez Date: Sun, 21 Jun 2026 23:34:01 +0200 Subject: [PATCH] fix(auto-combo): respect model visibility (isHidden) in auto-combo candidate pool (#4558) Integrated into release/v3.8.33 --- config/quality/file-size-baseline.json | 5 +- open-sse/services/autoCombo/virtualFactory.ts | 6 + open-sse/services/combo.ts | 8 +- .../settings/components/ComboDefaultsTab.tsx | 266 ++++++++++++++---- src/lib/db/models.ts | 38 +++ src/lib/localDb.ts | 1 + src/models/index.ts | 1 + .../auto-combo-hidden-models-4558.test.ts | 81 ++++++ 8 files changed, 347 insertions(+), 59 deletions(-) create mode 100644 tests/unit/auto-combo-hidden-models-4558.test.ts diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 460edee2bc..a45f92b833 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -129,7 +129,7 @@ "open-sse/services/batchProcessor.ts": 828, "open-sse/services/browserBackedChat.ts": 850, "open-sse/services/claudeCodeCompatible.ts": 1202, - "open-sse/services/combo.ts": 2695, + "open-sse/services/combo.ts": 2701, "open-sse/services/rateLimitManager.ts": 1035, "open-sse/services/tokenRefresh.ts": 1997, "open-sse/services/usage.ts": 3450, @@ -162,6 +162,7 @@ "src/app/(dashboard)/dashboard/providers/page.tsx": 1927, "src/app/(dashboard)/dashboard/runtime/RuntimePageClient.tsx": 1198, "src/app/(dashboard)/dashboard/settings/components/AppearanceTab.tsx": 819, + "src/app/(dashboard)/dashboard/settings/components/ComboDefaultsTab.tsx": 846, "src/app/(dashboard)/dashboard/settings/components/CompressionSettingsTab.tsx": 974, "src/app/(dashboard)/dashboard/settings/components/MemorySkillsTab.tsx": 898, "src/app/(dashboard)/dashboard/settings/components/PricingTab.tsx": 1012, @@ -181,7 +182,7 @@ "src/lib/db/apiKeys.ts": 1662, "src/lib/db/core.ts": 1825, "src/lib/db/migrationRunner.ts": 1125, - "src/lib/db/models.ts": 1221, + "src/lib/db/models.ts": 1259, "src/lib/db/providers.ts": 1050, "src/lib/db/proxies.ts": 1048, "src/lib/db/settings.ts": 1149, diff --git a/open-sse/services/autoCombo/virtualFactory.ts b/open-sse/services/autoCombo/virtualFactory.ts index 1891396ac8..fa535982c5 100644 --- a/open-sse/services/autoCombo/virtualFactory.ts +++ b/open-sse/services/autoCombo/virtualFactory.ts @@ -17,6 +17,7 @@ import { type AutoCategory, type AutoTier, } from "./suffixComposition"; +import { getHiddenModelsByProvider } from "@/models"; /** #4235 Phase B: optional category/tier overlay for `auto/:` combos. */ export interface AutoComboSpec { @@ -235,6 +236,7 @@ export async function createVirtualAutoCombo( const blockedProviders = new Set( Array.isArray(settings.blockedProviders) ? (settings.blockedProviders as string[]) : [] ); + const hiddenModelsMap = getHiddenModelsByProvider(); const validConnections = connections.filter(hasUsableConnectionCredential); @@ -250,6 +252,10 @@ export async function createVirtualAutoCombo( } if (!modelId) continue; // Skip providers without a model + // Skip models that the user has hidden in the dashboard + const hiddenModels = hiddenModelsMap.get(conn.provider); + if (hiddenModels?.has(modelId)) continue; + candidatePool.push({ provider: conn.provider, connectionId: conn.id, diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index d007dffdb7..e6ddd03c07 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -42,6 +42,7 @@ import { getHandoff, } from "../../src/lib/db/contextHandoffs.ts"; import { extractSessionAffinityKey } from "@/sse/services/auth"; +import { getHiddenModelsByProvider } from "@/models"; import { resolveModelLockoutSettings } from "../../src/lib/resilience/modelLockoutSettings"; import { fetchCodexQuota } from "./codexQuotaFetcher.ts"; import { @@ -318,6 +319,7 @@ export async function buildAutoCandidates( resetWindowConfig: ResetWindowConfig = resolveResetWindowConfig(null), resilienceSettings: ResilienceSettings | null = null ): Promise { + const hiddenModelsMap = getHiddenModelsByProvider(); const metrics = getComboMetrics(comboName); // Opt-in hard quota cutoff (default OFF). When disabled, candidates are never // dropped for low quota here — the soft quota penalty + connection cooldown still @@ -513,7 +515,11 @@ export async function buildAutoCandidates( }) ); - return candidates; + // Filter out candidates whose model is hidden by the user in the dashboard + return candidates.filter((c) => { + const hiddenModels = hiddenModelsMap.get(c.provider); + return !hiddenModels?.has(c.model); + }); } /** diff --git a/src/app/(dashboard)/dashboard/settings/components/ComboDefaultsTab.tsx b/src/app/(dashboard)/dashboard/settings/components/ComboDefaultsTab.tsx index 9ee7c5f14b..86a633473e 100644 --- a/src/app/(dashboard)/dashboard/settings/components/ComboDefaultsTab.tsx +++ b/src/app/(dashboard)/dashboard/settings/components/ComboDefaultsTab.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState, useEffect } from "react"; +import { useState, useEffect, useRef } from "react"; import { Card, Button, Input, Toggle } from "@/shared/components"; import { cn } from "@/shared/utils/cn"; import { @@ -102,7 +102,13 @@ export default function ComboDefaultsTab() { }); const [codexSessionAffinityTtlMs, setCodexSessionAffinityTtlMs] = useState(0); const [providerOverrides, setProviderOverrides] = useState({}); - const [newOverrideProvider, setNewOverrideProvider] = useState(""); + const [availableProviders, setAvailableProviders] = useState<{ id: string; provider: string }[]>( + [] + ); + const [dropdownOpen, setDropdownOpen] = useState(false); + const [searchQuery, setSearchQuery] = useState(""); + const [highlightedIdx, setHighlightedIdx] = useState(0); + const dropdownRef = useRef(null); const [saving, setSaving] = useState(false); const [status, setStatus] = useState<{ type: "success" | "error" | ""; message: string }>({ type: "", @@ -129,6 +135,27 @@ export default function ComboDefaultsTab() { Promise.all([ fetch("/api/settings/combo-defaults").then((res) => res.json()), fetch("/api/settings").then((res) => res.json()), + fetch("/api/providers") + .then((res) => res.json()) + .then((providers: any[]) => { + // Filter: include a provider only if at least one of its connections is active. + // Disabled providers (all connections inactive) are excluded. + const byProvider = new Map(); + for (const p of providers) { + if (!p.provider) continue; + const list = byProvider.get(p.provider) || []; + list.push(p); + byProvider.set(p.provider, list); + } + const activeProviders = Array.from(byProvider.entries()) + .filter(([, conns]) => conns.some((c) => c.isActive !== false)) + .map(([name]) => name) + .sort(); + setAvailableProviders(activeProviders.map((p) => ({ id: p, provider: p }))); + }) + .catch(() => { + /* providers fetch is non-critical */ + }), ]) .then(([comboData, settingsData]) => { setComboDefaults((prev) => ({ @@ -153,6 +180,17 @@ export default function ComboDefaultsTab() { .catch((err) => console.error("Failed to fetch combo defaults:", err)); }, []); + // Close dropdown on outside click + useEffect(() => { + const handleClickOutside = (e: MouseEvent) => { + if (dropdownRef.current && !dropdownRef.current.contains(e.target as Node)) { + setDropdownOpen(false); + } + }; + document.addEventListener("mousedown", handleClickOutside); + return () => document.removeEventListener("mousedown", handleClickOutside); + }, []); + const showStatus = (type: "success" | "error", message: string) => { setStatus({ type, message }); setTimeout(() => setStatus({ type: "", message: "" }), 2500); @@ -204,14 +242,16 @@ export default function ComboDefaultsTab() { } }; - const addProviderOverride = () => { - const name = newOverrideProvider.trim().toLowerCase(); - if (!name || providerOverrides[name]) return; - setProviderOverrides((prev) => ({ ...prev, [name]: { maxRetries: 1 } })); - setNewOverrideProvider(""); + const addProviderOverride = (name: string) => { + const trimmed = name.trim().toLowerCase(); + if (!trimmed || providerOverrides[trimmed]) return; + setProviderOverrides((prev) => ({ ...prev, [trimmed]: { maxRetries: 1 } })); + setDropdownOpen(false); + setSearchQuery(""); + setHighlightedIdx(0); }; - const removeProviderOverride = (provider) => { + const removeProviderOverride = (provider: string) => { setProviderOverrides((prev) => { const copy = { ...prev }; delete copy[provider]; @@ -219,6 +259,55 @@ export default function ComboDefaultsTab() { }); }; + // Reorder a provider override by rebuilding the object in the new order. + // direction: -1 = move up, +1 = move down + const moveProviderOverride = (provider: string, direction: -1 | 1) => { + setProviderOverrides((prev) => { + const keys = Object.keys(prev); + const idx = keys.indexOf(provider); + if (idx < 0) return prev; + const target = idx + direction; + if (target < 0 || target >= keys.length) return prev; + // Swap positions + [keys[idx], keys[target]] = [keys[target], keys[idx]]; + // Rebuild object in new order + const reordered: Record = {}; + for (const k of keys) { + reordered[k] = prev[k]; + } + return reordered; + }); + }; + + // Filtered provider list — excludes already-added ones, filtered by search query + const filteredProviders = availableProviders.filter( + (p) => + !providerOverrides[p.provider] && p.provider.toLowerCase().includes(searchQuery.toLowerCase()) + ); + + const handleDropdownKeyDown = (e: React.KeyboardEvent) => { + switch (e.key) { + case "ArrowDown": + e.preventDefault(); + setHighlightedIdx((prev) => Math.min(prev + 1, filteredProviders.length - 1)); + break; + case "ArrowUp": + e.preventDefault(); + setHighlightedIdx((prev) => Math.max(prev - 1, 0)); + break; + case "Enter": + e.preventDefault(); + if (filteredProviders[highlightedIdx]) { + addProviderOverride(filteredProviders[highlightedIdx].provider); + } + break; + case "Escape": + e.preventDefault(); + setDropdownOpen(false); + break; + } + }; + return (
@@ -625,57 +714,122 @@ export default function ComboDefaultsTab() {

{t("providerOverrides")}

{t("providerOverridesDesc")}

- {Object.entries(providerOverrides).map(([provider, config]: [string, any]) => ( -
- {provider} - - setProviderOverrides((prev) => ({ - ...prev, - [provider]: { ...prev[provider], maxRetries: parseInt(e.target.value) || 0 }, - })) - } - className="text-xs w-16" - aria-label={t("providerMaxRetriesAria", { provider })} - /> - {t("retries")} - -
- ))} + {/* Reorder arrows (combo-builder pattern) */} +
+ + +
+ {provider} + + setProviderOverrides((prev) => ({ + ...prev, + [provider]: { ...prev[provider], maxRetries: parseInt(e.target.value) || 0 }, + })) + } + className="text-xs w-16" + aria-label={t("providerMaxRetriesAria", { provider })} + /> + {t("retries")} + +
+ ) + )} -
- setNewOverrideProvider(e.target.value)} - onKeyDown={(e) => e.key === "Enter" && addProviderOverride()} - className="text-xs flex-1" - aria-label={t("newProviderNameAria")} - /> - + + {t("selectProviderPlaceholder") || "Select provider..."} + + + expand_more + + + + {dropdownOpen && ( +
+
+ { + setSearchQuery(e.target.value); + setHighlightedIdx(0); + }} + className="w-full px-2 py-1.5 text-xs rounded-md border border-border/50 bg-transparent outline-none focus:border-amber-500 transition-colors" + placeholder={t("searchProviderPlaceholder") || "Search providers..."} + aria-label={t("searchProviderAria") || "Search providers"} + onKeyDown={handleDropdownKeyDown} + autoFocus + /> +
+
    + {filteredProviders.length === 0 ? ( +
  • + {availableProviders.filter((p) => !providerOverrides[p.provider]).length === 0 + ? "All providers added" + : "No providers found"} +
    • + ) : ( + filteredProviders.map((p, idx) => ( +
  • addProviderOverride(p.provider)} + onMouseEnter={() => setHighlightedIdx(idx)} + > + {p.provider} +
    • + )) + )} +
+
+ )}
diff --git a/src/lib/db/models.ts b/src/lib/db/models.ts index 35afc39a85..d4cfc6b2dc 100644 --- a/src/lib/db/models.ts +++ b/src/lib/db/models.ts @@ -1101,6 +1101,44 @@ export function getModelIsHidden(providerId: string, modelId: string): boolean { return Boolean(co?.isHidden); } +/** + * Get a map of provider ID → set of hidden model IDs from all modelCompatOverrides + * and customModels. Used by auto-combo candidate building to skip user-hidden models. + * Single bulk DB query — not N+1 per model. + */ +export function getHiddenModelsByProvider(): Map> { + const db = getDbInstance(); + const result = new Map>(); + + // Query all rows from key_value for both namespaces + const rows = db + .prepare( + "SELECT key, value FROM key_value WHERE namespace IN ('modelCompatOverrides', 'customModels')" + ) + .all() as Array<{ key: string; value: string | null }>; + + for (const row of rows) { + if (!row.value) continue; + try { + const parsed = JSON.parse(row.value); + if (!Array.isArray(parsed)) continue; + for (const entry of parsed) { + if (entry && typeof entry === "object" && entry.isHidden) { + const modelId = entry.id; + if (typeof modelId === "string" && modelId.length > 0) { + if (!result.has(row.key)) result.set(row.key, new Set()); + result.get(row.key)!.add(modelId); + } + } + } + } catch { + // Skip malformed entries + } + } + + return result; +} + /** * #3782 — Check if a model was DELETED (trash) rather than merely eye-hidden. * diff --git a/src/lib/localDb.ts b/src/lib/localDb.ts index eee2234a10..a581e56b79 100755 --- a/src/lib/localDb.ts +++ b/src/lib/localDb.ts @@ -64,6 +64,7 @@ export { getModelUpstreamExtraHeaders, getModelIsHidden, setModelIsHidden, + getHiddenModelsByProvider, // Synced Available Models getSyncedAvailableModels, diff --git a/src/models/index.ts b/src/models/index.ts index 929981be86..09147c7b60 100755 --- a/src/models/index.ts +++ b/src/models/index.ts @@ -24,4 +24,5 @@ export { validateApiKey, isCloudEnabled, resolveProxyForProvider, + getHiddenModelsByProvider, } from "@/lib/localDb"; diff --git a/tests/unit/auto-combo-hidden-models-4558.test.ts b/tests/unit/auto-combo-hidden-models-4558.test.ts new file mode 100644 index 0000000000..03fd3dcb0b --- /dev/null +++ b/tests/unit/auto-combo-hidden-models-4558.test.ts @@ -0,0 +1,81 @@ +/** + * #4558 — Auto-combo must respect model visibility (isHidden). + * + * When an operator hides a model with the EYE/visibility toggle + * (`mergeModelCompatOverride(provider, model, { isHidden: true })`, written to + * the `modelCompatOverrides` key_value namespace), that model must be excluded + * from the AUTO-combo candidate pool. Both auto paths consume the same seam: + * - `open-sse/services/combo.ts::buildAutoCandidates` (combo.ts:322,520-521) + * - `open-sse/services/autoCombo/virtualFactory.ts` (virtualFactory.ts:239,256-257) + * via the new bulk `getHiddenModelsByProvider()` map (single query, not N+1). + * + * This guards that seam: a hidden model is present in the map's per-provider set + * while a visible sibling is not, and that toggling visibility back off + * (`isHidden: null`) removes it from the map again. Without the fix the map is + * empty and the auto pool would keep serving hidden models. + */ +import test, { before, after } from "node:test"; +import assert from "node:assert/strict"; +import os from "node:os"; +import path from "node:path"; +import fs from "node:fs"; + +// Hermetic DB: this test writes overrides into the `modelCompatOverrides` +// key_value namespace. Without an isolated DATA_DIR it would leak that state +// into the shared dev/CI database. Point DATA_DIR at a throwaway dir before any +// import that opens the SQLite handle (CLAUDE.md "Database Handles in Tests"). +const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-test-hidden-4558-")); +process.env.DATA_DIR = tmpDir; + +const { mergeModelCompatOverride, getHiddenModelsByProvider, getModelIsHidden } = await import( + "../../src/lib/localDb.ts" +); +const { resetDbInstance } = await import("../../src/lib/db/core.ts"); + +before(() => { + resetDbInstance(); +}); + +after(() => { + resetDbInstance(); + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +const PROVIDER = "openai"; +const HIDDEN_MODEL = "gpt-hidden-preview"; +const VISIBLE_MODEL = "gpt-visible-4o"; + +test("getHiddenModelsByProvider: empty before any model is hidden", () => { + const map = getHiddenModelsByProvider(); + assert.equal(map.get(PROVIDER)?.has(HIDDEN_MODEL) ?? false, false); +}); + +test("a hidden model lands in the provider's hidden set; a visible sibling does not", () => { + // Hide one model, leave a sibling visible (overridden for an unrelated reason). + mergeModelCompatOverride(PROVIDER, HIDDEN_MODEL, { isHidden: true }); + mergeModelCompatOverride(PROVIDER, VISIBLE_MODEL, { normalizeToolCallId: true }); + + // Sanity: the per-model read agrees. + assert.equal(getModelIsHidden(PROVIDER, HIDDEN_MODEL), true); + assert.equal(getModelIsHidden(PROVIDER, VISIBLE_MODEL), false); + + const map = getHiddenModelsByProvider(); + const hiddenForProvider = map.get(PROVIDER); + assert.ok(hiddenForProvider, "expected an entry for the provider"); + assert.equal(hiddenForProvider.has(HIDDEN_MODEL), true, "hidden model must be in the set"); + assert.equal( + hiddenForProvider.has(VISIBLE_MODEL), + false, + "visible model must NOT be in the hidden set" + ); +}); + +test("un-hiding a model (isHidden: null) removes it from the map", () => { + mergeModelCompatOverride(PROVIDER, HIDDEN_MODEL, { isHidden: null }); + const map = getHiddenModelsByProvider(); + assert.equal( + map.get(PROVIDER)?.has(HIDDEN_MODEL) ?? false, + false, + "un-hidden model must drop out of the hidden set" + ); +});