-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
fix(plugin): drop combo/ prefix and emit providerID on static entries #4378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1154,7 +1154,8 @@ const AUTO_COMBO_FALLBACK_OUTPUT = 8_192; | |||||||||||||
| * applies when the server omits them. Never 0. | ||||||||||||||
| */ | ||||||||||||||
| export function mapAutoComboToStaticEntry( | ||||||||||||||
| autoCombo: OmniRouteRawAutoCombo | ||||||||||||||
| autoCombo: OmniRouteRawAutoCombo, | ||||||||||||||
| providerID: string | ||||||||||||||
| ): OmniRouteStaticModelEntry { | ||||||||||||||
| const variant = autoCombo.variant; | ||||||||||||||
| const name = formatAutoComboName(variant, autoCombo.candidateCount); | ||||||||||||||
|
|
@@ -1168,6 +1169,7 @@ export function mapAutoComboToStaticEntry( | |||||||||||||
| : AUTO_COMBO_FALLBACK_OUTPUT; | ||||||||||||||
| return { | ||||||||||||||
| name, | ||||||||||||||
| providerID, | ||||||||||||||
| attachment: false, | ||||||||||||||
| reasoning: true, | ||||||||||||||
| temperature: true, | ||||||||||||||
|
|
@@ -2248,26 +2250,32 @@ export function slugifyComboName(name: string): string { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Build a combo's static-block key (`combo/<slug>`), guaranteeing uniqueness | ||||||||||||||
| * across an entire static catalog. If `<slug>` is already present in `used`, | ||||||||||||||
| * suffixes a short UUID-prefix disambiguator from `combo.id` so the second | ||||||||||||||
| * combo doesn't silently overwrite the first. Mutates `used` in place by | ||||||||||||||
| * recording the chosen key. Returns the final `combo/<...>` key. | ||||||||||||||
| * Build a combo's static-block key (bare slug like `MASTER`, `MASTER-LIGHT`), | ||||||||||||||
| * guaranteeing uniqueness across an entire static catalog. If `<slug>` is | ||||||||||||||
| * already present in `used`, suffixes a short UUID-prefix disambiguator from | ||||||||||||||
| * `combo.id` so the second combo doesn't silently overwrite the first. | ||||||||||||||
| * Mutates `used` in place by recording the chosen key. Returns the final | ||||||||||||||
| * bare key. | ||||||||||||||
| * | ||||||||||||||
| * Falls back to `combo/<id>` when the friendly name slugifies to the empty | ||||||||||||||
| * NOTE: the key MUST NOT carry a `combo/` namespace prefix — OpenCode | ||||||||||||||
| * parses model IDs on `/` to extract a provider prefix, and `combo/MASTER` | ||||||||||||||
| * would be treated as provider=`combo`, model=`MASTER`, causing a | ||||||||||||||
| * credentials-not-found error. See PR #4184. | ||||||||||||||
| * | ||||||||||||||
| * Falls back to bare `<id>` when the friendly name slugifies to the empty | ||||||||||||||
| * string (e.g. a combo named just punctuation). | ||||||||||||||
| */ | ||||||||||||||
| export function buildComboKey(combo: OmniRouteRawCombo, used: Set<string>): string { | ||||||||||||||
| const friendlyName = combo.name && combo.name.trim().length > 0 ? combo.name.trim() : combo.id; | ||||||||||||||
| let slug = slugifyComboName(friendlyName); | ||||||||||||||
| if (slug.length === 0) slug = combo.id; | ||||||||||||||
| let key = `combo/${slug}`; | ||||||||||||||
| let key = slug; | ||||||||||||||
| if (used.has(key)) { | ||||||||||||||
| const tail = combo.id.split("-")[0] ?? combo.id; | ||||||||||||||
| key = `combo/${slug}-${tail}`; | ||||||||||||||
| key = slug + "-" + tail; | ||||||||||||||
| // Defensive: in the (impossible) event the disambiguated key also | ||||||||||||||
| // collides, append the full id. | ||||||||||||||
| if (used.has(key)) key = `combo/${slug}-${combo.id}`; | ||||||||||||||
| if (used.has(key)) key = slug + "-" + combo.id; | ||||||||||||||
| } | ||||||||||||||
| used.add(key); | ||||||||||||||
| return key; | ||||||||||||||
|
|
@@ -2801,18 +2809,6 @@ export function createOmniRouteProviderHook( | |||||||||||||
| // models with curated names). | ||||||||||||||
| applyEnrichment(mapped, rawEnrichment.get(combo.id)); | ||||||||||||||
|
|
||||||||||||||
| // `Combo: ` prefix surfaces the combo nature in OC's model picker. | ||||||||||||||
| // Idempotent guard covers the case where enrichment overwrote | ||||||||||||||
| // mapped.name with an already-prefixed string. Mirrors the | ||||||||||||||
| // static-hook Combo:-prefix decoration. | ||||||||||||||
| if (!mapped.name.startsWith("Combo: ")) { | ||||||||||||||
| mapped.name = `Combo: ${mapped.name}`; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Optionally decorate combo name with its compression pipeline. | ||||||||||||||
| // Only fires when features.compressionMetadata: true, OmniRoute | ||||||||||||||
| // returned at least one default compression combo, AND the | ||||||||||||||
| // combo has resolvable members — claiming compression on an | ||||||||||||||
| // unroutable combo would mislead the picker. | ||||||||||||||
| if (hasMembers && defaultCompression && defaultCompression.pipeline.length > 0) { | ||||||||||||||
| const tag = formatCompressionPipeline(defaultCompression.pipeline); | ||||||||||||||
|
|
@@ -2862,7 +2858,7 @@ export function createOmniRouteProviderHook( | |||||||||||||
| for (const autoCombo of rawAutoCombos) { | ||||||||||||||
| if (!autoCombo || !autoCombo.id) continue; | ||||||||||||||
| if (autoCombo.isHidden === true) continue; | ||||||||||||||
| const entry = mapAutoComboToStaticEntry(autoCombo); | ||||||||||||||
| const entry = mapAutoComboToStaticEntry(autoCombo, resolved.providerId); | ||||||||||||||
| const key = autoComboModelId(autoCombo.variant); | ||||||||||||||
| const mapped: ModelV2 = { | ||||||||||||||
| id: key, | ||||||||||||||
|
|
@@ -3346,8 +3342,15 @@ function normaliseModalities(raw: unknown): OmniRouteModalityKind[] { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| export interface OmniRouteStaticModelEntry { | ||||||||||||||
| /** Owning provider id. MUST match the parent `provider.<id>` key so OC's | ||||||||||||||
| * static-catalog reader resolves credentials via `providerID` instead of | ||||||||||||||
| * parsing the model key on `/`. Without this, a bare-slug combo key like | ||||||||||||||
| * `MASTER` is misread as `providerID=MASTER, modelID=""` and the request fails | ||||||||||||||
| * with "Unable to determine provider". See PR #4184. */ | ||||||||||||||
| providerID: string; | ||||||||||||||
| /** Display label rendered in OC's model picker. Defaults to the model id. */ | ||||||||||||||
| name: string; | ||||||||||||||
|
|
||||||||||||||
| /** ISO date the model was released. Surfaces in OC's model card when present. */ | ||||||||||||||
| release_date?: string; | ||||||||||||||
| /** Model accepts image / file attachments. */ | ||||||||||||||
|
|
@@ -3545,7 +3548,7 @@ export function buildStaticProviderEntry( | |||||||||||||
| if (!displayName.startsWith(prefix)) displayName = `${prefix}${displayName}`; | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| const entry: OmniRouteStaticModelEntry = { name: displayName }; | ||||||||||||||
| const entry: OmniRouteStaticModelEntry = { name: displayName, providerID: opts.providerId }; | ||||||||||||||
|
|
||||||||||||||
| const attachment = caps.attachment ?? caps.vision; | ||||||||||||||
| if (typeof attachment === "boolean") entry.attachment = attachment; | ||||||||||||||
|
|
@@ -3717,13 +3720,9 @@ export function buildStaticProviderEntry( | |||||||||||||
| const hasMembers = memberEntries.length > 0; | ||||||||||||||
| const friendlyName = | ||||||||||||||
| combo.name && combo.name.trim().length > 0 ? combo.name.trim() : combo.id; | ||||||||||||||
| // `Combo: ` prefix surfaces the combo nature in OC's model picker — the | ||||||||||||||
| // catalog key (`combo/<slug>`) is already namespaced, but the picker | ||||||||||||||
| // shows `name`, so prefix the display string too. | ||||||||||||||
| const prefixedName = `Combo: ${friendlyName}`; | ||||||||||||||
| const displayName = | ||||||||||||||
| hasMembers && compressionSuffix ? `${prefixedName}${compressionSuffix}` : prefixedName; | ||||||||||||||
| const entry: OmniRouteStaticModelEntry = { name: displayName }; | ||||||||||||||
| hasMembers && compressionSuffix ? `${friendlyName} ${compressionSuffix}` : friendlyName; | ||||||||||||||
|
Comment on lines
3723
to
+3724
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
|
||||||||||||||
| const entry: OmniRouteStaticModelEntry = { name: displayName, providerID: opts.providerId }; | ||||||||||||||
|
|
||||||||||||||
| if (hasMembers) { | ||||||||||||||
| // LCD across capabilities — every member must support for the combo | ||||||||||||||
|
|
@@ -3790,9 +3789,9 @@ export function buildStaticProviderEntry( | |||||||||||||
| entry.tool_call = false; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Key under `combo/<slug>` (e.g. `combo/claude-primary`) so the | ||||||||||||||
| // namespace cleanly separates combos from raw provider/model pairs | ||||||||||||||
| // and so the key is copy/paste-friendly. Slug collisions across | ||||||||||||||
| // Key under bare slug (e.g. `claude-primary`) — no `combo/` prefix | ||||||||||||||
| // because OpenCode parses model IDs on `/` and would treat | ||||||||||||||
| // `combo/MASTER` as provider=`combo`. Slug collisions across | ||||||||||||||
| // combos are disambiguated with a short UUID-prefix suffix; see | ||||||||||||||
| // `buildComboKey` for the policy. | ||||||||||||||
| models[buildComboKey(combo, usedComboKeys)] = entry; | ||||||||||||||
|
|
@@ -3826,7 +3825,7 @@ export function buildStaticProviderEntry( | |||||||||||||
| for (const autoCombo of rawAutoCombos) { | ||||||||||||||
| if (!autoCombo || !autoCombo.id) continue; | ||||||||||||||
| if (autoCombo.isHidden === true) continue; | ||||||||||||||
| const entry = mapAutoComboToStaticEntry(autoCombo); | ||||||||||||||
| const entry = mapAutoComboToStaticEntry(autoCombo, opts.providerId); | ||||||||||||||
| // Use the variant as the key: "auto", "auto/coding", etc. | ||||||||||||||
| const key = autoComboModelId(autoCombo.variant); | ||||||||||||||
| if (models[key]) { | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -450,10 +450,10 @@ test("models() returns combo entries merged into the map", async () => { | |||||
| assert.ok(out["claude-primary"]); | ||||||
| assert.ok(out["claude-secondary"]); | ||||||
| assert.ok(out["gemini-3-flash"]); | ||||||
| assert.ok(out["combo/claude-tier"]); | ||||||
| assert.ok(out["claude-tier"]); | ||||||
|
|
||||||
| const combo = out["combo/claude-tier"]; | ||||||
| assert.equal(combo.name, "Combo: Claude Tier"); | ||||||
| const combo = out["claude-tier"]; | ||||||
| assert.equal(combo.name, "Claude Tier"); | ||||||
| assert.equal(combo.providerID, "omniroute"); | ||||||
| // LCD over claude-primary (200k, reasoning) + claude-secondary (100k, no reasoning) | ||||||
| assert.equal(combo.limit.context, 100_000); | ||||||
|
|
@@ -478,11 +478,11 @@ test("models(): combo with unknown member ids degrades to all-false LCD posture" | |||||
| { fetcher: modelsFetcher, combosFetcher } | ||||||
| ); | ||||||
| const out = await hook.models!({} as never, { auth: apiAuth("sk-z") as never }); | ||||||
| assert.ok(out["combo/phantom-combo"]); | ||||||
| assert.ok(out["phantom-combo"]); | ||||||
| // With zero resolvable members, LCD = all-false (defensive posture). | ||||||
| assert.equal(out["combo/phantom-combo"].capabilities.toolcall, false); | ||||||
| assert.equal(out["combo/phantom-combo"].capabilities.reasoning, false); | ||||||
| assert.equal(out["combo/phantom-combo"].limit.context, 0); | ||||||
| assert.equal(out["phantom-combo"].capabilities.toolcall, false); | ||||||
| assert.equal(out["phantom-combo"].capabilities.reasoning, false); | ||||||
| assert.equal(out["phantom-combo"].limit.context, 0); | ||||||
| }); | ||||||
|
|
||||||
| test("models(): hidden combos are excluded from the map", async () => { | ||||||
|
|
@@ -505,11 +505,11 @@ test("models(): hidden combos are excluded from the map", async () => { | |||||
| { fetcher: modelsFetcher, combosFetcher } | ||||||
| ); | ||||||
| const out = await hook.models!({} as never, { auth: apiAuth("sk-z") as never }); | ||||||
| assert.ok(out["combo/visible"]); | ||||||
| assert.ok(!out["combo/hidden"], "hidden combo must be omitted"); | ||||||
| assert.ok(out["visible"]); | ||||||
| assert.ok(!out["hidden"], "hidden combo must be omitted"); | ||||||
| }); | ||||||
|
|
||||||
| test("models(): combo name exactly matches raw model id → raw deleted, combo lives at combo/ key, no warn", async () => { | ||||||
| test("models(): combo name exactly matches raw model id → raw deleted, raw deleted, no warn", async () => { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a typo in the test description where
Suggested change
|
||||||
| // Combo.name === raw model id triggers the dedup deletion. This mirrors | ||||||
| // the real OmniRoute payload where /v1/models pre-mirrors combos as | ||||||
| // no-slash raw entries whose ids match /api/combos friendly names. | ||||||
|
|
@@ -529,10 +529,9 @@ test("models(): combo name exactly matches raw model id → raw deleted, combo l | |||||
| return hook.models!({} as never, { auth: apiAuth("sk-z") as never }); | ||||||
| }); | ||||||
|
|
||||||
| // Raw model deleted by combo-name dedup; combo surfaces under combo/<slug>. | ||||||
| assert.equal(out["claude-primary"], undefined, "raw deleted by combo-name dedup"); | ||||||
| assert.ok(out["combo/claude-primary"], "combo surfaces under combo/ namespace"); | ||||||
| assert.equal(out["combo/claude-primary"].name, "Combo: claude-primary"); | ||||||
| // Raw model replaced by combo of the same key; combo now lives at the bare slug. | ||||||
| assert.ok(out["claude-primary"], "combo surfaces under bare-slug key"); | ||||||
| assert.equal(out["claude-primary"].name, "claude-primary"); | ||||||
|
|
||||||
| // No collision warning fires — dedup makes keys disjoint. | ||||||
| const collisionWarns = warnings.filter((w) => { | ||||||
|
|
@@ -543,7 +542,7 @@ test("models(): combo name exactly matches raw model id → raw deleted, combo l | |||||
| }); | ||||||
|
|
||||||
| test("models(): two combos with same slug → second gets disambiguator suffix", async () => { | ||||||
| // Both combos slug to `claude` — second must get `combo/claude-<id-prefix>`. | ||||||
| // Both combos slug to `claude` — second must get `claude-<id-prefix>`. | ||||||
| const combos: OmniRouteRawCombo[] = [ | ||||||
| { | ||||||
| id: "uuid-a", | ||||||
|
|
@@ -566,8 +565,8 @@ test("models(): two combos with same slug → second gets disambiguator suffix", | |||||
|
|
||||||
| const out = await hook.models!({} as never, { auth: apiAuth("sk-z") as never }); | ||||||
| // First combo gets the bare slug; second gets disambiguated. | ||||||
| assert.ok(out["combo/claude"], "first combo at bare slug"); | ||||||
| assert.ok(out["combo/claude-uuid"], "second combo disambiguated by id prefix"); | ||||||
| assert.ok(out["claude"], "first combo at bare slug"); | ||||||
| assert.ok(out["claude-uuid"], "second combo disambiguated by id prefix"); | ||||||
| }); | ||||||
|
|
||||||
| test("models(): combos fetch fails → falls back to models-only, warn emitted, no throw", async () => { | ||||||
|
|
@@ -610,7 +609,7 @@ test("models(): combos cached + reused within TTL (one combo fetch per TTL windo | |||||
| const second = await hook.models!({} as never, { auth: apiAuth("sk-z") as never }); | ||||||
| assert.equal(combosFetcher.callCount(), 1, "combos fetched only once within TTL"); | ||||||
| assert.equal(modelsFetcher.callCount(), 1, "models fetched only once within TTL"); | ||||||
| assert.ok(second["combo/claude-tier"]); | ||||||
| assert.ok(second["claude-tier"]); | ||||||
| }); | ||||||
|
|
||||||
| test("models(): combos refetched after TTL expiry (same key as models)", async () => { | ||||||
|
|
@@ -702,7 +701,7 @@ test("models(): nested combo-ref context is the min of nested + raw members", as | |||||
| { fetcher: modelsFetcher, combosFetcher } | ||||||
| ); | ||||||
| const out = await hook.models!({} as never, { auth: apiAuth("sk-z") as never }); | ||||||
| const masterLight = out["combo/master-light"]; | ||||||
| const masterLight = out["master-light"]; | ||||||
| assert.ok(masterLight, "MASTER-LIGHT entry must exist"); | ||||||
| assert.equal( | ||||||
| masterLight.limit.context, | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deletion of the
Combo:prefix logic accidentally removed the introductory lines of the subsequent comment block, leaving only the trailing fragment// unroutable combo would mislead the picker.. Replacing it with a complete, concise comment restores readability.