-
-
Notifications
You must be signed in to change notification settings - Fork 190
WIP - Support thinking mode for Anthropic models #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@@ -39,36 +39,51 @@ def build_base_payload(chat_messages, temperature, model, stream) | |||
{ | |||
model: model, | |||
messages: chat_messages.map { |msg| format_message(msg) }, | |||
temperature: temperature, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO:
Hey @crmne, can you please let me know if this looks to be heading in a direction that you're happy with architecturally? Cheers! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @rhys117!
Thanks for tackling this feature!
Issue: Inconsistent Naming
We've got with_reasoning()
, reasoning_content
, default_reasoning_budget
, but Anthropic's API calls it "thinking" - and we're mixing both terms in capabilities. This breaks our clean API pattern.
Solution: Standardize on "thinking"
Let's match Anthropic's terminology throughout:
# Method
def with_thinking(budget: nil)
# budget parameter instead of separate config call
end
# Response
response.thinking # not reasoning_content
response.thinking? # boolean check
# Config
default_thinking_budget # not reasoning
# Error
UnsupportedThinkingError
Usage:
chat = RubyLLM.chat(model: 'claude-3-7-sonnet')
.with_thinking(budget: 2048)
.with_temperature(1.0)
response = chat.ask("Short story about a dog")
puts response.thinking
puts response.content
This keeps our API consistent and beautiful. Thoughts?
That all sounds good to me, I'll take a look at this again in the coming days |
@temperature = 0.7 | ||
@thinking = @config.default_thinking | ||
@thinking_budget = @config.default_thinking_budget | ||
@temperature = @config.default_temperature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This default_temperature
doesn't exist:
undefined method 'default_temperature' for an instance of RubyLLM::Configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been corrected, thanks to @hiemanshu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm waiting on this feature, and it's kinda working for me. I had to remove the default_temperature
and also implement a thinking?
to the Model::Info
in a dumb way but could be more robust:
class RubyLLM::Model::Info
...
def thinking?
id.include?("opus-4") || id.include?("sonnet-4") || id.include?("3-7-sonnet")
end
...
end
Any chance to move on with this PR?
@rhys117, I would be happy to help push this PR through the finish line. Let me know if you want. |
@pedrofs the best would be to implement it for all providers. Do you want to take on the challenge? |
Hey, @crmne. I can put some work by the end of July. And will be happy to help! Great work in the gem btw! |
Get thinking working with bedrock
Hey @crmne and @pedrofs, sorry I started this and haven't had time to finish it off. I've merged some changes from @hiemanshu here as well around adding bedrock support. I will work on the 'acts as' support tomorrow and clean some of this up. I'll be pretty time poor again afterwards, so it would be great if you want to pick this up @pedrofs! |
Sounds good! Do you want me to keep adding things on this branch or patch mine and move from there? |
Whatever is easiest, I've added you and Hiemanshu as collaborators to my fork so you can work directly off this branch if you like |
- Add 'thinking' to anthropic capabilities - Add 'thinking' to bedrock capabilities (for anthropic supported models) - Update models.json file to reflect changes
- incorrectly using 'supports_thinking?' instead of 'thinking?'
- Adds chat_thinking_spec.rb - Adds THINKING_MODELS and includes anthropic provider models - Adds NON_THINKING_MODELS and includes anthropic provider models - Adds cassette files
- use ** instead of using including unused parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a little more here to clean up than I remembered. I've gone through and corrected the pieces that I can see needed attention to get this working within the RubyLLM::Message class.
Can someone with bedrock keys please add to the spec appropriately?
Unfortunately, I did not have time to look into the 'acts as' support. @crmne, out of curiosity, what do you think about merging this without it and then working towards iterating on this in another pull request to avoid this one getting any larger?
Manual testing -
3.2.2 :007 > chat = RubyLLM::Chat.new(model: 'claude-sonnet-4-20250514').with_thinking
=>
#<RubyLLM::Chat:0x0000000100d73e98
...
3.2.2 :008 > chat.ask("Give me a short story about a cat.").thinking
=> "I'll write a short story about a cat. Let me think of an interesting premise - maybe something heartwarming or with a little adventure. I'll keep it concise but engaging."
# Streaming
3.2.2 :010 > chat.ask("Give me a short story about a cat.") { |chunk| p chunk.thinking }
nil
nil
nil
"I"
"'ll write a short story about a cat."
" Let me think of an interesting premise - maybe something heartwarming or"
" with a little adventure. I'll keep it concise but"
" engaging."
nil
...
@temperature = 0.7 | ||
@thinking = @config.default_thinking | ||
@thinking_budget = @config.default_thinking_budget | ||
@temperature = @config.default_temperature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been corrected, thanks to @hiemanshu
@@ -11,7 +11,7 @@ def completion_url | |||
"models/#{@model}:generateContent" | |||
end | |||
|
|||
def render_payload(messages, tools:, temperature:, model:, stream: false) # rubocop:disable Lint/UnusedMethodArgument |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discarding unused params using '**' is my preference here, but would be keen to hear others' opinions here please
Resolves #154
Description
Add thinking support for Anthropic models.
Implementation Details
models.json
Message
via.thinking
TODO:
acts_as
persistent models