mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
fix(combos): show embedding/rerank models and disambiguate duplicate names in builder options (#6975, #6957) (#6991)
Removes the leftover chat-only isChatCapable gate from addModelOption() (#6975) and adds a name-disambiguation pass at the end of buildModelOptions() so distinct model ids sharing the same upstream display name fall back to their id (#6957). Both proven with TDD repro tests (RED->GREEN).
This commit is contained in:
committed by
GitHub
parent
94328d5cb2
commit
9b43a00b60
2
changelog.d/fixes/6975-6957-combo-builder-models.md
Normal file
2
changelog.d/fixes/6975-6957-combo-builder-models.md
Normal file
@@ -0,0 +1,2 @@
|
||||
- **fix(combos):** embeddings-only and rerank-only models (e.g. JinaAI, Gemini auto-imported, OpenRouter custom, reranker models) no longer disappear from the combo builder's model picker — the leftover chat-only `isChatCapable` gate in `addModelOption()` has been removed (#6975).
|
||||
- **fix(combos):** when 2+ distinct model ids from the same provider would render an identical display name in the combo builder picker (e.g. Mistral's `codestral-latest`/`codestral-2508` aliases sharing one upstream catalog name), each colliding entry now falls back to its own id as the display label so every row stays visually distinguishable and findable (#6957).
|
||||
@@ -155,11 +155,6 @@ function toStringArray(value: unknown): string[] | undefined {
|
||||
return normalized.length > 0 ? normalized : undefined;
|
||||
}
|
||||
|
||||
function isChatCapable(supportedEndpoints: string[] | undefined): boolean {
|
||||
if (!supportedEndpoints || supportedEndpoints.length === 0) return true;
|
||||
return supportedEndpoints.includes("chat");
|
||||
}
|
||||
|
||||
function getSourcePriority(source: BuilderModelSource): number {
|
||||
switch (source) {
|
||||
case "imported":
|
||||
@@ -305,7 +300,6 @@ function addModelOption(
|
||||
const modelId = toStringOrNull(input.id);
|
||||
if (!modelId) return;
|
||||
if (getModelIsHidden(providerId, modelId)) return;
|
||||
if (!isChatCapable(input.supportedEndpoints)) return;
|
||||
|
||||
const nextSourcePriority = getSourcePriority(input.source);
|
||||
const existing = modelMap.get(modelId);
|
||||
@@ -449,9 +443,36 @@ function buildModelOptions(
|
||||
}
|
||||
}
|
||||
|
||||
disambiguateCollidingModelNames(modelMap);
|
||||
return modelMap;
|
||||
}
|
||||
|
||||
/**
|
||||
* #6957: some providers' own catalogs assign the identical display `name` to
|
||||
* several distinct model ids (e.g. Mistral's "codestral-latest" alias renders
|
||||
* under the same upstream name as its base "codestral-2508" model). Since the
|
||||
* builder picker renders `model.name` as the visible option text, two colliding
|
||||
* names make genuinely different models look like duplicates and hide aliases.
|
||||
* Run this after all merge loops have populated `modelMap`: for any name shared
|
||||
* by 2+ distinct ids, fall back every entry in that group to its own `id` as the
|
||||
* display name (display-only — `id`/`qualifiedModel` used for routing untouched).
|
||||
*/
|
||||
function disambiguateCollidingModelNames(modelMap: Map<string, ComboBuilderModelOption>): void {
|
||||
const idsByName = new Map<string, string[]>();
|
||||
for (const option of modelMap.values()) {
|
||||
const bucket = idsByName.get(option.name) || [];
|
||||
bucket.push(option.id);
|
||||
idsByName.set(option.name, bucket);
|
||||
}
|
||||
for (const [name, ids] of idsByName) {
|
||||
if (ids.length < 2) continue;
|
||||
for (const id of ids) {
|
||||
const option = modelMap.get(id);
|
||||
if (option && option.name === name) option.name = option.id;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function compareConnections(
|
||||
left: ComboBuilderConnectionOption,
|
||||
right: ComboBuilderConnectionOption
|
||||
|
||||
@@ -103,10 +103,12 @@ test("combo builder options route aggregates providers, connections, models and
|
||||
});
|
||||
|
||||
await modelsDb.addCustomModel("openai", "custom-ops", "Custom Ops");
|
||||
// #6975: embeddings-only models (supportedEndpoints without "chat") are no longer
|
||||
// dropped from the combo builder — they must appear like any other model.
|
||||
await modelsDb.addCustomModel(
|
||||
"openai",
|
||||
"text-embedding-hidden",
|
||||
"Hidden Embedding",
|
||||
"text-embedding-visible",
|
||||
"Text Embedding",
|
||||
"manual",
|
||||
"chat-completions",
|
||||
["embeddings"]
|
||||
@@ -148,9 +150,10 @@ test("combo builder options route aggregates providers, connections, models and
|
||||
false
|
||||
);
|
||||
assert.ok(openai.models.some((model) => model.id === "custom-ops"));
|
||||
// #6975: embeddings-only models must now appear in the combo builder output.
|
||||
assert.equal(
|
||||
openai.models.some((model) => model.id === "text-embedding-hidden"),
|
||||
false
|
||||
openai.models.some((model) => model.id === "text-embedding-visible"),
|
||||
true
|
||||
);
|
||||
assert.deepEqual(
|
||||
openai.connections.map((connection) => ({
|
||||
|
||||
130
tests/unit/repro-6957.test.ts
Normal file
130
tests/unit/repro-6957.test.ts
Normal file
@@ -0,0 +1,130 @@
|
||||
/**
|
||||
* Repro for #6957 — combo builder "2. Model" dropdown shows visually duplicated
|
||||
* "imported" rows and appears to be missing the "-latest" aliases for a native
|
||||
* Mistral provider with 2 API-key connections.
|
||||
*
|
||||
* Root cause (confirmed against the reporter's actual `GET /api/combos/builder/options`
|
||||
* payload, issue #6957): there is NO literal `model.id` collision — every synced
|
||||
* model id is already unique after `buildModelOptions()`/`getAllSyncedAvailableModels()`
|
||||
* dedup. The bug is that `ComboBuilderModelOption.name` (the text rendered in the
|
||||
* `<option>` in `src/app/(dashboard)/dashboard/combos/page.tsx:3217-3220`) is
|
||||
* populated straight from the upstream-synced `model.name`, which Mistral's own
|
||||
* /v1/models catalog resolves to a shared "canonical" display name for every alias
|
||||
* in a model family. So `codestral-2508`, `codestral-latest`,
|
||||
* `mistral-code-fim-latest` and `mistral-code-latest` are 4 DISTINCT ids that all
|
||||
* render as the literal text "codestral-2508 · imported" — indistinguishable in the
|
||||
* picker — and the `-latest` alias (e.g. `mistral-large-latest`) is present in the
|
||||
* payload but invisible/unfindable because it displays under its base model's name
|
||||
* instead of its own.
|
||||
*/
|
||||
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-6957-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
|
||||
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 { getComboBuilderOptions } = await import("../../src/lib/combos/builderOptions.ts");
|
||||
|
||||
test.after(() => {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// A trimmed slice of the reporter's actual payload (issue #6957 comment attachment
|
||||
// Untitled-1.json): 4 distinct ids in the "codestral" family that all resolve to
|
||||
// the same upstream display name, plus the "mistral-large" base/-latest pair.
|
||||
const MISTRAL_FAMILY_SLICE = [
|
||||
{ id: "codestral-2508", name: "codestral-2508" },
|
||||
{ id: "codestral-latest", name: "codestral-2508" },
|
||||
{ id: "mistral-code-fim-latest", name: "codestral-2508" },
|
||||
{ id: "mistral-code-latest", name: "codestral-2508" },
|
||||
{ id: "mistral-large-2512", name: "mistral-large-2512" },
|
||||
{ id: "mistral-large-latest", name: "mistral-large-2512" },
|
||||
];
|
||||
|
||||
test("#6957 native Mistral provider with 2 connections: synced models produce a unique id per model but ambiguous/colliding display names", async () => {
|
||||
// Native Mistral provider, 2 API-key connections — mirrors the reporter's setup
|
||||
// (providerId "mistral", connectionCount 2, both accounts auto-syncing the same
|
||||
// upstream catalog).
|
||||
const conn1 = await providersDb.createProviderConnection({
|
||||
provider: "mistral",
|
||||
authType: "apikey",
|
||||
name: "ac1",
|
||||
apiKey: "sk-mistral-ac1",
|
||||
});
|
||||
const conn2 = await providersDb.createProviderConnection({
|
||||
provider: "mistral",
|
||||
authType: "apikey",
|
||||
name: "ac2",
|
||||
apiKey: "sk-mistral-ac2",
|
||||
});
|
||||
|
||||
// Both connections sync the identical upstream catalog slice (same account
|
||||
// family just imported twice, matching "connectionCount: 2" in the reporter's
|
||||
// JSON — this is what produces the appearance of duplicates in the builder).
|
||||
for (const conn of [conn1, conn2]) {
|
||||
await modelsDb.replaceSyncedAvailableModelsForConnection(
|
||||
"mistral",
|
||||
conn.id,
|
||||
MISTRAL_FAMILY_SLICE.map((m) => ({ id: m.id, name: m.name, source: "imported" }))
|
||||
);
|
||||
}
|
||||
|
||||
const payload = await getComboBuilderOptions();
|
||||
const mistral = payload.providers.find((p) => p.providerId === "mistral");
|
||||
assert.ok(mistral, "mistral provider must appear in the combo builder output");
|
||||
assert.equal(mistral!.connectionCount, 2, "sanity: 2 connections, matching the reporter's setup");
|
||||
|
||||
const models = mistral!.models.filter((m) => MISTRAL_FAMILY_SLICE.some((f) => f.id === m.id));
|
||||
|
||||
// No literal id duplicates — confirms the earlier needs-info hypothesis (a
|
||||
// per-connection modelMap dedup gap) was WRONG; every id is already unique.
|
||||
const ids = models.map((m) => m.id);
|
||||
assert.equal(new Set(ids).size, ids.length, "sanity: model ids are already unique");
|
||||
assert.equal(models.length, MISTRAL_FAMILY_SLICE.length, "all 6 distinct models must be present");
|
||||
|
||||
// THE BUG: the picker renders `model.name` as the visible label
|
||||
// (page.tsx:3217-3220 `{model.name}{model.source ? ... : ""}`). Today,
|
||||
// `buildModelOptions()`/`addModelOption()` (builderOptions.ts) passes the
|
||||
// upstream-synced name straight through with no disambiguation, so 4 distinct
|
||||
// models render identical text and the "-latest" alias is indistinguishable
|
||||
// from its base model — reproducing both reported symptoms at once.
|
||||
const codestralLatest = models.find((m) => m.id === "codestral-latest")!;
|
||||
const codestralBase = models.find((m) => m.id === "codestral-2508")!;
|
||||
assert.notEqual(
|
||||
codestralLatest.name,
|
||||
codestralBase.name,
|
||||
"RED: 'codestral-latest' must render a distinguishable label from its base 'codestral-2508' " +
|
||||
"model — today both literally render 'codestral-2508 · imported', which is exactly the " +
|
||||
"visually-duplicated rows + invisible '-latest' alias reported in #6957"
|
||||
);
|
||||
|
||||
const mistralLargeLatest = models.find((m) => m.id === "mistral-large-latest")!;
|
||||
const mistralLargeBase = models.find((m) => m.id === "mistral-large-2512")!;
|
||||
assert.notEqual(
|
||||
mistralLargeLatest.name,
|
||||
mistralLargeBase.name,
|
||||
"RED: 'mistral-large-latest' must render a distinguishable label from its base " +
|
||||
"'mistral-large-2512' model"
|
||||
);
|
||||
|
||||
// Provider-wide: no two distinct model ids should ever render the same option text.
|
||||
const nameCollisions = new Map<string, string[]>();
|
||||
for (const model of models) {
|
||||
const bucket = nameCollisions.get(model.name) || [];
|
||||
bucket.push(model.id);
|
||||
nameCollisions.set(model.name, bucket);
|
||||
}
|
||||
const collidingNames = Array.from(nameCollisions.entries()).filter(([, ids]) => ids.length > 1);
|
||||
assert.equal(
|
||||
collidingNames.length,
|
||||
0,
|
||||
`RED: distinct model ids must not share an identical display name (collisions: ${JSON.stringify(collidingNames)})`
|
||||
);
|
||||
});
|
||||
35
tests/unit/repro-6975.test.ts
Normal file
35
tests/unit/repro-6975.test.ts
Normal file
@@ -0,0 +1,35 @@
|
||||
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-embed-6975-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
const modelsDb = await import("../../src/lib/db/models.ts");
|
||||
const { getComboBuilderOptions } = await import("../../src/lib/combos/builderOptions.ts");
|
||||
|
||||
test.after(() => {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("#6975 embeddings-only custom model must appear in the combo builder output", async () => {
|
||||
await modelsDb.addCustomModel("opencode", "zzz-embed-6975", "Embed Model 6975", "manual", "embeddings", [
|
||||
"embeddings",
|
||||
]);
|
||||
const payload = await getComboBuilderOptions();
|
||||
const m = payload.providers.flatMap((p) => p.models).find((m) => m.id === "zzz-embed-6975");
|
||||
assert.ok(m, "embeddings-only custom model must appear in the combo builder output");
|
||||
});
|
||||
|
||||
test("#6975 rerank-only custom model must appear in the combo builder output", async () => {
|
||||
await modelsDb.addCustomModel("opencode", "zzz-rerank-6975", "Rerank Model 6975", "manual", "rerank", [
|
||||
"rerank",
|
||||
]);
|
||||
const payload = await getComboBuilderOptions();
|
||||
const m = payload.providers.flatMap((p) => p.models).find((m) => m.id === "zzz-rerank-6975");
|
||||
assert.ok(m, "rerank-only custom model must appear in the combo builder output");
|
||||
});
|
||||
Reference in New Issue
Block a user