fix(llm): omit temperature for major-only Opus ids (claude-opus-5) - #5761
Open
husamemadH wants to merge 1 commit into
Open
fix(llm): omit temperature for major-only Opus ids (claude-opus-5)#5761husamemadH wants to merge 1 commit into
husamemadH wants to merge 1 commit into
Conversation
The version pattern in _anthropic_rejects_temperature() required a minor
component, so major-only ids like `claude-opus-5` never matched and the
guard reported that the model accepts `temperature`. Anthropic rejects the
field outright on Opus 4.7+, so every such call returned HTTP 400 and the
stream aborted with zero tokens ("the model returned an empty response").
Make the minor optional and read a missing minor as `.0`. The major is also
capped at 1-2 digits with a no-trailing-digit lookahead, mirroring the
minor: once the minor is optional, a greedy major would swallow the date in
`claude-3-opus-20240229` and read it as version 20240229, dropping
temperature from a model that accepts it.
Fixes odysseus-dev#5753
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
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
_anthropic_rejects_temperature()decides whether to sendtemperatureon the native Anthropic path — Opus 4.7+ returns HTTP 400 if the field is present at all. Its version pattern required a minor component (opus-(\d+)[-_.](\d{1,2})), so a major-only id likeclaude-opus-5never matched, the guard reported "this model accepts temperature", and the field was sent. Every such call fails with`temperature` is deprecated for this model, aborting the stream in ~0.4s with zero tokens — surfaced in the UI as "The model returned an empty response."This makes the minor optional and treats a missing minor as
.0, soclaude-opus-5parses as(5, 0)and is correctly gated as >= 4.7.The major is also capped at 1–2 digits with a no-trailing-digit lookahead, mirroring the minor. This is required once the minor becomes optional: with a greedy
(\d+)major,claude-3-opus-20240229(legacy Claude 3 Opus, date directly afteropus-) would parse as version20240229and silently lose its temperature — a model that accepts it. The old pattern avoided that only as a side effect of requiring a minor.Interactive chat never hit this: it leaves
temperatureasNoneso nothing is sent. The scheduled-task path callsstream_agent_loop()without a temperature and inherits itstemperature: float = 0.3default, which is sent — so background tasks broke while chat looked healthy.Target branch
dev, notmain.Linked Issue
Fixes #5753
Type of Change
Checklist
devHow to Test
Reproduce on
dev(the guard answersFalsefor a model that is newer than 4.7):Before:
rejects: False/temperature in payload: True(the0.3is the scheduled-task default).After:
rejects: True/temperature in payload: False.Full predicate table from the issue, plus the legacy Claude 3 Opus case:
claude-opus-5claude-opus-5-20260101claude-opus-4-8claude-opus-4-7claude-opus-4-6claude-opus-4-20250514octopus-4-8claude-3-opus-20240229Tests:
42 passed. Also ran
python -m pytest tests/ -k "llm_core or temperature or anthropic"— 235 passed.Note on the fix suggested in the issue
The issue proposes making the minor optional while leaving the major as
(\d+). That regressesclaude-3-opus-20240229toTrue(the greedy major swallows the date as version20240229), which fails the existing case attests/test_llm_core_anthropic_temp_omit.py:51. Capping the major at 1–2 digits handles both; the issue's table didn't include a legacy Claude 3 Opus id, which is why it looked clean.Verification
Verified at the payload level, which is where this bug lives — the field is either in the request body or it isn't:
_anthropic_rejects_temperature()and_build_anthropic_payload()against the real module in a container built fromdev, before and after the change. Before:claude-opus-5keepstemperature. After: it is omitted.octopus-4-8.tests/test_llm_core_anthropic_temp_omit.py+tests/test_llm_core_anthropic_temp_clamp.py: 42 passed.pytest tests/ -k "llm_core or temperature or anthropic": 235 passed, no regressions.The HTTP 400 itself is as described in #5753; this change removes the field that triggers it.
Disclosure
The patch was drafted with Claude Code. I reproduced the bug myself, reviewed the regex change line by line, and ran the tests before opening this. Flagging it per CONTRIBUTING.md's note on agent-generated PRs — the issue (#5753) was filed independently and this is a single targeted fix, not a bulk submission.
Files changed
src/llm_core.py— version pattern in_anthropic_rejects_temperature(), plus comments explaining both digit caps.tests/test_llm_core_anthropic_temp_omit.py— major-only ids added to the "rejects" set (claude-opus-5, dated, provider-prefixed, futureclaude-opus-6); payload test for the scheduled-task path; explicit payload guard for legacy Claude 3 Opus.No UI/rendering changes.