-
Notifications
You must be signed in to change notification settings - Fork 1.6k
add reasoning content to ChatCompletions #494
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # 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 |
This PR is stale because it has been open for 10 days with no activity. |
fix issue #415