From 607ad12e842cd021f9cb0a1107d4b2fd280eed41 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 23 Jun 2026 21:56:07 -0300 Subject: [PATCH] fix(executors): strip params unsupported by the target provider/model (#4658) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integrated into release/v3.8.36 — port rebuilt clean over release tip, release-green validated --- open-sse/executors/default.ts | 12 ++ open-sse/executors/github.ts | 8 ++ open-sse/translator/paramSupport.ts | 60 +++++++++ ...executors-strip-unsupported-params.test.ts | 121 ++++++++++++++++++ 4 files changed, 201 insertions(+) create mode 100644 open-sse/translator/paramSupport.ts create mode 100644 tests/unit/executors-strip-unsupported-params.test.ts diff --git a/open-sse/executors/default.ts b/open-sse/executors/default.ts index 034477ec48..0eceec4530 100644 --- a/open-sse/executors/default.ts +++ b/open-sse/executors/default.ts @@ -16,6 +16,7 @@ import { getGigachatAccessToken } from "../services/gigachatAuth.ts"; import { getRegistryEntry } from "../config/providerRegistry.ts"; import { mergeClientAnthropicBeta } from "../config/anthropicHeaders.ts"; import { applyProviderRequestDefaults } from "../services/providerRequestDefaults.ts"; +import { stripUnsupportedParams } from "../translator/paramSupport.ts"; import { detectFormat, getOpenAICompatibleType, @@ -651,6 +652,17 @@ export class DefaultExecutor extends BaseExecutor { ); } + // Config-driven strip of params unsupported by the target provider/model + // (e.g. claude-opus-4 deprecated `temperature` → Anthropic 400). Port from + // 9router#7ae9fff6 (fixes upstream #1748). Rules live in + // ../translator/paramSupport.ts so adding one means editing one table. + if (typeof withDefaults === "object" && withDefaults !== null) { + const bodyRecord = withDefaults as Record; + const outboundModel = + typeof bodyRecord.model === "string" ? bodyRecord.model : model; + stripUnsupportedParams(this.provider, outboundModel, bodyRecord); + } + // Apply modelIdPrefix from RegistryEntry (e.g. "accounts/fireworks/models/") // so registry can store short model IDs while the upstream API receives the full path. if (typeof withDefaults === "object" && withDefaults !== null) { diff --git a/open-sse/executors/github.ts b/open-sse/executors/github.ts index eadc46d90e..f4ab762f80 100644 --- a/open-sse/executors/github.ts +++ b/open-sse/executors/github.ts @@ -6,6 +6,7 @@ import { getGitHubCopilotRefreshHeaders, } from "../config/providerHeaderProfiles.ts"; import { sanitizeResponsesInputItems } from "../services/responsesInputSanitizer.ts"; +import { stripUnsupportedParams } from "../translator/paramSupport.ts"; export class GithubExecutor extends BaseExecutor { constructor() { @@ -126,6 +127,13 @@ export class GithubExecutor extends BaseExecutor { ); } + // Config-driven strip of params unsupported by the target provider/model. + // For GitHub Copilot this removes Claude-style `thinking` and + // `reasoning_effort` for Claude models that reject them upstream + // (Haiku 4.5 / Opus 4.7 — Opus 4.6 / Sonnet 4.6 keep them). + // Port from 9router#7ae9fff6 (fixes upstream #1748, #713). + stripUnsupportedParams("github", model, modifiedBody); + return modifiedBody; } diff --git a/open-sse/translator/paramSupport.ts b/open-sse/translator/paramSupport.ts new file mode 100644 index 0000000000..142f5528bb --- /dev/null +++ b/open-sse/translator/paramSupport.ts @@ -0,0 +1,60 @@ +// Strip request params a given provider/model rejects upstream (e.g. HTTP 400). +// Config-driven: add a rule here instead of scattering `delete body.x` across +// executors. Port from 9router#7ae9fff6 (fixes upstream #1748). +// +// Rule semantics: +// - `provider` (optional) limits the rule to a single provider id. +// - `match` is a RegExp tested against the model id OR a predicate (model -> boolean). +// - `drop` is the list of param keys to remove when the rule fires. +// +// A param is removed only when it is present (!== undefined). The helper never +// introduces new keys and never throws on null/undefined bodies — call sites +// can chain it without extra guards. + +type StripRule = { + provider?: string; + match: RegExp | ((model: string) => boolean); + drop: string[]; +}; + +const STRIP_RULES: StripRule[] = [ + // claude-opus-4 series: temperature is deprecated (Anthropic returns 400). #1748 + { match: /claude-opus-4/i, drop: ["temperature"] }, + // GitHub Copilot gpt-5.4: temperature unsupported. + { provider: "github", match: /gpt-5\.4/i, drop: ["temperature"] }, + // GitHub Copilot Claude (except opus/sonnet 4.6): thinking + reasoning_effort rejected. #713 + { + provider: "github", + match: (m: string) => + /claude/i.test(m) && !/claude.*(opus|sonnet).*4\.6/i.test(m), + drop: ["thinking", "reasoning_effort"], + }, +]; + +function matches(rule: StripRule, model: string): boolean { + return typeof rule.match === "function" ? rule.match(model) : rule.match.test(model); +} + +/** + * Remove unsupported params from `body` in place. Returns the same reference + * (or `body` unchanged when it is not a plain object / model is empty). + */ +export function stripUnsupportedParams( + provider: string | null | undefined, + model: string | null | undefined, + body: T +): T { + if (!model || !body || typeof body !== "object") return body; + const rec = body as unknown as Record; + for (const rule of STRIP_RULES) { + if (rule.provider && rule.provider !== provider) continue; + if (!matches(rule, model)) continue; + for (const key of rule.drop) { + if (rec[key] !== undefined) delete rec[key]; + } + } + return body; +} + +// Exported for unit tests only — do not import from production code. +export const __STRIP_RULES_FOR_TEST: ReadonlyArray = STRIP_RULES; diff --git a/tests/unit/executors-strip-unsupported-params.test.ts b/tests/unit/executors-strip-unsupported-params.test.ts new file mode 100644 index 0000000000..d1896408c4 --- /dev/null +++ b/tests/unit/executors-strip-unsupported-params.test.ts @@ -0,0 +1,121 @@ +// Unit tests for stripUnsupportedParams: per-(provider,model) strip of params +// the upstream rejects (HTTP 400). Port from 9router#7ae9fff6 (fixes #1748). +// +// Rule coverage: +// 1. claude-opus-4 series: temperature deprecated → Anthropic 400. +// 2. github + gpt-5.4: temperature unsupported. +// 3. github + Claude (except opus/sonnet 4.6): thinking + reasoning_effort rejected. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + stripUnsupportedParams, + __STRIP_RULES_FOR_TEST, +} from "../../open-sse/translator/paramSupport.ts"; + +test("stripUnsupportedParams: drops temperature for claude-opus-4 models (any provider)", () => { + const body = { model: "claude-opus-4-20250514", temperature: 0.7, max_tokens: 100 }; + stripUnsupportedParams("anthropic", "claude-opus-4-20250514", body); + assert.equal(body.temperature, undefined, "temperature must be stripped"); + assert.equal(body.max_tokens, 100, "other params must survive"); + assert.equal(body.model, "claude-opus-4-20250514", "model must not be touched"); +}); + +test("stripUnsupportedParams: drops temperature for claude-opus-4-1 (case insensitive)", () => { + const body: Record = { temperature: 0.5 }; + stripUnsupportedParams("anthropic", "CLAUDE-OPUS-4-1-20250805", body); + assert.equal(body.temperature, undefined); +}); + +test("stripUnsupportedParams: keeps temperature for claude-sonnet-4 (only opus-4 affected)", () => { + const body: Record = { temperature: 0.7 }; + stripUnsupportedParams("anthropic", "claude-sonnet-4-20250514", body); + assert.equal(body.temperature, 0.7, "sonnet-4 still accepts temperature"); +}); + +test("stripUnsupportedParams: keeps temperature for claude-opus-3 (regression guard)", () => { + const body: Record = { temperature: 0.7 }; + stripUnsupportedParams("anthropic", "claude-3-opus-20240229", body); + assert.equal(body.temperature, 0.7); +}); + +test("stripUnsupportedParams: github + gpt-5.4 strips temperature", () => { + const body: Record = { temperature: 1, max_completion_tokens: 200 }; + stripUnsupportedParams("github", "gpt-5.4", body); + assert.equal(body.temperature, undefined); + assert.equal(body.max_completion_tokens, 200); +}); + +test("stripUnsupportedParams: github + gpt-5 (non-5.4) keeps temperature", () => { + const body: Record = { temperature: 1 }; + stripUnsupportedParams("github", "gpt-5", body); + assert.equal(body.temperature, 1); +}); + +test("stripUnsupportedParams: github + Claude strips thinking + reasoning_effort", () => { + const body: Record = { + thinking: { type: "enabled" }, + reasoning_effort: "high", + temperature: 0.5, + }; + stripUnsupportedParams("github", "claude-3-5-sonnet", body); + assert.equal(body.thinking, undefined, "Copilot rejects Claude-style thinking"); + assert.equal(body.reasoning_effort, undefined); + assert.equal(body.temperature, 0.5, "non-targeted params survive"); +}); + +test("stripUnsupportedParams: github + claude opus 4.6 KEEPS thinking + reasoning_effort", () => { + const body: Record = { + thinking: { type: "enabled" }, + reasoning_effort: "high", + }; + stripUnsupportedParams("github", "claude-opus-4.6", body); + assert.equal(body.reasoning_effort, "high", "opus-4.6 supports reasoning_effort"); + // Note: thinking survives too on opus-4.6 per upstream rule. + assert.deepEqual(body.thinking, { type: "enabled" }); +}); + +test("stripUnsupportedParams: github + claude sonnet 4.6 KEEPS reasoning_effort", () => { + const body: Record = { reasoning_effort: "low" }; + stripUnsupportedParams("github", "claude-sonnet-4.6", body); + assert.equal(body.reasoning_effort, "low"); +}); + +test("stripUnsupportedParams: non-github provider + Claude does NOT strip thinking", () => { + // The github-Claude rule is provider-scoped. + const body: Record = { thinking: { type: "enabled" } }; + stripUnsupportedParams("anthropic", "claude-3-5-sonnet", body); + assert.deepEqual(body.thinking, { type: "enabled" }); +}); + +test("stripUnsupportedParams: only deletes when the key is present (never adds undefined)", () => { + const body: Record = { max_tokens: 50 }; + stripUnsupportedParams("anthropic", "claude-opus-4", body); + // No `temperature` key should be introduced. + assert.equal("temperature" in body, false); +}); + +test("stripUnsupportedParams: returns the same body reference (in-place mutation)", () => { + const body: Record = { temperature: 0.7 }; + const out = stripUnsupportedParams("anthropic", "claude-opus-4", body); + assert.equal(out, body); +}); + +test("stripUnsupportedParams: null/undefined body is a no-op", () => { + assert.equal(stripUnsupportedParams("anthropic", "claude-opus-4", null), null); + assert.equal(stripUnsupportedParams("anthropic", "claude-opus-4", undefined), undefined); +}); + +test("stripUnsupportedParams: missing model is a no-op", () => { + const body: Record = { temperature: 0.7 }; + stripUnsupportedParams("anthropic", "", body); + assert.equal(body.temperature, 0.7); +}); + +test("STRIP_RULES is non-empty and every rule has a drop list", () => { + assert.ok(__STRIP_RULES_FOR_TEST.length > 0); + for (const rule of __STRIP_RULES_FOR_TEST) { + assert.ok(Array.isArray(rule.drop) && rule.drop.length > 0); + assert.ok(typeof rule.match === "function" || rule.match instanceof RegExp); + } +});