fix: tool_choice=required bypasses format-specific parsers (#6821)

[AsadShahid04]

Problem

When tool_choice is set to "required" or a named function in a /v1/chat/completions or /v1/responses request, models that use non-JSON tool call formats (qwen3_coder XML, pythonic, harmony, DSML, etc.) have their tool calls silently ignored. The raw markup leaks into output_text instead of being parsed into structured function_call items. This is a 100% failure rate for affected parsers under tool_choice=required.

tool_choice: "auto" works correctly because it follows a different code path that sets the tool_call_parser.

Root Cause

In lib/llm/src/preprocessor.rs::apply_tool_calling_jail(), the match on tool_choice unconditionally activates Immediate JSON jail mode via tool_choice_required() / tool_choice_named() on the builder, WITHOUT also setting the tool_call_parser. The Immediate mode only understands raw JSON ({...} or [{...}]), so any model emitting a non-JSON format (XML <tool_call>...</tool_call>, pythonic, etc.) has its output silently dropped.

Fix

When a tool_call_parser is configured, always use marker-based jail mode regardless of the tool_choice value. This ensures the format-specific parser is invoked to detect and extract tool calls correctly. Immediate JSON mode is preserved as a fallback for required/named when no parser is configured (the model is expected to emit raw JSON in that case).

Files changed:

• lib/llm/src/preprocessor.rs — Prioritize marker-based parser over Immediate JSON mode when a parser is configured
• lib/llm/tests/test_jail.rs — Added regression test test_tool_choice_required_with_qwen3_coder_parser

Affected Parsers

qwen3_coder, nemotron_nano, pythonic, harmony, dsml, glm47, kimi_k2, and any future non-JSON parser.

Testing

• Added regression test covering tool_choice=required with qwen3_coder parser
• Existing tool_choice tests for JSON-only (no parser) paths pass unchanged
• Manually verified that tool_choice=auto behavior is unaffected

Fixes #6821

Summary by CodeRabbit

• New Features

  • Tool calling now supports format-specific parsers (e.g., XML-based parsers) alongside JSON-based parsing.

• Bug Fixes

  • Fixed tool-calling configuration to conditionally apply appropriate parsing modes based on parser availability.

• Tests

  • Added regression test coverage for XML-based tool parsing with required tool-choice behavior.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

👋 Hi AsadShahid04! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

[coderabbitai]

Walkthrough

This change fixes a bug where tool_choice: "required" was not parsing tool calls correctly. The preprocessor now conditionally applies marker-based parsing when a tool_call_parser is configured, instead of always using Immediate JSON modes for Named and Required tool choices, restoring proper XML tool call extraction for formats like qwen3_coder.

Changes

Cohort / File(s)	Summary
Tool-calling jail configuration fix lib/llm/src/preprocessor.rs	Updated apply_tool_calling_jail to conditionally select jail mode based on tool_call_parser presence. When a parser is configured, Named and Required tool choices now use marker-based parsing via builder.tool_call_parser(parser) instead of always defaulting to Immediate JSON jail modes.
Regression test for qwen3_coder parser lib/llm/tests/test_jail.rs	Added new async test test_tool_choice_required_with_qwen3_coder_parser that validates tool_choice: "required" correctly parses XML tool calls using qwen3_coder parser, ensuring parsed tool calls appear in output and raw XML markers do not leak into content.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the main issue: tool_choice=required bypasses format-specific parsers, which matches the core problem addressed in the changeset.
Description check	✅ Passed	The description comprehensively covers the problem, root cause, fix, affected files, parsers, and testing approach, closely following the template structure with all required sections filled.
Linked Issues check	✅ Passed	The code changes fully address the requirements in #6821: tool calls from non-JSON format parsers are now correctly extracted when tool_choice is required, and a regression test validates the qwen3_coder parser behavior.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the tool_choice=required parser bypass issue: the preprocessor logic targets the root cause, and the test directly validates the fix with qwen3_coder.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/llm/src/preprocessor.rs`:
- Around line 1116-1124: The parser-enabled branch for
ChatCompletionToolChoiceOption::Named currently calls
builder.tool_call_parser(parser) and drops the named.function.name enforcement,
allowing other tools; modify this path to keep marker-based parsing but enforce
the named tool by validating/filtering parsed calls against named.function.name
before emission — e.g., wrap or replace tool_call_parser with a small parser
wrapper that invokes the original parser and rejects or maps any parsed ToolCall
whose tool_name != named.function.name (or returns an error/empty choice), or
alternatively add a post-parse validation step that inspects the parsed
ToolCall.tool_name and only forwards/emit when it equals named.function.name;
ensure the change references ChatCompletionToolChoiceOption::Named,
tool_call_parser, builder.tool_call_parser, builder.tool_choice_named, and
named.function.name so the enforcement mirrors the fallback behavior.

In `@lib/llm/tests/test_jail.rs`:
- Around line 3064-3070: The test is constructing JailedStream directly so it
never exercises the tool_choice=required branch; change the test to call
OpenAIPreprocessor::apply_tool_calling_jail(...) with the parser and
Some(Required) (instead of using JailedStream::builder() directly) so the
Required decision path is executed and the same marker-based mode behavior is
validated; locate the test code that currently calls JailedStream::builder() and
replace that setup with a call to
OpenAIPreprocessor::apply_tool_calling_jail(parser, Some(Required), ...) and
assert the resulting jail state as before.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d81b1263-0a53-4caf-a242-e4ca9c1cd3aa

📥 Commits

Reviewing files that changed from the base of the PR and between aaa8a56 and 0cfba35.

📒 Files selected for processing (2)

• lib/llm/src/preprocessor.rs
• lib/llm/tests/test_jail.rs

[dagil-nvidia]

/ok to test 9888ada

[grahamking]

@AsadShahid04 can you fix the clippy errors? rust-clippy check.

[vvagias]

Rebased onto main and fixed CI (clippy lint + struct wrapper change).

[rmccorm4]

/ok to test 961d6c4

[MatejKosec]

Nice fix — the root cause analysis is spot-on and the approach (marker-based when parser is configured, immediate JSON as fallback) is clean. The named_tool_filter addition correctly preserves the tool_choice=named contract through the parser path.

One thing to address before merge: the markup leak when the filter drops all calls. A couple of nits inline as well.

[rmccorm4]

LGTM other than Matej's comments

[ishandhanani]

Fix lint please.

[huitianbai]

Fix lint please.

@ishandhanani @rmccorm4 Hi, I also encountered the same issue, and this PR seems not to filter function tools when tool_choice is "named“ function. I referred to sglang implementation (https://github.com/sgl-project/sglang/blob/main/python/sglang/srt/entrypoints/openai/serving_chat.py#L345).
And dynamo's logic: https://github.com/ai-dynamo/dynamo/blob/main/lib/llm/src/preprocessor/prompt/template/oai.rs#L425, it only emits all when tool choice is none. This will affect formatted prompt when tool choice is named function.

And I make a draft PR, maybe merge together #7808

[dagil-nvidia]

/ok to test 976a3f6

[vladnosiv]

Hi, I noticed this PR. Since I wrtied tool_choice support in december (#4722), I'm wondering what problem we're actually facing. In fact, what immediate JSON mode is using for all models is the intended behavior. Since it seems this is the only way to guarantee a tool call (except for structural tag for xgrammar).
This is achieved by passing through the guided decoding JSON schema generated for the tools. If the model does not generate JSON in this case, it means that there was no real forcing of the JSON schema.
For example, SGLang + Dynamo still doesn't seem to forcing json schema, here's a relevant PR from us: #6620

I didn't really understand how, after this PR, the tool call guarantee assumed for required/named modes is achieved.

[MatejKosec]

Hi, I noticed this PR. Since I wrtied tool_choice support in december (#4722), I'm wondering what problem we're actually facing. In fact, what immediate JSON mode is using for all models is the intended behavior. Since it seems this is the only way to guarantee a tool call (except for structural tag for xgrammar). This is achieved by passing through the guided decoding JSON schema generated for the tools. If the model does not generate JSON in this case, it means that there was no real forcing of the JSON schema. For example, SGLang + Dynamo still doesn't seem to forcing json schema, here's a relevant PR from us: #6620

I didn't really understand how, after this PR, the tool call guarantee assumed for required/named modes is achieved.

Hey Vlad, good to have the perspective of someone who built the original tool_choice support. You're right that true enforcement needs guided decoding wired through to the engine, which is what your PR #6620 was working toward.

Without it, the JSON mode before this PR doesn't actually guarantee a tool call since it is post-generation parsing — the model still generates freely, the jail just tries to interpret the output as JSON from token 1. There's no grammar constraint forcing the model to produce valid JSON. So the "guarantee" was already best-effort, while breaking 100% of the time for models that don't emit raw JSON (qwen3_coder, pythonic, harmony, etc.).

The other thing to consider is reasoning models, and how they have evolved over the past 6 to 12 months. Qwen3, DeepSeek-R1, GLM-5 all emit thinking tokens before their tool calls. Even a JSON-format model produces something like:

  <think>The user wants weather in Paris, I should call get_weather.</think>
  [{"name": "get_weather", "arguments": {"location": "Paris"}}]

The immediate JSON mode starts parsing from token 1 — it sees and fails because < isn't valid JSON. The actual tool call JSON is there, but the thinking prefix kills it. For non-JSON models like qwen3_coder the tool call itself is XML:

  <think>...</think>
  <tool_call>{"name": "get_weather", "arguments": {"location": "Paris"}}</tool_call>

The fix keeps Immediate JSON as the fallback when no parser is configured (simple JSON-only models that
don't think), and uses marker-based parsing when a parser is set. Both paths were already best-effort —
the difference is the marker-based path works across formats and improves compatibility with latest reasoning models.