From dd44abf28a31bc6d085fd6d8e6a2e715fb495dae Mon Sep 17 00:00:00 2001 From: excessivechaos Date: Wed, 5 Aug 2026 09:18:09 -0700 Subject: [PATCH] fix(catalog): scope effort-tier fallback to declared models and harden resolver Addresses reviewer findings on #9485: - CRITICAL #1: catalog no longer synthesizes unresolvable effort aliases for static reasoning models without declared tiers (cheaperinference, cline, etc.) - CRITICAL #2: tiered static models survive synced-coverage suppression so normal installs with synced DeepSeek base models still expose aliases - WARNING #3: registry suffix resolution short-circuits when the raw id matches a direct custom or synced model, preserving custom apiFormat/targetFormat - WARNING #4: empty synced effort array no longer erases the registry fallback - WARNING #5: isFlash check is robust to suffixed/prefixed model ids - Added regression tests for blast radius, custom-model shadowing, none-path, and suffixed isFlash --- open-sse/executors/base/reasoningEffort.ts | 6 +- src/app/api/v1/models/catalog.ts | 12 ++- src/app/api/v1/models/catalogHelpers.ts | 17 ++-- src/sse/services/model.ts | 20 ++++- tests/unit/deepseek-thinking-efforts.test.ts | 88 ++++++++++++++++++++ 5 files changed, 132 insertions(+), 11 deletions(-) diff --git a/open-sse/executors/base/reasoningEffort.ts b/open-sse/executors/base/reasoningEffort.ts index 1b341e95e7..cc5d4c94da 100644 --- a/open-sse/executors/base/reasoningEffort.ts +++ b/open-sse/executors/base/reasoningEffort.ts @@ -273,7 +273,11 @@ export function sanitizeReasoningEffortForProvider( // normalized API expects xhigh, not max (pi#4055). `none` is already the // OpenAI no-thinking carrier and passes through unchanged. if (provider === "deepseek") { - const isFlash = modelStr.toLowerCase() === "deepseek-v4-flash"; + // Match the Flash family even when the sanitizer sees a suffixed or prefixed + // id — exact-match would silently clamp Flash `low → high` if a future route + // forwards the raw catalog id (`deepseek-v4-flash-low`) before resolution + // (#9485 review). + const isFlash = modelStr.toLowerCase().startsWith("deepseek-v4-flash"); const mapped = effortStr === "xhigh" ? "max" diff --git a/src/app/api/v1/models/catalog.ts b/src/app/api/v1/models/catalog.ts index 32840526c4..e3fc75ea7a 100644 --- a/src/app/api/v1/models/catalog.ts +++ b/src/app/api/v1/models/catalog.ts @@ -728,13 +728,17 @@ async function buildUnifiedModelsResponseCore( // the fix, a provider with any synced model silently dropped ALL its // static models. const syncedForProvider = syncedModelIdsByCanonicalProvider.get(canonicalProviderId); + const hasDeclaredEffortTiers = + Array.isArray(model.supportedThinkingEfforts) && + model.supportedThinkingEfforts.length > 0; if ( shouldSuppressStaticModelBySyncedCoverage({ providerHasSynced: syncedForProvider !== undefined && syncedForProvider.size > 0, staticModelId: model.id, syncedModelIds: syncedForProvider ? [...syncedForProvider] : [], }) && - !isRegisteredEffortVariant(providerModels, model.id) + !isRegisteredEffortVariant(providerModels, model.id) && + !hasDeclaredEffortTiers ) continue; if (!providerSupportsModel(canonicalProviderId, model.id)) continue; @@ -749,7 +753,11 @@ async function buildUnifiedModelsResponseCore( canonicalProviderId, model.id, model.supportsReasoning, - model.supportedThinkingEfforts + model.supportedThinkingEfforts, + // Skip the canonical fallback for static models without declared tiers — + // otherwise the catalog synthesizes unresolvable `/-{tier}` + // ids for every static reasoning model across all providers (#9485 review). + !hasDeclaredEffortTiers ); const thinkingCapabilities = Object.keys(thinkingFields).length > 0 ? { capabilities: thinkingFields } : {}; diff --git a/src/app/api/v1/models/catalogHelpers.ts b/src/app/api/v1/models/catalogHelpers.ts index 71acac3628..fee39fd36b 100644 --- a/src/app/api/v1/models/catalogHelpers.ts +++ b/src/app/api/v1/models/catalogHelpers.ts @@ -84,19 +84,24 @@ export function getThinkingCapabilityFields( providerId: string, modelId: string, resolvedThinking?: boolean | null, - supportedThinkingEfforts?: readonly string[] + supportedThinkingEfforts?: readonly string[], + /** When true, skip the canonical effort-tier fallback — used for static registry + * models that declare `supportsReasoning` but no explicit tier list, so the + * catalog does not synthesize unresolvable `/-{tier}` ids. */ + skipCanonicalEffortFallback = false ): Record { const supportsThinking = resolvedThinking; if (typeof supportsThinking !== "boolean") return {}; + const hasDeclaredTiers = + supportedThinkingEfforts && supportedThinkingEfforts.length > 0; return { thinking: supportsThinking, supportsThinking, - ...(supportsThinking + ...(supportsThinking && (hasDeclaredTiers || !skipCanonicalEffortFallback) ? { - effort_tiers: - supportedThinkingEfforts && supportedThinkingEfforts.length > 0 - ? [...supportedThinkingEfforts] - : extendCodexGpt56EffortValues(providerId, modelId, CANONICAL_EFFORT_VALUES), + effort_tiers: hasDeclaredTiers + ? [...supportedThinkingEfforts!] + : extendCodexGpt56EffortValues(providerId, modelId, CANONICAL_EFFORT_VALUES), } : {}), }; diff --git a/src/sse/services/model.ts b/src/sse/services/model.ts index 959a9d18fe..2d02ce93d0 100644 --- a/src/sse/services/model.ts +++ b/src/sse/services/model.ts @@ -220,7 +220,11 @@ function copySyncedThinkingMetadata(metadata: RuntimeModelMeta, syncedMatch: any metadata.supportsThinking = syncedMatch.supportsThinking; } if (syncedMatch?.alwaysThinking === true) metadata.alwaysThinking = true; - if (Array.isArray(syncedMatch?.supportedThinkingEfforts)) { + // Only let a non-empty synced effort list override the static registry fallback; + // an empty array from an incomplete synced discovery must not erase registry-declared + // tiers (#9485 review). + if (Array.isArray(syncedMatch?.supportedThinkingEfforts) && + syncedMatch.supportedThinkingEfforts.length > 0) { metadata.supportedThinkingEfforts = syncedMatch.supportedThinkingEfforts; } if (typeof syncedMatch?.defaultThinkingEffort === "string") { @@ -260,12 +264,24 @@ async function lookupModelMeta( // #7694: no direct match on the raw modelId? try a synced-declared `-{effort}` // suffix before falling back to the literal id, so `/-` // resolves to the real base model + a resolved effort. + // #7694: no direct match on the raw modelId? try a synced-declared `-{effort}` + // suffix before falling back to the literal id, so `/-` + // resolves to the real base model + a resolved effort. let { modelId: resolvedModelId, effort } = resolveSyncedModelIdAndEffort( providerId, modelId, syncedModels ); - if (!effort && resolvedModelId === modelId) { + // Short-circuit registry suffix resolution when the raw id is already a direct + // custom or synced model — otherwise a model literally named + // `deepseek-v4-flash-low` gets rewritten to `deepseek-v4-flash` + effort `low` + // and its custom/synced metadata (apiFormat/targetFormat) is dropped (#9485 review). + if ( + !effort && + resolvedModelId === modelId && + !findCustomModelMeta(customModels, modelId) && + !findSyncedModelMeta(syncedModels, modelId) + ) { const registryResolution = resolveRegistryModelIdAndEffort(providerId, modelId); resolvedModelId = registryResolution.modelId; effort = registryResolution.effort; diff --git a/tests/unit/deepseek-thinking-efforts.test.ts b/tests/unit/deepseek-thinking-efforts.test.ts index dc904d19b0..832bc9e10e 100644 --- a/tests/unit/deepseek-thinking-efforts.test.ts +++ b/tests/unit/deepseek-thinking-efforts.test.ts @@ -10,6 +10,7 @@ process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "deepseek-efforts-tes const core = await import("../../src/lib/db/core.ts"); const providersDb = await import("../../src/lib/db/providers.ts"); +const modelsDb = await import("../../src/lib/db/models.ts"); const { getModelInfo } = await import("../../src/sse/services/model.ts"); const v1ModelsCatalog = await import("../../src/app/api/v1/models/catalog.ts"); const { REGISTRY } = await import("../../open-sse/config/providerRegistry.ts"); @@ -103,3 +104,90 @@ test("native DeepSeek preserves Flash low while clamping unsupported Pro low", ( ) as Record; assert.equal(pro.reasoning_effort, "high"); }); + + +test("non-DeepSeek static reasoning models do not advertise unresolvable effort aliases", async () => { + // cheaperinference declares deepseek-v4-flash/pro with supportsReasoning: true + // but no supportedThinkingEfforts — the catalog must NOT synthesize + // cheaperinference/deepseek-v4-flash-{low,high,...} ids for them (#9485 review #1). + await providersDb.createProviderConnection({ + provider: "cheaperinference", + authType: "apikey", + name: "cheaperinference-blast-radius", + apiKey: "cheaperinference-test-key", + isActive: true, + testStatus: "active", + }); + + const response = await v1ModelsCatalog.getUnifiedModelsResponse( + new Request("http://localhost/api/v1/models") + ); + const body = (await response.json()) as { data: Array<{ id: string }> }; + const ids = body.data.map((model) => model.id); + + // Static base models for cheaperinference should still be present + assert.ok( + ids.some((id) => id.endsWith("cheaperinference/deepseek-v4-flash")), + "cheaperinference/deepseek-v4-flash base entry should still be present" + ); + // But NO effort-suffixed aliases should be synthesized + assert.equal( + ids.some((id) => /cheaperinference\/deepseek-v4-flash-(none|low|medium|high|max|xhigh)$/.test(id)), + false, + "cheaperinference static reasoning models must not advertise unresolvable effort aliases" + ); + assert.equal( + ids.some((id) => /cheaperinference\/deepseek-v4-pro-(none|low|medium|high|max|xhigh)$/.test(id)), + false, + "cheaperinference static reasoning models must not advertise unresolvable effort aliases" + ); +}); + +test("custom model named deepseek-v4-flash-low is not rewritten by registry suffix resolution", async () => { + // A custom (DB) model literally named deepseek-v4-flash-low on the deepseek + // provider must not be silently rewritten to deepseek-v4-flash + effort low, + // which would drop its custom apiFormat/targetFormat metadata (#9485 review #3). + await modelsDb.addCustomModel( + "deepseek", + "deepseek-v4-flash-low", + "deepseek-v4-flash-low", + "manual", + "responses", + ["chat"], + "responses" + ); + + const info = await getModelInfo("ds/deepseek-v4-flash-low"); + // The model id should be preserved as the literal custom id, not rewritten + assert.equal(info.model, "deepseek-v4-flash-low"); + // The custom apiFormat must survive (not dropped by registry rewriting) + assert.equal(info.apiFormat, "responses"); + // No resolved effort should be injected — this is a distinct custom model + assert.equal(info.resolvedThinkingEffort, undefined); +}); + +test("none effort resolves and passes through the native DeepSeek sanitizer unchanged", async () => { + // The -none suffix resolves to base + effort "none", which reaches the native + // DeepSeek endpoint as reasoning_effort: "none" unchanged (#9485 review #8). + const flashNone = await getModelInfo("ds/deepseek-v4-flash-none"); + assert.equal(flashNone.model, "deepseek-v4-flash"); + assert.equal(flashNone.resolvedThinkingEffort, "none"); + + const sanitized = sanitizeReasoningEffortForProvider( + { model: "deepseek-v4-flash", reasoning_effort: "none" }, + "deepseek", + "deepseek-v4-flash" + ) as Record; + assert.equal(sanitized.reasoning_effort, "none"); +}); + +test("isFlash check is robust to suffixed model ids", () => { + // A suffixed id like deepseek-v4-flash-low must still be recognized as Flash + // so its low effort is preserved, not clamped to high (#9485 review #5). + const sanitizedSuffixed = sanitizeReasoningEffortForProvider( + { model: "deepseek-v4-flash-low", reasoning_effort: "low" }, + "deepseek", + "deepseek-v4-flash-low" + ) as Record; + assert.equal(sanitizedSuffixed.reasoning_effort, "low"); +});