fix(cli): align Claude adaptive thinking detection - #12617
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // Covers family-first IDs such as claude-opus-4.7 and version-first IDs such as claude-4.7-opus. | ||
| // Limit minors to two digits so release dates in IDs such as claude-opus-4-20250514 are not versions. | ||
| const version = /claude-(?:[a-z]+-)?(\d+)(?:[.-](\d{1,2}))?(?:[.@-]|$)/i.exec(apiId) | ||
| if (!version) return true |
There was a problem hiding this comment.
WARNING: Unversioned claude- IDs silently default to "modern adaptive"
When the regex doesn't match at all (!version), the function now returns true instead of the old false. In practice every current test case that reaches this function has a digit somewhere in the id (e.g. claude-fable-5 matches via major=5), so this fallback branch is currently untested. A future unversioned alias for a non-adaptive/legacy Claude model (no digits at all after claude-) would be misclassified as using modern adaptive thinking, which changes request shape (thinking/reasoning params) sent to the provider. Consider either keeping the previous safer default (false) for truly unversioned ids, or add a test that exercises this exact fallback path so a future regression is caught.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| model.api.id.includes("opus-4.7") || | ||
| model.api.id.includes("opus-4.8") || | ||
| anthropicClaude5(model.api.id) | ||
| anthropicUsesModernAdaptiveThinking(model.api.id) |
There was a problem hiding this comment.
SUGGESTION: Redundant substring checks now dead code
anthropicUsesModernAdaptiveThinking already returns true for ids containing opus-4.7 / opus-4.8 (major=4, minor>=7), so the explicit model.api.id.includes("opus-4.7") || model.api.id.includes("opus-4.8") checks on the two lines above are now always short-circuited by this call and can be removed for clarity.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Also note: this PR changes user-visible behavior (which Bedrock/Vertex/gateway Claude model ids get adaptive-thinking params) but doesn't include a changeset under Files Reviewed (2 files)
Fix these issues in Kilo Cloud Reviewed by claude-sonnet-5 · Input: 30 · Output: 14.2K · Cached: 796.4K Review guidance: REVIEW.md from base branch |
1 participant
What changed
Replace Kilo's family-specific Claude 5 matcher with OpenCode's generalized modern adaptive-thinking detector. This preserves adaptive Bedrock reasoning options for foundation and inference-profile forms such as
anthropic.claude-opus-5,us.anthropic.claude-opus-5-v1:0, andglobal.anthropic.claude-opus-5, while ensuring dated legacy Claude 4 IDs are not mistaken for modern model versions.This follows Kilo #12544 and adapts the parent OpenCode fix from anomalyco/opencode#38757 (
2b2aacc93975330f9fd045d4306f698b0c6a8f8f).Why adapted instead of cherry-picked
The upstream commit assumes the metadata-driven
reasoning_optionsinfrastructure introduced by anomalyco/opencode#36624 and subsequent provider-transform changes. Kilo has not backported that stack, and trial cherry-picks conflict across provider construction and shared tests. This PR therefore carries only the upstream detector needed for the Bedrock failure and keeps the regression coverage in Kilo-owned tests.