Add 'none' reasoning effort to ChatCompletionRequest#36238
Add 'none' reasoning effort to ChatCompletionRequest#36238DarkLight1337 merged 9 commits intovllm-project:mainfrom
Conversation
|
Hi @juliendenize, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
There was a problem hiding this comment.
Code Review
This pull request adds support for 'none' as a value for reasoning_effort. However, the implementation is incomplete and could lead to incorrect behavior. When reasoning_effort is set to 'none', include_reasoning still defaults to True, which should be False. I've left a comment with a suggested fix using a Pydantic validator. Additionally, this change lacks unit tests to verify the new behavior.
6a94ce3 to
93352eb
Compare
|
Hi @juliendenize, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
c120361 to
0f6c53f
Compare
|
Hi @juliendenize, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
Signed-off-by: Julien Denize <julien.denize@mistral.ai>
Signed-off-by: Julien Denize <julien.denize@mistral.ai>
0f6c53f to
2f9794b
Compare
|
Hi @juliendenize, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
Signed-off-by: Julien Denize <julien.denize@mistral.ai>
… crashing PR vllm-project#36238 added "none" as a valid `reasoning_effort` value for the Chat Completion API, but the Harmony (GPT-OSS) rendering path was not updated. Sending `reasoning_effort="none"` to a Harmony model raises `ValueError("Harmony does not support reasoning_effort='none'")`, returning a 500 error to the client. The root cause is that the `REASONING_EFFORT` lookup dict only maps "high", "medium", and "low" to Harmony's `ReasoningEffort` enum. `"none"` (and the Responses API's `"minimal"`) are not in the dict. Fix: - In `_make_request_with_harmony`, treat `reasoning_effort="none"` as unset (`None`) so the system message omits the reasoning directive. The protocol validator already sets `include_reasoning=False`, which strips reasoning from both streaming and non-streaming responses. - In `get_system_message`, guard the dict lookup with an `in` check so unsupported effort values are silently ignored rather than raising `KeyError`. This also prevents the same crash for the Responses API path where `effort="minimal"` would hit the same dict. Fixes vllm-project#37909 Signed-off-by: dubin555 <dubin555@gmail.com>
Purpose
This PR adds support to 'none' reasoning effort.
This is something that is actually supported by the OpenAI client.
It is added to the ChatCompletionRequest but not to harmony.