-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Reasoning parser #3859
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
Reasoning parser #3859
Conversation
…esponse behavior.
…model, also handle first response while separating reasoning.
…ive just in case.
self.think_start_token = "<think>" | ||
self.think_end_token = "</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.
Can we extend this to all reasoning models? Not just dpsk R1. There might be different thinking tokens.
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 different reasoning models need different parers, and I add docs for it.
|
However, I can not pass my tests with |
possible related issue: #3730 (comment) |
self.think_start_token = think_start_token | ||
self.think_end_token = think_end_token | ||
self.pattern = re.compile( | ||
rf"{self.think_start_token}(.*?){self.think_end_token}", re.DOTALL |
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.
The most recent tokenizer hardcodes the opening <think>
tag: https://huggingface.co/deepseek-ai/DeepSeek-R1/commit/8a58a132790c9935686eb97f042afa8013451c9f
This means the text coming back from inference won't include <think>
, this is why I updated #3202 to assume the model is reasoning until </think>
is seen, it also strips out <think>
to handle the old chat template.
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.
The PR added the start token if it is missing:
# Add the start token to the beginning of the text.
text = self.think_start_token + text
You can see it in detect_and_parse
docs/backend/reasoning_parser.md
Outdated
```bash | ||
python -m sglang.launch_server --host 0.0.0.0 \ | ||
--model-path deepseek-ai/DeepSeek-R1-Distill-Qwen-14B \ | ||
--enable-reasoning --reasoning-parser deepseek-r1 |
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.
Appreciate the docs I was too lazy to add!
Would you consider also supporting the separate_reasoning
contract? For my use case we want inference users to be able to control whether reasoning_content
is separated, rather than set it as default behavior on sglang launch, which I understand some sglang users will want to do.
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.
you mean add a separate_reasoning
parameter in sending requests?
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.
Separating reasoning and non-reasoning outputs is super useful, and would love for that to be a toggle rather than always on or always off.
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.
Happy to merge the great changes from this PR in #3202 to try and get best of both worlds?
Or visa versa, @ShaoZhang0115?
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.
Updated #3202 to combine functionality form this PR, and added some unittests.
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.
Microsoft has already shipped the separate_reasoning api to production and intends to keep it there, so would very much like to have it merged into main instead of maintaining a fork.
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.
could you give a reference?
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.
Uh, as in the functionality being deployed by Microsoft?
If you've got access to GitHub models, then this request should show the functionality (I used http-yac vscode extension):
POST https://models.inference.ai.azure.com/chat/completions
Content-Type: application/json
Authorization: Bearer {{github_pat}}
X-Auth-Provider: Github
{
"model": "deepseek-r1",
"messages": [
{"role": "system", "content": "You are a helpful assistant."},
{"role": "user", "content": "Who won the world series in 2020?"}
],
"separate_reasoning": true,
"stream": false
}
Expected Response:
{
"id": "51438c1eda364aeb9d7ccecfca078165",
"object": "chat.completion",
"created": 1740765748,
"model": "deepseek-r1",
"choices": [
{
"index": 0,
"message": {
"role": "assistant",
"content": "The Los Angeles Dodgers won the 2020 World Series, defeating the Tampa Bay Rays in six games. The series was held at Globe Life Field in Arlington, Texas, marking the first time the World Series was played entirely at a neutral site. Corey Seager was named the World Series Most Valuable Player (MVP). This victory ended the Dodgers' 32-year championship drought, their previous title having been won in 1988.",
"tool_calls": null,
"reasoning_content": "Okay, the user is asking who won the World Series in 2020. Let me think. The World Series is the championship series of Major League Baseball (MLB) in the United States and Canada. Each year, the champions of the American League and the National League compete in a best-of-seven playoff. \n\nFirst, I need to recall which teams played in the 2020 World Series. The 2020 season was unique because of the COVID-19 pandemic. The season was shortened to 60 games, and the playoffs were held in a bubble format to minimize travel and reduce the risk of infection. The World Series itself was held at a neutral site, which was Globe Life Field in Arlington, Texas. That was the first time the World Series was played entirely at a single neutral site.\n\nNow, thinking about the teams. In 2020, the American League champions were the Tampa Bay Rays. They won the AL pennant by defeating the Houston Astros in the AL Championship Series. On the National League side, the Los Angeles Dodgers won the NL pennant by beating the Atlanta Braves in the NL Championship Series. So the World Series was between the Dodgers and the Rays.\n\nWait, let me confirm that. The Rays were indeed the AL champions in 2020. The Dodgers had been a strong team in recent years but hadn't won the World Series since 1988. In 2020, they finally clinched the title. The series went to six games. The Dodgers won Game 6 to take the series 4-2. Corey Seager was named the World Series MVP. \n\nSo, putting it all together, the Los Angeles Dodgers won the 2020 World Series against the Tampa Bay Rays. It's important to note the context of the pandemic affecting the season structure, which might be a follow-up question from the user. But the direct answer is the Dodgers. I should make sure there's no confusion with other years. For example, the Dodgers also won in 2020, then again in 2020? Wait, no, they won in 2020. They might have won again more recently, but the question is specifically about 2020. \n\nDouble-checking a reliable source would be good, but based on my existing knowledge, I believe the Dodgers are correct. Let me think if there's any chance I mixed up the year. No, 2020 was their first since 1988. Then they won again in 2020? Wait, no, that can't be. Wait, no, they won in 2020. Wait, no, they won in 2020. Let me confirm. Yes, the Dodgers won the 2020 World Series. The Rays haven't won a World Series yet. So the answer is the Los Angeles Dodgers. \n\nYes, that's correct. The user might be interested in the MVP or the context, but the main answer is the Dodgers.\n"
},
"finish_reason": "stop"
}
],
"usage": {
"prompt_tokens": 19,
"total_tokens": 710,
"completion_tokens": 691,
"prompt_tokens_details": null
}
}
How does that work with grammars? Does the grammar kick-in only after the reasoning parser? |
Have a similar question about this as well, though I don't think it's specific to this PR or #3202 , the reasoning parsers (and tool parsers) operate at the level of text coming out of the underlying engine to the API layer. |
Ideally we would be able to pass a grammar for reasoning and a grammar for content, but I believe the default grammar behavior should apply only to the content. |
docs/backend/reasoning_parser.md
Outdated
for chunk in response: | ||
if chunk.choices[0].delta.content: | ||
content += chunk.choices[0].delta.content | ||
elif chunk.choices[0].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.
Is this functioning correctly now? When I test the feature for the vllm, it triggers an error from the OpenAI Python client.
Please note that it is not compatible with the OpenAI Python client library. You can use the requests library to make streaming requests.
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 is correct and tested. The variable is changed and OpenAI python client library supports custom variables.
class DeltaMessage(BaseModel):
role: Optional[str] = None
content: Optional[str] = None
reasoning_content: Optional[str] = None
tool_calls: Optional[List[ToolCall]] = Field(default=None, examples=[None])
return text, "" | ||
else: | ||
# Add the start token to the beginning of the text. | ||
text = self.think_start_token + text |
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 isn't backwards compatible with the old chat template which didn't include the <think>
token, would add it twice.
docs/backend/reasoning_parser.md
Outdated
```bash | ||
python -m sglang.launch_server --host 0.0.0.0 \ | ||
--model-path deepseek-ai/DeepSeek-R1-Distill-Qwen-14B \ | ||
--enable-reasoning --reasoning-parser deepseek-r1 |
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.
Uh, as in the functionality being deployed by Microsoft?
If you've got access to GitHub models, then this request should show the functionality (I used http-yac vscode extension):
POST https://models.inference.ai.azure.com/chat/completions
Content-Type: application/json
Authorization: Bearer {{github_pat}}
X-Auth-Provider: Github
{
"model": "deepseek-r1",
"messages": [
{"role": "system", "content": "You are a helpful assistant."},
{"role": "user", "content": "Who won the world series in 2020?"}
],
"separate_reasoning": true,
"stream": false
}
Expected Response:
{
"id": "51438c1eda364aeb9d7ccecfca078165",
"object": "chat.completion",
"created": 1740765748,
"model": "deepseek-r1",
"choices": [
{
"index": 0,
"message": {
"role": "assistant",
"content": "The Los Angeles Dodgers won the 2020 World Series, defeating the Tampa Bay Rays in six games. The series was held at Globe Life Field in Arlington, Texas, marking the first time the World Series was played entirely at a neutral site. Corey Seager was named the World Series Most Valuable Player (MVP). This victory ended the Dodgers' 32-year championship drought, their previous title having been won in 1988.",
"tool_calls": null,
"reasoning_content": "Okay, the user is asking who won the World Series in 2020. Let me think. The World Series is the championship series of Major League Baseball (MLB) in the United States and Canada. Each year, the champions of the American League and the National League compete in a best-of-seven playoff. \n\nFirst, I need to recall which teams played in the 2020 World Series. The 2020 season was unique because of the COVID-19 pandemic. The season was shortened to 60 games, and the playoffs were held in a bubble format to minimize travel and reduce the risk of infection. The World Series itself was held at a neutral site, which was Globe Life Field in Arlington, Texas. That was the first time the World Series was played entirely at a single neutral site.\n\nNow, thinking about the teams. In 2020, the American League champions were the Tampa Bay Rays. They won the AL pennant by defeating the Houston Astros in the AL Championship Series. On the National League side, the Los Angeles Dodgers won the NL pennant by beating the Atlanta Braves in the NL Championship Series. So the World Series was between the Dodgers and the Rays.\n\nWait, let me confirm that. The Rays were indeed the AL champions in 2020. The Dodgers had been a strong team in recent years but hadn't won the World Series since 1988. In 2020, they finally clinched the title. The series went to six games. The Dodgers won Game 6 to take the series 4-2. Corey Seager was named the World Series MVP. \n\nSo, putting it all together, the Los Angeles Dodgers won the 2020 World Series against the Tampa Bay Rays. It's important to note the context of the pandemic affecting the season structure, which might be a follow-up question from the user. But the direct answer is the Dodgers. I should make sure there's no confusion with other years. For example, the Dodgers also won in 2020, then again in 2020? Wait, no, they won in 2020. They might have won again more recently, but the question is specifically about 2020. \n\nDouble-checking a reliable source would be good, but based on my existing knowledge, I believe the Dodgers are correct. Let me think if there's any chance I mixed up the year. No, 2020 was their first since 1988. Then they won again in 2020? Wait, no, that can't be. Wait, no, they won in 2020. Wait, no, they won in 2020. Let me confirm. Yes, the Dodgers won the 2020 World Series. The Rays haven't won a World Series yet. So the answer is the Los Angeles Dodgers. \n\nYes, that's correct. The user might be interested in the MVP or the context, but the main answer is the Dodgers.\n"
},
"finish_reason": "stop"
}
],
"usage": {
"prompt_tokens": 19,
"total_tokens": 710,
"completion_tokens": 691,
"prompt_tokens_details": null
}
}
…ust model references
c8429d4
to
94bee72
Compare
Motivation
Rewrite #3202
Modifications
--enable-reasoning
and--reasoning-parser
options for deepseek r1 series models.reasoning_content
as in official api, ref: https://api-docs.deepseek.com/zh-cn/guides/reasoning_model, in both streaming and non-streaming chat completions.Example:
Get response:
Docs with be updated as soon as possible.
Checklist