From 5dbbd93e84e0e8f8b449b7750a95ffc68c2b85ee Mon Sep 17 00:00:00 2001 From: Aaron Scherer Date: Wed, 16 Sep 2026 07:01:47 -0500 Subject: [PATCH 1/6] feat(sse): retry without thinking when a model rejects it outright --- open-sse/config/providerFieldStrips.ts | 27 +++++++++++++++++++ open-sse/executors/base.ts | 27 +++++++++++++++++++ tests/unit/provider-field-strips.test.ts | 33 ++++++++++++++++++++++++ 3 files changed, 87 insertions(+) diff --git a/open-sse/config/providerFieldStrips.ts b/open-sse/config/providerFieldStrips.ts index dfa4344358d..f2419058047 100644 --- a/open-sse/config/providerFieldStrips.ts +++ b/open-sse/config/providerFieldStrips.ts @@ -53,6 +53,33 @@ export function detectUnsupportedParam(bodyText: string): string | null { return match?.[1] ?? null; } +/** + * Reasoning fields a request can carry into an OpenAI-compatible upstream. Ordered + * most to least common so the retry drops the real one first when several are set. + */ +export const REASONING_REQUEST_FIELDS: readonly string[] = [ + "reasoning_effort", + "reasoning", + "thinking", + "think", +]; + +/** + * Some upstreams reject thinking by naming the MODEL, not the offending field, so + * neither `findOffendingField` nor `detectUnsupportedParam` can see anything to strip. + * Ollama does this for non-thinking models such as Qwen3-Coder (an Instruct-only + * build): `"Qwen3-Coder:latest" does not support thinking`. Matches: + * - `"" does not support thinking` + * - `model X does not support reasoning` + */ +export const UNSUPPORTED_THINKING_RE = /does\s+not\s+support\s+(?:thinking|reasoning)\b/i; + +/** True when a 400 body says the model has no thinking mode at all. */ +export function isUnsupportedThinkingError(bodyText: string): boolean { + if (typeof bodyText !== "string" || !bodyText) return false; + return UNSUPPORTED_THINKING_RE.test(bodyText); +} + /** Immutably drop request fields Groq rejects with a 400. */ export function stripGroqUnsupportedFields>(body: T): T { if (!body || typeof body !== "object") return body; diff --git a/open-sse/executors/base.ts b/open-sse/executors/base.ts index 7f07fa56a3a..6895bf1363f 100644 --- a/open-sse/executors/base.ts +++ b/open-sse/executors/base.ts @@ -17,6 +17,8 @@ import { createCopilotIdentityFallback } from "./copilotIdentityFallback.ts"; import { findOffendingField, detectUnsupportedParam, + isUnsupportedThinkingError, + REASONING_REQUEST_FIELDS, stripGroqUnsupportedFields, } from "../config/providerFieldStrips.ts"; import { @@ -1597,6 +1599,31 @@ export class BaseExecutor { `Upstream 400 rejected ${offending} on ${url} — retrying without it` ); response = await fetchWithStartTimeout(url, { ...fetchOptions, body: retryBody }); + } else if (isUnsupportedThinkingError(errText)) { + // Upstream rejected thinking by naming the model, not the field, so the + // two detectors above find nothing to strip. Drop whichever reasoning + // field the request actually carries and retry once. Ollama does this + // for Instruct-only models (e.g. Qwen3-Coder). + const reasoningFields = REASONING_REQUEST_FIELDS.filter( + (field) => + !strippedFields.has(field) && + (transformedBody as Record)[field] !== undefined + ); + if (reasoningFields.length > 0) { + for (const field of reasoningFields) { + strippedFields.add(field); + delete (transformedBody as Record)[field]; + } + let retryBody = JSON.stringify(transformedBody); + if (usesClaudeCodeProtocol || this.provider === "claude") { + retryBody = await signRequestBody(retryBody); + } + log?.info?.( + "THINKING_UNSUPPORTED", + `Upstream 400: ${model} has no thinking mode on ${url} — retrying without ${reasoningFields.join(", ")}` + ); + response = await fetchWithStartTimeout(url, { ...fetchOptions, body: retryBody }); + } } else { // Auto-learn: detect "Unsupported parameter" errors and persist to DB // when the provider config has autoLearn enabled (#6625). diff --git a/tests/unit/provider-field-strips.test.ts b/tests/unit/provider-field-strips.test.ts index ffcbb869327..54faa9ac1be 100644 --- a/tests/unit/provider-field-strips.test.ts +++ b/tests/unit/provider-field-strips.test.ts @@ -3,6 +3,8 @@ import assert from "node:assert/strict"; import { findOffendingField, + isUnsupportedThinkingError, + REASONING_REQUEST_FIELDS, stripGroqUnsupportedFields, } from "../../open-sse/config/providerFieldStrips.ts"; @@ -80,3 +82,34 @@ test("stripGroqUnsupportedFields drops unsupported messages[].model and other me assert.equal("messageId" in out.messages[1], false); assert.equal("sender" in out.messages[1], false); }); + +test("isUnsupportedThinkingError matches upstreams that name the model, not the field", () => { + // Ollama's exact wording for an Instruct-only model. + assert.equal( + isUnsupportedThinkingError('"Qwen3-Coder:latest" does not support thinking'), + true + ); + assert.equal(isUnsupportedThinkingError("model gemma3 does not support reasoning"), true); + assert.equal(isUnsupportedThinkingError("does not\n support\tthinking"), true); + assert.equal(isUnsupportedThinkingError("DOES NOT SUPPORT THINKING"), true); +}); + +test("isUnsupportedThinkingError ignores unrelated and near-miss 400 bodies", () => { + assert.equal(isUnsupportedThinkingError("does not support tools"), false); + assert.equal(isUnsupportedThinkingError("does not support thinkingly"), false); + assert.equal(isUnsupportedThinkingError("supports thinking"), false); + assert.equal(isUnsupportedThinkingError("rate limit exceeded"), false); + assert.equal(isUnsupportedThinkingError(""), false); + assert.equal(isUnsupportedThinkingError(undefined), false); +}); + +test("neither existing detector can see the model-named thinking rejection", () => { + // The reason this branch exists: findOffendingField needs a literal field name. + assert.equal(findOffendingField('"Qwen3-Coder:latest" does not support thinking'), null); +}); + +test("REASONING_REQUEST_FIELDS covers the fields a request can carry", () => { + for (const field of ["reasoning_effort", "reasoning", "thinking", "think"]) { + assert.ok(REASONING_REQUEST_FIELDS.includes(field), `missing ${field}`); + } +}); From 316d671bc1e94f4686686f998d8b7dfba900196d Mon Sep 17 00:00:00 2001 From: Aaron Scherer Date: Wed, 16 Sep 2026 07:02:53 -0500 Subject: [PATCH 2/6] docs(changelog): add fragment for thinking-rejection retry --- changelog.d/features/strip-thinking-on-model-rejection.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/features/strip-thinking-on-model-rejection.md diff --git a/changelog.d/features/strip-thinking-on-model-rejection.md b/changelog.d/features/strip-thinking-on-model-rejection.md new file mode 100644 index 00000000000..4d169a1cb67 --- /dev/null +++ b/changelog.d/features/strip-thinking-on-model-rejection.md @@ -0,0 +1 @@ +- **feat(sse):** a request no longer dies when the upstream rejects thinking by naming the model instead of the offending field. OmniRoute now drops whichever reasoning field the body actually carries (`reasoning_effort`, `reasoning`, `thinking`, `think`) and retries once. Ollama rejects this way for Instruct-only models such as Qwen3-Coder, where the 400 reads `"Qwen3-Coder:latest" does not support thinking`. Neither existing detector could see anything to strip there, so the error went straight back to the client and every Claude Code request carrying a thinking block failed against a self-hosted non-thinking model. From 80d1e620ad1d570236f284d6288e4ceca11195c3 Mon Sep 17 00:00:00 2001 From: Aaron Scherer Date: Wed, 16 Sep 2026 07:04:03 -0500 Subject: [PATCH 3/6] docs(changelog): number the fragment for #13868 --- ...-rejection.md => 13868-strip-thinking-on-model-rejection.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename changelog.d/features/{strip-thinking-on-model-rejection.md => 13868-strip-thinking-on-model-rejection.md} (83%) diff --git a/changelog.d/features/strip-thinking-on-model-rejection.md b/changelog.d/features/13868-strip-thinking-on-model-rejection.md similarity index 83% rename from changelog.d/features/strip-thinking-on-model-rejection.md rename to changelog.d/features/13868-strip-thinking-on-model-rejection.md index 4d169a1cb67..938cd3bd561 100644 --- a/changelog.d/features/strip-thinking-on-model-rejection.md +++ b/changelog.d/features/13868-strip-thinking-on-model-rejection.md @@ -1 +1 @@ -- **feat(sse):** a request no longer dies when the upstream rejects thinking by naming the model instead of the offending field. OmniRoute now drops whichever reasoning field the body actually carries (`reasoning_effort`, `reasoning`, `thinking`, `think`) and retries once. Ollama rejects this way for Instruct-only models such as Qwen3-Coder, where the 400 reads `"Qwen3-Coder:latest" does not support thinking`. Neither existing detector could see anything to strip there, so the error went straight back to the client and every Claude Code request carrying a thinking block failed against a self-hosted non-thinking model. +- **feat(sse):** a request no longer dies when the upstream rejects thinking by naming the model instead of the offending field. OmniRoute now drops whichever reasoning field the body actually carries (`reasoning_effort`, `reasoning`, `thinking`, `think`) and retries once. Ollama rejects this way for Instruct-only models such as Qwen3-Coder, where the 400 reads `"Qwen3-Coder:latest" does not support thinking`. Neither existing detector could see anything to strip there, so the error went straight back to the client and every Claude Code request carrying a thinking block failed against a self-hosted non-thinking model. ([#13868](https://github.com/diegosouzapw/OmniRoute/pull/13868)) From 2df3241a4e340ab58596dc609f58aab7a3922cfe Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:30:34 -0300 Subject: [PATCH 4/6] fix(sse): fall through to auto-learn when thinking-unsupported retry has no field to strip Add regression coverage for the 400-recovery retry chain. --- open-sse/executors/base.ts | 40 +++--- .../base-thinking-unsupported-retry.test.ts | 119 ++++++++++++++++++ 2 files changed, 143 insertions(+), 16 deletions(-) create mode 100644 tests/unit/base-thinking-unsupported-retry.test.ts diff --git a/open-sse/executors/base.ts b/open-sse/executors/base.ts index 6895bf1363f..e5bcbc687b0 100644 --- a/open-sse/executors/base.ts +++ b/open-sse/executors/base.ts @@ -1599,31 +1599,39 @@ export class BaseExecutor { `Upstream 400 rejected ${offending} on ${url} — retrying without it` ); response = await fetchWithStartTimeout(url, { ...fetchOptions, body: retryBody }); - } else if (isUnsupportedThinkingError(errText)) { + } else if ( + isUnsupportedThinkingError(errText) && + REASONING_REQUEST_FIELDS.some( + (field) => + !strippedFields.has(field) && + (transformedBody as Record)[field] !== undefined + ) + ) { // Upstream rejected thinking by naming the model, not the field, so the // two detectors above find nothing to strip. Drop whichever reasoning // field the request actually carries and retry once. Ollama does this - // for Instruct-only models (e.g. Qwen3-Coder). + // for Instruct-only models (e.g. Qwen3-Coder). When the request carries + // no top-level reasoning field at all (e.g. Gemini's nested + // generationConfig.thinkingConfig), this condition is false and control + // falls through to the auto-learn `else` below instead of no-oping. const reasoningFields = REASONING_REQUEST_FIELDS.filter( (field) => !strippedFields.has(field) && (transformedBody as Record)[field] !== undefined ); - if (reasoningFields.length > 0) { - for (const field of reasoningFields) { - strippedFields.add(field); - delete (transformedBody as Record)[field]; - } - let retryBody = JSON.stringify(transformedBody); - if (usesClaudeCodeProtocol || this.provider === "claude") { - retryBody = await signRequestBody(retryBody); - } - log?.info?.( - "THINKING_UNSUPPORTED", - `Upstream 400: ${model} has no thinking mode on ${url} — retrying without ${reasoningFields.join(", ")}` - ); - response = await fetchWithStartTimeout(url, { ...fetchOptions, body: retryBody }); + for (const field of reasoningFields) { + strippedFields.add(field); + delete (transformedBody as Record)[field]; } + let retryBody = JSON.stringify(transformedBody); + if (usesClaudeCodeProtocol || this.provider === "claude") { + retryBody = await signRequestBody(retryBody); + } + log?.info?.( + "THINKING_UNSUPPORTED", + `Upstream 400: ${model} has no thinking mode on ${url} — retrying without ${reasoningFields.join(", ")}` + ); + response = await fetchWithStartTimeout(url, { ...fetchOptions, body: retryBody }); } else { // Auto-learn: detect "Unsupported parameter" errors and persist to DB // when the provider config has autoLearn enabled (#6625). diff --git a/tests/unit/base-thinking-unsupported-retry.test.ts b/tests/unit/base-thinking-unsupported-retry.test.ts new file mode 100644 index 00000000000..353764c36e3 --- /dev/null +++ b/tests/unit/base-thinking-unsupported-retry.test.ts @@ -0,0 +1,119 @@ +/** + * TDD for #13868 — BaseExecutor's 400-recovery chain must retry once, stripping the + * request's reasoning field, when an upstream rejects thinking by naming the MODEL + * rather than the offending field (Ollama does this for Instruct-only models such as + * Qwen3-Coder). Neither `findOffendingField` nor `detectUnsupportedParam` can see + * anything to strip in that error shape, so this exercises the dedicated + * `isUnsupportedThinkingError` branch end-to-end through the real retry chain. + * + * Mirrors the fetch-capture pattern in context-editing-relays.test.ts. + * + * Run: node --import tsx/esm --test tests/unit/base-thinking-unsupported-retry.test.ts + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { DefaultExecutor } from "../../open-sse/executors/default.ts"; + +/** First call returns `status` with `errorText`; subsequent calls return 200 OK. */ +function mockFetchErrorThenOk(status: number, errorText: string) { + const bodies: Array> = []; + const original = globalThis.fetch; + let calls = 0; + globalThis.fetch = (async (_url: unknown, init: { body?: unknown } = {}) => { + bodies.push(JSON.parse(String(init.body ?? "{}"))); + calls += 1; + if (calls === 1) { + return new Response(errorText, { + status, + headers: { "Content-Type": "application/json" }, + }); + } + return new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }) as typeof globalThis.fetch; + return { bodies, callCount: () => calls, restore: () => void (globalThis.fetch = original) }; +} + +const baseCredentials = { apiKey: "relay-key", baseUrl: "https://relay.example/v1" }; + +test("#13868: upstream 400 naming the model (not the field) as thinking-unsupported strips reasoning_effort and retries once", async () => { + const { bodies, callCount, restore } = mockFetchErrorThenOk( + 400, + '"Qwen3-Coder:latest" does not support thinking' + ); + try { + await new DefaultExecutor("anthropic-compatible-cc-myrelay").execute({ + model: "Qwen3-Coder:latest", + body: { + model: "Qwen3-Coder:latest", + messages: [{ role: "user", content: "hi" }], + max_tokens: 1, + reasoning_effort: "high", + }, + stream: false, + credentials: baseCredentials, + }); + } finally { + restore(); + } + assert.equal(callCount(), 2, "must retry exactly once after the thinking-unsupported 400"); + assert.equal(bodies[0]?.reasoning_effort, "high", "first attempt carried reasoning_effort"); + assert.equal( + "reasoning_effort" in (bodies[1] ?? {}), + false, + "retry must drop reasoning_effort entirely" + ); +}); + +test("#13868: the retry strips only the reasoning field actually present, not every REASONING_REQUEST_FIELDS entry", async () => { + const { bodies, callCount, restore } = mockFetchErrorThenOk( + 400, + "model gemma3 does not support reasoning" + ); + try { + await new DefaultExecutor("anthropic-compatible-cc-myrelay").execute({ + model: "gemma3", + body: { + model: "gemma3", + messages: [{ role: "user", content: "hi" }], + max_tokens: 1, + thinking: { type: "enabled" }, + }, + stream: false, + credentials: baseCredentials, + }); + } finally { + restore(); + } + assert.equal(callCount(), 2, "must retry exactly once"); + assert.equal("thinking" in (bodies[1] ?? {}), false, "retry must drop the present field"); + assert.equal( + "reasoning_effort" in (bodies[1] ?? {}), + false, + "a field never sent must not appear on retry either" + ); +}); + +test("#13868: an UNRELATED 400 does NOT strip reasoning fields or retry", async () => { + const { bodies, callCount, restore } = mockFetchErrorThenOk(400, "max_tokens: must be >= 1"); + try { + await new DefaultExecutor("anthropic-compatible-cc-myrelay").execute({ + model: "Qwen3-Coder:latest", + body: { + model: "Qwen3-Coder:latest", + messages: [{ role: "user", content: "hi" }], + max_tokens: 1, + reasoning_effort: "high", + }, + stream: false, + credentials: baseCredentials, + }); + } finally { + restore(); + } + assert.equal(callCount(), 1, "an unrelated 400 must not trigger the thinking-unsupported retry"); + assert.equal(bodies[0]?.reasoning_effort, "high", "the single attempt still carried the field"); +}); From ff85f88a6361fd5173c1cc8b0e1f7ed5cc7a9abe Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:50:59 -0300 Subject: [PATCH 5/6] refactor(sse): shrink the thinking-unsupported retry branch and apply prettier Reuses a single non-mutating field-presence check for the retry/fallthrough decision instead of duplicating the filter, and formats the touched files. --- open-sse/executors/base.ts | 33 +++++-------- .../base-thinking-unsupported-retry.test.ts | 46 ++++++++++++++++++- tests/unit/provider-field-strips.test.ts | 5 +- 3 files changed, 57 insertions(+), 27 deletions(-) diff --git a/open-sse/executors/base.ts b/open-sse/executors/base.ts index e5bcbc687b0..9f034652260 100644 --- a/open-sse/executors/base.ts +++ b/open-sse/executors/base.ts @@ -1583,6 +1583,11 @@ export class BaseExecutor { .text() .catch(() => ""); const offending = findOffendingField(errText); + const thinkingFields = REASONING_REQUEST_FIELDS.filter( + (field) => + !strippedFields.has(field) && + (transformedBody as Record)[field] !== undefined + ); if ( offending && !strippedFields.has(offending) && @@ -1599,27 +1604,11 @@ export class BaseExecutor { `Upstream 400 rejected ${offending} on ${url} — retrying without it` ); response = await fetchWithStartTimeout(url, { ...fetchOptions, body: retryBody }); - } else if ( - isUnsupportedThinkingError(errText) && - REASONING_REQUEST_FIELDS.some( - (field) => - !strippedFields.has(field) && - (transformedBody as Record)[field] !== undefined - ) - ) { - // Upstream rejected thinking by naming the model, not the field, so the - // two detectors above find nothing to strip. Drop whichever reasoning - // field the request actually carries and retry once. Ollama does this - // for Instruct-only models (e.g. Qwen3-Coder). When the request carries - // no top-level reasoning field at all (e.g. Gemini's nested - // generationConfig.thinkingConfig), this condition is false and control - // falls through to the auto-learn `else` below instead of no-oping. - const reasoningFields = REASONING_REQUEST_FIELDS.filter( - (field) => - !strippedFields.has(field) && - (transformedBody as Record)[field] !== undefined - ); - for (const field of reasoningFields) { + } else if (isUnsupportedThinkingError(errText) && thinkingFields.length > 0) { + // Ollama names the MODEL, not the field, as thinking-unsupported (e.g. + // Qwen3-Coder). When no reasoning field is present (e.g. Gemini's nested + // generationConfig.thinkingConfig), this falls through to auto-learn below. + for (const field of thinkingFields) { strippedFields.add(field); delete (transformedBody as Record)[field]; } @@ -1629,7 +1618,7 @@ export class BaseExecutor { } log?.info?.( "THINKING_UNSUPPORTED", - `Upstream 400: ${model} has no thinking mode on ${url} — retrying without ${reasoningFields.join(", ")}` + `Upstream 400: ${model} has no thinking mode on ${url} — retrying without ${thinkingFields.join(", ")}` ); response = await fetchWithStartTimeout(url, { ...fetchOptions, body: retryBody }); } else { diff --git a/tests/unit/base-thinking-unsupported-retry.test.ts b/tests/unit/base-thinking-unsupported-retry.test.ts index 353764c36e3..27a2855ebc1 100644 --- a/tests/unit/base-thinking-unsupported-retry.test.ts +++ b/tests/unit/base-thinking-unsupported-retry.test.ts @@ -14,6 +14,7 @@ import test from "node:test"; import assert from "node:assert/strict"; import { DefaultExecutor } from "../../open-sse/executors/default.ts"; +import { setGlobalAutoLearnEnabled } from "../../src/lib/db/paramFilters.ts"; /** First call returns `status` with `errorText`; subsequent calls return 200 OK. */ function mockFetchErrorThenOk(status: number, errorText: string) { @@ -37,7 +38,10 @@ function mockFetchErrorThenOk(status: number, errorText: string) { return { bodies, callCount: () => calls, restore: () => void (globalThis.fetch = original) }; } -const baseCredentials = { apiKey: "relay-key", baseUrl: "https://relay.example/v1" }; +const baseCredentials = { + apiKey: "relay-key", + providerSpecificData: { baseUrl: "https://relay.example/v1" }, +}; test("#13868: upstream 400 naming the model (not the field) as thinking-unsupported strips reasoning_effort and retries once", async () => { const { bodies, callCount, restore } = mockFetchErrorThenOk( @@ -117,3 +121,43 @@ test("#13868: an UNRELATED 400 does NOT strip reasoning fields or retry", async assert.equal(callCount(), 1, "an unrelated 400 must not trigger the thinking-unsupported retry"); assert.equal(bodies[0]?.reasoning_effort, "high", "the single attempt still carried the field"); }); + +test("#13868: a thinking-unsupported 400 with NO reasoning field on the body falls through to auto-learn instead of silently no-oping", async () => { + // Gemini-shaped case: the request carries no top-level REASONING_REQUEST_FIELDS entry + // (its thinking config is nested under generationConfig.thinkingConfig), so the + // isUnsupportedThinkingError branch has nothing to strip. Before this fix, that branch + // had no `else`, so an upstream 400 matching BOTH signals (thinking-unsupported wording + // AND a literal "Unsupported parameter" name) would still fall into the dead-end `if` + // and never reach the auto-learn detector below it, even though auto-learn is enabled. + setGlobalAutoLearnEnabled(true); + const { bodies, callCount, restore } = mockFetchErrorThenOk( + 400, + "model does not support thinking; Unsupported parameter: generationConfig" + ); + try { + await new DefaultExecutor("anthropic-compatible-cc-myrelay").execute({ + model: "gemini-shaped-model", + body: { + model: "gemini-shaped-model", + messages: [{ role: "user", content: "hi" }], + max_tokens: 1, + generationConfig: { thinkingConfig: { thinkingBudget: 100 } }, + }, + stream: false, + credentials: baseCredentials, + }); + } finally { + restore(); + setGlobalAutoLearnEnabled(false); + } + assert.equal( + callCount(), + 2, + "must fall through to auto-learn and retry once the thinking branch finds nothing to strip" + ); + assert.equal( + "generationConfig" in (bodies[1] ?? {}), + false, + "auto-learn must have stripped the field it detected" + ); +}); diff --git a/tests/unit/provider-field-strips.test.ts b/tests/unit/provider-field-strips.test.ts index 54faa9ac1be..4bbbee1379e 100644 --- a/tests/unit/provider-field-strips.test.ts +++ b/tests/unit/provider-field-strips.test.ts @@ -85,10 +85,7 @@ test("stripGroqUnsupportedFields drops unsupported messages[].model and other me test("isUnsupportedThinkingError matches upstreams that name the model, not the field", () => { // Ollama's exact wording for an Instruct-only model. - assert.equal( - isUnsupportedThinkingError('"Qwen3-Coder:latest" does not support thinking'), - true - ); + assert.equal(isUnsupportedThinkingError('"Qwen3-Coder:latest" does not support thinking'), true); assert.equal(isUnsupportedThinkingError("model gemma3 does not support reasoning"), true); assert.equal(isUnsupportedThinkingError("does not\n support\tthinking"), true); assert.equal(isUnsupportedThinkingError("DOES NOT SUPPORT THINKING"), true); From e0f2269bffaf35dc06a1f385ca1ee3f7e1708355 Mon Sep 17 00:00:00 2001 From: Aaron Scherer Date: Wed, 7 Oct 2026 23:41:22 -0300 Subject: [PATCH 6/6] fix(sse): port the thinking-unsupported retry into applyFieldDowngradeRecovery The release branch extracted the 400 field-downgrade chain out of base.ts into open-sse/executors/base/fieldDowngradeRecovery.ts, so the inline branch from this PR no longer had a home after merging the tip. Move it into a dedicated applyThinkingUnsupportedRecovery() helper that runs between the findOffendingField retry and auto-learn, serializing through the serializeBody callback (which already signs Claude Code protocol bodies). With no top-level reasoning field it still returns null so auto-learn runs. The executor-level test now drops the provider blocklist entry auto-learn persists, so a re-run against the same DATA_DIR no longer pre-strips generationConfig and skips the retry. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- .../executors/base/fieldDowngradeRecovery.ts | 37 ++++++++++++++++++- .../base-thinking-unsupported-retry.test.ts | 8 +++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/open-sse/executors/base/fieldDowngradeRecovery.ts b/open-sse/executors/base/fieldDowngradeRecovery.ts index d84a5d01f43..eec1e526552 100644 --- a/open-sse/executors/base/fieldDowngradeRecovery.ts +++ b/open-sse/executors/base/fieldDowngradeRecovery.ts @@ -10,7 +10,12 @@ import { isAutoLearnGloballyEnabled, } from "@/lib/db/paramFilters"; import { HTTP_STATUS } from "../../config/constants.ts"; -import { findOffendingField, detectUnsupportedParam } from "../../config/providerFieldStrips.ts"; +import { + findOffendingField, + detectUnsupportedParam, + isUnsupportedThinkingError, + REASONING_REQUEST_FIELDS, +} from "../../config/providerFieldStrips.ts"; type FieldDowngradeLog = { debug?: (tag: string, message: string) => void; @@ -69,6 +74,34 @@ async function applyAutoLearnRecovery( return response; } +/** + * Some upstreams (Ollama for Instruct-only models such as Qwen3-Coder) reject thinking by + * naming the MODEL, not the field. Drop every top-level reasoning field the body carries + * and retry once; null when no such field is present (e.g. Gemini's nested + * generationConfig.thinkingConfig), so the caller falls through to auto-learn. + */ +async function applyThinkingUnsupportedRecovery( + params: FieldDowngradeParams, + record: Record, + errText: string +): Promise { + const { url, model, body, fetchOptions, fetchFn, serializeBody, strippedFields, log } = params; + const thinkingFields = REASONING_REQUEST_FIELDS.filter( + (field) => !strippedFields.has(field) && record[field] !== undefined + ); + if (thinkingFields.length === 0 || !isUnsupportedThinkingError(errText)) return null; + for (const field of thinkingFields) { + strippedFields.add(field); + delete record[field]; + } + const retryBody = await serializeBody(body); + log?.info?.( + "THINKING_UNSUPPORTED", + `Upstream 400: ${model} has no thinking mode on ${url} — retrying without ${thinkingFields.join(", ")}` + ); + return fetchFn(url, { ...fetchOptions, body: retryBody }); +} + /** Returns the response to use going forward (the retry's when one was issued). */ export async function applyFieldDowngradeRecovery(params: FieldDowngradeParams): Promise { const { url, fetchOptions, fetchFn, serializeBody, strippedFields, log } = params; @@ -90,5 +123,7 @@ export async function applyFieldDowngradeRecovery(params: FieldDowngradeParams): log?.debug?.("FIELD_400", `Upstream 400 rejected ${offending} on ${url} — retrying without it`); return fetchFn(url, { ...fetchOptions, body: retryBody }); } + const thinkingRetry = await applyThinkingUnsupportedRecovery(params, record, errText); + if (thinkingRetry) return thinkingRetry; return (await applyAutoLearnRecovery(params, record, errText)) ?? response; } diff --git a/tests/unit/base-thinking-unsupported-retry.test.ts b/tests/unit/base-thinking-unsupported-retry.test.ts index 27a2855ebc1..23bd3f0da96 100644 --- a/tests/unit/base-thinking-unsupported-retry.test.ts +++ b/tests/unit/base-thinking-unsupported-retry.test.ts @@ -14,7 +14,10 @@ import test from "node:test"; import assert from "node:assert/strict"; import { DefaultExecutor } from "../../open-sse/executors/default.ts"; -import { setGlobalAutoLearnEnabled } from "../../src/lib/db/paramFilters.ts"; +import { + deleteParamFilterConfig, + setGlobalAutoLearnEnabled, +} from "../../src/lib/db/paramFilters.ts"; /** First call returns `status` with `errorText`; subsequent calls return 200 OK. */ function mockFetchErrorThenOk(status: number, errorText: string) { @@ -149,6 +152,9 @@ test("#13868: a thinking-unsupported 400 with NO reasoning field on the body fal } finally { restore(); setGlobalAutoLearnEnabled(false); + // Auto-learn persists "generationConfig" to this provider's blocklist; drop it so a + // re-run against the same DATA_DIR does not pre-strip the field and skip the retry. + deleteParamFilterConfig("anthropic-compatible-cc-myrelay"); } assert.equal( callCount(),