diff --git a/changelog.d/fixes/0000-responses-node-model-test.md b/changelog.d/fixes/0000-responses-node-model-test.md new file mode 100644 index 0000000000..c3191fc953 --- /dev/null +++ b/changelog.d/fixes/0000-responses-node-model-test.md @@ -0,0 +1 @@ +- **fix(dashboard):** model health tests for a provider node set to the Responses API now call `/v1/responses` with a Responses-shaped body instead of `/v1/chat/completions` — those models were reported as `Provider returned HTTP 200 but no text content` even though the same model answered normally through `/v1/responses` ([#13070](https://github.com/diegosouzapw/OmniRoute/issues/13070)) diff --git a/src/lib/api/modelTestRunner.ts b/src/lib/api/modelTestRunner.ts index c72248f55a..59790cc111 100644 --- a/src/lib/api/modelTestRunner.ts +++ b/src/lib/api/modelTestRunner.ts @@ -3,7 +3,9 @@ import { POST as postChatCompletion } from "@/app/api/v1/chat/completions/route" import { POST as postAudioTranscription } from "@/app/api/v1/audio/transcriptions/route"; import { handleValidatedEmbeddingRequestBody } from "@/app/api/v1/embeddings/route"; import { POST as postRerank } from "@/app/api/v1/rerank/route"; +import { POST as postResponses } from "@/app/api/v1/responses/route"; import { + buildComboTestPrompt, buildComboTestRequestBody, extractComboTestResponseText, extractComboTestStreamResult, @@ -29,6 +31,10 @@ const ZAI_WEB_PROVIDER_ID = "zai-web"; const ZAI_WEB_TEST_TIMEOUT_MS = 60_000; const SLOW_WEB_TEST_MODELS = new Set(["dola-pro"]); const STREAMING_CHAT_TEST_MAX_TOKENS = 64; +// Responses calls the same budget `max_output_tokens`; `max_tokens` is silently +// ignored on that endpoint, which would let a reasoning model spend the whole +// default budget before emitting any visible text. +const RESPONSES_TEST_MAX_OUTPUT_TOKENS = 256; function asRecord(value: unknown): Record { return value && typeof value === "object" && !Array.isArray(value) @@ -175,6 +181,26 @@ export function buildInternalChatRequest( }); } +export function buildInternalResponsesRequest( + testBody: Record, + signal: AbortSignal, + connectionId?: string +) { + return new Request(`${INTERNAL_ORIGIN}/v1/responses`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-Internal-Test": "combo-health-check", + "X-OmniRoute-No-Cache": "true", + "X-OmniRoute-Compression": "off", + "X-Request-Id": `model-test-${randomUUID()}`, + ...(connectionId ? { "X-OmniRoute-Connection": connectionId } : {}), + }, + body: JSON.stringify(testBody), + signal, + }); +} + export function buildInternalRerankRequest( testBody: Record, signal: AbortSignal, @@ -265,7 +291,22 @@ export function detectTestKind(modelStr: string, customModel: any, nodeApiType?: lowerModel.includes("text-embed") || lowerModel.includes("jina-clip") || lowerModel.includes("colbert")); - return { isRerank, isEmbedding, isAudioTranscription }; + // A Responses node answers on /v1/responses only. Without this the model fell + // through to the chat branch below, which posts a Chat Completions body to + // /v1/chat/completions: the route can still answer 200 while carrying nothing a + // Chat Completions reader recognises, so the model was marked unhealthy with + // "Provider returned HTTP 200 but no text content" (#13070). + // + // Last in the chain deliberately: a Responses-typed node can still host an + // embedding or rerank model, and those endpoints stay right for it. + const isResponses = + !isAudioTranscription && + !isRerank && + !isEmbedding && + (apiFormat === "responses" || + nodeType === "responses" || + supportedEndpoints.includes("responses")); + return { isRerank, isEmbedding, isAudioTranscription, isResponses }; } /** @@ -424,7 +465,7 @@ export async function runSingleModelTest( findCustomModelMetadata(providerId, fullModelStr), findProviderNodeApiType(providerId), ]); - const { isRerank, isEmbedding, isAudioTranscription } = detectTestKind( + const { isRerank, isEmbedding, isAudioTranscription, isResponses } = detectTestKind( fullModelStr, customModel, nodeApiType @@ -443,10 +484,22 @@ export async function runSingleModelTest( } : isAudioTranscription ? { model: fullModelStr } - : buildComboTestRequestBody(fullModelStr, isEmbedding, { - stream: !isEmbedding && streamChat, - maxTokens: !isEmbedding && streamChat ? STREAMING_CHAT_TEST_MAX_TOKENS : undefined, - }); + : isResponses + ? { + model: fullModelStr, + // Responses takes `input`, not `messages`. + input: buildComboTestPrompt(), + max_output_tokens: RESPONSES_TEST_MAX_OUTPUT_TOKENS, + // Non-streaming on purpose: the SSE reader below understands Chat + // Completions deltas and the `output_text`/`output[]` shapes, but not + // Responses stream events (`response.output_text.delta`), so a + // streamed answer would read as empty — the very failure being fixed. + stream: false, + } + : buildComboTestRequestBody(fullModelStr, isEmbedding, { + stream: !isEmbedding && streamChat, + maxTokens: !isEmbedding && streamChat ? STREAMING_CHAT_TEST_MAX_TOKENS : undefined, + }); // Per-model AbortController. We track whether the timeout fired so we can // distinguish "rate-limit queue aborted" (withRateLimit threw AbortError @@ -473,6 +526,9 @@ export async function runSingleModelTest( buildInternalAudioTranscriptionRequest(fullModelStr, signal, connectionId) ); } + if (isResponses) { + return postResponses(buildInternalResponsesRequest(testBody, signal, connectionId)); + } return postChatCompletion(buildInternalChatRequest(testBody, signal, connectionId)); }; @@ -577,7 +633,7 @@ export async function runSingleModelTest( // deactivated") would run outside runAsProbe and could still reach // markAccountUnavailable (#9817). const parsedResponse = await runAsProbe(() => - extractModelTestResponseText(res, !isEmbedding && !isRerank && streamChat) + extractModelTestResponseText(res, !isEmbedding && !isRerank && !isResponses && streamChat) ); responseText = parsedResponse.text; streamError = parsedResponse.error; diff --git a/src/lib/combos/testHealth.ts b/src/lib/combos/testHealth.ts index 0fcca804dd..b9897b5ce5 100644 --- a/src/lib/combos/testHealth.ts +++ b/src/lib/combos/testHealth.ts @@ -112,7 +112,7 @@ function getRandomFiveDigitNumber() { return COMBO_TEST_OPERAND_MIN + Math.floor(Math.random() * COMBO_TEST_OPERAND_RANGE); } -function buildComboTestPrompt() { +export function buildComboTestPrompt() { const left = getRandomFiveDigitNumber(); const right = getRandomFiveDigitNumber(); diff --git a/tests/unit/model-test-runner.test.ts b/tests/unit/model-test-runner.test.ts index c717ea0bb0..c8853b3c3a 100644 --- a/tests/unit/model-test-runner.test.ts +++ b/tests/unit/model-test-runner.test.ts @@ -74,6 +74,7 @@ test("detectTestKind defaults to a plain chat test for ordinary models", () => { isRerank: false, isEmbedding: false, isAudioTranscription: false, + isResponses: false, }); }); @@ -95,6 +96,7 @@ test("detectTestKind detects rerank by id and by metadata, and rerank wins over isRerank: true, isEmbedding: false, isAudioTranscription: false, + isResponses: false, }); // apiFormat metadata drives detection even when the id is opaque assert.equal(detectTestKind("vendor/opaque-model", { apiFormat: "rerank" }).isRerank, true); @@ -116,6 +118,7 @@ test("detectTestKind detects audio transcription from metadata, and it wins over isRerank: false, isEmbedding: false, isAudioTranscription: true, + isResponses: false, }); assert.equal( detectTestKind("vendor/opaque-model", { supportedEndpoints: ["audio-transcriptions"] }) @@ -152,6 +155,7 @@ test("detectTestKind falls back to the provider node's configured apiType", () = isRerank: false, isEmbedding: false, isAudioTranscription: false, + isResponses: false, }); // Per-model metadata still wins when present. diff --git a/tests/unit/responses-node-model-test-13070.test.ts b/tests/unit/responses-node-model-test-13070.test.ts new file mode 100644 index 0000000000..56f6088ccf --- /dev/null +++ b/tests/unit/responses-node-model-test-13070.test.ts @@ -0,0 +1,184 @@ +/** + * #13070 -- the dashboard's per-model health test ignored a provider node's + * `apiType: "responses"`. + * + * `detectTestKind` mapped a node's apiType to audio, rerank and embeddings only, + * so every text model on a Responses node fell through to the chat branch and + * `buildInternalChatRequest` posted a Chat Completions body to + * /v1/chat/completions. A Responses-native upstream can answer 200 to that and + * still carry nothing a Chat Completions reader recognises, so the model went + * red with "Provider returned HTTP 200 but no text content" while the same + * model answered normally through /v1/responses. + * + * The classification tests below are cheap, but on their own they prove + * nothing: reverting the dispatch in runSingleModelTest and leaving + * detectTestKind alone keeps them all green. The last test is the one that + * fails in that case -- it reads the body that actually leaves for the + * upstream and asserts it is Responses-shaped. + */ +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-13070-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const nodesDb = await import("../../src/lib/db/providers/nodes.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const runner = await import("../../src/lib/api/modelTestRunner.ts"); +const callLogs = await import("../../src/lib/usage/callLogs.ts"); + +const NODE_ID = "openai-compatible-responses-13070-0000-4000-8000-000000000000"; +const MODEL_ID = "opaque-text-model"; + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +// --------------------------------------------------------------------------- +// detectTestKind — a Responses node must be recognised, and must not steal the +// endpoints that were already right for it. +// --------------------------------------------------------------------------- + +test("detectTestKind reports a Responses node, whichever field carries the signal", () => { + // An imported model has no per-model metadata at all; the node's apiType is + // the only signal available, which is exactly the reported case. + assert.equal(runner.detectTestKind("vendor/opaque-guid", null, "responses").isResponses, true); + assert.equal( + runner.detectTestKind("vendor/opaque-guid", { apiFormat: "responses" }).isResponses, + true + ); + assert.equal( + runner.detectTestKind("vendor/opaque-guid", { supportedEndpoints: ["responses"] }).isResponses, + true + ); +}); + +test("detectTestKind leaves an ordinary chat model alone", () => { + const kind = runner.detectTestKind("openai/gpt-4o", null); + assert.equal(kind.isResponses, false); + assert.equal(kind.isRerank, false); + assert.equal(kind.isEmbedding, false); + assert.equal(kind.isAudioTranscription, false); +}); + +test("embeddings, rerank and audio still win over a Responses node type", () => { + // A Responses-typed node can host these too, and /v1/responses is the wrong + // endpoint for all three. Losing this ordering would break working setups + // rather than fix a broken one. + assert.equal( + runner.detectTestKind("baai/bge-m3", null, "responses").isEmbedding, + true, + "embedding id must still route to embeddings" + ); + assert.equal(runner.detectTestKind("baai/bge-m3", null, "responses").isResponses, false); + + assert.equal(runner.detectTestKind("jina/jina-reranker-v2", null, "responses").isRerank, true); + assert.equal( + runner.detectTestKind("jina/jina-reranker-v2", null, "responses").isResponses, + false + ); + + const audio = runner.detectTestKind( + "vendor/whisper", + { apiFormat: "audio-transcriptions" }, + "responses" + ); + assert.equal(audio.isAudioTranscription, true); + assert.equal(audio.isResponses, false); +}); + +// --------------------------------------------------------------------------- +// buildInternalResponsesRequest — the endpoint, and the bypass headers the +// other builders carry. A health check that lost X-Internal-Test would be +// rejected by strict mode instead of testing anything. +// --------------------------------------------------------------------------- + +test("buildInternalResponsesRequest targets /v1/responses with the health-check headers", async () => { + const controller = new AbortController(); + const req = runner.buildInternalResponsesRequest( + { model: "vendor/opaque", input: "hi" }, + controller.signal, + "conn-1" + ); + + assert.equal(new URL(req.url).pathname, "/v1/responses"); + assert.equal(req.method, "POST"); + assert.equal(req.headers.get("X-Internal-Test"), "combo-health-check"); + assert.equal(req.headers.get("X-OmniRoute-No-Cache"), "true"); + assert.equal(req.headers.get("X-OmniRoute-Compression"), "off"); + assert.equal(req.headers.get("X-OmniRoute-Connection"), "conn-1"); + assert.deepEqual(await req.json(), { model: "vendor/opaque", input: "hi" }); +}); + +test("buildInternalResponsesRequest omits the connection header when there is no connection", () => { + const req = runner.buildInternalResponsesRequest({ model: "m" }, new AbortController().signal); + assert.equal(req.headers.get("X-OmniRoute-Connection"), null); +}); + +// --------------------------------------------------------------------------- +// The wiring. Everything above passes against the unfixed runner as long as +// detectTestKind alone is changed; this one does not. +// --------------------------------------------------------------------------- + +test("a model on a Responses node is probed on the internal /v1/responses route", async () => { + await nodesDb.createProviderNode({ + id: NODE_ID, + type: "openai-compatible", + name: "Responses Node 13070", + prefix: "resp13070", + apiType: "responses", + baseUrl: "https://example.test/v1", + }); + const connection = await providersDb.createProviderConnection({ + provider: NODE_ID, + authType: "apikey", + name: "responses-node-13070", + apiKey: "sk-responses-node-13070", + isActive: true, + testStatus: "active", + }); + + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => + // A minimal Responses reply. `output_text` is a field the existing + // extractor already understands, which is why this fix needs no reader + // change -- only the request side was ever wrong. + new Response(JSON.stringify({ output_text: "4" }), { + status: 200, + headers: { "content-type": "application/json" }, + })) as typeof globalThis.fetch; + + try { + await runner.runSingleModelTest({ + providerId: NODE_ID, + modelId: MODEL_ID, + connectionId: String(connection.id), + timeoutMs: 15_000, + }); + } finally { + globalThis.fetch = originalFetch; + } + + await callLogs.waitForCallLogSaves(10_000); + const logs = await callLogs.getCallLogs({}); + const probe = logs.find((entry: { model?: string | null }) => + String(entry.model ?? "").includes(MODEL_ID) + ); + + assert.ok(probe, "the model test should have produced a call log entry"); + // This is the line from the report: the call log showed + // path=/v1/chat/completions for a Responses node. Asserting on the + // upstream request instead would prove nothing -- the router translates a + // chat body into Responses shape for such a node either way, so that + // assertion stays green with the dispatch below reverted. + assert.equal( + probe.path, + "/v1/responses", + `a Responses node must be probed on /v1/responses (call log says ${probe.path})` + ); +});