fix(core): omit deprecated temperature param for Claude 4.8+ - #6520
Merged
Conversation
Claude Opus 4.8 deprecated the `temperature` sampling parameter — the server returns 400 with "temperature is deprecated for this model" when it is sent. Add `modelRejectsTemperature()` version gate (mirroring the existing `modelRejectsManualThinking()` pattern) and conditionally omit `temperature` from the request body for models with major > 4 or 4.minor >= 8. Older models and unknown/unversioned ids retain the previous behavior (default temperature of 1). Fixes #6519
Collaborator
|
Thanks for the fix! (re-run — PR was updated since last review) Template: non-standard headings (Summary / Changes / Behavior instead of the PR template sections), but the content is comprehensive — behavior table, per-file breakdown, linked issue. Not blocking. Problem: observed bug ✓. Issue #6519 documents a real 400 error from the Anthropic API with the exact message Direction: aligned ✓. The fix mirrors the existing Size: 2 files changed — ~25 production lines in Approach: minimal and focused ✓. The diff does exactly one thing: adds Moving on to code review. 🔍 中文说明感谢修复！（重新审查——PR 在上次审查后有更新） 模板：标题格式非标准（用了 Summary / Changes / Behavior 而非模板中的段落），但内容完整——行为对照表、逐文件变更说明、关联 issue。不阻塞。 问题：已观测到的 bug ✓。Issue #6519 记录了 Anthropic API 对 方向：对齐 ✓。修复复用了已有的 规模：2 个文件变更—— 方案：最小且聚焦 ✓。diff 只做了一件事：新增 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code Review (re-run — PR updated)Independent proposal (before reading the diff): The fix needs a version gate on Comparison with the diff: The PR matches this approach exactly, including the warning log. The implementation is clean:
No correctness bugs, no security issues, no regressions. Follows existing conventions precisely. Reuse check: The new method reuses TestingUnit tests: 89/89 pass (including the 4 new temperature tests — Opus 4.8 omits, Opus 4.7 keeps, Sonnet 5 omits, unknown model keeps). Real-scenario test: No Anthropic API key available in CI — cannot do a live before/after API roundtrip. The unit tests directly verify the fix by inspecting the request object sent to the API ( 中文说明代码审查（重新审查——PR 已更新）独立方案（阅读 diff 前）： 修复需要一个针对 与 diff 的对比： PR 完全匹配该方案，包括警告日志。实现干净：
无正确性 bug，无安全问题，无回归。完全遵循已有代码规范。 测试单元测试： 89/89 通过（包括 4 个新增的 temperature 测试——Opus 4.8 省略、Opus 4.7 保留、Sonnet 5 省略、未知模型保留）。 真实场景测试： CI 环境无 Anthropic API key——无法进行实际的 before/after API 往返测试。单元测试通过检查发送给 API 的请求对象直接验证了修复（ — Qwen Code · qwen3.7-max |
Collaborator
|
This is a clean, textbook fix — and the updates since the last review make it even better. The problem is real (issue #6519 documents the exact 400 error from the Anthropic API), the solution mirrors an established pattern in the same file ( The 4 tests (up from 2 in the previous revision) cover all the boundaries: Opus 4.8 omits, Opus 4.7 keeps, Sonnet 5 omits (future-proofing for the 5.x family), and unknown models keep the fallback. All 89 tests in the file pass. One thing I'd note: the No concerns. Approving. 中文说明这是一个干净、标准的修复——上次审查后的更新让它更好了。 问题是真实的（issue #6519 记录了 Anthropic API 返回的确切 400 错误），方案复用了同文件中已有的模式（ 4 个测试（上次审查时是 2 个）覆盖了所有边界：Opus 4.8 省略、Opus 4.7 保留、Sonnet 5 省略（为 5.x 系列做前瞻性保护）、未知模型保留回退值。文件内全部 89 个测试通过。 一个备注： 无顾虑，通过。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jul 8, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
previously approved these changes
Jul 8, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
wenshao
requested changes
Jul 8, 2026
| return { | ||
| max_tokens: maxTokens, | ||
| temperature: getParam<number>('temperature', 'temperature') ?? 1, | ||
| ...(!this.modelRejectsTemperature() && { temperature: temperatureValue }), |
Collaborator
There was a problem hiding this comment.
[Critical] This only omits temperature for 4.8+, but Anthropic's Messages API docs say temperature, top_p, and top_k should be omitted on Claude Opus 4.7 and later, including Opus 4.8. With the current request shape, claude-opus-4-7 still sends configured temperature, and claude-opus-4-8 still sends configured top_p/top_k, so both paths can continue to return 400s after this fix.
Please make this a sampling-parameter gate for the affected model set and apply it to all three fields; update the Opus 4.7 test to assert omission and add coverage for topP/topK (and the major > 4 branch). See https://platform.claude.com/docs/en/build-with-claude/working-with-messages.
— GPT-5 via Qwen Code /review
wenshao
reviewed
Jul 8, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Review: omit deprecated temperature for Claude 4.8+
Clean, well-scoped fix. modelRejectsTemperature() mirrors the established modelRejectsManualThinking() gate precisely, the boundary is right (4.7 keeps / 4.8 drops), and the two tests exercise both sides of the 4.x boundary. Verified against the source:
- ✅
temperatureis the only sampling param sent unconditionally (?? 1);top_p/top_kareundefinedby default and stripped by JSON serialization — so temperature was indeed the sole trigger for the 400. The fix targets the right param. - ✅ No bypass path —
temperatureis set only inbuildSamplingParameters, so the gate fully controls whether it reaches the wire (spread intobuildRequestat L696). - ✅
{...false}spreads to zero keys, so the conditional spread is correct; the return type is alreadytemperature?: number, so omission is type-safe (no CI red). - ✅ Version parsing handles dated 4.8 ids (
claude-opus-4-8-20260101→ minor 8) and doesn't mis-gate dated 4.0 ids (claude-opus-4-20250514→ minor 0, keeps temperature).
Three non-blocking notes inline.
Test coverage: the added tests only cover the Opus 4.7/4.8 boundary. Since the gate also fires for the whole 5.x family via major > 4, a claude-sonnet-5 case (temperature omitted) and an unknown/unversioned id case (temperature kept) would pin that branch and prevent a future narrowing to opus-only from silently regressing.
| return { | ||
| max_tokens: maxTokens, | ||
| temperature: getParam<number>('temperature', 'temperature') ?? 1, | ||
| ...(!this.modelRejectsTemperature() && { temperature: temperatureValue }), |
Collaborator
There was a problem hiding this comment.
Two minor cleanups here — not bugs ({...false} correctly spreads to no keys):
-
Idiom consistency + a now-dead computation. The sibling spreads just below use the ternary form (
...(thinking ? { thinking } : {}), L697–698). Matching that here also lets you droptemperatureValue, which currently still runsgetParam(...)even when the value is discarded:...(this.modelRejectsTemperature() ? {} : { temperature: getParam<number>('temperature', 'temperature') ?? 1 }),
-
Silent drop of an explicit
temperature. A user who settemperature: 0(e.g. for determinism) on a 4.8+ model now has it dropped with no signal. Consider a once-per-generatordebugLogger.warn, latched likeeffortClampWarnedinresolveEffectiveEffort, so the omission is visible under--debug— consistent with how effort clamping is surfaced.
| ); | ||
| if (!parsed) return false; | ||
| const { major, minor } = parsed; | ||
| return major > 4 || (major === 4 && minor >= 8); |
Collaborator
There was a problem hiding this comment.
major > 4 fires for every 5.x family — Sonnet 5, Fable 5, Mythos 5 — not just Opus, whereas the linked issue #6519 and the PR's behavior table are Opus-4.8-specific. Worth confirming Anthropic dropped temperature family-wide at 5.x.
It mirrors modelRejectsManualThinking()'s major > 4, so it reads as deliberate, and it's the safe direction (omitting avoids the 400; sending would hard-fail). The only downside if some 5.x model still accepts temperature: a user-set value is silently omitted for it. A claude-sonnet-5 test would lock in the intended 5.x behavior.
…ests - Switch from boolean-spread to ternary form for idiom consistency with the thinking/output_config spreads at L697-698. - Add a once-per-generator debugLogger.warn when a user-configured temperature is silently dropped on 4.8+ (latched like effortClampWarned). - Add test coverage for claude-sonnet-5 (5.x family omits temperature) and unknown/unversioned model id (keeps temperature) to pin the major > 4 branch and prevent future narrowing.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts |
The warning log path (~9 lines: latch, conditional, formatted message) has zero test coverage. None of the 4 new tests spy on or assert against debugLogger. |
Add a spy on debugLogger and assert warn is called on the Opus 4.8 test (with user-set temperature) and NOT called on the Opus 4.7 or unknown-model tests. |
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jul 8, 2026
Collaborator
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jul 8, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Claude Opus 4.8 deprecated the
temperaturesampling parameter — the server returns 400 with"temperature is deprecated for this model"when it is sent.This PR adds a
modelRejectsTemperature()version gate (mirroring the existingmodelRejectsManualThinking()pattern) and conditionally omitstemperaturefrom the Anthropic API request body for Claude 4.8+ models.Changes
packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.tsmodelRejectsTemperature(): returnstruefor models withmajor > 4or4.minor >= 8buildSamplingParameters()to spreadtemperatureonly when the model accepts itpackages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.tsomits temperature on Opus 4.8 (deprecated)— verifies temperature is absentkeeps temperature on Opus 4.7 (still accepted)— verifies older models unaffectedBehavior
claude-opus-4-8temperature: 1sent → 400 errortemperatureomitted → ✓claude-opus-4-7temperature: 1sent → ✓temperature: 1sent → ✓ (unchanged)temperature: 1sent → ✓temperature: 1sent → ✓ (unchanged)Fixes #6519