fix(backend): keep combo routing from dispatching image requests to text-only targets (#8332) (#8360)

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)

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");

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<string, unknown>
+): 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;
   }
 

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<string, unknown> = {}) {
+  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"]);
+  }
+);

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(