From 3aaa3c987af790bfa6fd8dcb8a765f44304c443e Mon Sep 17 00:00:00 2001 From: Agnik47 <140933190+Agnik47@users.noreply.github.com> Date: Sat, 15 Aug 2026 02:18:45 +0530 Subject: [PATCH] fix(models): reject unknown model aliases instead of assuming OpenAI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getModelConfig` ended with an unconditional fallback returning `provider: "openai"` with the caller's string as the model id. The orchestrator derives the judge *provider* from that return value (`orchestrator/index.ts:90-91`), so any alias not in `MODEL_CONFIGS` and not matching a known prefix was silently routed to OpenAI. `--judge sonnet-4-5` (the registry key is `sonnet-4.5`), `opus4.5` or `gemini2.5-pro` therefore became an OpenAI judge using the typo verbatim. That surfaces either as a 401 when `OPENAI_API_KEY` is unset — pointing at the wrong provider entirely — or a 404 for a model the user believes is Anthropic, and only in the evaluate phase, after a full ingest has been paid for. Neither message mentions that the alias was unrecognised. The prefix branches also tested the original casing while the registry lookup used `alias.toLowerCase()`, so `GPT-4.5` missed the registry *and* the `gpt-` branch and landed in the fallback with `supportsTemperature: true` — wrong for a reasoning model. Match on `lowerAlias` in every branch, and return the normalised id rather than the caller's spelling, since providers reject a mis-cased model id. Replace the fallback with an error naming the alias and listing `listAvailableModels()`: reaching it means the alias matched no registry entry and no provider prefix, so its provider genuinely cannot be inferred and guessing is what caused the misattribution. Also resolve the answering model alongside the judge at the top of `Orchestrator.run`. It is otherwise first resolved in the answer phase, so an unusable alias would still only surface after ingest. Fixes #73 --- src/orchestrator/index.ts | 4 ++ src/utils/models.test.ts | 98 +++++++++++++++++++++++++++++++++++++++ src/utils/models.ts | 59 +++++++++++------------ 3 files changed, 132 insertions(+), 29 deletions(-) create mode 100644 src/utils/models.test.ts diff --git a/src/orchestrator/index.ts b/src/orchestrator/index.ts index 0db1244..a86327d 100644 --- a/src/orchestrator/index.ts +++ b/src/orchestrator/index.ts @@ -90,6 +90,10 @@ export class Orchestrator { const judgeModelInfo = resolveModel(judgeModel) const judgeName = judgeModelInfo.provider as JudgeName + // Resolve the answering model up front too. It is otherwise first resolved in + // the answer phase, so an unusable alias would only surface after a full ingest. + resolveModel(answeringModel) + logger.info(`Starting MemoryBench run: ${providerName} + ${benchmarkName}`) logger.info(`Run ID: ${runId}`) logger.info( diff --git a/src/utils/models.test.ts b/src/utils/models.test.ts new file mode 100644 index 0000000..ef9392d --- /dev/null +++ b/src/utils/models.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, test } from "bun:test" +import { getModelConfig, listAvailableModels, resolveModel } from "./models" + +describe("getModelConfig", () => { + test("returns the registered config for a known alias", () => { + const config = getModelConfig("gpt-4o") + + expect(config.id).toBe("gpt-4o") + expect(config.provider).toBe("openai") + }) + + test("matches a registered alias regardless of casing", () => { + expect(getModelConfig("GPT-4O")).toEqual(getModelConfig("gpt-4o")) + }) + + describe("provider prefix inference", () => { + test("routes claude- to anthropic", () => { + const config = getModelConfig("claude-opus-4.5") + + expect(config.provider).toBe("anthropic") + expect(config.id).toBe("claude-opus-4.5") + }) + + test("routes gemini- to google", () => { + expect(getModelConfig("gemini-9.9-pro").provider).toBe("google") + }) + + test("routes gpt- to openai", () => { + expect(getModelConfig("gpt-9-turbo").provider).toBe("openai") + }) + + test("treats o-series and gpt-5 as reasoning models without temperature", () => { + for (const alias of ["gpt-5-turbo", "o1-preview", "o3-mini", "o4-max"]) { + expect(getModelConfig(alias).supportsTemperature).toBe(false) + expect(getModelConfig(alias).maxTokensParam).toBe("max_completion_tokens") + } + }) + + test("keeps gemini-3 on its own temperature default", () => { + expect(getModelConfig("gemini-3-pro").defaultTemperature).toBe(1) + expect(getModelConfig("gemini-2.9-pro").defaultTemperature).toBe(0) + }) + + // Before the fix these matched the caller's casing, so an upper-case alias + // fell past every prefix branch into the silent OpenAI default. + test("infers the provider from a prefix regardless of casing", () => { + expect(getModelConfig("Claude-Sonnet-4-6").provider).toBe("anthropic") + expect(getModelConfig("Gemini-9.9-Pro").provider).toBe("google") + expect(getModelConfig("O3-Mini").supportsTemperature).toBe(false) + }) + + test("normalises the resolved model id to lower case", () => { + // Providers reject a mis-cased model id, so the id we send must be normalised. + expect(getModelConfig("Claude-Sonnet-4-6").id).toBe("claude-sonnet-4-6") + expect(getModelConfig("GPT-4.5").id).toBe("gpt-4.5") + }) + + test("does not treat an upper-case reasoning model as temperature-capable", () => { + expect(getModelConfig("GPT-5-Mini").supportsTemperature).toBe(false) + }) + }) + + describe("unrecognised aliases", () => { + // Each of these previously became an OpenAI judge using the typo verbatim. + const TYPOS = ["sonnet-4-5", "opus4.5", "gemini2.5-pro", "haiku", "", " "] + + for (const alias of TYPOS) { + test(`rejects ${JSON.stringify(alias)} instead of defaulting to openai`, () => { + expect(() => getModelConfig(alias)).toThrow(/Unknown model/) + }) + } + + test("names the offending alias and lists what is available", () => { + let message = "" + try { + getModelConfig("sonnet-4-5") + } catch (e) { + message = (e as Error).message + } + + expect(message).toContain("sonnet-4-5") + // The correct spelling is a registered alias, so the list points the user at it. + expect(message).toContain("sonnet-4.5") + expect(listAvailableModels().length).toBeGreaterThan(0) + }) + }) + + test("resolveModel delegates to getModelConfig", () => { + expect(resolveModel("gpt-4o")).toEqual(getModelConfig("gpt-4o")) + expect(() => resolveModel("not-a-model")).toThrow(/Unknown model/) + }) + + test("every registered alias resolves", () => { + for (const alias of listAvailableModels()) { + expect(() => getModelConfig(alias)).not.toThrow() + } + }) +}) diff --git a/src/utils/models.ts b/src/utils/models.ts index b29ac80..d808f1b 100644 --- a/src/utils/models.ts +++ b/src/utils/models.ts @@ -241,61 +241,63 @@ export function getModelConfig(alias: string): ModelConfig { return MODEL_CONFIGS[lowerAlias] } - // Fallback for unknown models - try to infer from prefix + // Fallback for unknown models - try to infer from prefix. Match on lowerAlias + // throughout: matching the caller's casing here would send `GPT-4.1` past every + // branch into a guess, and providers reject a mis-cased model id anyway. if ( - alias.startsWith("gpt-5") || - alias.startsWith("o1") || - alias.startsWith("o3") || - alias.startsWith("o4") + lowerAlias.startsWith("gpt-5") || + lowerAlias.startsWith("o1") || + lowerAlias.startsWith("o3") || + lowerAlias.startsWith("o4") ) { return { - id: alias, + id: lowerAlias, provider: "openai", - displayName: alias, + displayName: lowerAlias, supportsTemperature: false, defaultTemperature: 1, maxTokensParam: "max_completion_tokens", defaultMaxTokens: 1000, } } - if (alias.startsWith("gpt-")) { + if (lowerAlias.startsWith("gpt-")) { return { - id: alias, + id: lowerAlias, provider: "openai", - displayName: alias, + displayName: lowerAlias, supportsTemperature: true, defaultTemperature: 0, maxTokensParam: "maxTokens", defaultMaxTokens: 1000, } } - if (alias.startsWith("claude-")) { + if (lowerAlias.startsWith("claude-")) { return { - id: alias, + id: lowerAlias, provider: "anthropic", - displayName: alias, + displayName: lowerAlias, supportsTemperature: true, defaultTemperature: 0, maxTokensParam: "maxTokens", defaultMaxTokens: 1000, } } - if (alias.startsWith("gemini-3")) { + if (lowerAlias.startsWith("gemini-3")) { return { - id: alias, + id: lowerAlias, provider: "google", - displayName: alias, + displayName: lowerAlias, supportsTemperature: true, defaultTemperature: 1, maxTokensParam: "maxTokens", defaultMaxTokens: 1000, } } - if (alias.startsWith("gemini-")) { + if (lowerAlias.startsWith("gemini-")) { return { - id: alias, + id: lowerAlias, provider: "google", - displayName: alias, + displayName: lowerAlias, supportsTemperature: true, defaultTemperature: 0, maxTokensParam: "maxTokens", @@ -303,16 +305,15 @@ export function getModelConfig(alias: string): ModelConfig { } } - // Default fallback - return { - id: alias, - provider: "openai", - displayName: alias, - supportsTemperature: true, - defaultTemperature: 0, - maxTokensParam: "maxTokens", - defaultMaxTokens: 1000, - } + // No registry entry and no recognisable provider prefix: the provider genuinely + // cannot be inferred. Defaulting to OpenAI here used to route a typo'd Anthropic + // or Google alias to OpenAI under the user's original spelling, surfacing as a + // 401/404 from the wrong provider deep in the evaluate phase. + throw new Error( + `Unknown model "${alias}". It is not a registered alias and does not start with ` + + `a recognised provider prefix (gpt-, o1/o3/o4, claude-, gemini-), so its ` + + `provider cannot be determined. Available models: ${listAvailableModels().join(", ")}` + ) } // Legacy exports for backward compatibility