Repository navigation
fix(sse): echo the client-requested model, not the combo leaf (#18494) #18495
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
c086bb4
bf8d945
d436adb
b7ebca0
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 |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - **fix(sse):** the response-model echo (`echoRequestedModelName`, Codex CLI, Claude Code) now returns the model the client requested instead of the combo leaf target ([#18495](https://github.com/cheaper-inference/OmniRoute/pull/18495), closes [#18494](https://github.com/cheaper-inference/OmniRoute/issues/18494)) — thanks @xiaoyaner0201 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ import { | |
| injectMemoryAndSkills, | ||
| mergeInjectedFallbackOwnerNames, | ||
| } from "./chatCore/memorySkillsInjection.ts"; | ||
| import { resolveChatCoreRequestSetup } from "./chatCore/requestSetup.ts"; | ||
| import { resolveChatCoreRequestSetup, resolveClientEchoModel } from "./chatCore/requestSetup.ts"; | ||
| import { shouldSkipNativeCodexReactiveCompaction } from "./chatCore/nativeCodexCompaction.ts"; | ||
| import { | ||
| normalizeOpenAICompatibleTools, | ||
|
|
@@ -867,11 +867,12 @@ async function handleChatCoreInner({ | |
| // `opencode-zen/nemotron-3-ultra-free`) is not recognized by the client on `--resume`. | ||
| const isClaudeCodeClient = isClaudeCodeOriginatedHeaders(clientRawRequest?.headers); | ||
|
|
||
| const clientEchoModel = resolveClientEchoModel(clientRawRequest?.body, requestedModel); | ||
| let echoModel = | ||
| (settings.echoRequestedModelName === true || isCodexResponsesEcho || isClaudeCodeClient) && | ||
| typeof requestedModel === "string" && | ||
| requestedModel | ||
| ? requestedModel | ||
| typeof clientEchoModel === "string" && | ||
| clientEchoModel | ||
| ? clientEchoModel | ||
|
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.
|
||
| : null; | ||
| // Auto-echo the listing-valid form for bare requests to noAuth catalog | ||
| // providers so clients validating response.model against /v1/models don't warn. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| // The response-model echo (#1311 opt-in, #3697 Codex CLI) must echo the model name the | ||
| // client sent. Combo and alias dispatch rewrite the dispatched body.model to the resolved | ||
| // leaf target (chatHelpers.ts sends `${provider}/${model}`) before handleChatCore runs, so | ||
| // echoing body.model leaks the leaf target instead of the alias/combo the client asked for. | ||
| // clientRawRequest.body is the snapshot of what the client actually sent. | ||
| 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-echo-client-model-")); | ||
| process.env.DATA_DIR = TEST_DATA_DIR; | ||
|
|
||
| const { handleChatCore } = await import("../../open-sse/handlers/chatCore.ts"); | ||
| const settingsDb = await import("../../src/lib/db/settings.ts"); | ||
| const core = await import("../../src/lib/db/core.ts"); | ||
|
|
||
| const CLIENT_MODEL = "team/my-alias-combo"; | ||
| const LEAF_MODEL = "gpt-4o-mini"; | ||
|
|
||
| function noopLog() { | ||
| return { debug() {}, info() {}, warn() {}, error() {} }; | ||
| } | ||
|
|
||
| async function invoke({ | ||
| stream, | ||
| echoSetting = true, | ||
| userAgent = "unit-test", | ||
| clientModel = CLIENT_MODEL, | ||
| }) { | ||
| await settingsDb.updateSettings({ echoRequestedModelName: echoSetting }); | ||
| const originalFetch = globalThis.fetch; | ||
| globalThis.fetch = async () => | ||
| stream | ||
| ? new Response( | ||
| `data: {"id":"c1","object":"chat.completion.chunk","model":"${LEAF_MODEL}","choices":[{"index":0,"delta":{"role":"assistant","content":"ok"}}]}\n\n` + | ||
| `data: {"id":"c1","object":"chat.completion.chunk","model":"${LEAF_MODEL}","choices":[{"index":0,"delta":{},"finish_reason":"stop"}]}\n\ndata: [DONE]\n\n`, | ||
| { status: 200, headers: { "Content-Type": "text/event-stream" } } | ||
| ) | ||
| : new Response( | ||
| JSON.stringify({ | ||
| id: "c1", | ||
| object: "chat.completion", | ||
| model: LEAF_MODEL, | ||
| choices: [ | ||
| { index: 0, message: { role: "assistant", content: "ok" }, finish_reason: "stop" }, | ||
| ], | ||
| usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 }, | ||
| }), | ||
| { status: 200, headers: { "Content-Type": "application/json" } } | ||
| ); | ||
| try { | ||
| const messages = [{ role: "user", content: "hello" }]; | ||
| const result = await handleChatCore({ | ||
| // What combo/alias dispatch hands to chatCore: body.model already rewritten to the leaf. | ||
| body: { model: `openai/${LEAF_MODEL}`, stream, messages }, | ||
| modelInfo: { provider: "openai", model: LEAF_MODEL, extendedContext: false }, | ||
| credentials: { apiKey: "sk-test", providerSpecificData: {} }, | ||
| log: noopLog(), | ||
| clientRawRequest: { | ||
| endpoint: "/v1/chat/completions", | ||
| body: { model: clientModel, stream, messages }, | ||
| headers: new Headers({ | ||
| accept: stream ? "text/event-stream" : "application/json", | ||
| "user-agent": userAgent, | ||
| }), | ||
| }, | ||
| userAgent, | ||
| }); | ||
| assert.equal(result.success, true, JSON.stringify(result).slice(0, 300)); | ||
| const text = await result.response.text(); | ||
| return [...text.matchAll(/"model":"([^"]+)"/g)].map((m) => m[1]); | ||
| } finally { | ||
| globalThis.fetch = originalFetch; | ||
| } | ||
| } | ||
|
|
||
| test.after(() => { | ||
| try { | ||
| core.getDbInstance().close(); | ||
| } catch {} | ||
| fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); | ||
| }); | ||
|
|
||
| test("echo returns the client-requested model, not the rewritten leaf (non-streaming)", async () => { | ||
| const models = await invoke({ stream: false }); | ||
| assert.deepEqual([...new Set(models)], [CLIENT_MODEL]); | ||
| }); | ||
|
|
||
| test("echo returns the client-requested model, not the rewritten leaf (streaming)", async () => { | ||
| const models = await invoke({ stream: true }); | ||
| assert.ok(models.length > 0); | ||
| assert.deepEqual([...new Set(models)], [CLIENT_MODEL]); | ||
| }); | ||
|
|
||
| test("echo off: response model is not rewritten to the client name", async () => { | ||
| const models = await invoke({ stream: false, echoSetting: false }); | ||
| assert.ok(!models.includes(CLIENT_MODEL), JSON.stringify(models)); | ||
| }); |
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.
Script executed:
Repository: cheaper-inference/OmniRoute
Length of output: 13640
Script executed:
Repository: cheaper-inference/OmniRoute
Length of output: 14941
Script executed:
Repository: cheaper-inference/OmniRoute
Length of output: 10202
Script executed:
Repository: cheaper-inference/OmniRoute
Length of output: 1400
Script executed:
Repository: cheaper-inference/OmniRoute
Length of output: 2664
Keep the client's original model for the echo on both routes.
On
/v1/chat/completions, the route resolves the model alias onparsedBodybefore callinghandleChat.handleChatcapturesclientRawRequestfrom that object, soclientRawRequest.body.modelalready holds the resolved name.resolveClientEchoModelreads that field first, so the response echoes the resolved name instead of the alias the client sent.On
/v1/responses,withCodexPreferredModelreplaces the body with{ ...body, model }beforehandleChat. A bare Codex model therefore echoes with thecodex/prefix. The Responses keepalive frame also reads this rewritten value.Capture the client's original
modelin each route before any rewrite. Pass that value to the echo logic and to the Responses keepalive frame. The combo leaf case already works because the rewrite happens after the snapshot.