mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
The fusion single-survivor degrade path (added for #6454) returned the lone panel answer directly whenever only one panelist succeeded, ignoring an explicitly configured judgeModel. With default minPanel=2 and a 2-model panel, any single flaky panelist forced this path every request, so the configured judge never ran and the response .model reflected a panel member. The judge is now still invoked to synthesize a lone surviving answer when judgeModel is explicitly configured; the direct-answer shortcut is kept only for the implicit case (no judgeModel, judge defaults to panel[0]).
This commit is contained in:
committed by
GitHub
parent
c1e3590da7
commit
978e92e104
@@ -10,6 +10,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral
|
||||
|
||||
### 🐛 Bug Fixes
|
||||
|
||||
- **fix(providers):** `fusion` combo strategy silently returned a panel member's raw answer instead of the configured `config.judgeModel` synthesis ([#6455](https://github.com/diegosouzapw/OmniRoute/issues/6455)) — `handleFusionChat()`'s single-survivor "degrade gracefully" path (added for #6454) returned the lone panel answer directly whenever only one panelist succeeded, regardless of whether an explicit `judgeModel` was configured; with the default `minPanel: 2` and a 2-model panel, any single flaky/rate-limited panelist forced this path on every request, so the configured judge (e.g. `auto/claude-opus`) was never invoked and the client-visible `.model` reflected whichever panelist happened to survive. The judge is now still invoked to synthesize a lone surviving answer whenever `judgeModel` is explicitly configured; the cheap direct-answer shortcut is kept only for the implicit case (no `judgeModel` set, where the "judge" is just `panel[0]`). Regression guard: `tests/unit/fusion-judge-model-6455.test.ts` + updated `tests/unit/combo-fusion-strategy.test.ts`. (thanks @chirag127)
|
||||
- **fix(providers):** image/diffusion models discovered from an upstream catalog (e.g. HuggingFace's live `/v1/models`) are no longer advertised as chat models ([#6457](https://github.com/diegosouzapw/OmniRoute/issues/6457)) — the chat catalog builder defaulted synced models with no modality info to `endpoints: ["chat"]`, so `huggingface/stabilityai/stable-diffusion-xl-base-1.0` showed up in the chat `/v1/models` listing and returned `400 "not a chat model"` when called. `catalog.ts` now skips any synced model already registered as an image model for that provider (via the new `isRegisteredImageModel()`), leaving `getAllImageModels()` to list it with the correct `type: "image"`. Regression guard: `tests/unit/image-model-not-in-chat-catalog-6457.test.ts`.
|
||||
- **fix(test):** replace the bare `expect(true).toBe(true)` tautology in `playground-api-tab.test.tsx`'s SSE test and close the `check:test-masking` gap that let it slip through for a full cycle ([#6404](https://github.com/diegosouzapw/OmniRoute/issues/6404)) — a prior pass (#6548) had already swapped the literal to `expect(sendBtn).toBeDefined()`, but that stayed just as vacuous: the test's fetch mock returned an empty `/v1/models` list, so `ApiTab`'s Send button is always `disabled` (`!selectedModel`) and the SSE branch never runs — the "SSE infra is verified" comment was never true. The test now mocks a real model, drives the model `<select>` to enable Send, asserts `sendBtn.disabled === false` before clicking, and asserts the streamed SSE delta (`"Hello!"`) actually reached the response editor. Root cause on the detector side: `check-test-masking.mjs`'s tautology subcheck only compares base-vs-HEAD counts within a PR's own diff (`headExtTaut > baseExtTaut`) and no-ops locally when `GITHUB_BASE_SHA`/`GITHUB_BASE_REF` are unset ("sem base ref — pulando") — so a tautology merged once, or checked with a bare local run, was invisible forever after. Added a new always-on, PR-independent absolute-floor scan (`scanBareTautologies` + `countBareTautologies`) over every git-tracked test file for the bare `expect(true).toBe(true)` / `assert.equal(1,1)` / `assert.strictEqual(1,1)` patterns specifically (deliberately excluding `assert.ok(true)`, which has ~15 pre-existing verified-legitimate try/catch-fallback uses repo-wide and stays governed by the lenient diff-only subcheck) — verified zero pre-existing hits repo-wide once this file was fixed, so the new floor is safe to enforce unconditionally. Regression guard: `tests/unit/check-test-masking.test.ts` (new `scanBareTautologies`/`countBareTautologies` cases) + `tests/unit/ui/playground-api-tab.test.tsx`. (thanks @chirag127)
|
||||
- **fix(oauth):** Codex/ChatGPT (and every other OAuth provider) connection stays stuck showing "Auth Failed" even after a genuinely successful token refresh ([#6352](https://github.com/diegosouzapw/OmniRoute/issues/6352)) — `updateProviderCredentials()` (the shared `onPersist` callback for the manual "Refresh token" route, the reactive per-request refresh in `chat.ts`, and the Codex/Claude auth-file importers) correctly reused the stored `refresh_token`, persisted the new `access_token`, and replaced a rotated `refresh_token`, but never cleared the stale `testStatus`/`lastError*`/`errorCode` fields left over from a prior expired/invalid refresh or upstream 401/403 — only the separate background health-check sweep did that clearing. A successful refresh now resets `testStatus` to `"active"` and clears `lastError`, `lastErrorAt`, `lastErrorType`, `lastErrorSource`, and `errorCode` (an explicit `testStatus` from the caller still wins). Regression guard: `tests/unit/codex-oauth-refresh-persist-6352.test.ts`.
|
||||
|
||||
@@ -250,7 +250,8 @@ export async function handleFusionChat({
|
||||
// Honor user-supplied minPanel down to 1: with 1 survivor we still degrade
|
||||
// gracefully via the answers.length===1 branch below (issue #6454).
|
||||
const minPanel = Math.min(Math.max(1, cfg.minPanel), panel.length);
|
||||
const judge = judgeModel && judgeModel.trim() ? judgeModel.trim() : panel[0];
|
||||
const hasExplicitJudge = Boolean(judgeModel && judgeModel.trim());
|
||||
const judge = hasExplicitJudge ? (judgeModel as string).trim() : panel[0];
|
||||
log.info(
|
||||
"FUSION",
|
||||
`Combo "${comboName ?? ""}" | panel=${panel.length} [${panel.join(", ")}] | judge=${judge} | quorum=${minPanel}`
|
||||
@@ -335,11 +336,24 @@ export async function handleFusionChat({
|
||||
);
|
||||
}
|
||||
if (answers.length === 1) {
|
||||
// No explicit judgeModel configured: the "judge" is just panel[0], so
|
||||
// synthesizing from a single source through itself would be redundant —
|
||||
// answer directly with the lone survivor (issue #6454).
|
||||
if (!hasExplicitJudge) {
|
||||
log.info(
|
||||
"FUSION",
|
||||
`Only ${answers[0].model} succeeded — answering directly (no fusion)`
|
||||
);
|
||||
return handleSingleModel(body, answers[0].model);
|
||||
}
|
||||
// An explicit judgeModel IS configured: honor it even with a single
|
||||
// surviving panel answer, rather than silently substituting the panel
|
||||
// member for the configured judge (issue #6455). The judge still adds
|
||||
// value reviewing/polishing a lone source per its documented contract.
|
||||
log.info(
|
||||
"FUSION",
|
||||
`Only ${answers[0].model} succeeded — answering directly (no fusion)`
|
||||
`Only ${answers[0].model} succeeded — judging single answer with ${judge}`
|
||||
);
|
||||
return handleSingleModel(body, answers[0].model);
|
||||
}
|
||||
|
||||
// 4. Judge analyzes + writes one final answer (streams to client if requested).
|
||||
|
||||
@@ -168,7 +168,7 @@ test("fusion: proceeds on quorum without waiting for a straggler (grace window)"
|
||||
assert.ok(!/slow/.test(judgeText), "straggler answer should not appear in the judge prompt");
|
||||
});
|
||||
|
||||
test("fusion: returns the lone survivor directly when only one panel model succeeds", async () => {
|
||||
test("fusion: returns the lone survivor directly when only one panel model succeeds and no judgeModel is configured", async () => {
|
||||
const seen: string[] = [];
|
||||
const handleSingleModel = async (_b: Body, m: string) => {
|
||||
seen.push(m);
|
||||
@@ -176,6 +176,37 @@ test("fusion: returns the lone survivor directly when only one panel model succe
|
||||
return errResponse(500);
|
||||
};
|
||||
await handleComboChat({
|
||||
body: { messages: [{ role: "user", content: "Q" }] },
|
||||
combo: fusionCombo(["p/ok", "p/bad"], {
|
||||
// No judgeModel configured: the implicit "judge" is just panel[0], so
|
||||
// synthesizing a single source through itself is redundant.
|
||||
fusionTuning: { minPanel: 2, stragglerGraceMs: 50, panelHardTimeoutMs: 5000 },
|
||||
}),
|
||||
handleSingleModel,
|
||||
log,
|
||||
settings: {},
|
||||
allCombos: [],
|
||||
});
|
||||
// No judge call — single answer + no explicit judge means there is nothing to fuse.
|
||||
assert.ok(
|
||||
!seen.includes("p/judge"),
|
||||
"judge should not be invoked when only one panel model survives and no judgeModel is set"
|
||||
);
|
||||
});
|
||||
|
||||
// #6455: when an explicit judgeModel IS configured, the lone-survivor degrade
|
||||
// path used to silently return the raw panel answer, never invoking the
|
||||
// configured judge. See tests/unit/fusion-judge-model-6455.test.ts for the
|
||||
// full regression guard.
|
||||
test("fusion: honors an explicit judgeModel even with a single surviving panel answer", async () => {
|
||||
const seen: string[] = [];
|
||||
const handleSingleModel = async (_b: Body, m: string) => {
|
||||
seen.push(m);
|
||||
if (m === "p/ok") return okResponse("lone");
|
||||
if (m === "p/judge") return okResponse("JUDGED");
|
||||
return errResponse(500);
|
||||
};
|
||||
const res = await handleComboChat({
|
||||
body: { messages: [{ role: "user", content: "Q" }] },
|
||||
combo: fusionCombo(["p/ok", "p/bad"], {
|
||||
judgeModel: "p/judge",
|
||||
@@ -186,11 +217,12 @@ test("fusion: returns the lone survivor directly when only one panel model succe
|
||||
settings: {},
|
||||
allCombos: [],
|
||||
});
|
||||
// No judge call — single answer means there is nothing to fuse.
|
||||
assert.ok(
|
||||
!seen.includes("p/judge"),
|
||||
"judge should not be invoked when only one panel model survives"
|
||||
seen.includes("p/judge"),
|
||||
"explicit judgeModel should still be invoked to synthesize a single panel answer"
|
||||
);
|
||||
assert.equal(seen[seen.length - 1], "p/judge");
|
||||
assert.equal(res.status, 200);
|
||||
});
|
||||
|
||||
test("fusion: returns 503 when the whole panel fails", async () => {
|
||||
|
||||
142
tests/unit/fusion-judge-model-6455.test.ts
Normal file
142
tests/unit/fusion-judge-model-6455.test.ts
Normal file
@@ -0,0 +1,142 @@
|
||||
/**
|
||||
* Regression guard for #6455 — fusion combo silently returning a panel member
|
||||
* instead of the configured judgeModel's synthesis.
|
||||
*
|
||||
* Root cause: handleFusionChat()'s "degrade gracefully" path (added for #6454)
|
||||
* returned the lone surviving panel answer directly whenever only one panel
|
||||
* member succeeded — regardless of whether an explicit `config.judgeModel`
|
||||
* was configured. With a 2-model panel and the default `minPanel: 2`, any
|
||||
* single flaky/rate-limited panelist forces this path on *every* request, so
|
||||
* the configured judge (e.g. "auto/claude-opus") was never invoked and the
|
||||
* client-visible `.model` field always reflected whichever panel member
|
||||
* happened to survive that request (case (a): judge genuinely not run).
|
||||
*
|
||||
* Fix: when an explicit judgeModel is configured, still route the lone
|
||||
* surviving answer through the judge for synthesis instead of returning it
|
||||
* raw. Only the truly implicit case (no judgeModel set, "judge" defaults to
|
||||
* panel[0]) keeps the cheap direct-answer shortcut.
|
||||
*/
|
||||
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-fusion-6455-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "fusion-6455-test-secret";
|
||||
|
||||
const { handleComboChat } = await import("../../open-sse/services/combo.ts");
|
||||
|
||||
type Body = Record<string, unknown>;
|
||||
|
||||
function jsonResponse(model: string, content: string): Response {
|
||||
const body = JSON.stringify({ model, choices: [{ message: { role: "assistant", content } }] });
|
||||
return new Response(body, { status: 200, headers: { "Content-Type": "application/json" } });
|
||||
}
|
||||
|
||||
function errResponse(status = 500): Response {
|
||||
return new Response(JSON.stringify({ error: { message: "boom" } }), {
|
||||
status,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
const noop = () => {};
|
||||
const log = { info: noop, warn: noop, debug: noop, error: noop };
|
||||
|
||||
function fusionCombo(models: string[], extra: Record<string, unknown> = {}) {
|
||||
return {
|
||||
name: "fusion-free",
|
||||
strategy: "fusion",
|
||||
models: models.map((m) => ({ model: m })),
|
||||
config: extra,
|
||||
};
|
||||
}
|
||||
|
||||
test("6455: judge synthesizes even a single surviving panel answer when judgeModel is explicit", async () => {
|
||||
const calls: string[] = [];
|
||||
const handleSingleModel = async (_b: Body, m: string) => {
|
||||
calls.push(m);
|
||||
if (m === "gpt-5.5-panelist") return jsonResponse(m, "panel answer");
|
||||
if (m === "sonar-pro-panelist") return errResponse(503); // flaky panel member
|
||||
if (m === "auto/claude-opus") return jsonResponse("auto/claude-opus", "JUDGED FINAL");
|
||||
throw new Error(`unexpected model ${m}`);
|
||||
};
|
||||
|
||||
const res = await handleComboChat({
|
||||
body: { messages: [{ role: "user", content: "hi" }] },
|
||||
combo: fusionCombo(["gpt-5.5-panelist", "sonar-pro-panelist"], {
|
||||
judgeModel: "auto/claude-opus",
|
||||
}),
|
||||
handleSingleModel,
|
||||
log,
|
||||
settings: {},
|
||||
allCombos: [],
|
||||
});
|
||||
|
||||
// (a) The judge model MUST be invoked to synthesize, not skipped.
|
||||
assert.ok(
|
||||
calls.includes("auto/claude-opus"),
|
||||
`expected the configured judge to be invoked; calls were: ${calls.join(", ")}`
|
||||
);
|
||||
// The judge call is the last one — it consumes the panel answer(s).
|
||||
assert.equal(calls[calls.length - 1], "auto/claude-opus");
|
||||
|
||||
// (b) The response body reflects the judge's synthesis, not the raw panelist.
|
||||
assert.equal(res.status, 200);
|
||||
const json = (await res.json()) as { model?: string; choices?: Array<Record<string, unknown>> };
|
||||
assert.equal(json.model, "auto/claude-opus");
|
||||
const message = json.choices?.[0]?.message as { content?: string } | undefined;
|
||||
assert.equal(message?.content, "JUDGED FINAL");
|
||||
});
|
||||
|
||||
test("6455: panel still fans out to every member before degrading (no regression)", async () => {
|
||||
const calls: string[] = [];
|
||||
const handleSingleModel = async (_b: Body, m: string) => {
|
||||
calls.push(m);
|
||||
if (m === "p/a") return jsonResponse(m, "answer-a");
|
||||
if (m === "p/b") return jsonResponse(m, "answer-b");
|
||||
if (m === "p/judge") return jsonResponse("p/judge", "FUSED");
|
||||
throw new Error(`unexpected model ${m}`);
|
||||
};
|
||||
|
||||
const res = await handleComboChat({
|
||||
body: { messages: [{ role: "user", content: "hi" }] },
|
||||
combo: fusionCombo(["p/a", "p/b"], { judgeModel: "p/judge" }),
|
||||
handleSingleModel,
|
||||
log,
|
||||
settings: {},
|
||||
allCombos: [],
|
||||
});
|
||||
|
||||
assert.deepEqual(calls.slice(0, 2).sort(), ["p/a", "p/b"]);
|
||||
assert.equal(calls[2], "p/judge");
|
||||
assert.equal(res.status, 200);
|
||||
const json = (await res.json()) as { model?: string };
|
||||
assert.equal(json.model, "p/judge");
|
||||
});
|
||||
|
||||
test("6455: implicit judge (no judgeModel configured) still short-circuits a lone survivor", async () => {
|
||||
const calls: string[] = [];
|
||||
const handleSingleModel = async (_b: Body, m: string) => {
|
||||
calls.push(m);
|
||||
if (m === "p/ok") return jsonResponse(m, "lone");
|
||||
return errResponse(500);
|
||||
};
|
||||
|
||||
await handleComboChat({
|
||||
body: { messages: [{ role: "user", content: "hi" }] },
|
||||
combo: fusionCombo(["p/ok", "p/bad"]), // no judgeModel — defaults to panel[0]
|
||||
handleSingleModel,
|
||||
log,
|
||||
settings: {},
|
||||
allCombos: [],
|
||||
});
|
||||
|
||||
// Panel fan-out calls p/ok and p/bad; the lone survivor is then answered
|
||||
// directly with the client's original body (not a judge-synthesis turn) —
|
||||
// panel[0] IS the implicit judge, so there is nothing to gain by routing
|
||||
// it through a separate synthesis call.
|
||||
assert.deepEqual(calls, ["p/ok", "p/bad", "p/ok"]);
|
||||
});
|
||||
Reference in New Issue
Block a user