Skip to content

Fix Bedrock thinking effort mapping for Claude models (#851) - #855

Open
Edilbek wants to merge 2 commits into
crmne:mainfrom
Edilbek:fix/851-bedrock-thinking-effort
Open

Fix Bedrock thinking effort mapping for Claude models (#851)#855
Edilbek wants to merge 2 commits into
crmne:mainfrom
Edilbek:fix/851-bedrock-thinking-effort

Conversation

@Edilbek

@Edilbek Edilbek commented Jul 29, 2026

Copy link
Copy Markdown

What this does

Fixes #851.

Extended thinking on Bedrock Claude models (Sonnet 5, and every other Claude on Bedrock) raised RubyLLM::BadRequestError: The model returned the following errors: reasoning_effort: Extra inputs are not permitted whenever with_thinking(effort:) was used.

The Converse protocol branched on reasoningSupported.embedded metadata to decide how to send thinking params. That flag is unrelated to how a model accepts reasoning input, and it's false for every Bedrock Claude, so effort-based thinking always fell into a { reasoning_effort: ... } branch. Bedrock forwards additionalModelRequestFields straight to Anthropic, which rejects the unknown reasoning_effort key.

Bedrock actually publishes each model's accepted fields in converse.additionalRequestFieldsSchema. Claude models expose a reasoningConfig.budgetTokens schema and only accept a token budget, never an effort level. This PR:

  • Replaces the reasoning_embedded? check with reasoning_budget_schema, which reads that published schema.
  • Maps effort onto a budget: enumerated schemas use the token value they name for each level (e.g. Sonnet 5 low → 1024, medium → 40000, high → 63999); integer-range schemas spread low/medium/high across the range; efforts a schema doesn't enumerate clamp to its maximum.
  • Keeps sending reasoning_effort for models whose schema advertises no budget (e.g. openai.gpt-oss-120b).
  • Prefers an explicit budget: over effort:, and clamps a derived budget below max_output_tokens when one is set so it can't consume the whole output allowance.
  • Falls back to a sibling regional registry entry for the same foundation model, since Bedrock only stores the Converse schema on some regional entries — without this, eu./global./bare anthropic. ids still failed.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Performance improvement

Scope check

  • I read the Contributing Guide
  • This aligns with RubyLLM's focus on LLM communication
  • This isn't application-specific logic that belongs in user code
  • This benefits most users, not just my specific use case

Required for new features

N/A — bug fix.

Quality check

  • I ran overcommit --install and all hooks pass
  • I tested my changes thoroughly
    • For provider changes: Re-recorded VCR cassettes with bundle exec rake vcr:record[provider_name]
    • All tests pass: bundle exec rspec
  • I updated documentation if needed
  • I didn't modify auto-generated files manually (models.json, aliases.json)

AI-generated code

  • I used AI tools to help write this code
  • I have reviewed and understand all generated code (required if above is checked)

API changes

  • Breaking change
  • New public methods/classes
  • Changed method signatures
  • No API changes

Notes for the maintainer:

  • VCR cassettes not re-recorded. No Bedrock credentials were available to record a live effort-based thinking cassette. The change is covered by unit specs in spec/ruby_llm/protocols/converse/chat_spec.rb asserting the emitted payload, plus a WebMock-stubbed end-to-end reproduction. The derived payload shape matches what the existing budget-based Bedrock cassettes already send, but you may want to add { provider: :bedrock, model: 'claude-sonnet-5' } to the thinking models and record one.

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.30%. Comparing base (091b16a) to head (1b7c9af).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
lib/ruby_llm/protocols/converse/chat.rb 81.25% 1 Missing and 5 partials ⚠️
lib/ruby_llm/protocols/converse.rb 83.33% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #855      +/-   ##
==========================================
+ Coverage   83.18%   83.30%   +0.12%     
==========================================
  Files         173      175       +2     
  Lines        8461     8798     +337     
  Branches     1373     1423      +50     
==========================================
+ Hits         7038     7329     +291     
- Misses        925      941      +16     
- Partials      498      528      +30     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread lib/ruby_llm/protocols/converse/chat.rb Outdated
Comment thread lib/ruby_llm/protocols/converse/chat.rb
@Edilbek
Edilbek requested a review from justwiebe July 29, 2026 17:11
@justwiebe

Copy link
Copy Markdown

Looks good, although I'm not a reviewer on the project

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Sonnet 5 Extra inputs are not permitted

2 participants