Skip to content
Merged
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
79 changes: 69 additions & 10 deletions src/adapters/anthropic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,16 +408,68 @@ const ADAPTIVE_THINKING_FAMILY_MINIMUMS: Record<string, readonly [major: number,
fable: [0, 0],
};

function usesAdaptiveThinking(modelId: string): boolean {
// Minor is 1-2 digits with a non-digit lookahead so date-pinned ids ("claude-opus-4-20250514")
// parse as minor 0 instead of minor 20250514; suffixed ids ("claude-opus-4-8[1m]") still match.
const match = /^claude-([a-z]+)-(\d+)(?:-(\d{1,2}))?(?!\d)/.exec(modelId);
if (!match) return false;
const minimum = ADAPTIVE_THINKING_FAMILY_MINIMUMS[match[1]];
/**
* Family/version parse for a Claude model id, tolerant of a routing prefix.
*
* `parsed.modelId` is not always bare, and the slash can fall on either side.
* A `modelMap` entry may point at a routed destination such as
* `anthropic/claude-sonnet-5` (prefix), while a custom provider may expose a
* native id such as `claude-sonnet-5/variant` (suffix); both survive routing's
* known-id decoding. So this matches the segment that actually begins with
* `claude-` rather than assuming it is the first or the last one. A capability
* predicate that quietly returns false is worse than one that throws — the
* request just goes out wrong.
*
* Minor is 1-2 digits with a non-digit lookahead so date-pinned ids
* ("claude-opus-4-20250514") parse as minor 0 instead of minor 20250514;
* suffixed ids ("claude-opus-4-8[1m]") still match.
*/
function claudeFamilyVersion(modelId: string): { family: string; major: number; minor: number } | undefined {
// Find the segment that actually starts with `claude-`, rather than assuming it is either
// the first (breaks `anthropic/claude-sonnet-5`) or the last (breaks `claude-sonnet-5/variant`,
// where the slash carries a vendor suffix rather than a routing prefix).
const match = /(?:^|\/)claude-([a-z]+)-(\d+)(?:-(\d{1,2}))?(?!\d)/.exec(modelId);
if (!match) return undefined;
return {
family: match[1]!,
major: Number(match[2]),
minor: match[3] === undefined ? 0 : Number(match[3]),
};
Comment on lines +427 to +437

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Parse dot-separated Claude minor versions.

Line 431 parses anthropic/claude-sonnet-4.5 as version 4.0. The optional minor group only accepts -5, and the negative lookahead permits the . after 4. This makes meetsFamilyMinimum() evaluate the wrong version for model IDs already used in tests/anthropic-reasoning.test.ts.

Accept both - and . before the minor version. Add a dot-separated ID to the capability regression cases.

Proposed fix
-  const match = /(?:^|\/)claude-([a-z]+)-(\d+)(?:-(\d{1,2}))?(?!\d)/.exec(modelId);
+  const match = /(?:^|\/)claude-([a-z]+)-(\d+)(?:[-.](\d{1,2}))?(?!\d)/.exec(modelId);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function claudeFamilyVersion(modelId: string): { family: string; major: number; minor: number } | undefined {
// Find the segment that actually starts with `claude-`, rather than assuming it is either
// the first (breaks `anthropic/claude-sonnet-5`) or the last (breaks `claude-sonnet-5/variant`,
// where the slash carries a vendor suffix rather than a routing prefix).
const match = /(?:^|\/)claude-([a-z]+)-(\d+)(?:-(\d{1,2}))?(?!\d)/.exec(modelId);
if (!match) return undefined;
return {
family: match[1]!,
major: Number(match[2]),
minor: match[3] === undefined ? 0 : Number(match[3]),
};
function claudeFamilyVersion(modelId: string): { family: string; major: number; minor: number } | undefined {
// Find the segment that actually starts with `claude-`, rather than assuming it is either
// the first (breaks `anthropic/claude-sonnet-5`) or the last (breaks `claude-sonnet-5/variant`,
// where the slash carries a vendor suffix rather than a routing prefix).
const match = /(?:^|\/)claude-([a-z]+)-(\d+)(?:[-.](\d{1,2}))?(?!\d)/.exec(modelId);
if (!match) return undefined;
return {
family: match[1]!,
major: Number(match[2]),
minor: match[3] === undefined ? 0 : Number(match[3]),
};
🧰 Tools
🪛 OpenGrep (1.26.0)

[ERROR] 431-431: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.

(coderabbit.command-injection.exec-js)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/adapters/anthropic.ts` around lines 427 - 437, Update claudeFamilyVersion
to accept either a hyphen or dot separator before the optional minor version, so
IDs such as claude-sonnet-4.5 produce minor 5 while preserving existing
hyphenated parsing. Add a dot-separated model ID regression case to the
capability tests covering meetsFamilyMinimum().

}

function meetsFamilyMinimum(
modelId: string,
minimums: Record<string, readonly [major: number, minor: number]>,
): boolean {
const parsed = claudeFamilyVersion(modelId);
if (!parsed) return false;
const minimum = minimums[parsed.family];
if (!minimum) return false;
const major = Number(match[2]);
const minor = match[3] === undefined ? 0 : Number(match[3]);
return major > minimum[0] || (major === minimum[0] && minor >= minimum[1]);
return parsed.major > minimum[0] || (parsed.major === minimum[0] && parsed.minor >= minimum[1]);
}

function usesAdaptiveThinking(modelId: string): boolean {
return meetsFamilyMinimum(modelId, ADAPTIVE_THINKING_FAMILY_MINIMUMS);
}

/**
* Claude families that (a) think by DEFAULT when the request omits `thinking`,
* and (b) accept an explicit `thinking: {type: "disabled"}` to turn it off.
*
* Deliberately NOT `usesAdaptiveThinking()`, which answers a different question
* (which wire shape a family accepts). The two sets differ in both directions:
* Fable always thinks and REJECTS an explicit disable, while Opus 4.7/4.8 use
* the adaptive wire but leave thinking off when the field is omitted, so they
* need no disable at all. Seeded with the family where the defect reproduces
* (#545); widen only with vendor evidence, since a wrong entry here turns a
* silent truncation into a 400.
*/
const EXPLICIT_THINKING_DISABLE_FAMILY_MINIMUMS: Record<string, readonly [major: number, minor: number]> = {
sonnet: [5, 0],
};

function supportsExplicitThinkingDisable(modelId: string): boolean {
return meetsFamilyMinimum(modelId, EXPLICIT_THINKING_DISABLE_FAMILY_MINIMUMS);
}

/** `output_config.effort` accepts low|medium|high|xhigh|max — "minimal" is rejected with a 400. */
Expand Down Expand Up @@ -766,7 +818,14 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
// `reasoning` is a Codex effort string; "none" is the disable sentinel (see parser.ts
// REASONING_EFFORTS). A bare truthy check would treat "none" as truthy and wrongly enable
// extended thinking (and strip temperature/top_p), so gate on a real, non-disable effort.
if (typeof parsed.options.reasoning === "string" && parsed.options.reasoning !== "none") {
//
// "none" is not the same as absent. Omitting `thinking` lets a default-on model think
// anyway, and thinking shares the caller's `max_tokens` — which truncates a small-budget
// request before it can emit its stop sequence (#545). Say "disabled" out loud where the
// model both defaults to thinking and accepts being told not to.
if (parsed.options.reasoning === "none" && supportsExplicitThinkingDisable(parsed.modelId)) {
body.thinking = { type: "disabled" };
} else if (typeof parsed.options.reasoning === "string" && parsed.options.reasoning !== "none") {
if (usesAdaptiveThinking(parsed.modelId)) {
// Adaptive-thinking models replace the token budget with an effort knob and reject
// `thinking.type: "enabled"` outright. `max_tokens` still caps thinking plus visible
Expand Down
8 changes: 7 additions & 1 deletion src/claude/inbound.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,13 @@ export function anthropicToResponsesTranslation(raw: unknown, cc?: OcxClaudeCode
const thinking = raw.thinking;
const outputConfigEffort = effortFromOutputConfig(raw.output_config);
const thinkingDisabled = isRec(thinking) && thinking.type === "disabled";
if (!thinkingDisabled && (isRec(thinking) || outputConfigEffort !== undefined)) {
if (thinkingDisabled) {
// An explicit "disabled" is an instruction, not an absence. Dropping it made this
// indistinguishable from a request that never mentioned thinking — and for models that
// think by default, omission means thinking is ON, sharing the caller's max_tokens (#545).
// "none" is the parser's disable sentinel (parser.ts REASONING_EFFORTS).
body.reasoning = { effort: "none", summary: "none" };
} else if (isRec(thinking) || outputConfigEffort !== undefined) {
const reasoning: Rec = { summary: "auto" };
if (outputConfigEffort !== undefined) {
// Adaptive wire: /effort arrives as output_config.effort (devlog 080).
Expand Down
85 changes: 85 additions & 0 deletions tests/anthropic-reasoning.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { describe, expect, test } from "bun:test";
import { createAnthropicAdapter as createAnthropicAdapterProduction } from "../src/adapters/anthropic";
import { parseRequest } from "../src/responses/parser";
import { anthropicToResponsesBody } from "../src/claude/inbound";
import type { OcxParsedRequest, OcxProviderConfig } from "../src/types";
import { withTestTranslatorBudget } from "./helpers/translator-budget";

Expand Down Expand Up @@ -136,13 +137,74 @@ describe("anthropic extended-thinking gate", () => {
expect(b.output_config).toBeUndefined();
});

// The adaptive-wire predicate shares the id parse with the #545 disable gate, so a
// slash-carrying id must still pick the ADAPTIVE shape. Getting this wrong sends obsolete
// manual `thinking.enabled` to a model that rejects it — a 400, not a silent truncation.
test.each([
"anthropic/claude-sonnet-5",
"claude-sonnet-5/variant",
"claude-opus-4-8/vendor-suffix",
])("adaptive-thinking model %s keeps the adaptive wire shape", async (modelId) => {
const b = await bodyOf(parsed("high", {}, modelId));
expect(b.thinking).toEqual({ type: "adaptive" });
expect(b.output_config).toEqual({ effort: "high" });
});

test("adaptive-thinking model with reasoning 'none' sends no thinking config", async () => {
const b = await bodyOf(parsed("none", { temperature: 0.3 }, "claude-fable-5"));
expect(b.thinking).toBeUndefined();
expect(b.output_config).toBeUndefined();
expect(b.temperature).toBe(0.3);
});

// #545: Claude Desktop's Auto Mode classifier sends thinking:{type:"disabled"} with
// max_tokens:64. Omitting the field lets a default-on model think anyway, and thinking
// shares that 64-token budget — so generation stopped before the stop sequence and the
// client retried. Say "disabled" out loud, but only where the vendor accepts it.
test.each([
"claude-sonnet-5",
"claude-sonnet-5-20260101",
"claude-sonnet-5[1m]",
// A modelMap entry can point at a routed destination, which custom-provider routing
// decodes back into a slash-carrying native id. An id-shape miss here is silent: the
// request simply goes out without the disable and the model thinks anyway.
"anthropic/claude-sonnet-5",
"openrouter/anthropic/claude-sonnet-5",
// The slash can also carry a vendor SUFFIX rather than a routing prefix, so the family
// segment is not reliably first or last. Both directions are real routed shapes.
"claude-sonnet-5/variant",
])("%s + reasoning 'none' sends an explicit thinking disable (#545)", async (modelId) => {
const b = await bodyOf(parsed("none", { maxOutputTokens: 64, stopSequences: ["</block>"] }, modelId));
expect(b.thinking).toEqual({ type: "disabled" });
expect(b.output_config).toBeUndefined();
// The caller's own limits must survive untouched — they were never the defect.
expect(b.max_tokens).toBe(64);
expect(b.stop_sequences).toEqual(["</block>"]);
});

test("Sonnet 5 with reasoning OMITTED still omits thinking (#545)", async () => {
// Absence is not a disable instruction: only an explicit "none" earns the explicit field.
const b = await bodyOf(parsed(undefined, {}, "claude-sonnet-5"));
expect(b.thinking).toBeUndefined();
});

test.each([
"claude-fable-5",
"claude-opus-4-7",
"claude-opus-4-8",
"claude-haiku-4-5",
"claude-sonnet-4-6",
"anthropic/claude-fable-5",
"claude-fable-5/foo",
"not-a-claude-model",
])("%s + 'none' sends NO explicit disable (#545 gate stays narrow)", async (modelId) => {
// Fable always thinks and rejects an explicit disable; the Opus 4.7/4.8 adaptive wire
// leaves thinking off when omitted. Widening the gate to every adaptive family would
// trade a silent truncation for a 400.
const b = await bodyOf(parsed("none", {}, modelId));
expect(b.thinking).toBeUndefined();
});

test("drops reconstructed Responses reasoning signatures when switching into Anthropic", async () => {
const b = await bodyOf(parseRequest({
model: "anthropic/claude-sonnet-4.5",
Expand All @@ -169,3 +231,26 @@ describe("anthropic extended-thinking gate", () => {
expect(messages).toEqual([{ role: "user", content: "continue on anthropic" }]);
});
});

describe("Claude Desktop classifier round trip (#545)", () => {
test("thinking:disabled survives inbound translation to the outbound Anthropic body", async () => {
// The reporter's exact shape: a permission classifier with a 64-token budget that must
// close its XML tag. Before the fix, "disabled" was dropped at the inbound hop and the
// outbound request omitted `thinking` entirely, so Sonnet 5 thought anyway and spent the
// budget before emitting </block>. Claude Code then retried, up to five times.
const inbound = anthropicToResponsesBody({
model: "claude-sonnet-5",
max_tokens: 64,
stop_sequences: ["</block>"],
thinking: { type: "disabled" },
system: "decide whether this tool call is allowed",
messages: [{ role: "user", content: "<request>ls</request>" }],
});

const body = await bodyOf(parseRequest(inbound));

expect(body.thinking).toEqual({ type: "disabled" });
expect(body.max_tokens).toBe(64);
expect(body.stop_sequences).toEqual(["</block>"]);
});
});
11 changes: 8 additions & 3 deletions tests/claude-inbound.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ describe("claude inbound translation", () => {
test("thinking variants", () => {
const base = { model: "m", max_tokens: 10, messages: [{ role: "user", content: "hi" }] };
expect((anthropicToResponsesBody({ ...base, thinking: { type: "adaptive" } }) as any).reasoning).toEqual({ summary: "auto" });
expect((anthropicToResponsesBody({ ...base, thinking: { type: "disabled" } }) as any).reasoning).toBeUndefined();
// "disabled" and omitted must NOT collapse to the same state: for a model that thinks by
// default, omission means thinking is ON and shares the caller's max_tokens (#545).
expect((anthropicToResponsesBody({ ...base, thinking: { type: "disabled" } }) as any).reasoning).toEqual({ effort: "none", summary: "none" }); // justified: sibling assertions in this test use the same cast
expect((anthropicToResponsesBody(base) as any).reasoning).toBeUndefined();
expect(effortForThinkingBudget(1024)).toBe("low");
expect(effortForThinkingBudget(8192)).toBe("medium");
Expand Down Expand Up @@ -126,10 +128,13 @@ describe("claude inbound translation", () => {
thinking: { type: "enabled", budget_tokens: 1024 },
output_config: { effort: "xhigh" },
}))).toEqual({ summary: "auto", effort: "xhigh" });
// disabled thinking suppresses effort entirely (subagent wire, claude-code#65863)
// disabled thinking suppresses effort entirely (subagent wire, claude-code#65863).
// Still suppressed — "high" never reaches the wire — but now stated explicitly as the
// "none" disable sentinel instead of by absence, so a default-on model is told to stop
// rather than left to think anyway (#545).
expect(reasoningOf(anthropicToResponsesBody({
...base, thinking: { type: "disabled" }, output_config: { effort: "high" },
}))).toBeUndefined();
}))).toEqual({ effort: "none", summary: "none" });
// unknown effort strings are dropped so downstream defaults win
expect(reasoningOf(anthropicToResponsesBody({
...base, thinking: { type: "adaptive" }, output_config: { effort: "turbo" },
Expand Down
12 changes: 12 additions & 0 deletions tests/cursor-effort-suffix.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ describe("Cursor per-model reasoning-effort suffix", () => {
expect(modelIdFor("cursor/claude-4.6-opus")).toBe("claude-4.6-opus-max");
});

// #545 made Claude Desktop's `thinking:{type:"disabled"}` survive translation as the "none"
// sentinel instead of being dropped. For a modelMap that routes such a request to Cursor,
// that changes the selected tier — pin it so the cross-provider effect is deliberate.
//
// Cursor has no "off" for a reasoning model, so the lowest tier is the closest honest
// reading of "do not think". Dropping the instruction sent these to the model's TOP tier,
// which is the opposite of what the caller asked for.
test("an explicit 'none' picks the lowest tier, not the top one (#545)", () => {
expect(modelIdFor("cursor/claude-opus-4-8", "none")).toBe("claude-opus-4-8-low");
expect(modelIdFor("cursor/claude-opus-4-8")).toBe("claude-opus-4-8-max");
});

test("single-tier models always use their one tier", () => {
expect(modelIdFor("cursor/gpt-5.5-extra", "low")).toBe("gpt-5.5-extra-high");
expect(modelIdFor("cursor/claude-4.6-sonnet", "high")).toBe("claude-4.6-sonnet-medium");
Expand Down
Loading