fix(anthropic): normalize sampling params under extended thinking (temperature/top_p) - #3780
Conversation
…prevent E415 hard-link tarball rejection
Bumps [esbuild](https://github.com/evanw/esbuild) from 0.28.0 to 0.28.1. - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md) - [Commits](evanw/esbuild@v0.28.0...v0.28.1) --- updated-dependencies: - dependency-name: esbuild dependency-version: 0.28.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Claude models with extended thinking (e.g. Opus 4.8 via the Claude Code provider) return HTTP 400 when the request carries non-default sampling params: Anthropic only allows `temperature` = 1 and `top_p` >= 0.95 (or unset) while thinking is enabled/adaptive. The VS Code Copilot "Ollama" provider sends temperature 0.7 / top_p 0.9, and thinking is injected by per-model requestDefaults *after* the translator/constraint passes, so the existing enforceThinkingTemperature (run only inside the CC bridge) never fired on the executor path — and it never handled top_p at all. - enforceThinkingTemperature now also drops top_p when thinking is active (in addition to pinning temperature to 1). - Call it at the final dispatch chokepoint in BaseExecutor, before fingerprinting/CCH signing, for `claude` and Claude-Code-compatible providers — the single point every Claude routing mode (grouped/raw/ combo) and the native passthrough share, so all are covered. Verified against production: temperature 0.7 + top_p 0.9 to Opus 4.8 now succeeds where it previously 400'd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the Claude Code API constraints to drop the top_p parameter and pin temperature to 1 when extended thinking is active, preventing API errors from non-default sampling parameters. It also adds corresponding unit tests to verify this behavior. The review feedback suggests adding defensive checks in both the executor and the constraint utility to ensure that the request body is a valid, non-null object before attempting to access or modify its properties, preventing potential runtime crashes.
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.
| if (this.provider === "claude" || isClaudeCodeCompatible(this.provider)) { | ||
| enforceThinkingTemperature(transformedBody as Record<string, unknown>); | ||
| } |
There was a problem hiding this comment.
If transformedBody is null, undefined, or not an object, casting it to Record<string, unknown> and passing it to enforceThinkingTemperature will cause a runtime crash (TypeError) when accessing body.thinking. We should add a defensive check to ensure transformedBody is a valid non-null object before calling enforceThinkingTemperature.
| if (this.provider === "claude" || isClaudeCodeCompatible(this.provider)) { | |
| enforceThinkingTemperature(transformedBody as Record<string, unknown>); | |
| } | |
| if ( | |
| (this.provider === "claude" || isClaudeCodeCompatible(this.provider)) && | |
| transformedBody && | |
| typeof transformedBody === "object" && | |
| !Array.isArray(transformedBody) | |
| ) { | |
| enforceThinkingTemperature(transformedBody as Record<string, unknown>); | |
| } |
| export function enforceThinkingTemperature(body: Record<string, unknown>): void { | ||
| const thinking = body.thinking as Record<string, unknown> | undefined; | ||
| if (thinking?.type === "enabled" || thinking?.type === "adaptive") { | ||
| body.temperature = 1; | ||
| if (body.top_p !== undefined) { | ||
| delete body.top_p; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
To make enforceThinkingTemperature robust and prevent potential runtime errors from unexpected or improperly typed inputs (especially when called with type assertions like as Record<string, unknown>), we should add a defensive guard clause at the beginning of the function to verify that body is a valid non-null object.
| export function enforceThinkingTemperature(body: Record<string, unknown>): void { | |
| const thinking = body.thinking as Record<string, unknown> | undefined; | |
| if (thinking?.type === "enabled" || thinking?.type === "adaptive") { | |
| body.temperature = 1; | |
| if (body.top_p !== undefined) { | |
| delete body.top_p; | |
| } | |
| } | |
| } | |
| export function enforceThinkingTemperature(body: Record<string, unknown>): void { | |
| if (!body || typeof body !== "object" || Array.isArray(body)) return; | |
| const thinking = body.thinking as Record<string, unknown> | undefined; | |
| if (thinking?.type === "enabled" || thinking?.type === "adaptive") { | |
| body.temperature = 1; | |
| if (body.top_p !== undefined) { | |
| delete body.top_p; | |
| } | |
| } | |
| } |
Code Review SummaryStatus: No New Issues Found | Recommendation: Address existing review comments before merge Files Reviewed (3 files)
Open Review Items (2 prior comments, not re-raised here)
Other Observations (not in diff)No observations outside the diff. Fix these issues in Kilo Cloud Reviewed by step-3.7-flash-20260528 · 1,190,061 tokens |
…pic-thinking-sampling-params
|
Merged into Sharp root-cause: Validated locally: |
…-size Sync with release/v3.8.24 (branch was 29 commits behind v3.8.23). Two review fixes pushed to the PR branch: - Restore reasoningTokenBufferEnabled in comboRuntimeConfigSchema — the branch had dropped it (present on the v3.8.23 fork point and on the current release; diegosouzapw#3588/diegosouzapw#3700 reasoning-buffer combo config). Out of scope for the strict-mode feature; restored to avoid a regression. - Re-baseline file-size for the feature growth (ApiManagerPageClient.tsx, apiKeys.ts, schemas.ts) plus the diegosouzapw#3780 base.ts carry-over. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
Resolve file-size-baseline.json conflict in the _rebaseline keys block: keep this PR's combo_quota_audit key + release's v3824_3776 key (#3776); drop the now-redundant drift_3780 key (release's #3776 key already documents the base.ts 1205->1218 carry-over from #3780). frozen values auto-merged (combo.ts 5131 mine, base.ts 1218 both, chatCore.ts 5808 theirs). Verified on the merged tree: all 14 Fast Quality Gates pass (incl. file-size: chatCore 5736<=5808, combo 5131<=5131), W1 streaming hook intact, quota-streaming 8 / complexity-router 6 / scoring-clamp 7 / combo-routing-engine 81 green. Pre-existing release app-tsc noise in chatCore (3026-4380, from #3775) is untouched by this PR.
…contributor credits - Restructure [3.8.24] into ✨ Features / 🔒 Security / 🐛 Fixed / 📝 Maintenance - Add bullets for every PR landed since v3.8.23 that was missing: marketplace (#3656), strict-mode CC defaults (#3776), emergency-fallback flag (#3752), xhigh effort (#3756), Codex memory WS (#3749), IPv6 egress (#3777), marketplace SSRF (#3774), CodeQL/Dependabot (#3778), anthropic sampling (#3780), thinking passthrough (#3775), mcp dist entry (#3765), streamed tool args (#3762), logs light-mode (#3760), clean-history purge (#3751), quality-gates (#3757), docs gaps (#3453), file-size re-baseline (#3770), E415 publish guard, i18n prune - Move misplaced #3775 bullet out of [Unreleased] into [3.8.24] - Date [3.8.23] header (TBD -> 2026-06-12, the release tag date)
…+ complexity routing (#3779) Deep audit of the combo + quota-shared system, delivered as 4 TDD waves. - Wave 1: repair 5 dead/broken rules — streaming USD recording, pool-usage provider resolution, provider-diversity wiring, maxComboDepth threading, scoring clamp/NaN-safety (incl. connectionDensity). - Wave 2: validate every auto-router strategy (cost / latency / sla-aware / lkgp / selectWithStrategy + aliases) and the predictive-TTFT decision. - Wave 3: E2E coverage — 3-hop priority failover, per-target timeout failover, real strategy:auto dispatch. - Wave 4: complexity-aware routing (2026, opt-in) over the existing specificity detector, plus revival of the dead tierAffinity / specificityMatch scoring factors (require-in-ESM root cause -> static import). Proxy/credential isolation verified clean (each target uses its own credentials+proxy via AsyncLocalStorage). file-size reconciled (combo.ts re-baseline after extracting buildComplexityRoutingHint to complexityRouter.ts; base.ts release-drift from #3780). Fast Quality Gates + semgrep green.
…mperature/top_p) (diegosouzapw#3780) Claude with extended thinking rejects non-default sampling params (temperature must be 1, top_p >= 0.95 or unset) with HTTP 400. Clients like the VS Code Copilot Ollama BYOK provider send temperature 0.7 + top_p 0.9, breaking every Claude+thinking request across grouped/raw/combo. enforceThinkingTemperature now also drops top_p (in addition to pinning temperature=1) when thinking is enabled/adaptive, and is called at the final dispatch chokepoint in BaseExecutor (before fingerprint/CCH signing) for claude + claude-code-compatible providers — the single point every routing mode converges on. No-op when thinking is inactive. Tests: claude-code-parity.test.ts +3 cases (31/31). typecheck:core + eslint clean. Integrated into release/v3.8.24.
…contributor credits - Restructure [3.8.24] into ✨ Features / 🔒 Security / 🐛 Fixed / 📝 Maintenance - Add bullets for every PR landed since v3.8.23 that was missing: marketplace (diegosouzapw#3656), strict-mode CC defaults (diegosouzapw#3776), emergency-fallback flag (diegosouzapw#3752), xhigh effort (diegosouzapw#3756), Codex memory WS (diegosouzapw#3749), IPv6 egress (diegosouzapw#3777), marketplace SSRF (diegosouzapw#3774), CodeQL/Dependabot (diegosouzapw#3778), anthropic sampling (diegosouzapw#3780), thinking passthrough (diegosouzapw#3775), mcp dist entry (diegosouzapw#3765), streamed tool args (diegosouzapw#3762), logs light-mode (diegosouzapw#3760), clean-history purge (diegosouzapw#3751), quality-gates (diegosouzapw#3757), docs gaps (diegosouzapw#3453), file-size re-baseline (diegosouzapw#3770), E415 publish guard, i18n prune - Move misplaced diegosouzapw#3775 bullet out of [Unreleased] into [3.8.24] - Date [3.8.23] header (TBD -> 2026-06-12, the release tag date)
…+ complexity routing (diegosouzapw#3779) Deep audit of the combo + quota-shared system, delivered as 4 TDD waves. - Wave 1: repair 5 dead/broken rules — streaming USD recording, pool-usage provider resolution, provider-diversity wiring, maxComboDepth threading, scoring clamp/NaN-safety (incl. connectionDensity). - Wave 2: validate every auto-router strategy (cost / latency / sla-aware / lkgp / selectWithStrategy + aliases) and the predictive-TTFT decision. - Wave 3: E2E coverage — 3-hop priority failover, per-target timeout failover, real strategy:auto dispatch. - Wave 4: complexity-aware routing (2026, opt-in) over the existing specificity detector, plus revival of the dead tierAffinity / specificityMatch scoring factors (require-in-ESM root cause -> static import). Proxy/credential isolation verified clean (each target uses its own credentials+proxy via AsyncLocalStorage). file-size reconciled (combo.ts re-baseline after extracting buildComplexityRoutingHint to complexityRouter.ts; base.ts release-drift from diegosouzapw#3780). Fast Quality Gates + semgrep green.
Problem
Claude models that run with extended thinking (e.g. Opus 4.8 via the Claude Code provider) return HTTP 400 when the request carries non-default sampling params:
The VS Code Copilot "Ollama" BYOK provider sends
temperature: 0.7andtop_p: 0.9, so every request to a thinking-enabled Claude model fails. It reproduces across all routing modes (grouped / raw / combo) because thinking is applied at the provider/model layer they all share.Root cause
thinkingis injected by per-modelrequestDefaults(applyProviderRequestDefaults) after the translator/constraint passes run. The existingenforceThinkingTemperatureonly runs inside the Claude Code bridge path (buildAndSignClaudeCodeRequest), which is not on the executor path these requests take — so it never fired. And even when it did fire, it pinnedtemperaturebut never handledtop_pat all.Fix
enforceThinkingTemperaturenow also dropstop_pwhen thinking is enabled/adaptive (in addition to pinningtemperature = 1).BaseExecutor— before fingerprinting/CCH signing serialize the body — forclaudeand Claude-Code-compatible providers. Single point every Claude routing mode (grouped/raw/combo) and the native passthrough share. No-op when thinking isn't active.Unit tests extended (
tests/unit/claude-code-parity.test.ts), eslint + typecheck clean.