-
Notifications
You must be signed in to change notification settings - Fork 1.3k
add reasoning content to ChatCompletions #494
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?
add reasoning content to ChatCompletions #494
Conversation
42aeffc
to
5a88d0e
Compare
5a88d0e
to
2ea9d85
Compare
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.
Looks reasonable - can you please add:
- Unit tests
- A working example that shows this?
if not emit_reasoning_content: | ||
emit_reasoning_content = True | ||
|
||
reasoning_content_title = "# reasoning content\n\n" |
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 doesn't seem right - why hardcode?
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.
It's a markdown title for splitting the content and reasoning content. It's a constant value so have to hardcode.
The whole output are like below:
# reasoning content
Okay, I need to write a haiku about recursion in programming. Let's start by recalling what a haiku is. It's a three-line poem with a 5-7-5 syllable structure. The first line has five syllables, the second seven, and the third five again. The theme here is recursion in programming, so I should focus on elements that capture the essence of recursion.
.....
# content
**Haiku on Recursion:**
Function calls itself,
Base case breaks the looping chain—
Stack grows, then falls back.
Another way is use <think></think>
The whole output would be like this:
<think>
Okay, I need to write a haiku about recursion in programming. Let's start by recalling what a haiku is. It's a three-line poem with a 5-7-5 syllable structure. The first line has five syllables, the second seven, and the third five again. The theme here is recursion in programming, so I should focus on elements that capture the essence of recursion.
.....
</think>
**Haiku on Recursion:**
Function calls itself,
Base case breaks the looping chain—
Stack grows, then falls back.
Which way do you prefer?
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 think neither? IMO it would be better to emit a separate item for reasoning. For example, I was trying something like this in #581. What do you think?
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.
Yeah, I agree with you.
I just found out openai-python 1.7.6 already added the types needed for emit reasoning content.
from openai.types.responses import (
ResponseReasoningItem,
ResponseReasoningSummaryTextDeltaEvent,
ResponseReasoningSummaryPartAddedEvent,
ResponseReasoningSummaryPartDoneEvent,
ResponseReasoningSummaryTextDoneEvent
)
So we can emit ResponseReasoningSummaryTextDeltaEvent for reasoning content or create some class like ResponseReasoningTextDeltaEvent in this repo?
What do you think?
if content is not None: | ||
if not emit_content and is_reasoning_model: | ||
emit_content = True | ||
content_title = "\n\n# content\n\n" |
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.
same here?
@@ -62,9 +65,16 @@ async def handle_stream( | |||
continue | |||
|
|||
delta = chunk.choices[0].delta | |||
reasoning_content = None | |||
content = None | |||
if hasattr(delta, "reasoning_content"): |
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.
when would this be true?
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.
When using reasoning model like deepseek-reasoner
deepseek reasoning model
OK |
fix issue #415