mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
fix(openai): strip reasoning_effort when GPT-5.x models carry function tools (#7101)
* fix(openai): strip reasoning_effort when GPT-5.x tools present (port from 9router#2540)
Raw api.openai.com Chat Completions rejects GPT-5.x reasoning models that carry both function tools and an active reasoning_effort with HTTP 400 ("Function tools with reasoning_effort are not supported ... Please use /v1/responses instead"). The existing forceResponsesUpstream guard only reroutes openai-compatible-* connections carrying MCP/tool_search tool shapes; the plain openai provider had no equivalent guard, so gpt-5.x models used with a coding client (function tools + any explicit reasoning effort) still hit the upstream 400. Add stripGpt5ReasoningWhenTools() (gpt5SamplingGuard.ts), wired into chatCore.ts alongside the existing sampling guard, to drop reasoning_effort/reasoning when function tools are present and reasoning is active, letting the request succeed on /v1/chat/completions.
Reported-by: Tech Solution (@techsolutionmta) (https://github.com/decolua/9router/issues/2540)
* fix(openai): scope reasoning-strip guard to /chat/completions only
stripGpt5ReasoningWhenTools gated on provider+model-name alone, so once
#7242 routes the public GPT-5.6 family to /v1/responses (targetFormat
"openai-responses", which natively supports tools + reasoning), the two
PRs would compose into the worst of both worlds: routed to the endpoint
that supports reasoning, but reasoning stripped anyway. Pass the
request's already-resolved targetFormat into the guard and skip the
strip whenever it is not going out over /chat/completions, so the
guard tracks the actual upstream surface instead of a model-name list
that would need updating for every future GPT-5.x family.
Reported-by: Tech Solution (@techsolutionmta) (https://github.com/decolua/9router/issues/2540)
This commit is contained in:
committed by
GitHub
parent
606aa9a7b0
commit
6cdb77a0c2
1
changelog.d/fixes/2540-gpt5-tools-reasoning-effort.md
Normal file
1
changelog.d/fixes/2540-gpt5-tools-reasoning-effort.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(openai):** strip `reasoning_effort`/`reasoning` for GPT-5.x models on the raw `openai` Chat Completions surface when the request carries function `tools` — upstream rejects that combination with HTTP 400 ("Function tools with reasoning_effort are not supported ... Please use /v1/responses instead"), and the dashboard has no `reasoning_effort:"none"` override to work around it client-side — thanks @techsolutionmta
|
||||
@@ -110,7 +110,10 @@ import { normalizeMimoThinking } from "../services/mimoThinking.ts";
|
||||
import { normalizeClaudeAdaptiveThinking } from "../services/claudeAdaptiveThinking.ts";
|
||||
import { normalizeClaudeHaikuConstraints } from "../services/claudeHaikuConstraints.ts";
|
||||
import { echoModelInObject } from "../services/responseModelEcho.ts";
|
||||
import { stripGpt5SamplingWhenReasoning } from "../services/gpt5SamplingGuard.ts";
|
||||
import {
|
||||
stripGpt5SamplingWhenReasoning,
|
||||
stripGpt5ReasoningWhenTools,
|
||||
} from "../services/gpt5SamplingGuard.ts";
|
||||
import { getUnsupportedParams, REGISTRY } from "../config/providerRegistry.ts";
|
||||
import { supportsMaxTokens, getResolvedModelCapabilities } from "@/lib/modelCapabilities.ts";
|
||||
import { normalizeThinkingForModel } from "@/shared/constants/modelSpecs.ts";
|
||||
@@ -2107,6 +2110,22 @@ export async function handleChatCore({
|
||||
log
|
||||
);
|
||||
|
||||
// GPT-5.x reasoning models on the raw openai Chat Completions surface reject function
|
||||
// `tools` combined with an active `reasoning_effort`: HTTP 400 "Function tools with
|
||||
// reasoning_effort are not supported ... Please use /v1/responses instead." This used to
|
||||
// be true for every GPT-5.x model on the plain `openai` provider, but #7242 (targetFormat
|
||||
// "openai-responses" on GPT_5_6_API_CAPABILITIES) now routes the GPT-5.6 family to
|
||||
// /v1/responses instead, which accepts tools + reasoning natively — so the strip must not
|
||||
// fire there. Pass the already-resolved `targetFormat` so the guard gates on the actual
|
||||
// upstream surface for this request instead of a model-name list. Port of 9router#2540.
|
||||
translatedBody = stripGpt5ReasoningWhenTools(
|
||||
translatedBody,
|
||||
provider,
|
||||
finalModelToUpstream,
|
||||
targetFormat,
|
||||
log
|
||||
);
|
||||
|
||||
// Rename max_tokens to max_completion_tokens if not supported (#1961)
|
||||
if (!supportsMaxTokens({ provider, model })) {
|
||||
if (translatedBody.max_tokens !== undefined) {
|
||||
|
||||
@@ -19,6 +19,8 @@
|
||||
* Azure Foundry reasoning matrix, openai-python#2072.
|
||||
*/
|
||||
|
||||
import { FORMATS } from "../translator/formats.ts";
|
||||
|
||||
type JsonRecord = Record<string, unknown>;
|
||||
|
||||
const SAMPLING_PARAMS = ["temperature", "top_p"] as const;
|
||||
@@ -79,3 +81,76 @@ export function stripGpt5SamplingWhenReasoning<T extends Record<string, unknown>
|
||||
);
|
||||
return next as T;
|
||||
}
|
||||
|
||||
const REASONING_FIELDS = ["reasoning_effort", "reasoning"] as const;
|
||||
|
||||
/**
|
||||
* True when the request carries a non-empty `tools` array holding at least one
|
||||
* function-shaped tool entry (`{type:"function", ...}` or a bare `{name, ...}`
|
||||
* without a `type`, the OpenAI Chat Completions convention).
|
||||
*/
|
||||
function hasFunctionTools(record: JsonRecord): boolean {
|
||||
if (!Array.isArray(record.tools) || record.tools.length === 0) return false;
|
||||
return record.tools.some((toolValue) => {
|
||||
const tool = asRecord(toolValue);
|
||||
if (!tool) return false;
|
||||
const toolType = typeof tool.type === "string" ? tool.type : "";
|
||||
return toolType === "" || toolType === "function";
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Raw api.openai.com Chat Completions rejects GPT-5.x reasoning models that
|
||||
* carry BOTH function `tools` and an active `reasoning_effort` with HTTP 400:
|
||||
* "Function tools with reasoning_effort are not supported for <model> in
|
||||
* /v1/chat/completions. Please use /v1/responses instead." Historically the
|
||||
* plain `openai` provider always stayed on `/chat/completions` for every
|
||||
* GPT-5.x model, so this combination reached the upstream 400 with no way to
|
||||
* recover other than dropping the reasoning fields.
|
||||
*
|
||||
* That is no longer true for every GPT-5.x model: the public GPT-5.6 family
|
||||
* is tagged with `targetFormat: "openai-responses"` (see
|
||||
* `GPT_5_6_API_CAPABILITIES` in `config/providers/shared.ts`, closes #2540 /
|
||||
* 9router#2547) and is routed to `/v1/responses` instead, which natively
|
||||
* accepts tools + reasoning together — /v1/responses is literally the
|
||||
* endpoint the 400 message tells callers to use. Gate on the resolved
|
||||
* `targetFormat` (the fact chatCore already computed for this request)
|
||||
* rather than a model-name list: only strip when the request is actually
|
||||
* going out over `/chat/completions`. If a future GPT-5.x family also moves
|
||||
* to `/responses`, this guard keeps working with no change needed here.
|
||||
* Port of 9router#2540.
|
||||
*/
|
||||
export function stripGpt5ReasoningWhenTools<T extends Record<string, unknown>>(
|
||||
body: T,
|
||||
provider: string | null | undefined,
|
||||
model: string | null | undefined,
|
||||
targetFormat: string | null | undefined,
|
||||
log?: { warn?: (tag: string, message: string) => void } | null
|
||||
): T {
|
||||
if (provider !== "openai") return body;
|
||||
if (typeof model !== "string" || !/^gpt-5/i.test(model)) return body;
|
||||
// Already routed to /v1/responses (e.g. GPT-5.6, #7242) — that endpoint
|
||||
// supports tools + reasoning natively, nothing to strip.
|
||||
if (targetFormat === FORMATS.OPENAI_RESPONSES) return body;
|
||||
|
||||
const record = asRecord(body);
|
||||
if (!record) return body;
|
||||
if (!hasFunctionTools(record)) return body;
|
||||
if (!hasActiveReasoning(record, model)) return body;
|
||||
|
||||
const stripped: string[] = [];
|
||||
for (const field of REASONING_FIELDS) {
|
||||
if (Object.hasOwn(record, field)) stripped.push(field);
|
||||
}
|
||||
if (stripped.length === 0) return body;
|
||||
|
||||
const next: JsonRecord = { ...record };
|
||||
for (const field of stripped) delete next[field];
|
||||
|
||||
log?.warn?.(
|
||||
"PARAMS",
|
||||
`Stripped ${stripped.join(", ")} for ${model} (function tools + reasoning_effort ` +
|
||||
`are rejected on /v1/chat/completions; use /v1/responses instead)`
|
||||
);
|
||||
return next as T;
|
||||
}
|
||||
|
||||
172
tests/unit/gpt5-tools-reasoning-guard.test.ts
Normal file
172
tests/unit/gpt5-tools-reasoning-guard.test.ts
Normal file
@@ -0,0 +1,172 @@
|
||||
/**
|
||||
* GPT-5 tools+reasoning guard — `stripGpt5ReasoningWhenTools`.
|
||||
*
|
||||
* On the raw `openai` Chat Completions surface, GPT-5.x reasoning models reject a
|
||||
* request that carries BOTH function tools and an active `reasoning_effort` with
|
||||
* HTTP 400: "Function tools with reasoning_effort are not supported for
|
||||
* <model> in /v1/chat/completions. Please use /v1/responses instead."
|
||||
* (port of 9router#2540). OmniRoute's `forceResponsesUpstream` guard only fires
|
||||
* for `openai-compatible-*` connections carrying MCP/tool_search tool shapes —
|
||||
* the plain `openai` provider has no equivalent guard, so this scenario still
|
||||
* reaches the upstream 400 today. Strip `reasoning_effort`/`reasoning` when
|
||||
* function tools are present so the request succeeds on /v1/chat/completions.
|
||||
*
|
||||
* The guard is passed the request's already-resolved `targetFormat` (chatCore
|
||||
* resolves it once via `resolveChatCoreTargetFormat` before this guard runs) so it
|
||||
* gates on the actual upstream surface for THIS request rather than a model-name
|
||||
* list. This matters because #7242 (closes #2540 upstream / 9router#2547) tags the
|
||||
* public GPT-5.6 family with `targetFormat: "openai-responses"` and routes it to
|
||||
* `/v1/responses` instead — an endpoint that accepts tools + reasoning natively —
|
||||
* so stripping must NOT fire for GPT-5.6 requests once that routing is in effect.
|
||||
* Without this composition, #7101's strip and #7242's reroute would combine into
|
||||
* the worst of both worlds: routed to the endpoint that supports reasoning, but
|
||||
* with reasoning silently dropped anyway.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { stripGpt5ReasoningWhenTools } from "../../open-sse/services/gpt5SamplingGuard.ts";
|
||||
|
||||
// Chat Completions models in this suite use gpt-5.4/gpt-5.5 (targetFormat "openai"),
|
||||
// which stay on /chat/completions and must keep being stripped. gpt-5.6-sol is reserved
|
||||
// for the /v1/responses composition cases below, where stripping must NOT happen.
|
||||
|
||||
test("strips reasoning_effort for openai gpt-5.x on /chat/completions when function tools are present", () => {
|
||||
const body = {
|
||||
model: "gpt-5.4-sol",
|
||||
reasoning_effort: "high",
|
||||
tools: [{ type: "function", function: { name: "read_file" } }],
|
||||
messages: [],
|
||||
};
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", "gpt-5.4-sol", "openai");
|
||||
assert.equal(result.reasoning_effort, undefined);
|
||||
});
|
||||
|
||||
test("strips nested reasoning.effort for openai gpt-5.x on /chat/completions when function tools are present", () => {
|
||||
const body = {
|
||||
model: "gpt-5.4-sol",
|
||||
reasoning: { effort: "medium" },
|
||||
tools: [{ type: "function", function: { name: "read_file" } }],
|
||||
};
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", "gpt-5.4-sol", "openai");
|
||||
assert.equal(result.reasoning, undefined);
|
||||
});
|
||||
|
||||
test("keeps reasoning_effort=none untouched (already non-reasoning mode)", () => {
|
||||
const body = {
|
||||
model: "gpt-5.4-sol",
|
||||
reasoning_effort: "none",
|
||||
tools: [{ type: "function", function: { name: "read_file" } }],
|
||||
};
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", "gpt-5.4-sol", "openai");
|
||||
assert.equal(result.reasoning_effort, "none");
|
||||
});
|
||||
|
||||
test("keeps reasoning_effort when there are no tools", () => {
|
||||
const body = { model: "gpt-5.4-sol", reasoning_effort: "high", messages: [] };
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", "gpt-5.4-sol", "openai");
|
||||
assert.equal(result.reasoning_effort, "high");
|
||||
});
|
||||
|
||||
test("keeps reasoning_effort when tools array is empty", () => {
|
||||
const body = { model: "gpt-5.4-sol", reasoning_effort: "high", tools: [] };
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", "gpt-5.4-sol", "openai");
|
||||
assert.equal(result.reasoning_effort, "high");
|
||||
});
|
||||
|
||||
test("non-openai provider is untouched", () => {
|
||||
const body = {
|
||||
model: "gpt-5.4-sol",
|
||||
reasoning_effort: "high",
|
||||
tools: [{ type: "function", function: { name: "x" } }],
|
||||
};
|
||||
const result = stripGpt5ReasoningWhenTools(body, "codex", "gpt-5.4-sol", "openai");
|
||||
assert.equal(result.reasoning_effort, "high");
|
||||
});
|
||||
|
||||
test("non-gpt-5 openai model is untouched", () => {
|
||||
const body = {
|
||||
model: "gpt-4o",
|
||||
reasoning_effort: "high",
|
||||
tools: [{ type: "function", function: { name: "x" } }],
|
||||
};
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", "gpt-4o", "openai");
|
||||
assert.equal(result.reasoning_effort, "high");
|
||||
});
|
||||
|
||||
test("returns the same reference when nothing to strip", () => {
|
||||
const body = { model: "gpt-5.4-sol", tools: [{ type: "function" }], messages: [] };
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", "gpt-5.4-sol", "openai");
|
||||
assert.equal(result, body);
|
||||
});
|
||||
|
||||
test("logs the stripped fields when a logger is provided", () => {
|
||||
const calls: Array<[string, string]> = [];
|
||||
const log = { warn: (tag: string, message: string) => calls.push([tag, message]) };
|
||||
stripGpt5ReasoningWhenTools(
|
||||
{
|
||||
model: "gpt-5.4-sol",
|
||||
reasoning_effort: "high",
|
||||
tools: [{ type: "function", function: { name: "x" } }],
|
||||
},
|
||||
"openai",
|
||||
"gpt-5.4-sol",
|
||||
"openai",
|
||||
log
|
||||
);
|
||||
assert.equal(calls.length, 1);
|
||||
assert.equal(calls[0][0], "PARAMS");
|
||||
assert.match(calls[0][1], /reasoning_effort/);
|
||||
});
|
||||
|
||||
// --- Composition with #7242 (GPT-5.6 → /v1/responses) ---
|
||||
//
|
||||
// #7242 tags the public GPT-5.6 family with targetFormat "openai-responses" so it is
|
||||
// routed to /v1/responses, which natively supports tools + reasoning together. If this
|
||||
// guard ignored targetFormat and only looked at provider+model-name (the pre-#7242
|
||||
// shape), a GPT-5.6 request with tools + reasoning_effort would still get its reasoning
|
||||
// silently stripped even though it is no longer going to /chat/completions — the worst
|
||||
// of both worlds. These cases prove the composition holds.
|
||||
|
||||
test("gpt-5.6 routed to /v1/responses (targetFormat openai-responses) keeps reasoning_effort", () => {
|
||||
const body = {
|
||||
model: "gpt-5.6-sol",
|
||||
reasoning_effort: "high",
|
||||
tools: [{ type: "function", function: { name: "read_file" } }],
|
||||
messages: [],
|
||||
};
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", "gpt-5.6-sol", "openai-responses");
|
||||
assert.equal(result.reasoning_effort, "high");
|
||||
assert.equal(result, body, "no-op path should return the same reference");
|
||||
});
|
||||
|
||||
test("gpt-5.6 routed to /v1/responses keeps nested reasoning.effort too", () => {
|
||||
const body = {
|
||||
model: "gpt-5.6-sol",
|
||||
reasoning: { effort: "medium" },
|
||||
tools: [{ type: "function", function: { name: "read_file" } }],
|
||||
};
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", "gpt-5.6-sol", "openai-responses");
|
||||
assert.deepEqual(result.reasoning, { effort: "medium" });
|
||||
});
|
||||
|
||||
test("gpt-5.4/gpt-5.5 stay on /chat/completions (targetFormat openai) and keep stripping", () => {
|
||||
for (const model of ["gpt-5.4-sol", "gpt-5.5-pro"]) {
|
||||
const body = {
|
||||
model,
|
||||
reasoning_effort: "high",
|
||||
tools: [{ type: "function", function: { name: "read_file" } }],
|
||||
};
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", model, "openai");
|
||||
assert.equal(result.reasoning_effort, undefined, `${model} should still be stripped`);
|
||||
}
|
||||
});
|
||||
|
||||
test("if gpt-5.6 were ever NOT routed to /v1/responses, the strip would still apply (defense in depth)", () => {
|
||||
const body = {
|
||||
model: "gpt-5.6-sol",
|
||||
reasoning_effort: "high",
|
||||
tools: [{ type: "function", function: { name: "read_file" } }],
|
||||
};
|
||||
const result = stripGpt5ReasoningWhenTools(body, "openai", "gpt-5.6-sol", "openai");
|
||||
assert.equal(result.reasoning_effort, undefined);
|
||||
});
|
||||
Reference in New Issue
Block a user