Add fix-tool-choice-enforcement mod - #380
Closed
tea24864 wants to merge 1 commit into
Closed
Conversation
Restores tool_choice enforcement on /v1/chat/completions. On affected builds, tool_choice="required" and named tool choice decode free-form: the model answers in prose and no tool call is produced. A "required" request is also malformed on the way out, reporting finish_reason "tool_calls" with tool_calls null, which breaks any client that branches on finish_reason. Enforcement is an xgrammar structural tag built in Parser.adjust_request. Two independent breaks keep it from reaching the sampler: 1. Nothing on the chat-completions path calls adjust_request. The only call site is the harmony branch of the Responses API. 2. On builds whose ParserManager.get_parser collapses a reasoning and tool adapter that share one engine, glm45 + glm47 among them, the returned engine class has no structural-tag code at all. Part 1 adds the call and is model-agnostic. Part 2 gives the collapsed GLM engine the tag its delegating counterpart would have applied, mirroring DelegatingParser._apply_structural_tag rather than inventing a second policy. Both parts are skipped when the installed vLLM does not need them. Measured on a 4x DGX Spark TP4 cluster with tool-eval-bench hardmode, GLM-5.3-Flash EXL3 6bpw: 159 to 161 points of 176, with the tool_choice=required compliance scenario flipping from fail to pass across three trials. Diffed scenario by scenario, the net gain is entirely this fix. Adds tests/test_tool_choice_enforcement_mod.sh, covering placement, source drift in either file, idempotency, non-GLM images, and an upstream that already applies the call.
Author
|
Closing this: I opened it against the wrong repository. This was meant for my own fork, not upstream. Sorry for the noise. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
On affected builds,
tool_choice="required"and named tool choice on/v1/chat/completionsdecode free-form. The model answers in prose and no toolcall is produced.
A
requiredrequest is also malformed on the way out:finish_reasonis"tool_calls"whiletool_callsisnull, because the serving layer forcesthat value and then finds nothing to report. Any OpenAI-compatible client that
branches on
finish_reasonbreaks on it.Reproduced on
vllm-node-b12xbuilds0.1.dev20489+ga50ebee1d.d20260904and0.1.dev20596+g2a979314d.d20260907, serving GLM-5.3-Flash with--reasoning-parser glm45 --tool-call-parser glm47 --enable-auto-tool-choice.Cause
Enforcement is an xgrammar structural tag built in
Parser.adjust_request. Twoindependent breaks keep it from reaching the sampler.
adjust_request. The onlycall site in
vllm/entrypoints/is the harmony branch of the Responses API,next to a TODO about unifying it with the non-harmony branch.
DelegatingParser.adjust_requestis the only implementation that calls_apply_structural_tag. WhenParserManager.get_parsercollapses areasoning and tool adapter that share one engine, which
glm45+glm47do, it returns the engine class itself, and that class has no structural-tag
code at all.
Fixing either half alone changes nothing on a collapsing build. Part 1 was
applied on its own and both probes returned byte-identical output.
The second break is specific to the builds this mod targets. Stock
vllm-project/vllmhas no_get_parser_engine_clsbranch, soget_parseralways returns a
DelegatingParsersubclass and part 1 alone is enough there.The mod README documents how to tell the two apart.
What the mod does
parser.adjust_request(request)in_create_chat_completion, after the parser is built and before the request isrendered, matching where the Responses API makes the call and where the tag
has to land to reach
to_sampling_params. Skipped when the installed vLLMalready calls it.
Glm47MoeParseranadjust_requestthat keepsthe engine's behaviour and then applies the tag, mirroring
DelegatingParser._apply_structural_tagrather than inventing a secondpolicy, including the
VLLM_ENFORCE_STRICT_TOOL_CALLINGcheck thatAbstractToolParser.get_structural_tagapplies. Skipped whenvllm/parser/glm47_moe.pyis not installed.Blast radius
The tag builder returns
Nonefortool_choice: "none", and for"auto"unless a tool declares
strict, so ordinary auto traffic decodes as before.Confirmed live: an auto request still calls the tool when the question needs
one, still answers directly when it does not, and the reasoning block is intact
either way.
VLLM_ENFORCE_STRICT_TOOL_CALLING=0disables the whole path, as upstream.The mod is opt-in. No shipped recipe declares it.
Validation
4x DGX Spark TP4 cluster,
tool-eval-benchv2.6.1 hardmode, 88 scenarios,GLM-5.3-Flash EXL3 6bpw,
0.1.dev20489:TC-45 is
tool_choice=required Compliance. Diffed scenario by scenario, tenscenarios moved and the nine besides TC-45 cancel to exactly zero, so the net
gain is entirely this fix. EXL3 4bpw moved 154 to 158 points with the same flip.
Across 17 scored hardmode runs, TC-45 tracks the engine build rather than the
quantization or checkpoint, including a
0.1.dev20596NVFP4-Spark run thatfails the same way and rules out the EXL3 checkpoints as the cause.
Tests
Adds
tests/test_tool_choice_enforcement_mod.sh, following the existing modtest convention. It covers call placement inside the
parser_clsguard andahead of rendering, no leakage into the streaming path, a self-contained
appended block, idempotency, fail-closed on source drift in either file,
non-GLM images, an upstream that already applies the call, and a missing vLLM
install.
All top-level recipes still dry-run clean.
🤖 Generated with Claude Code
https://claude.ai/code/session_01A5YuMY6DErgC5h2Uztz93b