[openai] Fix missing tool usage check (system message) #24768
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a bug that prevented tool usage with gpt-oss models. The issue was caused by the commentary channel being incorrectly removed from the system message, which is necessary for tool call functionality. The fix involves passing a with_custom_tools flag to the get_system_message function, determined by whether tools are present in the request. This change correctly preserves the commentary channel when tools are used. The fix is straightforward and effectively resolves the bug.
levunet
force-pushed
the
feat/fix_tool_check
branch
from
04938e5 to
ecafa3c
Compare
There was a problem hiding this comment.
Looks good to me, it would be great to add a unit test for this, there are some examples here the test likely doesn't even need the model to run, just that the messages are being generated properly here.
There was a problem hiding this comment.
A small nit would be that this will activate the commentary channel even if an empty list of tools is passed in, which is not uncommon. We could avoid that by using some like bool(request.tools) instead of request.tools is not None. But, I separately discovered this issue, Alec pointed me to this PR, and agree that this fix is needed to get accurate tool calling in Chat Completions with gpt-oss models.
Without this PR, I regularly see the models outputting non-built-in tool calls to the analysis channel, which isn't where they should go.
levunet
force-pushed
the
feat/fix_tool_check
branch
from
ecafa3c to
f9043ac
Compare
levunet
requested review from
DarkLight1337,
NickLucche,
robertgshaw2-redhat and
simon-mo
as code owners
github-project-automation
bot
moved this from To Triage
to Ready
in gpt-oss Issues & Enhancements
chaunceyjiang
added
the
ready
levunet
force-pushed
the
feat/fix_tool_check
branch
from
9f989c4 to
38382b4
Compare
Fixed a bug where the commentary value was missing in Invalid Channel due to the absence of with_custom_tools value when fetching the system message. Signed-off-by: kyt <[email protected]>
levunet
force-pushed
the
feat/fix_tool_check
branch
from
f42b60e to
33cc22e
Compare
|
This PR substantially improves Chat Completions streaming tool call handling for Harmony models, especially for gpt-oss-20b. Without this PR, the 20b model often (and 120b sometimes) outputs tool calls to the It may not reduce 100% of cases where tool calls inappropriately go to the |
|
@bbrowning |
aarnphm
left a comment
aarnphm
left a comment
There was a problem hiding this comment.
Hopefully this fixes. Thanks for this!
|
Just a note that the CI failures look unrelated, as both are failing in a disk space check on the docker build jobs. This PR doesn't change anything that would impact free disk space on the builder nodes. |
|
Any updates on this? Or has it been fixed somewhere else? |
|
I can confirm that by applying openai/harmony#76, #24768 and #24954 , codex works very well with gpt-oss-120b. |
|
Hey guys, any news here? Looking forward for this merge! |
github-project-automation
bot
moved this from Ready
to Done
in gpt-oss Issues & Enhancements
|
Sorry for the delay, merging now |
…24768) Signed-off-by: kyt <[email protected]> Signed-off-by: Rahul Tuli <[email protected]>
Signed-off-by: kyt <[email protected]> Signed-off-by: yewentao256 <[email protected]>
…24768) Signed-off-by: kyt <[email protected]> Signed-off-by: Tomer Asida <[email protected]>
…24768) Signed-off-by: kyt <[email protected]> Signed-off-by: Karan Goel <[email protected]>
…24768) Signed-off-by: kyt <[email protected]>
…24768) Signed-off-by: kyt <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>
@borishim could you share how you made that work besides applying the PRs? Is this with "--tool-call-parser openai --enable-auto-tool-choice" and responses API? Many thanks! |
Here are the switches I used: Note that I applied the patches against 0.10.2 docker images. Also, current codex cli expects chat completion API for gpt-oss. So I think that you should use chat completion API. |
|
I found a bug in vllm 0.11.0 where, after installing FlashInfer and using it by default for sampling, specific sentences would repeat infinitely or fail to generate responses during answer generation. If you need it, you can use the following method and it will work normally: VLLM_USE_FLASHINFER_SAMPLER=0 Also, I think this might be a minor mistake, but you should probably use 'openai_gptoss' for the reasoning-parser. |
…24768) Signed-off-by: kyt <[email protected]>
…24768) Signed-off-by: kyt <[email protected]>
…24768) Signed-off-by: kyt <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>
Sync to upstream's [v0.11.0](https://github.com/vllm-project/vllm/releases/tag/v0.11.0) release + a cherry pick of vllm-project/vllm#24768 This PR targets CUDA but may also be sufficient for ROCM. Dockerfile updates: - general updates to match upstream's Dockerfile - nvcc, nvrtc and cuobjdump were addded for deepgemm JIT requirementes: neuralmagic/nm-vllm-ent@2a545c8 - missing paths were added for triton JIT: neuralmagic/nm-vllm-ent@b3027fc Tests: Branch in nm-cicd: https://github.com/neuralmagic/nm-cicd/tree/sync-v0.11-cuda accept-sync: https://github.com/neuralmagic/nm-cicd/actions/runs/18270550524 -- please ignore unit tests, they need to be updated to v1. Image tested: quay.io/vllm/automation-vllm:cuda-18270550524 Image validation: https://github.com/neuralmagic/nm-cicd/actions/runs/18271507914 Whisper runs: https://github.com/neuralmagic/nm-cicd/actions/runs/18281815955/job/52046560584 https://github.com/neuralmagic/nm-cicd/actions/runs/18281511979
10 participants
Purpose
Fixed a bug where the commentary value was missing in Invalid Channel due to the absence of with_custom_tools value when fetching the system message.