diff --git a/changelog.d/fixes/8332-combo-vision-fallback.md b/changelog.d/fixes/8332-combo-vision-fallback.md new file mode 100644 index 0000000000..40f784c397 --- /dev/null +++ b/changelog.d/fixes/8332-combo-vision-fallback.md @@ -0,0 +1 @@ +- fix(backend): keep combo routing from dispatching image requests to text-only targets (#8332) diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index 935bbcfd81..ed1a3aece5 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -178,6 +178,7 @@ import { resolveShadowTargets, scheduleShadowRouting } from "./combo/shadowRouti import { attemptCompatRejectedFallback } from "./combo/comboCompatFallback.ts"; import { applyContextRequirements } from "./combo/contextRequirements.ts"; import { + computeCompatRejectedTargets, filterTargetsByRequestCompatibility, resolveComboRuntimeUnits, resolveComboTargets, @@ -2924,8 +2925,7 @@ async function handleRoundRobinCombo({ // BEFORE availability is known; if every compat-kept target then turns out to be // runtime-unavailable, we must reconsider these before returning 503, instead of // permanently dropping a compat-rejected-but-healthy provider. - const compatKeptSet = new Set(filteredTargets); - const compatRejectedTargets = evalRankedTargets.filter((target) => !compatKeptSet.has(target)); + const compatRejectedTargets = computeCompatRejectedTargets(evalRankedTargets, filteredTargets, body); let modelCount = filteredTargets.length; if (modelCount === 0) { return comboModelNotFoundResponse("Round-robin combo has no executable targets"); diff --git a/open-sse/services/combo/comboStructure.ts b/open-sse/services/combo/comboStructure.ts index cdd6bec347..a53550f2de 100644 --- a/open-sse/services/combo/comboStructure.ts +++ b/open-sse/services/combo/comboStructure.ts @@ -501,6 +501,38 @@ function hasOnlyContextWindowFailures(reasons: string[]): boolean { return reasons.length > 0 && reasons.every((reason) => reason === "context_window"); } +/** + * #8332: vision is a hard requirement, not a soft preference — a target whose vision + * support is not confirmed can never succeed on an image_url request. Callers + * reconsidering compat-rejected targets (fallback tiers, degrade-to-unfiltered) MUST + * exclude these via this predicate. + */ +export function isVisionIncompatibleTarget( + target: ResolvedComboTarget, + requirements: RequestCompatibilityRequirements +): boolean { + if (!requirements.requiresVision) return false; + const capabilities = getResolvedModelCapabilities(target.modelStr); + return capabilities.supportsVision !== true; +} + +/** + * Builds the #6238 last-resort fallback candidate set: ranked targets the compat + * pre-filter rejected, minus anything rejected for vision (#8332 — see + * isVisionIncompatibleTarget). + */ +export function computeCompatRejectedTargets( + rankedTargets: ResolvedComboTarget[], + compatKeptTargets: ResolvedComboTarget[], + body: Record +): ResolvedComboTarget[] { + const requirements = deriveRequestCompatibilityRequirements(body); + const keptSet = new Set(compatKeptTargets); + return rankedTargets.filter( + (target) => !keptSet.has(target) && !isVisionIncompatibleTarget(target, requirements) + ); +} + function getTargetCompatibilityFailures( target: ResolvedComboTarget, requirements: RequestCompatibilityRequirements @@ -633,6 +665,11 @@ export function filterTargetsByRequestCompatibility( .map((entry) => `${entry.target.modelStr}(${entry.reasons.join("+")})`) .join(", ")}` ); + // #8332: vision is never safe to guess — this safety net must not resurrect a + // vision-rejected target, even if nothing else remains. + if (requirements.requiresVision) { + return targets.filter((target) => !isVisionIncompatibleTarget(target, requirements)); + } return targets; } diff --git a/tests/unit/8332-combo-vision-fallback.test.ts b/tests/unit/8332-combo-vision-fallback.test.ts new file mode 100644 index 0000000000..2e04b8194c --- /dev/null +++ b/tests/unit/8332-combo-vision-fallback.test.ts @@ -0,0 +1,199 @@ +// Regression guard for #8332: combo routing's #6238 last-resort compat-fallback +// tier dispatched an image_url-bearing request to a text-only (vision-incapable) +// target. filterTargetsByRequestCompatibility correctly rejects such targets +// up front (comboStructure.ts), but the round-robin path rebuilt its rejected +// set from the raw evalRankedTargets list (discarding the per-target rejection +// reasons), so `attemptCompatRejectedFallback` reconsidered a vision-rejected +// target exactly like any other compat-rejected-but-healthy target and sent it +// the unmodified image body — producing the upstream +// `400: unknown variant "image_url", expected "text"`. +// +// The fix makes vision a hard capability requirement for the fallback tier: +// a target rejected because it cannot confirm vision support must never be +// reconsidered, even as a last resort. +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-rr-compat-8332-")); +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; +process.env.DATA_DIR = TEST_DATA_DIR; + +const { handleComboChat } = await import("../../open-sse/services/combo.ts"); +const core = await import("../../src/lib/db/core.ts"); +const settingsDb = await import("../../src/lib/db/settings.ts"); +const { saveModelsDevCapabilities, clearModelsDevCapabilities } = + await import("../../src/lib/modelsDevSync.ts"); +const { resetAllComboMetrics } = await import("../../open-sse/services/comboMetrics.ts"); +const { resetAllCircuitBreakers } = await import("../../src/shared/utils/circuitBreaker.ts"); +const { resetAll: resetAllSemaphores } = + await import("../../open-sse/services/rateLimitSemaphore.ts"); + +function createLog() { + return { + info: () => {}, + warn: () => {}, + error: () => {}, + debug: () => {}, + }; +} + +function okResponse(body: unknown = { choices: [{ message: { content: "ok" } }] }) { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, + }); +} + +function capabilityEntry(limitContext: unknown, overrides: Record = {}) { + return { + tool_call: true, + reasoning: false, + attachment: false, + structured_output: true, + temperature: true, + modalities_input: JSON.stringify(["text"]), + modalities_output: JSON.stringify(["text"]), + knowledge_cutoff: null, + release_date: null, + last_updated: null, + status: null, + family: null, + open_weights: false, + limit_context: limitContext, + limit_input: limitContext, + limit_output: 4096, + interleaved_field: null, + ...overrides, + }; +} + +const imageRequestBody = { + messages: [ + { + role: "user", + content: [ + { type: "text", text: "What is in this image?" }, + { type: "image_url", image_url: { url: "https://example.com/cat.png" } }, + ], + }, + ], +}; + +test.beforeEach(() => { + resetAllComboMetrics(); + resetAllCircuitBreakers(); + resetAllSemaphores(); + clearModelsDevCapabilities(); +}); + +test.after(() => { + resetAllComboMetrics(); + resetAllCircuitBreakers(); + resetAllSemaphores(); + clearModelsDevCapabilities(); + settingsDb.clearAllLKGP(); + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + if (ORIGINAL_DATA_DIR === undefined) { + delete process.env.DATA_DIR; + } else { + process.env.DATA_DIR = ORIGINAL_DATA_DIR; + } +}); + +test( + "round-robin last-resort compat fallback never dispatches an image_url request " + + "to a vision-incapable target (#8332)", + async () => { + // rr-blind is text-only (attachment:false -> supportsVision:false) -> the + // vision-requiring request makes the compat pre-filter reject it. rr-vision-a + // and rr-vision-b are vision-capable -> kept, but both are simulated + // runtime-unavailable, forcing recordedAttempts === 0 and triggering the + // #6238 last-resort tier. Only the vision-rejected rr-blind is "healthy". + saveModelsDevCapabilities({ + openai: { + "rr-blind": capabilityEntry(128000, { attachment: false }), + "rr-vision-a": capabilityEntry(128000, { attachment: true }), + "rr-vision-b": capabilityEntry(128000, { attachment: true }), + }, + }); + + const attempted: string[] = []; + + const result = await handleComboChat({ + body: imageRequestBody, + combo: { + name: "rr-compat-fallback-8332-vision", + strategy: "round-robin", + models: ["openai/rr-blind", "openai/rr-vision-a", "openai/rr-vision-b"], + config: { maxRetries: 0, concurrencyPerModel: 1, queueTimeoutMs: 1000 }, + }, + handleSingleModel: async (_body, modelStr) => { + attempted.push(modelStr); + return okResponse({ choices: [{ message: { content: `served by ${modelStr}` } }] }); + }, + // Only the vision-rejected rr-blind is "available"; the vision-capable + // compat-kept targets are all runtime-unavailable. + isModelAvailable: async (modelStr) => modelStr === "openai/rr-blind", + log: createLog(), + settings: null, + relayOptions: null, + allCombos: null, + }); + + // Before the fix, rr-blind (vision-incapable but "available") was dispatched + // the raw image_url body as the #6238 last-resort fallback. After the fix it + // must never be attempted, and since no other target is available, the combo + // must surface its normal exhaustion error instead of a corrupted 200. + assert.deepEqual( + attempted, + [], + "vision-incapable rr-blind must never receive the image_url body, even as a last-resort fallback" + ); + assert.notEqual(result.status, 200, "must not silently succeed via the vision-incapable target"); + } +); + +test( + "round-robin last-resort compat fallback still serves a healthy non-vision-rejected " + + "target for an image request (#8332 — does not overcorrect #6238)", + async () => { + // rr-no-tools is tool-incapable (irrelevant here; no tools requested) but IS + // vision-capable, so it is not rejected for vision. rr-vision-a is also + // vision-capable and compat-kept, but runtime-unavailable, forcing the + // #6238 last-resort tier. This proves the vision-only exclusion does not + // also swallow legitimate non-vision-rejection fallback targets. + saveModelsDevCapabilities({ + openai: { + "rr-vision-a": capabilityEntry(128000, { attachment: true }), + }, + }); + + const attempted: string[] = []; + + const result = await handleComboChat({ + body: imageRequestBody, + combo: { + name: "rr-compat-fallback-8332-sanity", + strategy: "round-robin", + models: ["openai/rr-vision-a"], + config: { maxRetries: 0, concurrencyPerModel: 1, queueTimeoutMs: 1000 }, + }, + handleSingleModel: async (_body, modelStr) => { + attempted.push(modelStr); + return okResponse({ choices: [{ message: { content: `served by ${modelStr}` } }] }); + }, + isModelAvailable: async () => true, + log: createLog(), + settings: null, + relayOptions: null, + allCombos: null, + }); + + assert.equal(result.status, 200); + assert.deepEqual(attempted, ["openai/rr-vision-a"]); + } +); diff --git a/tests/unit/combo-vision-aware-routing.test.ts b/tests/unit/combo-vision-aware-routing.test.ts index f617537743..ae84b3ce08 100644 --- a/tests/unit/combo-vision-aware-routing.test.ts +++ b/tests/unit/combo-vision-aware-routing.test.ts @@ -15,8 +15,13 @@ * known-multimodal families (pixtral, llava, qwen-vl, gpt-4o, …) resolve to * `true` when there is no synced/registry/spec data. * B) the combo filter treats anything that is not confirmed `=== true` as - * vision-incompatible for image requests, while the existing - * "keep all when none qualify" fallback prevents any regression. + * vision-incompatible for image requests. Unlike the other requirement + * dimensions (tools, structured output, context window), the "keep all when + * none qualify" degrade-to-unfiltered safety net does NOT apply to vision + * (#8332): dispatching an image body to a confirmed-non-vision model can + * never succeed upstream, so a combo with zero confirmed-vision members must + * return no targets for an image request rather than resurrecting a + * guaranteed-to-fail one. */ import test from "node:test"; import assert from "node:assert/strict"; @@ -95,14 +100,23 @@ test("image request: combo drops the non-vision target, keeps the vision target" assert.ok(!ids.includes("mistral/ministral-14b-latest"), "non-vision target must be dropped"); }); -test("image request with NO confirmed-vision target: keep all (fallback, no regression)", () => { - const out = filterTargetsByRequestCompatibility( - [target("mistral/ministral-14b-latest"), target("groq/llama-3.1-8b-instant")], - imageBody, - noopLog - ); - assert.equal(out.length, 2, "must not strip every target when none is confirmed vision"); -}); +test( + "image request with NO confirmed-vision target: strip all (#8332 — never dispatch " + + "an image body to a confirmed-non-vision target, even as a last resort)", + () => { + const out = filterTargetsByRequestCompatibility( + [target("mistral/ministral-14b-latest"), target("groq/llama-3.1-8b-instant")], + imageBody, + noopLog + ); + assert.equal( + out.length, + 0, + "an image request with zero confirmed-vision targets must yield no executable targets, " + + "not a guaranteed-to-fail dispatch to a text-only model" + ); + } +); test("text-only request: targets are untouched by the vision filter", () => { const out = filterTargetsByRequestCompatibility(