diff --git a/changelog.d/fixes/8134-github-t5-fallback-filter.md b/changelog.d/fixes/8134-github-t5-fallback-filter.md new file mode 100644 index 0000000000..cba43ee2a8 --- /dev/null +++ b/changelog.d/fixes/8134-github-t5-fallback-filter.md @@ -0,0 +1 @@ +- fix(providers): filter unsupported family-fallback candidates against the provider catalog (#8134) diff --git a/open-sse/services/modelFamilyFallback.ts b/open-sse/services/modelFamilyFallback.ts index 81f569764e..3ebfb92bfe 100644 --- a/open-sse/services/modelFamilyFallback.ts +++ b/open-sse/services/modelFamilyFallback.ts @@ -140,6 +140,38 @@ export function isContextOverflowError(status: number, errorMessage: string): bo // ── Fallback Resolution ────────────────────────────────────────────────────── +/** + * All notation forms a family-fallback candidate might be registered under + * in a provider's catalog: the literal hyphen form, dot-notation variants + * (`claude-opus-4-8` -> `claude-opus-4.8`), and — for dated snapshot ids + * like `claude-opus-4-5-20251101` — the same variants with the trailing + * `-YYYYMMDD` snapshot suffix stripped, so a dated candidate can still + * resolve to a provider's undated catalog entry (`claude-opus-4.5`). + */ +function candidateNotationVariants(candidate: string): string[] { + const variants = [ + candidate, + candidate.replace(/-(\d+)-(\d+)$/, "-$1.$2"), + candidate.replace(/-(\d+)-(\d+)-/, "-$1.$2-"), + ]; + + const dateStripped = candidate.replace(/-\d{8}$/, ""); + if (dateStripped !== candidate) { + variants.push(dateStripped, dateStripped.replace(/-(\d+)-(\d+)$/, "-$1.$2")); + } + + return variants; +} + +/** + * Resolve a family candidate against the provider's supported model ids. + * Returns the matching notation (preferring the first variant found) or + * `null` if the candidate is absent from the catalog under every notation. + */ +function resolveCandidateNotation(candidate: string, supportedIds: Set): string | null { + return candidateNotationVariants(candidate).find((variant) => supportedIds.has(variant)) ?? null; +} + /** * Get the next fallback model from the same family. * @@ -170,11 +202,12 @@ export function getNextFamilyFallback( for (const candidate of family) { let resolvedCandidate = candidate; if (supportedIds && !supportedIds.has(candidate)) { - // Try dot-notation variants: claude-opus-4-8 → claude-opus-4.8 - const dotVariant = candidate.replace(/-(\d+)-(\d+)$/, "-$1.$2"); - const dotVariant2 = candidate.replace(/-(\d+)-(\d+)-/, "-$1.$2-"); - if (supportedIds.has(dotVariant)) resolvedCandidate = dotVariant; - else if (supportedIds.has(dotVariant2)) resolvedCandidate = dotVariant2; + const match = resolveCandidateNotation(candidate, supportedIds); + // Provider catalog is known but this candidate has no match under any + // notation — it is provably unsupported, so skip it instead of + // returning an id the provider will just 400 on again. + if (!match) continue; + resolvedCandidate = match; } const fullCandidate = `${prefix}${resolvedCandidate}`; if (!triedModels.has(fullCandidate)) { diff --git a/tests/unit/8134-github-t5-fallback-filter.test.ts b/tests/unit/8134-github-t5-fallback-filter.test.ts new file mode 100644 index 0000000000..6de8f13416 --- /dev/null +++ b/tests/unit/8134-github-t5-fallback-filter.test.ts @@ -0,0 +1,70 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { getRegistryEntry } from "../../open-sse/config/providerRegistry.ts"; + +const { getNextFamilyFallback } = await import("../../open-sse/services/modelFamilyFallback.ts"); + +// Regression for #8134 — GitHub Copilot ("github", alias "gh") T5 family fallback +// returned "claude-opus-4-6" verbatim even though the github registry catalog +// (Opus 4.8 / 4.8-fast / 4.7 / 4.5) has NO 4.6 tier under any dot/hyphen +// notation. getNextFamilyFallback() resolved `supportedIds` from the provider's +// registry but only used it to try notation variants of a candidate, never to +// filter out a candidate that is provably absent from the catalog — so the +// unsupported id fell through and was returned anyway, costing a 3rd wasted +// upstream round-trip before the family was exhausted. +// +// Fix: when the provider registry is resolved, getNextFamilyFallback() now +// skips (continue) any family candidate that has no match in supportedIds +// under ANY notation (hyphen, dot, or a dated-snapshot id with the date +// suffix stripped) instead of returning it unfiltered. + +test("#8134: github claude-opus-4.8 fallback chain never returns an unsupported tier (claude-opus-4-6)", () => { + const github = getRegistryEntry("github"); + assert.ok(github, "expected the github registry entry to resolve"); + const githubIds = new Set(github.models.map((m) => m.id)); + assert.ok( + !githubIds.has("claude-opus-4-6") && !githubIds.has("claude-opus-4.6"), + "fixture assumption broken: github registry now has a 4.6 tier" + ); + + const tried = new Set(["github/claude-opus-4.8"]); + const first = getNextFamilyFallback("github/claude-opus-4.8", tried); + assert.ok(first, "expected a first fallback candidate"); + const firstBareId = first.replace(/^github\//, ""); + assert.ok( + githubIds.has(firstBareId), + `first fallback "${first}" is not in github's registered model catalog: ${[...githubIds].join(", ")}` + ); + + tried.add(first); + const second = getNextFamilyFallback(first, tried); + assert.ok(second, "expected a second fallback candidate (family must not be silently exhausted)"); + const secondBareId = second.replace(/^github\//, ""); + assert.ok( + githubIds.has(secondBareId), + `second fallback "${second}" is not in github's registered model catalog: ${[...githubIds].join(", ")}` + ); + assert.notEqual(secondBareId, "claude-opus-4-6"); + assert.notEqual(secondBareId, "claude-opus-4.6"); +}); + +test("#8134: getNextFamilyFallback never returns a candidate absent from the resolved provider's catalog", () => { + const github = getRegistryEntry("github"); + assert.ok(github); + const githubIds = new Set(github.models.map((m) => m.id)); + + let current = "github/claude-opus-4.8"; + const tried = new Set([current]); + for (let hop = 0; hop < 5; hop++) { + const next = getNextFamilyFallback(current, tried); + if (!next) break; + const bareId = next.replace(/^github\//, ""); + assert.ok( + githubIds.has(bareId), + `hop ${hop + 1}: "${next}" is not in github's registered model catalog` + ); + tried.add(next); + current = next; + } +});