mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-21 14:22:14 +03:00
fix(guardrails): resolve nested combo-ref hops before vision-bridge decision (#13927)
getComboVisionBridgeDecision() treated any top-level combo-ref step as an unconditional "process", without ever resolving the referenced combo's real leaf models. A pass-through combo whose only member is a combo-ref to an all-vision-capable inner combo was wrongly routed through the describe-and-replace path, and with no describer model configured every image was replaced with the literal stub text. Recursively resolve combo-ref steps to their real leaf models (depth-guarded by the same MAX_COMBO_DEPTH used by the flatten dispatch path, plus a visited-set cycle guard) and fold their vision capability into the same accumulation used for direct model steps. An unresolvable combo-ref (not found / empty / circular / depth-exceeded) is conservatively treated as a single non-vision-capable leaf instead of forcing the whole combo to "process".
This commit is contained in:
1
changelog.d/fixes/13927-combo-ref-vision-bridge.md
Normal file
1
changelog.d/fixes/13927-combo-ref-vision-bridge.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(guardrails): resolve nested `combo-ref` steps to their real leaf models when deciding vision-bridge behavior, so a pass-through combo pointing at an all-vision-capable inner combo skips the describe-and-replace path instead of stripping raw images (#13927)
|
||||
@@ -31,11 +31,79 @@ import {
|
||||
isProviderConnectionUsable,
|
||||
hasUsableCredentialsForModel,
|
||||
} from "./visionBridgeCredentials";
|
||||
import { MAX_COMBO_DEPTH } from "@omniroute/open-sse/services/combo/comboPredicates.ts";
|
||||
|
||||
export { isProviderConnectionUsable, hasUsableCredentialsForModel };
|
||||
|
||||
type ComboVisionBridgeDecision = "process" | "skip" | "not-combo" | "no-vision";
|
||||
|
||||
type LeafVisionTally = { hasVision: boolean; hasNonVision: boolean };
|
||||
|
||||
/// Evaluate a single `kind: "model"` step's proven vision capability.
|
||||
/// Returns null when the step lacks a valid model string (malformed step).
|
||||
function evaluateModelStepCapability(s: Record<string, unknown>): "vision" | "non-vision" | null {
|
||||
const targetModel = s.model;
|
||||
if (typeof targetModel !== "string") return null;
|
||||
const provider =
|
||||
typeof s.providerId === "string"
|
||||
? s.providerId
|
||||
: typeof s.provider === "string"
|
||||
? s.provider
|
||||
: null;
|
||||
const caps = getResolvedModelCapabilities({ provider, model: targetModel });
|
||||
return caps.supportsVision === true ? "vision" : "non-vision";
|
||||
}
|
||||
|
||||
/// Recursively resolve a `combo-ref` step to its real leaf models' vision
|
||||
/// capability, reusing the same MAX_COMBO_DEPTH guard as the flatten dispatch
|
||||
/// path (open-sse/services/combo/comboStructure.ts) plus a visited-set cycle
|
||||
/// guard, so this request-hot-path lookup can never recurse unbounded or loop
|
||||
/// on a cyclic combo-ref chain.
|
||||
///
|
||||
/// Unresolvable cases (combo not found, empty/invalid models, depth exceeded,
|
||||
/// or a cycle) fall back to treating the combo-ref step as a single
|
||||
/// non-vision-capable leaf -- conservative, but no longer forces the WHOLE
|
||||
/// outer combo to "process" the way the old unconditional shortcut did.
|
||||
async function resolveComboRefVisionCapability(
|
||||
comboName: string,
|
||||
visited: Set<string>,
|
||||
depth: number
|
||||
): Promise<LeafVisionTally> {
|
||||
const fallback: LeafVisionTally = { hasVision: false, hasNonVision: true };
|
||||
if (depth > MAX_COMBO_DEPTH || visited.has(comboName)) return fallback;
|
||||
|
||||
const { getComboByName } = await import("@/lib/db/combos");
|
||||
const nestedCombo = await getComboByName(comboName);
|
||||
if (!nestedCombo) return fallback;
|
||||
|
||||
const nestedVisited = new Set(visited);
|
||||
nestedVisited.add(comboName);
|
||||
|
||||
const nestedRawModels = (nestedCombo as Record<string, unknown>).models;
|
||||
if (!Array.isArray(nestedRawModels) || nestedRawModels.length === 0) return fallback;
|
||||
|
||||
const tally: LeafVisionTally = { hasVision: false, hasNonVision: false };
|
||||
let hasLeaf = false;
|
||||
for (const step of nestedRawModels) {
|
||||
const s = step as Record<string, unknown>;
|
||||
if (s.kind === "combo-ref" && typeof s.comboName === "string") {
|
||||
hasLeaf = true;
|
||||
const nested = await resolveComboRefVisionCapability(s.comboName, nestedVisited, depth + 1);
|
||||
tally.hasVision = tally.hasVision || nested.hasVision;
|
||||
tally.hasNonVision = tally.hasNonVision || nested.hasNonVision;
|
||||
continue;
|
||||
}
|
||||
if (s.kind === "model") {
|
||||
hasLeaf = true;
|
||||
const capability = evaluateModelStepCapability(s);
|
||||
if (capability === "vision") tally.hasVision = true;
|
||||
else tally.hasNonVision = true;
|
||||
}
|
||||
}
|
||||
|
||||
return hasLeaf ? tally : fallback;
|
||||
}
|
||||
|
||||
export function resolveVisionComboName(mapping: Record<string, unknown>): string | null {
|
||||
const comboName = mapping.comboName ?? mapping.name ?? null;
|
||||
return typeof comboName === "string" && comboName.length > 0 ? comboName : null;
|
||||
@@ -75,37 +143,43 @@ export async function getComboVisionBridgeDecision(
|
||||
if (!Array.isArray(rawModels)) return "process";
|
||||
|
||||
// 4. Check each target for vision support
|
||||
// combo-ref → conservative (process images)
|
||||
// combo-ref → recursively resolve the referenced combo's real leaf
|
||||
// models (depth/cycle-guarded); unresolvable → conservative non-vision leaf
|
||||
// model step with no native vision → process images
|
||||
// all model steps with native vision → safe to skip
|
||||
// zero vision-capable model steps → "no-vision" (reroute-eligible)
|
||||
let hasModelStep = false;
|
||||
let hasVisionCapableStep = false;
|
||||
let hasNonVisionStep = false;
|
||||
const rootComboName =
|
||||
typeof (combo as Record<string, unknown>).name === "string"
|
||||
? ((combo as Record<string, unknown>).name as string)
|
||||
: model;
|
||||
for (const step of rawModels) {
|
||||
const s = step as Record<string, unknown>;
|
||||
if (s.kind === "combo-ref") return "process";
|
||||
if (s.kind === "combo-ref") {
|
||||
hasModelStep = true;
|
||||
if (typeof s.comboName !== "string") {
|
||||
hasNonVisionStep = true;
|
||||
continue;
|
||||
}
|
||||
const nested = await resolveComboRefVisionCapability(
|
||||
s.comboName,
|
||||
new Set([rootComboName]),
|
||||
1
|
||||
);
|
||||
if (nested.hasVision) hasVisionCapableStep = true;
|
||||
if (nested.hasNonVision) hasNonVisionStep = true;
|
||||
continue;
|
||||
}
|
||||
if (s.kind === "model") {
|
||||
hasModelStep = true;
|
||||
const targetModel = s.model;
|
||||
if (typeof targetModel === "string") {
|
||||
const provider =
|
||||
typeof s.providerId === "string"
|
||||
? s.providerId
|
||||
: typeof s.provider === "string"
|
||||
? s.provider
|
||||
: null;
|
||||
const caps = getResolvedModelCapabilities({
|
||||
provider,
|
||||
model: targetModel,
|
||||
});
|
||||
if (caps.supportsVision === true) {
|
||||
hasVisionCapableStep = true;
|
||||
} else {
|
||||
hasNonVisionStep = true;
|
||||
}
|
||||
const capability = evaluateModelStepCapability(s);
|
||||
if (capability === null) return "process";
|
||||
if (capability === "vision") {
|
||||
hasVisionCapableStep = true;
|
||||
} else {
|
||||
return "process";
|
||||
hasNonVisionStep = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
148
tests/unit/combo-ref-vision-bridge-13927.test.ts
Normal file
148
tests/unit/combo-ref-vision-bridge-13927.test.ts
Normal file
@@ -0,0 +1,148 @@
|
||||
/**
|
||||
* Repro probe for #13927: combo-ref hop strips image parts.
|
||||
*
|
||||
* getComboVisionBridgeDecision() treats ANY top-level `combo-ref` step as an
|
||||
* unconditional "process" (describe images as text) without ever resolving
|
||||
* the nested combo to see whether its actual leaf targets support vision.
|
||||
* For a pass-through combo whose single member is a combo-ref to another
|
||||
* combo made entirely of vision-capable models, this wrongly triggers the
|
||||
* describe-and-replace path instead of "skip" (pass the image through raw),
|
||||
* which is what happens for the flat inner combo or the leaf model called
|
||||
* directly.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
process.env.DATA_DIR = `/tmp/omniroute-test-13927-${Date.now()}`;
|
||||
|
||||
const { getComboVisionBridgeDecision } = await import("../../src/lib/guardrails/visionBridge.ts");
|
||||
const combosDb = await import("../../src/lib/db/combos.ts");
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
|
||||
test.after(() => {
|
||||
core.resetDbInstance();
|
||||
});
|
||||
|
||||
test("#13927: nested combo-ref to an all-vision-capable inner combo should resolve to skip, not process", async () => {
|
||||
// Inner combo: flat, all leaf targets are proven vision-capable
|
||||
// (command-code/gpt-5.5 is asserted vision-capable elsewhere, e.g.
|
||||
// tests/unit/vision-bridge-native-skip.test.ts).
|
||||
await combosDb.createCombo({
|
||||
name: "inner-vision-combo-13927",
|
||||
models: [{ providerId: "command-code", model: "gpt-5.5", weight: 1 }],
|
||||
});
|
||||
|
||||
// Outer combo: single member is a combo-ref to the inner (all-vision) combo
|
||||
// -- exactly the "preset/default -> preset/deepseek-flash-latest" shape from
|
||||
// the issue.
|
||||
await combosDb.createCombo({
|
||||
name: "outer-passthrough-combo-13927",
|
||||
models: [{ kind: "combo-ref", comboName: "inner-vision-combo-13927" }],
|
||||
});
|
||||
|
||||
const innerDecision = await getComboVisionBridgeDecision("inner-vision-combo-13927");
|
||||
assert.equal(
|
||||
innerDecision,
|
||||
"skip",
|
||||
"sanity: the flat inner combo (all vision-capable targets) must resolve to skip"
|
||||
);
|
||||
|
||||
const outerDecision = await getComboVisionBridgeDecision("outer-passthrough-combo-13927");
|
||||
assert.equal(
|
||||
outerDecision,
|
||||
"skip",
|
||||
"BUG #13927: outer combo whose only member is a combo-ref to an all-vision-capable " +
|
||||
"combo must ALSO resolve to skip (image should pass through raw), not 'process' " +
|
||||
"(which describes the image as text and drops the raw bytes before the leaf ever sees them)"
|
||||
);
|
||||
});
|
||||
|
||||
test("#13927: combo-ref to a mixed-capability inner combo resolves to process", async () => {
|
||||
// Inner combo mixes one vision-capable and one non-vision-capable leaf.
|
||||
await combosDb.createCombo({
|
||||
name: "inner-mixed-combo-13927",
|
||||
models: [
|
||||
{ providerId: "command-code", model: "gpt-5.5", weight: 1 },
|
||||
{ providerId: "mistral", model: "mistral-large-latest", weight: 1 },
|
||||
],
|
||||
});
|
||||
|
||||
await combosDb.createCombo({
|
||||
name: "outer-mixed-ref-combo-13927",
|
||||
models: [{ kind: "combo-ref", comboName: "inner-mixed-combo-13927" }],
|
||||
});
|
||||
|
||||
const decision = await getComboVisionBridgeDecision("outer-mixed-ref-combo-13927");
|
||||
assert.equal(
|
||||
decision,
|
||||
"process",
|
||||
"a combo-ref to a mixed inner combo must resolve real leaf capabilities and still " +
|
||||
"describe images (some real leaves cannot see them)"
|
||||
);
|
||||
});
|
||||
|
||||
test("#13927: combo-ref to a zero-vision inner combo resolves to no-vision", async () => {
|
||||
await combosDb.createCombo({
|
||||
name: "inner-zero-vision-combo-13927",
|
||||
models: [{ providerId: "mistral", model: "mistral-large-latest", weight: 1 }],
|
||||
});
|
||||
|
||||
await combosDb.createCombo({
|
||||
name: "outer-zero-vision-ref-combo-13927",
|
||||
models: [{ kind: "combo-ref", comboName: "inner-zero-vision-combo-13927" }],
|
||||
});
|
||||
|
||||
const decision = await getComboVisionBridgeDecision("outer-zero-vision-ref-combo-13927");
|
||||
assert.equal(
|
||||
decision,
|
||||
"no-vision",
|
||||
"a combo-ref to an all-non-vision inner combo must behave like a text-only model " +
|
||||
"(reroute-eligible), matching the flat-combo 'no-vision' outcome"
|
||||
);
|
||||
});
|
||||
|
||||
test("#13927: circular combo-ref chain falls back safely and never throws", async () => {
|
||||
// A -> B -> A. Neither combo has a real leaf model, only cyclic combo-refs,
|
||||
// so both sides of the cycle hit the visited-set guard.
|
||||
await combosDb.createCombo({
|
||||
name: "circular-a-13927",
|
||||
models: [{ kind: "combo-ref", comboName: "circular-b-13927" }],
|
||||
});
|
||||
await combosDb.createCombo({
|
||||
name: "circular-b-13927",
|
||||
models: [{ kind: "combo-ref", comboName: "circular-a-13927" }],
|
||||
});
|
||||
|
||||
await assert.doesNotReject(async () => {
|
||||
const decision = await getComboVisionBridgeDecision("circular-a-13927");
|
||||
// Implementation-plan contract: an unresolvable/circular combo-ref step is
|
||||
// folded in as a non-vision-capable leaf rather than forcing the whole
|
||||
// combo back to "process" -- with no other real leaf present, that lands
|
||||
// on "no-vision" (safe, reroute-eligible), never a crash.
|
||||
assert.equal(decision, "no-vision");
|
||||
});
|
||||
});
|
||||
|
||||
test("#13927: circular combo-ref mixed with a real vision-capable leaf resolves to process", async () => {
|
||||
// A real vision-capable leaf plus an unresolvable circular combo-ref: the
|
||||
// circular hop still contributes a conservative non-vision leaf, so the
|
||||
// combo is "mixed" and must describe images, not skip.
|
||||
await combosDb.createCombo({
|
||||
name: "circular-c-13927",
|
||||
models: [{ kind: "combo-ref", comboName: "circular-d-13927" }],
|
||||
});
|
||||
await combosDb.createCombo({
|
||||
name: "circular-d-13927",
|
||||
models: [{ kind: "combo-ref", comboName: "circular-c-13927" }],
|
||||
});
|
||||
await combosDb.createCombo({
|
||||
name: "outer-circular-mixed-combo-13927",
|
||||
models: [
|
||||
{ providerId: "command-code", model: "gpt-5.5", weight: 1 },
|
||||
{ kind: "combo-ref", comboName: "circular-c-13927" },
|
||||
],
|
||||
});
|
||||
|
||||
const decision = await getComboVisionBridgeDecision("outer-circular-mixed-combo-13927");
|
||||
assert.equal(decision, "process");
|
||||
});
|
||||
Reference in New Issue
Block a user