diff --git a/src/app/api/models/route.ts b/src/app/api/models/route.ts index fb0d127893..2518c2538f 100644 --- a/src/app/api/models/route.ts +++ b/src/app/api/models/route.ts @@ -127,7 +127,9 @@ export async function handleGetModels(request: Request, dependencies: GetModelsD let syncedModelIdsByCanonicalProvider = new Map>(); let resolveCanonicalProviderIdForStatic: (alias: string) => string = (alias) => alias; try { - const syncedModelsByProvider = await getAllActiveSyncedModels(); + const syncedModelsByProvider = await getAllActiveSyncedModels({ + customModelsByProvider: capabilitySnapshot.customModelsByProvider, + }); const { aliasToProviderId, providerIdToAlias } = buildAliasMaps(); const resolve = (aliasOrId: string, fallbackProviderId?: string) => resolveCanonicalProviderIdFromMaps(aliasToProviderId, aliasOrId, fallbackProviderId); diff --git a/src/lib/db/models.ts b/src/lib/db/models.ts index 597d8d7063..e7e4f74831 100644 --- a/src/lib/db/models.ts +++ b/src/lib/db/models.ts @@ -62,6 +62,9 @@ export type { SyncedAvailableModel } from "./models/synced"; export { getCustomModelVisionOverride, listCustomModelVisionOverrides, + buildCustomModelVisionOverrides, + readCustomModelsByProvider, + type CustomModelsByProvider, type CustomModelVisionOverrideMap, type CustomModelVisionDatabase, type CustomModelVisionOverrideReadOptions, diff --git a/src/lib/db/models/activeSyncedCatalog.ts b/src/lib/db/models/activeSyncedCatalog.ts index c25d0f27c2..3e111bdbf8 100644 --- a/src/lib/db/models/activeSyncedCatalog.ts +++ b/src/lib/db/models/activeSyncedCatalog.ts @@ -422,7 +422,18 @@ export async function getActiveSyncedCatalog( * Return non-empty synced catalogs grouped by provider, restricted to active * connections. This is the authoritative live source for /v1/models. */ -export async function getAllActiveSyncedModels(): Promise> { +export interface GetAllActiveSyncedModelsOptions { + /** + * `customModels` rows already read by the caller's request-level capability + * snapshot. When present the listing reuses them instead of reading the + * namespace a second time (#16929 operator endpoint overlay). + */ + customModelsByProvider?: Readonly>; +} + +export async function getAllActiveSyncedModels( + options: GetAllActiveSyncedModelsOptions = {} +): Promise> { try { const connections = await getRawProviderConnections({ isActive: true }, undefined, undefined, [ "id", @@ -449,16 +460,20 @@ export async function getAllActiveSyncedModels(): Promise = {}; - // #16929: one customModels read for the whole listing (not one per provider). - let customModelsByProvider: Record = {}; - try { - const all = await getCustomModels(); - if (all && typeof all === "object" && !Array.isArray(all)) { - customModelsByProvider = all as Record; + // #16929: one customModels read for the whole listing (not one per provider), + // and none at all when the caller's capability snapshot already holds the rows. + let customModelsByProvider: Readonly> = + options.customModelsByProvider ?? {}; + if (!options.customModelsByProvider) { + try { + const all = await getCustomModels(); + if (all && typeof all === "object" && !Array.isArray(all)) { + customModelsByProvider = all as Record; + } + } catch { + // An unavailable custom overlay must not empty the entire live model + // catalog: keep the raw synced entries just as dispatch does. } - } catch { - // An unavailable custom overlay must not empty the entire live model - // catalog: keep the raw synced entries just as dispatch does. } await Promise.all( diff --git a/src/lib/db/models/customVisionOverride.ts b/src/lib/db/models/customVisionOverride.ts index bd3be90dc8..d8fe03b142 100644 --- a/src/lib/db/models/customVisionOverride.ts +++ b/src/lib/db/models/customVisionOverride.ts @@ -175,40 +175,67 @@ export function getCustomModelVisionOverride( } } -/** Bulk-load explicit custom-model vision overrides with one SQLite query. */ -export function listCustomModelVisionOverrides( +/** Provider → parsed `customModels` rows, the shape `getCustomModels()` returns without a provider. */ +export type CustomModelsByProvider = Readonly>; + +/** + * Bulk-load every provider's `customModels` row with one SQLite query. + * + * One request-level snapshot shares this single read between the vision + * overrides and the #16929 operator endpoint overlay of the live listing, so + * `/api/models` never re-reads the namespace per consumer. Malformed rows are + * skipped individually; a failing read yields an empty map (fail open). + */ +export function readCustomModelsByProvider( options: CustomModelVisionOverrideReadOptions = {} -): CustomModelVisionOverrideMap { +): CustomModelsByProvider { try { const db = options.getDatabase?.() ?? getDbInstance(); const rows = db .prepare("SELECT key, value FROM key_value WHERE namespace = 'customModels'") .all(); - const result = new Map>(); + const result: Record = {}; for (const row of rows) { const { key, value } = getKeyValue(row); if (!key || !value) continue; try { - const models = JSON.parse(value) as unknown; - if (!Array.isArray(models)) continue; - const byModel = new Map(); - for (const candidate of models) { - if (!candidate || typeof candidate !== "object" || Array.isArray(candidate)) continue; - const { id, supportsVision } = candidate as { - id?: unknown; - supportsVision?: unknown; - }; - if (typeof id === "string" && typeof supportsVision === "boolean") { - byModel.set(id, supportsVision); - } - } - if (byModel.size > 0) result.set(key, byModel); + result[key] = JSON.parse(value) as unknown; } catch { // Malformed custom-model rows do not participate in capability resolution. } } return result; } catch { - return new Map>(); + return {}; } } + +/** Derive the explicit custom-model vision overrides from already-read `customModels` rows. */ +export function buildCustomModelVisionOverrides( + customModelsByProvider: CustomModelsByProvider +): CustomModelVisionOverrideMap { + const result = new Map>(); + for (const [key, models] of Object.entries(customModelsByProvider)) { + if (!Array.isArray(models)) continue; + const byModel = new Map(); + for (const candidate of models) { + if (!candidate || typeof candidate !== "object" || Array.isArray(candidate)) continue; + const { id, supportsVision } = candidate as { + id?: unknown; + supportsVision?: unknown; + }; + if (typeof id === "string" && typeof supportsVision === "boolean") { + byModel.set(id, supportsVision); + } + } + if (byModel.size > 0) result.set(key, byModel); + } + return result; +} + +/** Bulk-load explicit custom-model vision overrides with one SQLite query. */ +export function listCustomModelVisionOverrides( + options: CustomModelVisionOverrideReadOptions = {} +): CustomModelVisionOverrideMap { + return buildCustomModelVisionOverrides(readCustomModelsByProvider(options)); +} diff --git a/src/lib/modelCapabilityResolutionSnapshot.ts b/src/lib/modelCapabilityResolutionSnapshot.ts index 7e2a6ef450..26ba658c7b 100644 --- a/src/lib/modelCapabilityResolutionSnapshot.ts +++ b/src/lib/modelCapabilityResolutionSnapshot.ts @@ -16,7 +16,9 @@ import { type ModelCompatVisionOverrideMap, } from "@/lib/db/models/compat"; import { - listCustomModelVisionOverrides, + buildCustomModelVisionOverrides, + readCustomModelsByProvider, + type CustomModelsByProvider, type CustomModelVisionOverrideMap, type CustomModelVisionOverrideReadOptions, listSyncedAvailableModelVision, @@ -41,6 +43,13 @@ export interface ModelCapabilityResolutionSnapshot { readonly reasoningEffortsOverrides: NestedReasoningEffortsOverrideMap; readonly contextOverrides: NestedOverrideMap; readonly customVisionOverrides: CustomModelVisionOverrideMap; + /** + * The `customModels` rows the vision overrides were derived from, exposed so + * listing consumers (#16929 operator endpoint overlay) reuse this one read + * instead of querying the namespace again. Optional: hand-built snapshots + * may omit it, and consumers then fall back to their own read. + */ + readonly customModelsByProvider?: CustomModelsByProvider; readonly compatVisionOverrides: ModelCompatVisionOverrideMap; /** #14081: positive-only vision verdicts from synced custom-node model rows. */ readonly syncedAvailableModelVision: SyncedAvailableModelVisionMap; @@ -100,13 +109,16 @@ export function createModelCapabilityResolutionSnapshot( setNestedOverride(contextOverrides, entry.provider, entry.modelId, entry.realContext); } + const customModelsByProvider = readCustomModelsByProvider(options.customModelVision); + return { synced, maxTokenOverrides, maxInputTokenOverrides, reasoningEffortsOverrides, contextOverrides, - customVisionOverrides: listCustomModelVisionOverrides(options.customModelVision), + customVisionOverrides: buildCustomModelVisionOverrides(customModelsByProvider), + customModelsByProvider, compatVisionOverrides: listModelCompatVisionOverrides(), syncedAvailableModelVision: listSyncedAvailableModelVision(), }; diff --git a/stryker.conf.json b/stryker.conf.json index 8e2262b7c9..5b7532bd61 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -658,7 +658,11 @@ "tests/unit/provider-alias-combo-pool-12844.test.ts", "tests/unit/model-capacity-failure-scope-15788.test.ts", "tests/unit/opencode-free-synced-catalog-and-no-connection-15540.test.ts", - "tests/unit/provider-node-local-dispatch-13452.test.ts" + "tests/unit/provider-node-local-dispatch-13452.test.ts", + "tests/unit/content-policy-refusal.test.ts", + "tests/unit/stream-timing-late-output.test.ts", + "tests/unit/chat-empty-stream-recovery.test.ts", + "tests/unit/provider-error-rules-opt-in-flags-16797.test.ts" ], "nodeArgs": [ "--import", diff --git a/tests/unit/admin-audit-events.test.ts b/tests/unit/admin-audit-events.test.ts index d521d2a0d4..d668bb0aba 100644 --- a/tests/unit/admin-audit-events.test.ts +++ b/tests/unit/admin-audit-events.test.ts @@ -248,6 +248,7 @@ test("deleting the final provider connection removes imported models but preserv id: "manual-model", name: "Manual", source: "manual", + endpointCapabilitySource: "operator", apiFormat: "chat-completions", supportedEndpoints: ["chat"], }, diff --git a/tests/unit/api-models-shared-custom-models-read.test.ts b/tests/unit/api-models-shared-custom-models-read.test.ts new file mode 100644 index 0000000000..025ccfcdf5 --- /dev/null +++ b/tests/unit/api-models-shared-custom-models-read.test.ts @@ -0,0 +1,102 @@ +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"; + +// #16929 follow-up: /api/models must bulk-read the `customModels` namespace once per +// request. The capability snapshot owns that read and the live listing reuses it. +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-shared-custom-read-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.SQLITE_FILE = path.join(TEST_DATA_DIR, "storage.sqlite"); + +const core = await import("../../src/lib/db/core.ts"); +const modelsDb = await import("../../src/lib/db/models.ts"); +const activeSynced = await import("../../src/lib/db/models/activeSyncedCatalog.ts"); +const snapshotModule = await import("../../src/lib/modelCapabilityResolutionSnapshot.ts"); + +const CUSTOM_MODELS_SQL = "FROM key_value WHERE namespace = 'customModels'"; + +function countCustomModelReads(run: () => Promise): Promise<{ result: T; reads: number }> { + const db = core.getDbInstance(); + const originalPrepare = db.prepare; + const callPrepare = originalPrepare.bind(db); + let reads = 0; + (db as unknown as { prepare: typeof db.prepare }).prepare = ((sql: string) => { + if (String(sql).replace(/\s+/g, " ").includes(CUSTOM_MODELS_SQL)) reads++; + return callPrepare(sql); + }) as typeof db.prepare; + return run() + .then((result) => ({ result, reads })) + .finally(() => { + (db as unknown as { prepare: typeof db.prepare }).prepare = originalPrepare; + }); +} + +test.before(() => { + const db = core.getDbInstance(); + const insert = db.prepare( + "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES ('customModels', ?, ?)" + ); + insert.run( + "openai", + JSON.stringify([ + { id: "gpt-4o", supportsVision: false }, + { id: "no-flag" }, + { id: "vision-on", supportsVision: true }, + ]) + ); + insert.run("broken", "{not json"); + insert.run("empty", JSON.stringify([])); +}); + +test.after(() => { + core.resetDbInstance(); + try { + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } catch { + /* best-effort */ + } +}); + +test("readCustomModelsByProvider parses every row once and skips malformed JSON", () => { + const rows = modelsDb.readCustomModelsByProvider(); + assert.deepEqual(Object.keys(rows).sort(), ["empty", "openai"]); + assert.equal((rows.openai as unknown[]).length, 3); +}); + +test("buildCustomModelVisionOverrides from pre-read rows equals the direct bulk read", () => { + const direct = modelsDb.listCustomModelVisionOverrides(); + const derived = modelsDb.buildCustomModelVisionOverrides(modelsDb.readCustomModelsByProvider()); + assert.deepEqual( + [...derived].map(([p, m]) => [p, [...m]]), + [...direct].map(([p, m]) => [p, [...m]]) + ); + assert.equal(derived.get("openai")?.get("gpt-4o"), false); + assert.equal(derived.get("openai")?.get("vision-on"), true); + assert.equal(derived.get("openai")?.has("no-flag"), false); +}); + +test("the capability snapshot reads customModels once and exposes the rows", async () => { + const { result: snapshot, reads } = await countCustomModelReads(async () => + snapshotModule.createModelCapabilityResolutionSnapshot() + ); + assert.equal(reads, 1, "one bulk read per snapshot"); + assert.ok(snapshot.customModelsByProvider, "snapshot must expose the rows it read"); + assert.equal(snapshot.customVisionOverrides.get("openai")?.get("gpt-4o"), false); + assert.equal((snapshot.customModelsByProvider?.openai as unknown[]).length, 3); +}); + +test("getAllActiveSyncedModels reuses the snapshot rows instead of re-reading", async () => { + const snapshot = snapshotModule.createModelCapabilityResolutionSnapshot(); + const shared = await countCustomModelReads(() => + activeSynced.getAllActiveSyncedModels({ + customModelsByProvider: snapshot.customModelsByProvider, + }) + ); + assert.equal(shared.reads, 0, "a pre-read map must suppress the second namespace read"); + + const standalone = await countCustomModelReads(() => activeSynced.getAllActiveSyncedModels()); + assert.equal(standalone.reads, 1, "without a caller snapshot the listing still bulk-reads once"); + assert.deepEqual(shared.result, standalone.result); +}); diff --git a/tests/unit/managed-model-import.test.ts b/tests/unit/managed-model-import.test.ts index 03bd22baca..baaa309567 100644 --- a/tests/unit/managed-model-import.test.ts +++ b/tests/unit/managed-model-import.test.ts @@ -169,6 +169,7 @@ test("sync keeps a same-id manual model as the user-configurable metadata overri id: "shared-model", name: "Operator configuration", source: "manual", + endpointCapabilitySource: "operator", apiFormat: "responses", supportedEndpoints: ["responses"], targetFormat: "claude",