fix(routing): normalize thinking:disabled for combo-substituted models that reject it (#3554)

[diegosouzapw]

Closes #3554

Problem: When a combo/route substitutes the upstream model after the client already chose its thinking value (e.g. combo claude-opus-4-8 → claude-fable-5), OmniRoute forwarded thinking:{type:"disabled"} verbatim. Claude Code sends disabled for internal title/name-generation calls; claude-fable-5 defaults to adaptive thinking and rejects it with an upstream 400 (26 confirmed in a 575-request production sample, per the reporter).

Fix: Added a per-model rejectsThinkingDisabled capability flag (set on claude-fable-5) and an exported pure helper normalizeThinkingForModel(body, modelId). In chatCore.ts, right after the upstream model is finalized (post combo/alias resolution, common to both the Claude passthrough and translated paths), thinking.type:"disabled" is stripped when the resolved target rejects it. Models that accept disabled (opus/sonnet) and any non-disabled thinking (enabled/adaptive) are untouched.

Test (Rule #18): tests/unit/combo-thinking-disabled-fable5-3554.test.ts (7 tests) — RED before the helper existed; locks the flag, the strip-for-fable-5 behavior, and the preserve-for-opus/sonnet + preserve-enabled/adaptive + unknown-model + no-thinking no-op cases. typecheck:core clean; passthrough-thinking + thinking-budget suites still green.

Reported by @Forcerecon with an excellent root-cause analysis.

[gemini-code-assist]

Code Review

This pull request addresses issue #3554 where combo model substitution can forward an invalid thinking.type: "disabled" parameter to target models (like claude-fable-5) that reject it, causing upstream 400 errors. To resolve this, a new rejectsThinkingDisabled flag has been added to the model specifications, and a normalizeThinkingForModel utility has been introduced to strip the invalid thinking field when the target model rejects it. This utility is integrated into the chat core handler, and comprehensive unit tests have been added to verify the behavior. No review comments were provided, so there is no additional feedback to address.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[kilo-code-bot]

Code Review Summary

Status: 2 Issues Found | Recommendation: FYI (PR already merged)

Issue Details (click to expand)

WARNING

File	Line	Issue
open-sse/handlers/chatCore.ts	3562	Silent parameter mutation without logging — When normalizeThinkingForModel strips thinking.type:"disabled", no log entry is emitted. The same file logs warnings for other parameter-stripping (e.g., line 3606 logs Stripped unsupported params for ${model}). This inconsistency makes it harder to debug cases where a combo substitution silently changes a client's thinking preference. Consider adding a log?.warn?.("PARAMS", "Stripped thinking.type:disabled for ${model} because it rejects it") for parity with existing param-stripping observability.

SUGGESTION

File	Line	Issue
tests/unit/combo-thinking-disabled-fable5-3554.test.ts	1-61	Test coverage gap — Unit tests verify normalizeThinkingForModel in isolation but do not exercise the full chatCore.ts request path. A test that simulates a combo routing claude-opus-4-8 → claude-fable-5 with thinking:{type:"disabled"} and asserts the outgoing request body has thinking removed would provide stronger integration-level guarantees that the helper is called at the right point in the pipeline.

Other Observations (not in diff)

File	Line	Issue
src/shared/constants/modelSpecs.ts	430	Future flag inheritance via prefix matching — getModelSpec uses case-insensitive prefix matching (lower.startsWith(key.toLowerCase())). If a future model variant like claude-fable-5-turbo or claude-fable-5-thinking is registered, it will automatically inherit rejectsThinkingDisabled: true because the prefix match hits claude-fable-5. This is existing behavior in getModelSpec, but worth noting since the new flag could be silently inherited by model variants that actually do accept thinking.type:"disabled". Consider whether prefix matching is appropriate for this flag or if exact/alias matching only should be used.

Files Reviewed (4 files)

• CHANGELOG.md — 0 issues
• open-sse/handlers/chatCore.ts — 1 issue
• src/shared/constants/modelSpecs.ts — 1 observation (existing code)
• tests/unit/combo-thinking-disabled-fable5-3554.test.ts — 1 issue

---

_{Reviewed by step-3.7-flash-20260528 · 801,067 tokens}