Repository navigation
feat: Enable structural tags by default and improve Kimi K3 guidance … - #15916
Conversation
There was a problem hiding this comment.
Questions for the author:
- components/src/dynamo/common/configuration/groups/runtime_args.py: The default now enables structural tags with scope=always, which makes supported vLLM/SGLang/Rust chat preprocessing paths attempt structural-tag guidance for auto tool requests that previously used the conservative off/auto default. Please provide the per-request latency/allocation or grammar-build cost evidence for those supported inference paths, especially auto tool requests with typical tool lists, so the runtime impact of enabling this by default can be assessed.
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: The previously reported SGLang explicit-null tool_choice issue is still present:
request.get("tool_choice", "auto")leaves a JSON null asNone, so the newtool_choice_kindclassification falls through to"other"instead of the OpenAI default"auto". A request withtool_choice:null, tools, andparallel_tool_calls:falsetherefore skips structural-tag guidance on the Python SGLang path while omitted/auto requests enforce it. - Original discussion: Verified still present at the reviewed head: the SGLang path still reads
tool_choice = request.get("tool_choice", "auto"), so an explicit JSON null remainsNoneand is classified asotherinstead of the OpenAI defaultauto. A supported request withtool_choice:null, tools, andparallel_tool_calls:falsetherefore bypasses the default structural-tag single-call/tool grammar on this path. NormalizeNoneto"auto"before deriving the SGLang tool-choice kind.
Questions for the author:
- components/src/dynamo/common/configuration/groups/runtime_args.py: The defaults now enable structural tags with scope
always, so supported vLLM/SGLang/Rust chat preprocessing paths attempt structural-tag guidance for auto tool requests that previously used the conservative off/auto default. Please provide the per-request latency/allocation or grammar-build cost evidence for those supported inference paths, especially auto tool requests with typical tool lists, so the runtime impact of enabling this by default can be assessed. - components/src/dynamo/common/configuration/groups/runtime_args.py: The deployment default still enables structural tags with
scope=always, so supported vLLM/SGLang/Rust chat preprocessing paths now attempt structural-tag guidance for auto tool requests that previously used the conservative off/auto default. Please provide the per-request latency/allocation or grammar-build cost evidence for those supported inference paths, especially auto tool requests with typical tool lists, so the runtime impact of enabling this by default can be assessed.
Explicit-null
|
✅ Dynamo PR CI passed — run 37830028154 (attempt 2) on
|
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: Verified still present at the reviewed head: the SGLang path still reads
tool_choice = request.get("tool_choice", "auto"), so an explicit JSON null remainsNoneand is classified asotherinstead of the OpenAI defaultauto. A supported request withtool_choice:null, tools, andparallel_tool_calls:falsetherefore bypasses the default structural-tag single-call/tool grammar on this path. NormalizeNoneto"auto"before deriving the SGLang tool-choice kind. - Original discussion: Verified still present at the reviewed head: the SGLang path still reads an explicit JSON null
tool_choiceasNone, sotool_choice_kindfalls through tootherinstead of the OpenAI defaultauto. A request withtool_choice:null, tools, andparallel_tool_calls:falsetherefore skips structural-tag guidance on the Python SGLang path while omitted/auto requests enforce it; normalizeNoneto"auto"before deriving the SGLang tool-choice kind.
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: The explicit-null SGLang
tool_choicedefect remains:request.get("tool_choice", "auto")leaves null asNone, classifying it asotherand bypassing structural-tag guidance instead of applying the OpenAIautodefault. - Original discussion: The explicit-null SGLang tool_choice defect remains present: JSON null stays None and is classified as "other" rather than OpenAI's default "auto", so eligible requests bypass structural-tag guidance. The author acknowledged this and is tracking the fix in #15922.
- Original discussion: The previously reported
runtime_structural_tag_optionsdocstring remains at line 14 and still only restates the function name and body. - Original discussion: The previously reported
should_attempt_structural_tagdocstring remains at line 33 and still merely expands the function name into a sentence. - Original discussion: The reasoning-gate assertion remains multiplied across the disaggregation capability matrix even though gate publication does not branch on the disaggregation dimensions and is already covered by its dedicated prerequisite test.
- Original discussion: The redundant
separate_reasoningaxis remains ontest_tool_structural_tag_enforced_after_one_reasoning_boundary; it still doubles every grammar case without affecting the compiled grammar or asserted reasoning boundary. - Original discussion: Verified still present: explicit JSON null
tool_choiceremainsNoneand is classified asother, causing the Python SGLang path to skip structural-tag guidance that omitted orautorequests receive. NormalizeNoneto"auto"before deriving the tool-choice kind. - Original discussion: Verified still present: the SGLang path leaves an explicit JSON null tool_choice as None and classifies it as "other" rather than the OpenAI default "auto". Requests with tools and parallel_tool_calls:false therefore bypass the default structural-tag constraint. Normalize None to "auto" before deriving the tool-choice kind.
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: The previously reported SGLang explicit-null tool_choice defect remains present: build_tool_call_guided_decoding still reads request.get("tool_choice", "auto"), so an explicit JSON null is left as None and classified as "other" rather than the OpenAI default "auto". Supported requests with tool_choice:null, tools, and structural-tag activation conditions therefore bypass the structural-tag guidance that omitted/auto tool_choice receives.
- Original discussion: Verified still present:
build_tool_call_guided_decodingstill readstool_choice = request.get("tool_choice", "auto"), so an explicit JSON null remainsNoneand is classified asotherinstead of the OpenAI defaultauto. A supported SGLang request withtool_choice:null, tools, andparallel_tool_calls:falsetherefore bypasses structural-tag guidance that omitted orautorequests receive. NormalizeNoneto"auto"before computing the forced-choice flags andtool_choice_kind. - Original discussion: Verified still present: the SGLang path leaves an explicit JSON null
tool_choiceasNone, so it is classified asotherinstead of the OpenAI defaultauto. Eligible requests with tools andparallel_tool_calls:falsetherefore bypass structural-tag guidance on this path; normalizeNoneto"auto"before deriving the tool-choice kind.
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: The
runtime_structural_tag_optionsdocstring is still present and still only restates that the function reads structural-tag options from runtime config. - Original discussion: The
should_attempt_structural_tagdocstring is still present and still only expands the function name into a sentence. - Original discussion: The comment above the guided-decoding assertion is still present and still narrates the assertion that follows.
- Original discussion: The module docstring in
test_pinned_vllm_structural_tag_registry.pyis still present and still duplicates the file and test names. - Original discussion: The reasoning-gate publication assertion is still embedded in
test_runtime_config_publishes_supported_disagg_capabilities, so it remains multiplied across the disaggregation capability matrix even though gate publication does not branch on those disaggregation dimensions and is already covered bytest_structural_tag_reasoning_policy_advertises_only_an_available_gate. - Original discussion: The redundant
separate_reasoningaxis remains ontest_tool_structural_tag_enforced_after_one_reasoning_boundary; it still doubles each grammar case even thoughseparate_reasoningonly affects response splitting and does not change the compiled structural-tag grammar or the asserted reasoning boundary. - Original discussion: The redundant docstring in
test_forced_structural_tag_returns_tool_callis still present and still restates the test name without preserving additional rationale. - Original discussion: The redundant docstring in
test_named_tool_choice_creates_json_array_parseris still present and still only narrates the named-choice setup plus theJsonArrayParserassertion. - Original discussion: The redundant docstring in
test_required_creates_json_array_parseris still present and still restates the test name and immediateJsonArrayParserassertion. - Original discussion: Verified still present at the reviewed head:
build_tool_call_guided_decodingstill readstool_choice = request.get("tool_choice", "auto"), so an explicit JSON null remainsNoneand is classified asotherinstead of the OpenAI defaultauto. A supported SGLang request withtool_choice:null, tools, andparallel_tool_calls:falsetherefore bypasses structural-tag guidance that omitted orautorequests receive. - Original discussion: Verified still present at the reviewed head: the SGLang path still reads
tool_choice = request.get("tool_choice", "auto"), so an explicit JSON null remainsNoneand is classified asotherrather than the OpenAI defaultauto. Eligible requests with tools andparallel_tool_calls:falsetherefore bypass structural-tag guidance that omitted orautorequests receive. - Original discussion: Verified still present: the SGLang path still reads
tool_choice = request.get("tool_choice", "auto"), so an explicit JSON null remainsNoneand is classified asotherinstead of the OpenAI defaultauto. Eligible SGLang requests withtool_choice:null, tools, andparallel_tool_calls:falsetherefore bypass structural-tag guidance that omitted orautorequests receive; normalizeNoneto"auto"before deriving the forced-choice flags and tool-choice kind. - Original discussion:
test_forced_structural_tag_returns_tool_callstill crosses both forced tool-choice cases with both preprocessing locations;use_poolonly selects where the same preprocessing runs and does not alter required-versus-named handling, so the pairwise required-inline/named-pool coverage remains sufficient. - Original discussion:
test_hyv4_structural_tag_matches_checkpoint_tokensstill crossesstream_responsethrough every suffix and tool-choice grammar case even though streaming is only consulted at postprocessor construction after the same HYV4 grammar has already been built and compiled. One streaming representative plus the batch suffix/tool-choice matrix preserves the distinct contracts. - Original discussion:
test_structural_tag_respects_named_tool_choicestill includesrequiredin thetool_choicematrix even though it asserts the same observable asautohere: both declared tools are accepted. The existing shared guidance-policy test covers required activation, while retaining auto and named preserves the baseline-versus-filtered-name behavior.
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: The SGLang Python path still treats an explicit JSON null tool_choice as neither omitted nor "auto": build_tool_call_guided_decoding reads request.get("tool_choice", "auto"), so a present null becomes None, tool_choice_kind becomes "other", and configured structural-tag guidance is skipped for requests that are semantically auto. Normalize None to "auto" before deriving the tool-choice kind and SGLang tool_choice.
- Original discussion: The comment before the
structural_tagassertion intest_forced_structural_tag_returns_tool_callis still present and still narrates the immediately following assertion. - Original discussion: The docstring on
runtime_structural_tag_optionsis still present and still only restates that the function reads structural-tag options from runtime config. - Original discussion: The docstring on
should_attempt_structural_tagis still present and still only expands the function name into a sentence; the branching below expresses the policy. - Original discussion: The docstring on
test_forced_structural_tag_returns_tool_callis still present and still restates the test function name without adding a lasting constraint or rationale. - Original discussion: The docstring on
test_named_tool_choice_creates_json_array_parseris still present and still just narrates the named-tool-choice setup and the followingJsonArrayParserassertion. - Original discussion: The docstring on
test_required_creates_json_array_parseris still present and still restates the test name and immediateJsonArrayParserassertion. - Original discussion: The module docstring in
test_pinned_vllm_structural_tag_registry.pyis still present and still restates the file and test names. - Original discussion: The reasoning-gate assertion is still inside test_runtime_config_publishes_supported_disagg_capabilities, so it remains multiplied across the disaggregation capability matrix even though gate publication does not branch on those dimensions and is already covered by test_structural_tag_reasoning_policy_advertises_only_an_available_gate.
- Original discussion: The separate_reasoning axis is still present on test_tool_structural_tag_enforced_after_one_reasoning_boundary; it still doubles the grammar cases even though separate_reasoning only affects response splitting and does not change the compiled structural-tag grammar or asserted reasoning boundary.
- Original discussion: The two-line comment above
test_auto_schema_enforces_omitted_strictis still present and still restates the adjacent tests and assertions about omitted versus explicitstrict: falsehandling. - Original discussion: Verified still present at the reviewed head: the Python SGLang path still reads
tool_choice = request.get("tool_choice", "auto"), so an explicit JSON null remainsNoneand is classified asotherinstead of the OpenAI defaultauto. Eligible SGLang requests with tools andparallel_tool_calls:falsetherefore bypass structural-tag guidance that omitted orautorequests receive; normalizeNoneto"auto"before deriving the forced-choice flags and tool-choice kind. - Original discussion: Verified still present at the reviewed head: the SGLang preprocessing path still reads
tool_choice = request.get("tool_choice", "auto"), so an explicit JSON null remainsNoneand is classified asotherrather than the OpenAI defaultauto. Eligible requests with tools andparallel_tool_calls:falsetherefore bypass structural-tag guidance that omitted orautorequests receive; normalizeNoneto"auto"before deriving the forced-choice flags and tool-choice kind. - Original discussion: test_forced_structural_tag_returns_tool_call still crosses both forced tool-choice cases with both preprocessing locations; use_pool only selects where the same preprocessing runs and does not alter required-versus-named handling, so pairwise required-inline and named-pool coverage remains sufficient.
- Original discussion: test_hyv4_structural_tag_matches_checkpoint_tokens still crosses stream_response through every suffix and tool-choice grammar case even though streaming is only consulted at postprocessor construction after the same HYV4 grammar has already been built and compiled. One streaming representative plus the batch suffix/tool-choice matrix preserves the distinct contracts.
- Original discussion: test_structural_tag_respects_named_tool_choice still includes required in the tool_choice matrix even though it asserts the same observable as auto here: both declared tools are accepted. The shared guidance-policy test covers required activation, while retaining auto and named preserves the baseline-versus-filtered-name behavior.
…(#13654)
(cherry picked from commit 3213fe6)
Overview:
CP for #13654
Refs DIS-2743