fix(anthropic): use adaptive thinking for Claude 4.6+ served outside the canonical registry - #9840
fix(anthropic): use adaptive thinking for Claude 4.6+ served outside the canonical registry#9840hammadxcm wants to merge 2 commits into
Conversation
…egistry
Claude Opus 4.7 and 4.8 (and Sonnet 4.6, Fable) require adaptive thinking
and reject the deprecated `thinking: {type: "enabled"}` shape with a 400:
"thinking.type.enabled" is not supported for this model. Use
"thinking.type.adaptive" and "output_config.effort" ...
The canonical model registry only carries `thinking_mode` for the
first-party `anthropic/` model IDs, so the same models served through
other providers (Bedrock, Vertex, Azure) or through fully custom
Anthropic-compatible providers fall through to the enabled shape and 400.
This is why 4.6 worked but 4.7/4.8 failed via Azure AI Foundry.
Add a model-name fallback used only when the registry has no thinking_mode:
Claude Opus/Sonnet 4.6 and later infer adaptive, the Fable family infers
always-on adaptive, and pre-4.6 / date-suffixed legacy names are left on
the enabled path. The version parse is forward-compatible, so future
Claude releases (4.9, 5.x) are covered without another change.
Fixes aaif-goose#9746
Signed-off-by: hammadxcm <hammadkhanxcm@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83fd44b486
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let after = name.split_once(family)?.1.trim_start_matches(['-', '.']); | ||
| let mut parts = after.split(['-', '.']); | ||
| let major = parts.next()?.parse().ok()?; | ||
| let minor = parts.next().and_then(|p| p.parse().ok()).unwrap_or(0); |
There was a problem hiding this comment.
When the model is an official date-suffixed Claude 4 ID such as claude-opus-4-20250514 or claude-sonnet-4-20250514, this parser treats the release date as the minor version ((4, 20250514)), so the new >= (4, 6) fallback classifies pre-4.6 models as adaptive. The canonical entries for claude-opus-4/claude-sonnet-4 have reasoning=true but no thinking_mode, so requests with thinking enabled now send thinking.type=adaptive instead of the previous enabled-budget shape, which can make these older Claude 4 models reject the request.
Useful? React with 👍 / 👎.
kimnamu
left a comment
kimnamu
left a comment
There was a problem hiding this comment.
Thanks for tackling this, @hammadxcm — framing the fix at the decision point (thinking_type_for_provider) rather than as a registry data patch is the right call. I'm not a maintainer, just another Bedrock/Anthropic-provider user who reviewed this carefully and ran the branch locally (cargo test -p goose --lib providers::formats::anthropic, 38/38 green). A few things I verified that you may find useful:
What I confirmed is solid
- The separator-agnostic parse and the
major <= 9guard correctly keep the legacyclaude-3-*-<date>names off adaptive — the date lands in major, so(20240229, 0)is filtered out. Verified:claude-3-opus-20240229 -> None. - Bedrock-style prefixes are handled transparently:
split_once("opus"/"sonnet")discards everything before the family token, sous.anthropic.claude-sonnet-4-7-v1:0parses the same asclaude-sonnet-4-7. Nice.
One regression to close (same as Codex's P2) — date-suffixed Claude 4 IDs
The date guard only covers the case where the date follows a single-digit major (claude-3-...). For the official date-suffixed Claude 4 IDs, the date lands in minor, so it slips through >= (4, 6):
claude-opus-4-20250514 -> (4, 20250514) -> Adaptive (this is Opus 4.0)
claude-sonnet-4-20250514 -> (4, 20250514) -> Adaptive
claude-opus-4-1-20250805 -> (4, 20250805) -> Adaptive (Opus 4.1)
us.anthropic.claude-sonnet-4-20250514-v1:0 -> Adaptive
These are real, in-registry IDs — canonical_models.json carries claude-opus-4-20250514 / claude-sonnet-4-20250514 with reasoning=true, thinking_mode=None, and the repo's own Bedrock fixture uses us.anthropic.claude-sonnet-4-20250514-v1:0. Because their canonical thinking_mode is None, they fall into exactly this new fallback, so a pre-4.6 model that used to send enabled would now send adaptive — the mirror-image 400 of the one you're fixing.
A one-character bound on minor (symmetric with your existing major bound) fixes it while staying forward-compatible with a hypothetical 4.10+. I applied this locally and it's green across the board. To lock the boundary in, here are the negatives to add to test_inferred_adaptive_thinking_mode (these fail on the current branch and pass with the bound — I verified both directions):
// Official date-suffixed Claude 4 IDs (pre-4.6): the date must not
// be parsed as the minor version and classified as adaptive.
"claude-opus-4-20250514",
"claude-sonnet-4-20250514",
"claude-opus-4-1-20250805",Red (current branch):
assertion `left == right` failed: claude-opus-4-20250514 should not infer adaptive
left: Some(Adaptive)
right: None
Green (with minor < 100): test result: ok. 38 passed; 0 failed.
Minor doc nit (non-blocking): the claude_family_version doc comment only mentions the date-in-major case (claude-3-opus-20240229); worth a half-line noting the date-in-minor case is handled by the minor bound in the caller, so the next reader doesn't re-introduce it.
Really clean, well-scoped fix otherwise — with the minor bound I'd love to see it land.
(Reviewed with the help of an AI agent (Claude Code); I ran the branch and verified every claim above myself.)
| }; | ||
|
|
||
| claude_family_version(&name, family) | ||
| .filter(|&(major, minor)| major <= 9 && (major, minor) >= (4, 6)) |
There was a problem hiding this comment.
Date-suffixed Claude 4 IDs (claude-opus-4-20250514, claude-opus-4-1-20250805, us.anthropic.claude-sonnet-4-20250514-v1:0) parse the date as minor and slip past >= (4, 6). These are real in-registry IDs with thinking_mode=None, so they hit this fallback and would flip a pre-4.6 model to adaptive (the mirror 400). A minor bound (symmetric with your major <= 9) fixes it and still allows a future 4.10+. Verified locally: red without it, 38/38 green with it.
| .filter(|&(major, minor)| major <= 9 && (major, minor) >= (4, 6)) | |
| .filter(|&(major, minor)| major <= 9 && minor < 100 && (major, minor) >= (4, 6)) |
Date-suffixed Claude 4 IDs such as claude-opus-4-20250514 parsed the release date as the minor version (4, 20250514), which passed the >= (4, 6) check and wrongly inferred adaptive thinking for these pre-4.6 models. They are real in-registry IDs with no thinking_mode, so they reach this fallback. Bound the minor version (minor < 100, symmetric with major <= 9) so 8-digit dates are rejected while future 4.10+ releases still infer adaptive. Add the date-suffixed IDs as regression cases. Thanks to @kimnamu and the codex review for catching this. Signed-off-by: hammadxcm <hammadkhanxcm@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1ff0541d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mode = canonical_thinking_mode(provider_name, &model_config.model_name) | ||
| .or_else(|| inferred_adaptive_thinking_mode(&model_config.model_name)); |
There was a problem hiding this comment.
When the only match is this new fallback (for example a custom-provider model like claude-opus-4.8-fast, which the parser test says should be covered, or a future claude-opus-4-9), canonical_reasoning still returns None because there is no canonical entry and ModelConfig::new(...).with_canonical_limits(custom_provider) leaves reasoning unset. As a result the subsequent reasoning != Some(true) check returns Disabled, so the request still omits thinking/output_config unless the caller explicitly set reasoning: true; the fallback needs to mark these inferred Claude modes as reasoning-capable too.
Useful? React with 👍 / 👎.
|
Thanks for digging into this and for engaging so carefully with the review feedback — appreciated. After looking at it closely, I think this fix is in the wrong layer, so I'm going to close it. The short version: the canonical model map ( Adding a parallel inference path in
If you hit a model that genuinely does not resolve (e.g. the Bedrock-style Closing for now, but a PR that improves the canonical prefix/suffix normalization (or adds missing entries) for those provider-prefixed Bedrock names would be very welcome. Thanks again! |
3 participants
Summary
Fixes #9746
Claude Opus 4.7 and 4.8 require adaptive thinking and reject the deprecated manual shape with a 400:
thinking_type_for_providerdecides betweenadaptiveandenabledfrom the canonical model registry'sthinking_mode. That field is only populated for the first-partyanthropic/model IDs (anthropic/claude-opus-4.7,anthropic/claude-opus-4.8, …). The same models served through another provider — Bedrock, Vertex, Azure, or a fully custom Anthropic-compatible endpoint — have no registrythinking_mode, so they fall through to{"type": "enabled"}and 400. That's exactly why the reporter saw 4.6 work but 4.7/4.8 fail via Azure AI Foundry.A pure registry/data fix wouldn't help a custom provider whose model isn't in the registry at all, so the fix belongs at the decision point.
Changes
crates/goose/src/providers/formats/anthropic.rs:inferred_adaptive_thinking_mode(model_name), used only as a fallback when the registry returns nothinking_mode. Claude Opus/Sonnet 4.6 and later inferAdaptive; the Fable family infersAlwaysOnAdaptive; pre-4.6 and date-suffixed legacy names (claude-3-opus-20240229, Sonnet 4.5) stay on the enabled path.opus-4-7,opus-4.7,opus-4.8-fast) and forward-compatible, so future releases (4.9, 5.x) are covered without another change — addressing the issue's "should apply to any future Claude".Testing
test_inferred_adaptive_thinking_mode— unit coverage for the name parser across separators, suffixes, the Fable family, and the pre-4.6 / legacy-date negatives.test_thinking_type_adaptive_for_non_registry_provider— Opus 4.7/4.8 through a provider with no registry entry now resolve toAdaptive, while Opus 4.5 still resolves toEnabled.cargo fmt,cargo test -p goose --lib providers::formats::anthropic(38/38),cargo clippy -p goose --lib --tests --features aws-providers -- -D warningsall clean.Note
The build-time registry inference (
inferred_thinking_modeinbuild_canonical_models.rs) has the same first-party-only matching. I kept this PR to the runtime path since that's what fixes custom/compatible providers and doesn't require regenerating the wholecanonical_models.json; aligning the build-time inference could be a small follow-up.