Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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))
27 changes: 27 additions & 0 deletions open-sse/config/providerFieldStrips.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
* - `"<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);
}

/** Immutably drop request fields Groq rejects with a 400. */
export function stripGroqUnsupportedFields<T extends Record<string, unknown>>(body: T): T {
if (!body || typeof body !== "object") return body;
Expand Down
24 changes: 24 additions & 0 deletions open-sse/executors/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import { createCopilotIdentityFallback } from "./copilotIdentityFallback.ts";
import {
findOffendingField,
detectUnsupportedParam,
isUnsupportedThinkingError,
REASONING_REQUEST_FIELDS,
stripGroqUnsupportedFields,
} from "../config/providerFieldStrips.ts";
import {
Expand Down Expand Up @@ -1581,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<string, unknown>)[field] !== undefined
);
if (
offending &&
!strippedFields.has(offending) &&
Expand All @@ -1597,6 +1604,23 @@ export class BaseExecutor {
`Upstream 400 rejected ${offending} on ${url} — retrying without it`
);
response = await fetchWithStartTimeout(url, { ...fetchOptions, body: retryBody });
} 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<string, unknown>)[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 ${thinkingFields.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).
Expand Down
163 changes: 163 additions & 0 deletions tests/unit/base-thinking-unsupported-retry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
/**
* 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 { 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);
}
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"
);
});
30 changes: 30 additions & 0 deletions tests/unit/provider-field-strips.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import assert from "node:assert/strict";

import {
findOffendingField,
isUnsupportedThinkingError,
REASONING_REQUEST_FIELDS,
stripGroqUnsupportedFields,
} from "../../open-sse/config/providerFieldStrips.ts";

Expand Down Expand Up @@ -80,3 +82,31 @@ 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}`);
}
});
Loading