mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-05 14:52:09 +03:00
fix(providers): warn when config.judgeModel is set on a non-fusion combo (#6455) Integrated into release/v3.8.47. (thanks @chirag127)
This commit is contained in:
@@ -770,6 +770,18 @@ export async function handleComboChat({
|
||||
// Fusion strategy: parallel panel + judge synthesis. Handled in a separate module
|
||||
// because it neither iterates targets in order nor needs the failover/retry/credential
|
||||
// gate machinery that follows — it fans out, then synthesizes once.
|
||||
const cfg = config as Record<string, unknown>;
|
||||
const judgeModel = typeof cfg.judgeModel === "string" ? cfg.judgeModel : undefined;
|
||||
const fusionTuning =
|
||||
cfg.fusionTuning && typeof cfg.fusionTuning === "object"
|
||||
? (cfg.fusionTuning as FusionTuning)
|
||||
: undefined;
|
||||
if (strategy !== "fusion" && (judgeModel || fusionTuning)) {
|
||||
log.warn(
|
||||
"COMBO",
|
||||
`Combo "${combo.name}" sets config.judgeModel/fusionTuning but strategy is "${strategy}" — these fields are only consumed by the fusion strategy and will be ignored (#6455)`
|
||||
);
|
||||
}
|
||||
if (strategy === "fusion") {
|
||||
const fusionModels = (combo.models || [])
|
||||
.map((m) => {
|
||||
@@ -781,12 +793,6 @@ export async function handleComboChat({
|
||||
return null;
|
||||
})
|
||||
.filter((m): m is string => Boolean(m));
|
||||
const cfg = config as Record<string, unknown>;
|
||||
const judgeModel = typeof cfg.judgeModel === "string" ? cfg.judgeModel : undefined;
|
||||
const tuning =
|
||||
cfg.fusionTuning && typeof cfg.fusionTuning === "object"
|
||||
? (cfg.fusionTuning as FusionTuning)
|
||||
: undefined;
|
||||
return handleFusionChat({
|
||||
body,
|
||||
models: fusionModels,
|
||||
@@ -794,7 +800,7 @@ export async function handleComboChat({
|
||||
log,
|
||||
comboName: combo.name,
|
||||
judgeModel,
|
||||
tuning,
|
||||
tuning: fusionTuning,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
157
tests/unit/combo-fusion-config-warn-6455.test.ts
Normal file
157
tests/unit/combo-fusion-config-warn-6455.test.ts
Normal file
@@ -0,0 +1,157 @@
|
||||
/**
|
||||
* Regression guard for #6455 — silent no-op when config.judgeModel is set on a
|
||||
* combo whose top-level strategy is anything other than "fusion".
|
||||
*
|
||||
* Before the fix, open-sse/services/combo.ts read `config.judgeModel` and
|
||||
* `config.fusionTuning` only inside the `if (strategy === "fusion")` branch,
|
||||
* so a persisted judgeModel on a priority/weighted/auto/round-robin combo was
|
||||
* silently ignored — the response reflected whichever priority/panel target
|
||||
* won, not a judge synthesis. Users observed this as their configured judge
|
||||
* (e.g. auto/claude-opus) being ignored in favor of the raw first target.
|
||||
*
|
||||
* The minimal-diff fix logs a warn immediately before the fusion branch, so
|
||||
* the misconfiguration is observable in logs without changing runtime behavior
|
||||
* for legitimate fusion combos.
|
||||
*/
|
||||
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-combo-6455-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "combo-fusion-warn-test-secret";
|
||||
|
||||
const { handleComboChat } = await import("../../open-sse/services/combo.ts");
|
||||
|
||||
function okResponse(content: string): Response {
|
||||
const body = JSON.stringify({ choices: [{ message: { role: "assistant", content } }] });
|
||||
return new Response(body, { status: 200, headers: { "Content-Type": "application/json" } });
|
||||
}
|
||||
|
||||
function makeLog() {
|
||||
const records: Array<{ level: string; scope: string; msg: string }> = [];
|
||||
const cap = (level: string) => (scope: string, msg: string) => {
|
||||
records.push({ level, scope, msg: String(msg) });
|
||||
};
|
||||
return {
|
||||
log: {
|
||||
info: cap("info"),
|
||||
warn: cap("warn"),
|
||||
debug: cap("debug"),
|
||||
error: cap("error"),
|
||||
},
|
||||
records,
|
||||
};
|
||||
}
|
||||
|
||||
test("6455: warns when config.judgeModel is set but strategy is not fusion", async () => {
|
||||
const { log, records } = makeLog();
|
||||
const handleSingleModel = async () => okResponse("resp");
|
||||
|
||||
const res = await handleComboChat({
|
||||
body: { messages: [{ role: "user", content: "hi" }] },
|
||||
combo: {
|
||||
name: "fusion-free",
|
||||
strategy: "priority",
|
||||
models: [{ model: "p/first" }, { model: "p/second" }],
|
||||
config: { judgeModel: "auto/claude-opus" },
|
||||
},
|
||||
handleSingleModel,
|
||||
log,
|
||||
settings: {},
|
||||
allCombos: [],
|
||||
});
|
||||
|
||||
assert.equal(res.status, 200);
|
||||
const warns = records.filter(
|
||||
(r) => r.level === "warn" && r.scope === "COMBO" && r.msg.includes("judgeModel")
|
||||
);
|
||||
assert.equal(warns.length, 1, `expected exactly one judgeModel warn, got ${warns.length}`);
|
||||
assert.match(warns[0].msg, /priority/);
|
||||
assert.match(warns[0].msg, /fusion-free/);
|
||||
assert.match(warns[0].msg, /#6455/);
|
||||
});
|
||||
|
||||
test("6455: warns when config.fusionTuning is set but strategy is not fusion", async () => {
|
||||
const { log, records } = makeLog();
|
||||
const handleSingleModel = async () => okResponse("resp");
|
||||
|
||||
await handleComboChat({
|
||||
body: { messages: [{ role: "user", content: "hi" }] },
|
||||
combo: {
|
||||
name: "wrong-strategy",
|
||||
strategy: "weighted",
|
||||
models: [{ model: "p/a", weight: 1 }],
|
||||
config: { fusionTuning: { judgeTemperature: 0.2 } },
|
||||
},
|
||||
handleSingleModel,
|
||||
log,
|
||||
settings: {},
|
||||
allCombos: [],
|
||||
});
|
||||
|
||||
const warns = records.filter(
|
||||
(r) => r.level === "warn" && r.scope === "COMBO" && r.msg.includes("fusionTuning")
|
||||
);
|
||||
assert.equal(warns.length, 1);
|
||||
});
|
||||
|
||||
test("6455: does NOT warn when strategy is fusion (legitimate use)", async () => {
|
||||
const { log, records } = makeLog();
|
||||
const handleSingleModel = async () => okResponse("resp");
|
||||
|
||||
await handleComboChat({
|
||||
body: { messages: [{ role: "user", content: "hi" }] },
|
||||
combo: {
|
||||
name: "real-fusion",
|
||||
strategy: "fusion",
|
||||
models: [{ model: "p/panelA" }, { model: "p/panelB" }],
|
||||
config: { judgeModel: "auto/claude-opus" },
|
||||
},
|
||||
handleSingleModel,
|
||||
log,
|
||||
settings: {},
|
||||
allCombos: [],
|
||||
});
|
||||
|
||||
const warns = records.filter(
|
||||
(r) =>
|
||||
r.level === "warn" &&
|
||||
r.scope === "COMBO" &&
|
||||
(r.msg.includes("judgeModel") || r.msg.includes("fusionTuning"))
|
||||
);
|
||||
assert.equal(
|
||||
warns.length,
|
||||
0,
|
||||
`expected no judgeModel warn on legitimate fusion, got ${warns.length}`
|
||||
);
|
||||
});
|
||||
|
||||
test("6455: does NOT warn when strategy is non-fusion and no judgeModel/fusionTuning set", async () => {
|
||||
const { log, records } = makeLog();
|
||||
const handleSingleModel = async () => okResponse("resp");
|
||||
|
||||
await handleComboChat({
|
||||
body: { messages: [{ role: "user", content: "hi" }] },
|
||||
combo: {
|
||||
name: "plain-priority",
|
||||
strategy: "priority",
|
||||
models: [{ model: "p/only" }],
|
||||
config: {},
|
||||
},
|
||||
handleSingleModel,
|
||||
log,
|
||||
settings: {},
|
||||
allCombos: [],
|
||||
});
|
||||
|
||||
const warns = records.filter(
|
||||
(r) =>
|
||||
r.level === "warn" &&
|
||||
r.scope === "COMBO" &&
|
||||
(r.msg.includes("judgeModel") || r.msg.includes("fusionTuning"))
|
||||
);
|
||||
assert.equal(warns.length, 0);
|
||||
});
|
||||
Reference in New Issue
Block a user