From 7dbe850daafd8986610bca57afb0cfd8fbfeb0b0 Mon Sep 17 00:00:00 2001 From: Markus Hartung Date: Wed, 16 Sep 2026 18:36:52 +0200 Subject: [PATCH] fix(providers): honor operator-set endpoint overrides for local models (#13078) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merged. An operator-set endpoint override that is ignored for local models is the worst kind of setting — it looks applied and is not. Honouring it is the whole fix. Validated as a combined board first (this PR merged with the 21 siblings of the same wave on the release tip): eslint with the frozen suppressions, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-counts, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 176 passing / 0 failing focused node:test cases across the 25 test files the wave touches and the dashboard test under Vitest (2/0). Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run. Thank you. --- ...13078-local-provider-embedding-override.md | 1 + src/app/api/provider-models/route.ts | 2 +- src/lib/db/models.ts | 41 +++-- .../providerModels/syncedEndpointRouting.ts | 58 ++++-- .../local-provider-embedding-override.test.ts | 173 ++++++++++++++++++ 5 files changed, 251 insertions(+), 24 deletions(-) create mode 100644 changelog.d/fixes/13078-local-provider-embedding-override.md create mode 100644 tests/unit/local-provider-embedding-override.test.ts diff --git a/changelog.d/fixes/13078-local-provider-embedding-override.md b/changelog.d/fixes/13078-local-provider-embedding-override.md new file mode 100644 index 0000000000..76585ac930 --- /dev/null +++ b/changelog.d/fixes/13078-local-provider-embedding-override.md @@ -0,0 +1 @@ +- **fix(providers):** honor an operator-set endpoint override (`PUT /api/provider-models`) for a local model whose own `/v1/models` response carries no capability data of its own (llama.cpp included) — the override previously only took effect when a matching `customModels` entry already existed, so declaring a brand-new local model as embeddings-capable silently did nothing. `updateCustomModel()` gains an opt-in `createIfMissing` mode; every other caller's existing contract is unchanged. Also accepts a collapsed single-slash id for path-based local models ([#13078](https://github.com/diegosouzapw/OmniRoute/pull/13078)). diff --git a/src/app/api/provider-models/route.ts b/src/app/api/provider-models/route.ts index 81447f421e..0b9b4fbe99 100644 --- a/src/app/api/provider-models/route.ts +++ b/src/app/api/provider-models/route.ts @@ -258,7 +258,7 @@ export async function PUT(request) { } } - const model = await updateCustomModel(provider, modelId, updates); + const model = await updateCustomModel(provider, modelId, updates, { createIfMissing: true }); if (!model) { const rawKeys = Object.keys(raw); diff --git a/src/lib/db/models.ts b/src/lib/db/models.ts index e30868c680..1a6c57ea3f 100644 --- a/src/lib/db/models.ts +++ b/src/lib/db/models.ts @@ -686,20 +686,37 @@ function applyTriStateBooleanOverride( export async function updateCustomModel( providerId: string, modelId: string, - updates: Record = {} + updates: Record = {}, + options: { createIfMissing?: boolean } = {} ) { const db = getDbInstance(); const row = db .prepare("SELECT value FROM key_value WHERE namespace = 'customModels' AND key = ?") .get(providerId); - if (!row) return null; - const value = getKeyValue(row).value; - if (!value) return null; + const value = row ? getKeyValue(row).value : null; + const models: JsonRecord[] = value ? JSON.parse(value) : []; + let index = models.findIndex((m: JsonRecord) => m.id === modelId); - const models = JSON.parse(value); - const index = models.findIndex((m: JsonRecord) => m.id === modelId); - if (index === -1) return null; + if (index === -1) { + if (!options.createIfMissing) return null; + // A model discovered via sync/passthrough (syncedAvailableModels) has no + // customModels row until an operator explicitly overrides one of its + // fields -- PUT /api/provider-models is exactly that "set an override" + // action, so upsert here (same default shape as addCustomModel()) instead + // of 404ing on the very save it exists to serve. Observed live: a + // llama.cpp connection's auto-discovered embedding model had no way to be + // marked "supports embeddings" because it had never been explicitly + // imported as a custom model first. + models.push({ + id: modelId, + name: modelId, + source: "manual", + apiFormat: "chat-completions", + supportedEndpoints: ["chat"], + }); + index = models.length - 1; + } const current = models[index]; const currentCompat = (current as JsonRecord).compatByProtocol as CompatByProtocolMap | undefined; @@ -770,10 +787,12 @@ export async function updateCustomModel( models[index] = next; - db.prepare("UPDATE key_value SET value = ? WHERE namespace = 'customModels' AND key = ?").run( - JSON.stringify(models), - providerId - ); + // INSERT OR REPLACE (not UPDATE): the createIfMissing path above may be + // writing this provider's customModels row for the first time, and an + // UPDATE...WHERE would silently match zero rows in that case. + db.prepare( + "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES ('customModels', ?, ?)" + ).run(providerId, JSON.stringify(models)); finishModelCatalogWriteWithBackup(); return next; diff --git a/src/lib/providerModels/syncedEndpointRouting.ts b/src/lib/providerModels/syncedEndpointRouting.ts index c33776fe3d..46687a2350 100644 --- a/src/lib/providerModels/syncedEndpointRouting.ts +++ b/src/lib/providerModels/syncedEndpointRouting.ts @@ -1,4 +1,4 @@ -import { getSyncedAvailableModelsByConnection } from "@/lib/db/models"; +import { getAllCustomModels, getSyncedAvailableModelsByConnection } from "@/lib/db/models"; import { isSelfHostedChatProvider, resolveProviderId } from "@/shared/constants/providers"; export type LocalSyncedEndpointRoute = { @@ -14,18 +14,52 @@ export async function resolveLocalSyncedEndpointRoute( const slashIndex = modelStr.indexOf("/"); if (slashIndex <= 0 || slashIndex === modelStr.length - 1) return null; - const provider = resolveProviderId(modelStr.slice(0, slashIndex)); - const model = modelStr.slice(slashIndex + 1); + const providerPrefix = modelStr.slice(0, slashIndex); + const provider = resolveProviderId(providerPrefix); if (!isSelfHostedChatProvider(provider)) return null; - const byConnection = await getSyncedAvailableModelsByConnection(provider); - const connectionIds = Object.entries(byConnection) - .filter(([, models]) => - models.some( - (candidate) => candidate.id === model && candidate.supportedEndpoints?.includes(endpoint) - ) - ) - .map(([connectionId]) => connectionId); + const rawSuffix = modelStr.slice(slashIndex + 1); + // Some self-hosted servers (llama.cpp included) report models by absolute + // filesystem path, so the raw id itself already starts with "/" -- + // "/" then reads as "//models/foo.gguf" (a double + // slash). That IS the byte-for-byte round-trip-safe id the catalog + // displays, but an operator who naturally collapses it to a single slash + // ("/models/foo.gguf") should still resolve -- try the raw model + // id both as given and with a leading "/" restored. + const modelCandidates = rawSuffix.startsWith("/") ? [rawSuffix] : [rawSuffix, `/${rawSuffix}`]; - return connectionIds.length > 0 ? { provider, model, connectionIds } : null; + // Most local servers' own /v1/models response carries no capability data at + // all (llama.cpp included -- unlike Ollama's /api/show, there is nothing to + // probe), so a discovered model's synced cache entry below often has no + // supportedEndpoints of its own. An operator-set override (PUT + // /api/provider-models, keyed by the exact catalog id the client used) is + // the explicit "this model does support embeddings" declaration for + // exactly that case -- honor it here the same way the /v1/models catalog + // already merges customModels on top of synced entries, instead of only + // trusting the un-annotated raw sync cache. + const customModelsForProvider = (await getAllCustomModels())[provider]; + const byConnection = await getSyncedAvailableModelsByConnection(provider); + + for (const model of modelCandidates) { + const overrideEndpoints = Array.isArray(customModelsForProvider) + ? ( + customModelsForProvider as Array<{ id?: unknown; supportedEndpoints?: unknown }> + ).find((entry) => entry.id === modelStr || entry.id === `${providerPrefix}/${model}`) + ?.supportedEndpoints + : undefined; + const hasOverride = Array.isArray(overrideEndpoints) && overrideEndpoints.includes(endpoint); + + const connectionIds = Object.entries(byConnection) + .filter(([, models]) => + models.some( + (candidate) => + candidate.id === model && (hasOverride || candidate.supportedEndpoints?.includes(endpoint)) + ) + ) + .map(([connectionId]) => connectionId); + + if (connectionIds.length > 0) return { provider, model, connectionIds }; + } + + return null; } diff --git a/tests/unit/local-provider-embedding-override.test.ts b/tests/unit/local-provider-embedding-override.test.ts new file mode 100644 index 0000000000..765ee91e26 --- /dev/null +++ b/tests/unit/local-provider-embedding-override.test.ts @@ -0,0 +1,173 @@ +// A self-hosted server like llama.cpp reports NO capability data on its own +// /v1/models response -- unlike Ollama (already fixed for this in #11087 via +// an /api/show probe), llama.cpp has no equivalent endpoint to probe. So a +// discovered embedding model's synced cache entry (syncedAvailableModels) +// never gets supportedEndpoints of its own, and the operator's only recourse +// is the explicit "Supported endpoints" override in the Providers UI. +// +// That override turned out to be broken end to end: +// +// 1. PUT /api/provider-models (updateCustomModel) 404'd for any model that +// had never been explicitly imported into customModels before -- exactly +// the case for every auto-discovered model, so the very save the UI +// offers for this situation failed silently. +// 2. Even with a saved override, /v1/embeddings' dynamic-provider routing +// (resolveLocalSyncedEndpointRoute) only ever consulted the raw, +// un-annotated syncedAvailableModels cache -- never the customModels +// override the /v1/models catalog itself already merges in for display. +// +// Both are pinned here against the real user-observed shape: a llama-cpp +// connection with one synced model (source: "imported", no +// supportedEndpoints), an operator-set override marking it embeddings- +// capable, and a request through the same code path createEmbeddingResponse +// uses (parseEmbeddingModel's provider miss -> resolveLocalSyncedEndpointRoute). + +import test, { beforeEach } 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-local-embedding-override-") +); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const modelsDb = await import("../../src/lib/db/models.ts"); +const { resolveLocalSyncedEndpointRoute } = await import( + "../../src/lib/providerModels/syncedEndpointRouting.ts" +); + +const PROVIDER = "llama-cpp"; +const ALIAS = "llamacpp"; +const RAW_MODEL_ID = "/models/Qwen3-Embedding-4B-Q8_0.gguf"; +const ALIAS_MODEL_ID = `${ALIAS}/${RAW_MODEL_ID}`; +const CONNECTION_ID = "c7e7371e-709d-4f3d-a704-b0767bf7f1bc"; + +beforeEach(() => { + core.getDbInstance() + .prepare("DELETE FROM key_value WHERE namespace IN ('customModels', 'syncedAvailableModels')") + .run(); +}); + +function seedSyncedModelWithNoCapabilityData() { + core.getDbInstance() + .prepare( + "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES ('syncedAvailableModels', ?, ?)" + ) + .run( + `${PROVIDER}:${CONNECTION_ID}`, + // Exact shape llama.cpp discovery actually persists: no supportedEndpoints, + // no apiFormat -- the raw /v1/models response has nothing to carry them. + JSON.stringify([{ id: RAW_MODEL_ID, name: RAW_MODEL_ID, source: "imported" }]) + ); +} + +test("updateCustomModel: createIfMissing upserts instead of 404ing on a never-imported model", async () => { + const before = await modelsDb.getCustomModels(PROVIDER); + assert.deepEqual(before, [], "sanity: no customModels row exists yet for this provider"); + + const result = await modelsDb.updateCustomModel( + PROVIDER, + ALIAS_MODEL_ID, + { supportedEndpoints: ["embeddings"] }, + { createIfMissing: true } + ); + + assert.ok(result, "must return the created/updated model, not null"); + assert.deepEqual(result!.supportedEndpoints, ["embeddings"]); + + const after = await modelsDb.getCustomModels(PROVIDER); + assert.equal(after.length, 1); + assert.equal(after[0].id, ALIAS_MODEL_ID); +}); + +test("updateCustomModel: without createIfMissing, still returns null for a never-imported model (unchanged contract)", async () => { + const result = await modelsDb.updateCustomModel("some-other-provider", "never/imported", { + supportedEndpoints: ["embeddings"], + }); + assert.equal(result, null, "the PATCH isHidden caller relies on this null to trigger its own compat-override fallback"); +}); + +test("resolveLocalSyncedEndpointRoute: an operator override rescues a model with no supportedEndpoints of its own", async () => { + seedSyncedModelWithNoCapabilityData(); + + const beforeOverride = await resolveLocalSyncedEndpointRoute(ALIAS_MODEL_ID, "embeddings"); + assert.equal( + beforeOverride, + null, + "without an override, the un-annotated synced entry must not match" + ); + + await modelsDb.updateCustomModel( + PROVIDER, + ALIAS_MODEL_ID, + { supportedEndpoints: ["embeddings"] }, + { createIfMissing: true } + ); + + const afterOverride = await resolveLocalSyncedEndpointRoute(ALIAS_MODEL_ID, "embeddings"); + assert.ok(afterOverride, "the explicit override must make the route resolve"); + assert.equal(afterOverride!.provider, PROVIDER); + assert.equal(afterOverride!.model, RAW_MODEL_ID); + assert.deepEqual(afterOverride!.connectionIds, [CONNECTION_ID]); +}); + +test("resolveLocalSyncedEndpointRoute: an override for a different model does not leak onto this one", async () => { + seedSyncedModelWithNoCapabilityData(); + await modelsDb.updateCustomModel( + PROVIDER, + `${ALIAS}/some-other-model.gguf`, + { supportedEndpoints: ["embeddings"] }, + { createIfMissing: true } + ); + + const route = await resolveLocalSyncedEndpointRoute(ALIAS_MODEL_ID, "embeddings"); + assert.equal(route, null, "an override on an unrelated model id must not match this one"); +}); + +test("resolveLocalSyncedEndpointRoute: a caller who collapses the double slash to a single slash still resolves", async () => { + // llama.cpp's own /v1/models reports models by absolute filesystem path + // ("/models/Qwen3-Embedding-4B-Q8_0.gguf"), so "/" reads as + // "llamacpp//models/..." -- correct, but easy for an operator to naturally + // collapse to a single slash when typing/pasting it by hand. + seedSyncedModelWithNoCapabilityData(); + await modelsDb.updateCustomModel( + PROVIDER, + ALIAS_MODEL_ID, // saved under the catalog's own double-slash id + { supportedEndpoints: ["embeddings"] }, + { createIfMissing: true } + ); + + const singleSlashId = `${ALIAS}/models/Qwen3-Embedding-4B-Q8_0.gguf`; // one slash, not two + assert.notEqual(singleSlashId, ALIAS_MODEL_ID, "sanity: this really is the collapsed form"); + + const route = await resolveLocalSyncedEndpointRoute(singleSlashId, "embeddings"); + assert.ok(route, "the single-slash form must still resolve to the same model/connection"); + assert.equal(route!.provider, PROVIDER); + assert.equal(route!.model, RAW_MODEL_ID, "the resolved model id must be the real leading-slash form"); + assert.deepEqual(route!.connectionIds, [CONNECTION_ID]); +}); + +test("resolveLocalSyncedEndpointRoute: a model's own genuine supportedEndpoints still work with no override needed", async () => { + core.getDbInstance() + .prepare( + "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES ('syncedAvailableModels', ?, ?)" + ) + .run( + `${PROVIDER}:${CONNECTION_ID}`, + JSON.stringify([ + { + id: RAW_MODEL_ID, + name: RAW_MODEL_ID, + source: "imported", + supportedEndpoints: ["embeddings"], + }, + ]) + ); + + const route = await resolveLocalSyncedEndpointRoute(ALIAS_MODEL_ID, "embeddings"); + assert.ok(route, "a genuinely-annotated synced entry must resolve without needing an override"); + assert.deepEqual(route!.connectionIds, [CONNECTION_ID]); +});