Repository navigation
Conversation
…er tool arguments Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
The parse-only jinja2.Environment in _referenced_template_variables triggered bandit B701 (jinja2_autoescape_false). It only parses templates into an AST and never renders, so autoescape is immaterial to behavior; set autoescape=True to satisfy the security scan. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
…used-kwargs guard The guard treated every chat_template_kwargs key as a caller control, but the servers add keys of their own: Responses derives reasoning_effort and thinking from reasoning.effort, and the Anthropic adapter sends both clear_thinking (GLM templates) and drop_thinking (DeepSeek-V4) because it cannot know which one the template reads. Qwen3 templates read neither reasoning_effort nor thinking, so test_reasoning_effort was rejected with a 400, and a GLM template would reject every Anthropic request that keeps earlier-turn thinking. - validate_chat_template_kwargs takes injected_keys; apply_chat_template and async_apply_chat_template pass them through. ChatCompletionRequest gains a server-populated injected_chat_template_kwargs field so the exemption survives the edge -> worker relay; the Anthropic adapter fills it and the Responses render site derives it. Caller keys stay strict. - enable_thinking and thinking join ALWAYS_ALLOWED: the reasoning parsers read them from chat_template_kwargs regardless of the template, so a template that ignores them is not a no-op. Fixes test_disaggregated_videomme[qwen3vl_2b_instruct], whose evaluator passes enable_thinking=False to a template without the toggle. - The guard raises UnusedChatTemplateKwargsError (a ValueError); the disaggregated server maps it to HTTP 400 instead of the generic 500. - The tool-call parity test builds its request the way the server does, falling back to the raw body when lazy validation rejects object arguments, and pins which path each representation takes. - Register the guard tests in l0_cpu.yml; the router rejection test uses a key that is not parser-consumed. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
…enderers Strict-parse requests deliver assistant tool_calls as a single-use pydantic ValidatorIterator. The jinja template path already materializes it via _parse_assistant_message_content, but the native renderer path passed it through unmaterialized, so tokenizers that index message['tool_calls'] failed with "'ValidatorIterator' object is not subscriptable" (test_native_renderer_keeps_raw_arguments in CI). Materialize it to a list in the native path without normalizing the raw arguments. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
…mpty_content NemotronV3ReasoningParser reads force_nonempty_content from chat_template_kwargs after generation, while the standard Nemotron template never references the key. Pin that the guard accepts it on both render paths and that the same kwargs drive the parser's reasoning-to-content swap. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
brnguyen2
added
the
api-compatible
1 task done
This branch has not been deployed
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.
Description
Follow-up to #19351, requested in this review thread: #19351 exempted
force_nonempty_contentfrom the unused-chat_template_kwargsguard (commit df286e0) but did not add the regression test.Stacked on #19351 — only the last commit (
e7b4521f92) is new. Do not review the rest here.NemotronV3ReasoningParserreadsforce_nonempty_contentfromchat_template_kwargsafter generation to decide whether an empty-content response gets its reasoning swapped intocontent. The standard Nemotron template never references the key, so a guard that only accepts template-referenced kwargs rejected previously valid requests.New
TestParserConsumedControlsintests/unittest/llmapi/apps/test_chat_template_kwargs_guard.py:force_nonempty_contenton the server render path (apply_chat_template),render_chat_request_for_tokenizer), where it must reach the renderer unpruned since the parser reads the same dict,ReasoningParserFactory.create_reasoning_parser("nemotron-v3", ...)), pinning the consumer the exemption exists for.The test file is already registered in
l0_cpu.yml(whole-file entry), so the new tests run without a test-list change.Test Coverage
tests/unittest/llmapi/apps/test_chat_template_kwargs_guard.py— full file passes locally (48/48), plustests/unittest/llmapi/test_reasoning_parser.py(207/207).PR Checklist