feat(sse): retry without thinking when a model rejects it outright

changelog.d/features/13868-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. ([#13868](https://github.com/diegosouzapw/OmniRoute/pull/13868))

open-sse/config/providerFieldStrips.ts

@@ -71,6 +71,33 @@ export function detectUnsupportedParam(bodyText: string): string | null {
   return name && !NON_STRIPPABLE_PARAMS.has(name.toLowerCase()) ? name : 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:
+ *   - `"<model>" 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);
+}
+
 /**
  * Anthropic's 400 message when a request carries an `advisor_redacted_result` whose
  * `encrypted_content` was produced by a different organization.

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<string, unknown>,
+  errText: string
+): Promise<Response | null> {
+  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<Response> {
   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;
 }

tests/unit/base-thinking-unsupported-retry.test.ts

@@ -0,0 +1,169 @@
+/**
+ * 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";
+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) {
+  const bodies: Array<Record<string, unknown>> = [];
+  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",
+  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(
+    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");
+});
+
+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);
+    // 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(),
+    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"
+  );
+});

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";
 
@@ -81,6 +83,34 @@ test("stripGroqUnsupportedFields drops unsupported messages[].model and other me
   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}`);
+  }
+});
+
 // Routing envelopes must become valid Groq Chat Completions fields.
 test("Groq GPT-OSS routing low/none uses low without foreign envelopes", () => {
   for (const effort of ["low", "none"]) {