From c79caa913a2e9b0c1f5151d067beeff1eea145e1 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 11 Sep 2026 22:04:31 -0300 Subject: [PATCH] fix(db): scope model-hidden overrides by modality (#12172) (#13258) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merged as part of the 39-PR owner batch of 2026-09-11, validated as a unit. Boarded into one consolidated worktree cut from `release/v3.8.51` with the other 38 — zero conflicts between them. - ESLint over every changed file: no errors (the only finding was one suppression entry the batch emptied, pruned on #13243) - `typecheck:core` clean; `check:dashboard-typecheck` OK (206 pre-existing, within baseline); `check:changelog-integrity` OK - complexity 2821 / baseline 3218 and cognitive-complexity 1272 / baseline 1437 — both under baseline - 256 assertions green: 246 under node:test and 10 under vitest, which is where `tests/unit/**/*.test.tsx` actually runs - `check-file-size`: `chatCore.ts` rebaselined 6144 → 6146 for #13278 and #13276, annotated and landed on #13243 ⚠️ base-red inherited: #12732 — the provider count (356 in the docs vs the 358 the modules define) and `open-sse/utils/stream.ts` at 3115 > frozen 3098 both reproduce on the pure tip with zero contribution from this batch. --- .../12172-model-id-collision-chat-image.md | 1 + .../[id]/hooks/useModelVisibilityHandlers.ts | 8 +- src/app/api/provider-models/route.ts | 13 ++- src/app/api/v1/models/catalog.ts | 46 ++++++----- src/lib/db/models.ts | 59 +++++++++++-- src/lib/db/models/compat.ts | 48 ++++++++++- .../model-hidden-modality-scope-12172.test.ts | 82 +++++++++++++++++++ 7 files changed, 224 insertions(+), 33 deletions(-) create mode 100644 changelog.d/fixes/12172-model-id-collision-chat-image.md create mode 100644 tests/unit/model-hidden-modality-scope-12172.test.ts diff --git a/changelog.d/fixes/12172-model-id-collision-chat-image.md b/changelog.d/fixes/12172-model-id-collision-chat-image.md new file mode 100644 index 0000000000..ef06abfd79 --- /dev/null +++ b/changelog.d/fixes/12172-model-id-collision-chat-image.md @@ -0,0 +1 @@ +- fix(db): scope model visibility overrides by modality so hiding a Chat model no longer hides an identically-ID'd Image/Embeddings/etc. model (#12172) diff --git a/src/app/(dashboard)/dashboard/providers/[id]/hooks/useModelVisibilityHandlers.ts b/src/app/(dashboard)/dashboard/providers/[id]/hooks/useModelVisibilityHandlers.ts index d49bbc989f..33c58a008d 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/hooks/useModelVisibilityHandlers.ts +++ b/src/app/(dashboard)/dashboard/providers/[id]/hooks/useModelVisibilityHandlers.ts @@ -212,7 +212,10 @@ export function useModelVisibilityHandlers({ { method: "PATCH", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ isHidden: hidden }), + // #12172: this page manages only Chat models — scope the hide/unhide to + // "chat" so it never suppresses an identically-ID'd model registered + // under a different modality's registry (e.g. Image). + body: JSON.stringify({ isHidden: hidden, modality: "chat" }), } ); if (!res.ok) { @@ -239,7 +242,8 @@ export function useModelVisibilityHandlers({ const res = await fetch(`/api/provider-models?provider=${encodeURIComponent(providerKey)}`, { method: "PATCH", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ isHidden: hidden, modelIds }), + // #12172: same "chat"-only scoping as the single-model toggle above. + body: JSON.stringify({ isHidden: hidden, modelIds, modality: "chat" }), }); if (!res.ok) { const detail = await res.text().catch(() => ""); diff --git a/src/app/api/provider-models/route.ts b/src/app/api/provider-models/route.ts index beba1dbc1f..81447f421e 100644 --- a/src/app/api/provider-models/route.ts +++ b/src/app/api/provider-models/route.ts @@ -412,6 +412,17 @@ export async function PATCH(request) { ); } + // #12172: optional modality scope (e.g. "chat", "images") so hiding a model on one + // registry surface does not also hide an identically-ID'd model on another one. + // Omitted = legacy "hide everywhere" behavior, unchanged for existing callers. + if (typeof body.modality !== "undefined" && typeof body.modality !== "string") { + return Response.json( + { error: { message: "modality must be a string when provided", type: "validation_error" } }, + { status: 400 } + ); + } + const modality = typeof body.modality === "string" && body.modality ? body.modality : undefined; + const modelIds = normalizeRequestedModelIds(searchParams, body); if (modelIds.length === 0) { return Response.json( @@ -428,7 +439,7 @@ export async function PATCH(request) { for (const modelId of modelIds) { const updatedModel = await updateCustomModel(provider, modelId, { isHidden: body.isHidden }); if (!updatedModel) { - mergeModelCompatOverride(provider, modelId, { isHidden: body.isHidden }); + mergeModelCompatOverride(provider, modelId, { isHidden: body.isHidden, modality }); } } diff --git a/src/app/api/v1/models/catalog.ts b/src/app/api/v1/models/catalog.ts index 4de6cd21fa..87991da4d3 100644 --- a/src/app/api/v1/models/catalog.ts +++ b/src/app/api/v1/models/catalog.ts @@ -296,17 +296,21 @@ async function buildUnifiedModelsResponseCore( } }; try { - // #9147: `getModelIsHidden()` is a SQLite read per call (custom row + compat list) - // and the build consults it ~16× per entry. Bulk-load the hidden-model map once - // (one query — `getHiddenModelsByProvider`) and resolve from memory for the whole - // build. A provider absent from the map has no hidden models at all — `false`, - // no on-demand fallback (that would reintroduce the per-call SQLite reads). - // Deliberately kept INSIDE this try block (not hoisted above it): the builder's - // own catch below is what converts a build-time failure into a sanitized 500 - // Response instead of a rejected promise — hoisting this bulk read above the - // try would let a crash here propagate as an unhandled rejection instead - // (catalogCache.ts's in-flight coalescing does not fully consume rejections). - const hiddenModelsByProvider = getHiddenModelsByProvider(); + // #9147/#12172: bulk-load the hidden-model map once PER MODALITY (memoized below, + // one SQLite query per modality actually used) instead of `getModelIsHidden()`'s + // per-call read — per-modality because chat/images/etc. registries can share a + // literal model id and must be hideable independently (#12172). Deliberately kept + // INSIDE this try block: the builder's catch below sanitizes a build-time failure + // into a 500 instead of a rejected promise. + const hiddenModelsByModality = new Map>>(); + const getHiddenModelsForModality = (modality: string): Map> => { + let m = hiddenModelsByModality.get(modality); + if (!m) { + m = getHiddenModelsByProvider(modality); + hiddenModelsByModality.set(modality, m); + } + return m; + }; let settings: Record = {}; try { settings = await getSettings(); @@ -434,7 +438,8 @@ async function buildUnifiedModelsResponseCore( const isModelHiddenBulk = ( providerKey: string | null | undefined, modelId: string, - canonicalProviderId?: string | null + canonicalProviderId?: string | null, + modality: string = "chat" ): boolean => { if (!providerKey || !modelId) return false; const canonical = canonicalProviderId || resolveCanonicalProviderId(providerKey); @@ -443,8 +448,9 @@ async function buildUnifiedModelsResponseCore( const keysToCheck = [providerKey, canonical, alias, nodePrefix].filter((k): k is string => Boolean(k) ); + const hiddenModelsForModality = getHiddenModelsForModality(modality); for (const key of keysToCheck) { - const hiddenSet = hiddenModelsByProvider.get(key); + const hiddenSet = hiddenModelsForModality.get(key); if (hiddenSet?.has(modelId)) return true; } return false; @@ -1467,7 +1473,7 @@ async function buildUnifiedModelsResponseCore( if (!isProviderActive(embModel.provider)) continue; const rawModelId = getSpecialtyModelRelativeId(embModel.id, embModel.provider); if (!providerSupportsModel(embModel.provider, rawModelId)) continue; - if (isModelHiddenBulk(embModel.provider, rawModelId)) continue; + if (isModelHiddenBulk(embModel.provider, rawModelId, null, "embeddings")) continue; const existingEmbedding = findEquivalentSpecialtyModel( embModel.provider, rawModelId, @@ -1510,7 +1516,7 @@ async function buildUnifiedModelsResponseCore( if (!isProviderActive(imgModel.provider)) continue; const rawModelId = getSpecialtyModelRelativeId(imgModel.id, imgModel.provider); if (!providerSupportsModel(imgModel.provider, rawModelId)) continue; - if (isModelHiddenBulk(imgModel.provider, rawModelId)) continue; + if (isModelHiddenBulk(imgModel.provider, rawModelId, null, "images")) continue; models.push({ id: imgModel.id, object: "model", @@ -1530,7 +1536,7 @@ async function buildUnifiedModelsResponseCore( if (!isProviderActive(rerankModel.provider)) continue; const rawModelId = getSpecialtyModelRelativeId(rerankModel.id, rerankModel.provider); if (!providerSupportsModel(rerankModel.provider, rawModelId)) continue; - if (isModelHiddenBulk(rerankModel.provider, rawModelId)) continue; + if (isModelHiddenBulk(rerankModel.provider, rawModelId, null, "rerank")) continue; if (hasEquivalentSpecialtyModel(rerankModel.provider, rawModelId, "rerank", rerankModel.id)) { continue; } @@ -1549,7 +1555,7 @@ async function buildUnifiedModelsResponseCore( if (!isProviderActive(audioModel.provider)) continue; const rawModelId = getSpecialtyModelRelativeId(audioModel.id, audioModel.provider); if (!providerSupportsModel(audioModel.provider, rawModelId)) continue; - if (isModelHiddenBulk(audioModel.provider, rawModelId)) continue; + if (isModelHiddenBulk(audioModel.provider, rawModelId, null, "audio")) continue; models.push({ id: audioModel.id, object: "model", @@ -1565,7 +1571,7 @@ async function buildUnifiedModelsResponseCore( if (!isProviderActive(modModel.provider)) continue; const rawModelId = getSpecialtyModelRelativeId(modModel.id, modModel.provider); if (!providerSupportsModel(modModel.provider, rawModelId)) continue; - if (isModelHiddenBulk(modModel.provider, rawModelId)) continue; + if (isModelHiddenBulk(modModel.provider, rawModelId, null, "moderation")) continue; models.push({ id: modModel.id, object: "model", @@ -1580,7 +1586,7 @@ async function buildUnifiedModelsResponseCore( if (!isProviderActive(videoModel.provider)) continue; const rawModelId = getSpecialtyModelRelativeId(videoModel.id, videoModel.provider); if (!providerSupportsModel(videoModel.provider, rawModelId)) continue; - if (isModelHiddenBulk(videoModel.provider, rawModelId)) continue; + if (isModelHiddenBulk(videoModel.provider, rawModelId, null, "videos")) continue; models.push({ id: videoModel.id, object: "model", @@ -1601,7 +1607,7 @@ async function buildUnifiedModelsResponseCore( if (!isProviderActive(musicModel.provider)) continue; const rawModelId = getSpecialtyModelRelativeId(musicModel.id, musicModel.provider); if (!providerSupportsModel(musicModel.provider, rawModelId)) continue; - if (isModelHiddenBulk(musicModel.provider, rawModelId)) continue; + if (isModelHiddenBulk(musicModel.provider, rawModelId, null, "music")) continue; models.push({ id: musicModel.id, object: "model", diff --git a/src/lib/db/models.ts b/src/lib/db/models.ts index 8b878af1bb..9ea0354192 100644 --- a/src/lib/db/models.ts +++ b/src/lib/db/models.ts @@ -27,6 +27,8 @@ import { isCompatProtocolKey, sanitizeUpstreamHeadersMap, removeModelCompatOverride, + mergeModelCompatOverride, + isOverrideHiddenForModality, type CompatByProtocolMap, type ModelCompatProtocolKey, type ModelCompatOverride, @@ -894,22 +896,30 @@ export function getModelPreserveOpenAIDeveloperRole( /** * Check if the model is flagged as hidden from the public catalog. + * `modality` (default "chat") scopes the check to one endpoint/registry — see + * {@link isOverrideHiddenForModality} — so an identically-ID'd model in a different + * modality's registry (e.g. Chat vs Image, #12172) is not silently suppressed too. */ -export function getModelIsHidden(providerId: string, modelId: string): boolean { +export function getModelIsHidden( + providerId: string, + modelId: string, + modality: string = "chat" +): boolean { const m = getCustomModelRow(providerId, modelId); if (m && Object.prototype.hasOwnProperty.call(m, "isHidden")) { return Boolean(m.isHidden); } const co = readCompatList(providerId).find((e) => e.id === modelId); - return Boolean(co?.isHidden); + return isOverrideHiddenForModality(co, modality); } /** * 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. + * and customModels, scoped to one `modality` (default "chat", matching every + * pre-#12172 caller's original chat-only intent). Used by auto-combo candidate + * building to skip user-hidden models. Single bulk DB query — not N+1 per model. */ -export function getHiddenModelsByProvider(): Map> { +export function getHiddenModelsByProvider(modality: string = "chat"): Map> { const db = getDbInstance(); const visibilityByProvider = new Map>(); const rows = db @@ -928,13 +938,33 @@ export function getHiddenModelsByProvider(): Map> { if (!entry || typeof entry !== "object") continue; const modelId = (entry as { id?: unknown }).id; if (typeof modelId !== "string" || modelId.length === 0) continue; - if (!Object.prototype.hasOwnProperty.call(entry, "isHidden")) continue; + const record = entry as { isHidden?: unknown; hiddenModalities?: unknown }; + const hasHiddenInfo = + Object.prototype.hasOwnProperty.call(record, "isHidden") || + (namespace === "modelCompatOverrides" && + record.hiddenModalities && + typeof record.hiddenModalities === "object"); + if (!hasHiddenInfo) continue; + // #12172: customModels rows have no modality scope (single user-managed + // entry) — legacy global isHidden applies to every modality unchanged. + const isHidden = + namespace === "modelCompatOverrides" + ? isOverrideHiddenForModality( + { + isHidden: Boolean(record.isHidden), + hiddenModalities: record.hiddenModalities as + | Record + | undefined, + }, + modality + ) + : Boolean(record.isHidden); let visibility = visibilityByProvider.get(row.key); if (!visibility) { visibility = new Map(); visibilityByProvider.set(row.key, visibility); } - visibility.set(modelId, Boolean((entry as { isHidden?: unknown }).isHidden)); + visibility.set(modelId, isHidden); } } catch { // Skip malformed entries @@ -957,7 +987,12 @@ export function getHiddenModelsByProvider(): Map> { * row when one exists, otherwise on the compat-override list. Setting * `hidden = false` is a no-op when the model is already visible. */ -export function setModelIsHidden(providerId: string, modelId: string, hidden: boolean): void { +export function setModelIsHidden( + providerId: string, + modelId: string, + hidden: boolean, + modality?: string +): void { const customRow = getCustomModelRow(providerId, modelId); if (customRow) { if (hidden) { @@ -968,6 +1003,14 @@ export function setModelIsHidden(providerId: string, modelId: string, hidden: bo return; } + // #12172: a modality-scoped write never touches the legacy all-modalities + // `isHidden` flag — it only sets/clears that one modality's override, so an + // identically-ID'd model in a different modality's registry is unaffected. + if (modality) { + mergeModelCompatOverride(providerId, modelId, { isHidden: hidden, modality }); + return; + } + const list = readCompatList(providerId); const idx = list.findIndex((e) => e.id === modelId); if (hidden) { diff --git a/src/lib/db/models/compat.ts b/src/lib/db/models/compat.ts index 9022bd7a82..dd22654396 100644 --- a/src/lib/db/models/compat.ts +++ b/src/lib/db/models/compat.ts @@ -114,11 +114,36 @@ export type ModelCompatOverride = { compatByProtocol?: CompatByProtocolMap; upstreamHeaders?: Record; isHidden?: boolean; + /** + * #12172: per-modality visibility override, keyed by endpoint/modality id + * (e.g. "chat", "images", "embeddings", ...). A key present here always wins + * over the legacy top-level `isHidden` for that specific modality — this is + * what lets an operator hide a model from Chat without also suppressing an + * identically-ID'd model in the Image (or any other) registry. A modality + * with no entry here falls back to `isHidden` (the pre-#12172 "hide + * everywhere" behavior), so existing rows keep working unchanged. + */ + hiddenModalities?: Record; apiFormat?: string; targetFormat?: string; supportsVision?: boolean; }; +/** + * Resolve whether an override hides its model for a given modality. + * Precedence: an explicit `hiddenModalities[modality]` entry always wins; + * otherwise fall back to the legacy all-modalities `isHidden` flag. + */ +export function isOverrideHiddenForModality( + override: Pick | null | undefined, + modality: string +): boolean { + if (!override) return false; + const scoped = override.hiddenModalities?.[modality]; + if (scoped !== undefined) return Boolean(scoped); + return Boolean(override.isHidden); +} + export function readCompatList(providerId: string): ModelCompatOverride[] { const db = getDbInstance(); const row = db @@ -171,6 +196,13 @@ export type ModelCompatPatch = { /** Replace top-level extra headers for override-only rows; omit to leave unchanged. */ upstreamHeaders?: Record | null; isHidden?: boolean | null; + /** + * #12172: when set alongside `isHidden`, scopes the write to that one + * modality (see {@link ModelCompatOverride.hiddenModalities}) instead of + * the legacy all-modalities flag. `isHidden: null` with a `modality` clears + * just that modality's override (reverting it to inherit the legacy flag). + */ + modality?: string | null; apiFormat?: string | null; targetFormat?: string | null; supportsVision?: boolean | null; @@ -230,7 +262,17 @@ export function mergeModelCompatOverride( const hasVideoUrlFlag = Object.prototype.hasOwnProperty.call(next, "preserveVideoUrl"); const hasTopUpstream = next.upstreamHeaders && Object.keys(next.upstreamHeaders).length > 0; if ("isHidden" in patch) { - if (patch.isHidden === null) { + const modality = typeof patch.modality === "string" && patch.modality ? patch.modality : null; + if (modality) { + const hiddenModalities = { ...(next.hiddenModalities || {}) }; + if (patch.isHidden === null) { + delete hiddenModalities[modality]; + } else { + hiddenModalities[modality] = Boolean(patch.isHidden); + } + if (Object.keys(hiddenModalities).length > 0) next.hiddenModalities = hiddenModalities; + else delete next.hiddenModalities; + } else if (patch.isHidden === null) { delete next.isHidden; } else { next.isHidden = Boolean(patch.isHidden); @@ -257,7 +299,9 @@ export function mergeModelCompatOverride( next.supportsVision = Boolean(patch.supportsVision); } } - const hasHiddenFlag = Object.prototype.hasOwnProperty.call(next, "isHidden"); + const hasHiddenFlag = + Object.prototype.hasOwnProperty.call(next, "isHidden") || + (!!next.hiddenModalities && Object.keys(next.hiddenModalities).length > 0); const hasApiFormat = Object.prototype.hasOwnProperty.call(next, "apiFormat"); const hasTargetFormat = Object.prototype.hasOwnProperty.call(next, "targetFormat"); const hasVisionFlag = Object.prototype.hasOwnProperty.call(next, "supportsVision"); diff --git a/tests/unit/model-hidden-modality-scope-12172.test.ts b/tests/unit/model-hidden-modality-scope-12172.test.ts new file mode 100644 index 0000000000..44f497f399 --- /dev/null +++ b/tests/unit/model-hidden-modality-scope-12172.test.ts @@ -0,0 +1,82 @@ +/** + * Regression test for #12172 — a model ID collision between the Chat registry and a + * specialty modality registry (Image) prevented independent visibility toggling, + * because `modelCompatOverrides` was keyed only by (providerId, modelId) with no + * modality/endpoint field: hiding the chat model also hid the identically-ID'd + * image model, and vice versa. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-12172-modality-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "modality-collision-test-secret"; + +const { setModelIsHidden, getModelIsHidden, getHiddenModelsByProvider } = await import( + "../../src/lib/db/models.ts" +); +const { resetDbInstance } = await import("../../src/lib/db/core.ts"); +const { codexProvider } = await import( + "../../open-sse/config/providers/registry/codex/index.ts" +); +const { IMAGE_PROVIDERS } = await import("../../open-sse/config/imageRegistry.ts"); + +test.after(() => { + resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("#12172: codex chat and image registries collide on the same model id (premise)", () => { + const chatModel = codexProvider.models.find((m) => m.id === "gpt-5.6-sol"); + const imageModel = IMAGE_PROVIDERS.codex.models.find((m) => m.id === "gpt-5.6-sol"); + assert.ok(chatModel, "codex chat registry must define gpt-5.6-sol (premise check)"); + assert.ok(imageModel, "codex image registry must define gpt-5.6-sol (premise check)"); +}); + +test("#12172: hiding the codex CHAT model gpt-5.6-sol must not suppress the codex IMAGE model", () => { + setModelIsHidden("codex", "gpt-5.6-sol", true, "chat"); + + const chatHidden = getHiddenModelsByProvider("chat").get("codex"); + const imageHidden = getHiddenModelsByProvider("images").get("codex"); + + assert.equal(chatHidden?.has("gpt-5.6-sol"), true, "chat model must be hidden after the toggle"); + assert.equal( + imageHidden?.has("gpt-5.6-sol") ?? false, + false, + "BUG #12172: hiding the chat model must not also hide the identically-ID'd image model" + ); + + assert.equal(getModelIsHidden("codex", "gpt-5.6-sol", "chat"), true); + assert.equal(getModelIsHidden("codex", "gpt-5.6-sol", "images"), false); +}); + +test("#12172: unhiding one modality does not affect the other", () => { + setModelIsHidden("codex", "gpt-5.6-terra", true, "chat"); + setModelIsHidden("codex", "gpt-5.6-terra", true, "images"); + assert.equal(getModelIsHidden("codex", "gpt-5.6-terra", "chat"), true); + assert.equal(getModelIsHidden("codex", "gpt-5.6-terra", "images"), true); + + setModelIsHidden("codex", "gpt-5.6-terra", false, "chat"); + assert.equal( + getModelIsHidden("codex", "gpt-5.6-terra", "chat"), + false, + "chat toggle must not be affected by the images toggle" + ); + assert.equal( + getModelIsHidden("codex", "gpt-5.6-terra", "images"), + true, + "images toggle must remain hidden after unhiding chat only" + ); +}); + +test("#12172: a legacy (no-modality) hide keeps suppressing every modality — backward compatible", () => { + setModelIsHidden("codex", "gpt-5.6-luna", true); + + assert.equal(getModelIsHidden("codex", "gpt-5.6-luna", "chat"), true); + assert.equal(getModelIsHidden("codex", "gpt-5.6-luna", "images"), true); + assert.equal(getHiddenModelsByProvider("chat").get("codex")?.has("gpt-5.6-luna"), true); + assert.equal(getHiddenModelsByProvider("images").get("codex")?.has("gpt-5.6-luna"), true); +});