-
Notifications
You must be signed in to change notification settings - Fork 420
models - litellm - start and stop reasoning #947
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
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.
Would an integ test be helpful here? Ensuring that we get back reasoning content and normal content in the stream?
) | ||
|
||
if choice.delta.content: | ||
if started_reasoning: |
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.
Can a model switch between streaming reasoning content and text content in a single streamed response? If so, I think we might need to change this implementation:
Reasoning: Let me think about this problem
Text: I have thought about this problem, let me think about a solution
Reasoning: Im thinking about a solution
Text: I have thought about a solution
Not sure if any models follow this pattern?
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.
Initially I tried to account for this but the code was a bit more complex. Since I wasn't aware of any model supporting this pattern, I decided instead to take a simpler approach and just implement for the pattern where reasoning is streamed first. I'm open to this being more flexible though.
afa31f7
to
40bd6ff
Compare
|
||
|
||
def test_stream_reasoning(skip_for, model): | ||
skip_for([cohere, gemini, llama, mistral, openai, writer], "reasoning is not supported") |
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.
Note, reasoning is supported in gemini but there is a similar bug to what we are fixing here for litellm. We need to yield a content stop block. Will address in a follow up PR.
Description
We need to explicitly mark the start and stop of the reasoning content stream in the litellm model provider. With these changes, users can now do the following:
The result of the above script will look something like:
Related Issues
#523
Documentation PR
N/A
Type of Change
Bug fix
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run prepare
: Updated the unit tests.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.