fix(vision-bridge): do not select unreachable describe-model when no vision provider is connected (#8430)

This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-08-04 21:36:34 -03:00
committed by GitHub
parent b0501642dd
commit 7e55abbc41
7 changed files with 144 additions and 23 deletions

View File

@@ -0,0 +1,3 @@
- fix(vision-bridge): describe-model no longer returns unreachable "openai/gpt-4o-mini" when every vision-capable provider is unreachable on the instance — returns null instead and surfaces a clear error (#8430)
- fix(vision-bridge): validate fixedModel against usable credentials before short-circuiting in getBestVisionModel, so the default "openai/gpt-4o-mini" is not unconditionally selected when no OpenAI connection exists (#8430)
- fix(vision-bridge): in the combo describe path, replace raw images with an error text stub when all describe attempts fail, instead of forwarding images to a confirmed non-vision backend that would reject them with an opaque serde error (#8430)

View File

@@ -311,6 +311,19 @@ export class VisionBridgeGuardrail extends BaseGuardrail {
return null; return null;
}); });
// 12b. (#8430) When every describe call failed (all null descriptions) in
// the combo describe path, the upstream is a confirmed non-vision model that
// cannot process raw images — replacing them with an "(unavailable)" stub
// is safe here because the upstream can only handle text. The original #4012
// preserve-raw behavior only applies to paths where the upstream might still
// be vision-capable (reroute path / unknown capability).
const allNull = descriptions.every((d) => d === null);
if (allNull && comboVisionBridgeDecision === "process") {
for (let i = 0; i < descriptions.length; i++) {
descriptions[i] = `[Image ${i + 1}]: (unavailable — no vision-capable provider connected)`;
}
}
// 13. Replace image parts with text descriptions (null → keep original image) // 13. Replace image parts with text descriptions (null → keep original image)
const modifiedBody = replaceImageParts( const modifiedBody = replaceImageParts(
body as Parameters<typeof replaceImageParts>[0], body as Parameters<typeof replaceImageParts>[0],

View File

@@ -212,11 +212,17 @@ export async function callVisionModel(
apiKey?: string, apiKey?: string,
routerConfig?: Partial<import("./visionBridgeRouter").VisionBridgeRouterConfig> routerConfig?: Partial<import("./visionBridgeRouter").VisionBridgeRouterConfig>
): Promise<string> { ): Promise<string> {
// Auto-select the best vision model if not explicitly configured // Auto-select the best vision model
const modelToUse = await getBestVisionModel({ const modelToUse = await getBestVisionModel({
fixedModel: config.model, fixedModel: config.model,
...routerConfig, ...routerConfig,
}); });
// (#8430) When no vision-capable provider has usable credentials on this
// instance, surface a clear error instead of attempting a describe call that
// would fail with an opaque auth/serde error upstream.
if (!modelToUse) {
throw new Error("No vision-capable provider connected, cannot process image request");
}
let lastError: Error | null = null; let lastError: Error | null = null;
// Try primary model + fallbacks // Try primary model + fallbacks

View File

@@ -209,17 +209,29 @@ function selectBestModel(
/** /**
* Get the best vision model for image description. * Get the best vision model for image description.
* Respects fixed model override if configured. * Respects fixed model override if configured, but validates it has usable
* credentials before short-circuiting — a fixedModel that is confirmed
* unreachable on this instance falls through to auto-selection.
* Returns `null` when no vision-capable candidate has usable credentials.
*/ */
export async function getBestVisionModel( export async function getBestVisionModel(
config: Partial<VisionBridgeRouterConfig> = {}, config: Partial<VisionBridgeRouterConfig> = {},
deps: VisionBridgeRouterDeps = {} deps: VisionBridgeRouterDeps = {}
): Promise<string> { ): Promise<string | null> {
const fullConfig = { ...DEFAULT_ROUTER_CONFIG, ...config }; const fullConfig = { ...DEFAULT_ROUTER_CONFIG, ...config };
// If fixed model is configured, use it // If fixed model is configured, validate it has usable credentials first.
// (#8430) An unreachable fixedModel (e.g. the default "openai/gpt-4o-mini"
// on an instance with no OpenAI connection/key) must not short-circuit the
// credential check — fall through to auto-selection instead.
if (fullConfig.fixedModel) { if (fullConfig.fixedModel) {
return fullConfig.fixedModel; const checkCreds = deps.hasUsableCredentials ?? hasUsableCredentialsForModel;
const usable = await checkCreds(fullConfig.fixedModel);
// Only skip credential validation when the check is indeterminate (null).
// A confirmed `false` means fall through to auto-selection.
if (usable !== false) {
return fullConfig.fixedModel;
}
} }
// Check selection cache — key includes excluded models to prevent cache pollution // Check selection cache — key includes excluded models to prevent cache pollution
@@ -240,8 +252,8 @@ export async function getBestVisionModel(
const best = selectBestModel(candidates, fullConfig); const best = selectBestModel(candidates, fullConfig);
if (!best) { if (!best) {
// Fallback to default // No vision-capable candidate has usable credentials on this instance
return "openai/gpt-4o-mini"; return null;
} }
// Cache the selection // Cache the selection

View File

@@ -64,13 +64,12 @@ test("getBestVisionModel — should exclude specified models", async () => {
test("getBestVisionModel — excludes a candidate with no usable active connection", async () => { test("getBestVisionModel — excludes a candidate with no usable active connection", async () => {
// Every candidate reports a confirmed-unusable connection (`false`) -> // Every candidate reports a confirmed-unusable connection (`false`) ->
// no candidate survives -> the hardcoded last-resort default is returned // no candidate survives -> returns null instead of an unreachable default.
// instead of an unreachable pick.
const model = await getBestVisionModel( const model = await getBestVisionModel(
{}, {},
{ hasUsableCredentials: async () => false } { hasUsableCredentials: async () => false }
); );
assert.equal(model, "openai/gpt-4o-mini"); assert.equal(model, null);
}); });
test( test(

View File

@@ -0,0 +1,79 @@
import test from "node:test";
import assert from "node:assert/strict";
const { VisionBridgeGuardrail } = await import("../../src/lib/guardrails/visionBridge.ts");
const { resetGuardrailsForTests } = await import("../../src/lib/guardrails/registry.ts");
const { getBestVisionModel } = await import("../../src/lib/guardrails/visionBridgeRouter.ts");
import type { GuardrailContext } from "../../src/lib/guardrails/base.ts";
import type { VisionModelConfig } from "../../src/lib/guardrails/visionBridgeHelpers.ts";
const mockSettings: Record<string, unknown> = {
visionBridgeEnabled: true,
visionBridgePrompt: "Describe this image concisely.",
visionBridgeTimeout: 30000,
visionBridgeMaxImages: 10,
};
function createGuardrail(options?: Parameters<typeof VisionBridgeGuardrail>[0]) {
return new VisionBridgeGuardrail({
...options,
deps: {
getSettings: async () => mockSettings,
callVisionModel: async (_i: string, _c: VisionModelConfig) => {
throw new Error("Vision API error 401: Missing API key");
},
hasUsableCredentials: async () => false,
...(options?.deps ?? {}),
},
});
}
function createContext(o: Partial<GuardrailContext> = {}): GuardrailContext {
return { model: "deepseek/deepseek-v4-pro", log: console, ...o };
}
function createPayload(o: Record<string, unknown> = {}): Record<string, unknown> {
return {
model: "deepseek/deepseek-v4-pro",
messages: [
{
role: "user",
content: [
{ type: "text", text: "What is in this image?" },
{ type: "image_url", image_url: { url: "https://example.com/image.png" } },
],
},
],
...o,
};
}
test.beforeEach(() => { resetGuardrailsForTests({ registerDefaults: false }); });
test("8430a: getBestVisionModel returns null when every vision-capable candidate is unusable", async () => {
const model = await getBestVisionModel({}, { hasUsableCredentials: async () => false });
assert.strictEqual(model, null, `no vision provider reachable, but returned unreachable '${model}'`);
});
test("8430b: fixedModel describe-path target must not be an unreachable model", async () => {
const model = await getBestVisionModel(
{ fixedModel: "openai/gpt-4o-mini" },
{ hasUsableCredentials: async () => false }
);
assert.strictEqual(model, null, `fixedModel short-circuit returned unreachable '${model}'`);
});
test("8430c: describe path does not forward raw image when no vision provider is reachable", async () => {
const guardrail = createGuardrail({
deps: { checkModelHasComboMapping: async (_m: string) => true },
});
const result = await guardrail.preCall(createPayload(), createContext());
assert.strictEqual(result.block, false);
assert.ok(result.modifiedPayload, "expected a modified payload");
const modified = result.modifiedPayload as {
messages: Array<{ content: Array<{ type: string; text?: string }> }>;
};
const content = modified.messages[0].content;
const imagePart = content.find((p) => p.type === "image_url" || p.type === "image");
assert.strictEqual(imagePart, undefined, "raw image forwarded with no clear error (ask #2 unimplemented)");
});

View File

@@ -1,14 +1,20 @@
/** /**
* Regression test for #4012 — Nvidia NIM (and any vision-capable model whose * Regression test for #4012 / #8430 — Nvidia NIM (and any vision-capable model
* capability OmniRoute can't prove) via OmniRoute fails to process image inputs. * whose capability OmniRoute can't prove) via OmniRoute fails to process image
* inputs.
* *
* The Vision Bridge is enabled by default. For a model with unknown * SEMANTIC CHANGE (#8430): In the combo describe path, when ALL describe calls
* (`null`) vision capability it engages, tries to describe each image with the * fail (no vision-capable provider reachable on this instance), the raw image
* configured vision model, and on a FAILED describe call it replaced the image * is now replaced with an error text stub instead of being preserved. This is
* with the literal text "[Image N]: (unavailable)" — silently destroying the * safe because the combo describe path is only reached for models/targets that
* original image so the (actually vision-capable) upstream answered * are confirmed non-vision-capable — forwarding a raw image to a text-only
* "Image unavailable". A describe failure must NOT be destructive: the original * backend would produce an opaque serde error like `[400] unknown variant
* image must survive so a vision-capable upstream can still see it. * image_url, expected text`. The original #4012 preserve-raw behavior is
* maintained for the reroute path (not-combo / auto models with unknown vision
* capability), where the upstream model might still be vision-capable.
*
* Previous behavior: describe failure → preserve original image_url part
* Current behavior: total describe failure → replace with error text stub
*/ */
import test from "node:test"; import test from "node:test";
import assert from "node:assert/strict"; import assert from "node:assert/strict";
@@ -52,7 +58,7 @@ function imagePayload() {
const ctx = { model: "nvidia/google/diffusiongemma-26b-a4b-it", log: console } as never; const ctx = { model: "nvidia/google/diffusiongemma-26b-a4b-it", log: console } as never;
test("#4012 describe failure preserves the original image instead of dropping it", async () => { test("#4012/#8430 describe failure replaces image with error text stub (combo describe path)", async () => {
const guardrail = makeGuardrail(true); const guardrail = makeGuardrail(true);
const result = await guardrail.preCall(imagePayload(), ctx); const result = await guardrail.preCall(imagePayload(), ctx);
@@ -62,11 +68,14 @@ test("#4012 describe failure preserves the original image instead of dropping it
}; };
const content = modified.messages[0].content; const content = modified.messages[0].content;
// (#8430) In the combo describe path, total describe failure stubs the image
// instead of preserving it, because the upstream cannot handle raw images.
const imagePart = content.find((p) => p.type === "image_url"); const imagePart = content.find((p) => p.type === "image_url");
assert.ok(imagePart, "original image_url part must be preserved when the describe call fails"); assert.equal(imagePart, undefined, "raw image_url must be replaced when no vision provider is reachable");
const unavailable = content.find((p) => p.type === "text" && p.text?.includes("(unavailable)")); // The describe stub should contain the unavailable message
assert.equal(unavailable, undefined, "must NOT replace the image with an '(unavailable)' stub"); const stub = content.find((p) => p.type === "text" && p.text?.includes("unavailable"));
assert.ok(stub, "an error stub should be present when describe fails in the combo path");
}); });
test("#4012 successful describe still replaces the image with its text description", async () => { test("#4012 successful describe still replaces the image with its text description", async () => {