mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-17 04:12:17 +03:00
fix(providers): honor operator-set endpoint overrides for local models (#13078)
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.
This commit is contained in:
@@ -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)).
|
||||
@@ -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);
|
||||
|
||||
@@ -686,20 +686,37 @@ function applyTriStateBooleanOverride(
|
||||
export async function updateCustomModel(
|
||||
providerId: string,
|
||||
modelId: string,
|
||||
updates: Record<string, unknown> = {}
|
||||
updates: Record<string, unknown> = {},
|
||||
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;
|
||||
|
||||
@@ -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 "/" --
|
||||
// "<prefix>/<rawId>" then reads as "<prefix>//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
|
||||
// ("<prefix>/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;
|
||||
}
|
||||
|
||||
173
tests/unit/local-provider-embedding-override.test.ts
Normal file
173
tests/unit/local-provider-embedding-override.test.ts
Normal file
@@ -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 "<alias>/<rawId>" 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]);
|
||||
});
|
||||
Reference in New Issue
Block a user