fix(providers): keep image/diffusion models out of the chat models catalog (#6457) (#6606)

This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-07-07 21:45:15 -03:00
committed by GitHub
parent ebdfe727a6
commit c1e3590da7
4 changed files with 129 additions and 1 deletions

View File

@@ -10,6 +10,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral
### 🐛 Bug Fixes
- **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`.
- **fix(resilience):** `headroom` combo routing did not always select the Codex account with the most free quota ([#6379](https://github.com/diegosouzapw/OmniRoute/issues/6379)) — `orderTargetsByHeadroom` (`open-sse/services/combo/quotaStrategies.ts`) already loaded the per-connection DB snapshot (with decrypted credentials) via `expandTargetsByQuotaAwareConnections`, but discarded it before calling `getSaturation`; for Codex, `fetchCodexSaturation` forwards straight to `fetchCodexQuota(connectionId, connection)`, which needs `connection` (or a prior `registerCodexConnection()` call, which never happens before headroom ranking runs) to read `accessToken` — so it returned `null` for every candidate, saturation failed open to `0` across the board, and ranking fell back to the original combo order regardless of actual free quota. `getSaturation()` and the headroom `SaturationFetcher` seam now accept and thread the loaded connection snapshot through to `fetchCodexQuota`. Kilo's dup flag vs #5903 was a false positive — that issue is about session-sticky reset-aware/least-used selection, not headroom's Codex saturation lookup. Regression guard: `tests/unit/headroom-codex-quota-snapshot-6379.test.ts`. (thanks @eidoog)

View File

@@ -723,6 +723,19 @@ export function getImageModelAliases() {
return IMAGE_MODEL_ALIASES;
}
/**
* #6457 — precise provider+modelId membership check against the image registry.
* Unlike getImageModelEntry() (which also resolves bare aliases and unprefixed
* ids by scanning every provider), this only answers "is `modelId` registered
* as an image model under this exact `providerId`?" — used by the chat catalog
* builder to keep upstream-discovered models (e.g. HuggingFace's live
* `/v1/models`, which returns image/diffusion models with no modality field)
* out of the chat listing when they are already known image-only models.
*/
export function isRegisteredImageModel(providerId, modelId) {
return Boolean(findImageModelConfig(providerId, modelId));
}
export function getImageModelEntry(modelStr) {
if (!modelStr) return null;

View File

@@ -12,7 +12,7 @@ import {
import { extractAliasBackedModels } from "./aliasBackedModels";
import { appendNoThinkingVariants } from "@omniroute/open-sse/utils/noThinkingAlias";
import { getAllEmbeddingModels } from "@omniroute/open-sse/config/embeddingRegistry";
import { getAllImageModels } from "@omniroute/open-sse/config/imageRegistry";
import { getAllImageModels, isRegisteredImageModel } from "@omniroute/open-sse/config/imageRegistry";
import { getAllRerankModels } from "@omniroute/open-sse/config/rerankRegistry";
import { getAllAudioModels } from "@omniroute/open-sse/config/audioRegistry";
import { getAllModerationModels } from "@omniroute/open-sse/config/moderationRegistry";
@@ -849,6 +849,18 @@ async function buildUnifiedModelsResponseCore(
for (const sm of syncedModels) {
if (!providerSupportsModel(canonicalProviderId, sm.id)) continue;
if (getModelIsHidden(providerId, sm.id)) continue;
// #6457: some upstream discovery catalogs (e.g. HuggingFace's live
// `/v1/models`) return image/diffusion models with no modality info,
// so `endpoints` below would default to ["chat"] and misrepresent
// them as chat-capable. Skip any synced model that is already a
// registered image model for this provider — getAllImageModels()
// below adds the correctly-typed `type: "image"` entry instead.
if (
isRegisteredImageModel(canonicalProviderId, sm.id) ||
isRegisteredImageModel(providerId, sm.id)
) {
continue;
}
// #6328: apply hidePaidModels to synced provider rows too. Synced rows
// rarely carry pricing metadata, so shouldHidePaid() falls through to
// the FREE_MODEL_IDS_BY_PROVIDER catalog — providers with a curated

View File

@@ -0,0 +1,102 @@
// Regression test for #6457 — huggingface/stabilityai/stable-diffusion-xl-base-1.0
// (an image/diffusion model) was listed as a CHAT model in GET /v1/models, so firing
// POST /v1/chat/completions with it hit the upstream and returned a raw HuggingFace
// "[400] The requested model '...' is not a chat model." error.
//
// Root cause: the synced-provider-models loop in catalog.ts (fed by live discovery —
// e.g. HuggingFace's own `/v1/models`) defaults a model's `endpoints` to `["chat"]`
// whenever the upstream discovery payload carries no modality/endpoint info, which is
// exactly what HuggingFace's live catalog returns for image models. That produced a
// second, bogus chat-typed entry for the SAME id already correctly listed with
// `type: "image"` by the imageRegistry loop — and catalogDedupe.ts keys on
// (id, type, subtype), so the two distinct-`type` entries both survived.
//
// Fix: skip a synced model in the chat-catalog loop when it is already a registered
// image model for that exact provider (open-sse/config/imageRegistry.ts
// isRegisteredImageModel()) — the imageRegistry loop still adds the correctly-typed
// `type: "image"` entry.
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-image-chat-6457-"));
process.env.DATA_DIR = TEST_DATA_DIR;
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "catalog-test-secret-6457";
const core = await import("../../src/lib/db/core.ts");
const providersDb = await import("../../src/lib/db/providers.ts");
const modelsDb = await import("../../src/lib/db/models.ts");
const v1ModelsCatalog = await import("../../src/app/api/v1/models/catalog.ts");
async function resetStorage() {
core.resetDbInstance();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
}
test.beforeEach(async () => {
await resetStorage();
});
test.after(async () => {
core.resetDbInstance();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
});
async function seedHuggingFaceConnection() {
return providersDb.createProviderConnection({
provider: "huggingface",
authType: "apikey",
name: `huggingface-${Math.random().toString(16).slice(2, 8)}`,
apiKey: "hf-key",
isActive: true,
testStatus: "active",
});
}
test("#6457 image/diffusion model discovered via live sync is NOT listed as a chat model", async () => {
const connection = await seedHuggingFaceConnection();
// Simulate what HuggingFace's live `/v1/models` discovery persists for an
// image/diffusion model: no supportedEndpoints/modality info at all — the exact
// upstream shape that made the synced-models loop default to `["chat"]`.
await modelsDb.replaceSyncedAvailableModelsForConnection("huggingface", connection.id, [
{ id: "stabilityai/stable-diffusion-xl-base-1.0", name: "Stable Diffusion XL (HF)" },
{ id: "meta-llama/llama-3.1-8b-instruct", name: "Llama 3.1 8B" },
]);
const response = await v1ModelsCatalog.getUnifiedModelsResponse(
new Request("http://localhost/api/v1/models")
);
assert.equal(response.status, 200);
const body = (await response.json()) as {
data: Array<{ id: string; type?: string }>;
};
const imageModelEntries = body.data.filter((m) =>
m.id.includes("stabilityai/stable-diffusion-xl-base-1.0")
);
assert.ok(imageModelEntries.length > 0, "the image model must still be listed somewhere");
for (const entry of imageModelEntries) {
assert.equal(
entry.type,
"image",
`every listing of the diffusion model must be type:"image", got ${JSON.stringify(entry)}`
);
}
// A real chat model synced alongside it must still be listed as chat (no `type`,
// per the OpenAI-compatible convention used throughout this catalog).
const chatModelEntries = body.data.filter((m) =>
m.id.includes("meta-llama/llama-3.1-8b-instruct")
);
assert.ok(chatModelEntries.length > 0, "the real chat model must still be listed");
for (const entry of chatModelEntries) {
assert.equal(entry.type, undefined, "the real chat model must not carry a non-chat type");
}
});