feat: Support deployment-level thinking defaults for chat templates - #11047
Conversation
Signed-off-by: Indrajit Bhosale <iamindrajitb@gmail.com>
Signed-off-by: Indrajit Bhosale <iamindrajitb@gmail.com>
Signed-off-by: Indrajit Bhosale <iamindrajitb@gmail.com>
WalkthroughAdds a deployment-level ChangesDefault Thinking Mode Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/bindings/python/rust/backend.rs (1)
297-345: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAppend the new
backend.WorkerConfigparameter instead of inserting it mid-signature.This changes the positional constructor contract for
backend.WorkerConfig(...): every argument afterreasoning_parsernow shifts one slot. Please movedefault_thinking_modeto the end of the PyO3 signature/parameter list and updatelib/bindings/python/src/dynamo/_core.pyito match.Suggested fix
#[pyo3(signature = ( namespace, component = "backend".to_string(), endpoint = "generate".to_string(), model_name = String::new(), served_model_name = None, model_input = ModelInput::Tokens, endpoint_types = "chat,completions".to_string(), custom_jinja_template = None, tool_call_parser = None, reasoning_parser = None, - default_thinking_mode = None, exclude_tools_when_tool_choice_none = true, enable_local_indexer = true, enable_kv_routing = true, metrics_labels = Vec::new(), runtime = None, disaggregation_mode = DisaggregationMode::Aggregated, health_check_payload = None, structural_tag_mode = "off".to_string(), structural_tag_scope = "auto".to_string(), structural_tag_schema = "auto".to_string(), route_to_encoder = false, + default_thinking_mode = None, ))] @@ custom_jinja_template: Option<String>, tool_call_parser: Option<String>, reasoning_parser: Option<String>, - default_thinking_mode: Option<String>, exclude_tools_when_tool_choice_none: bool, enable_local_indexer: bool, enable_kv_routing: bool, metrics_labels: Vec<(String, String)>, runtime: Option<RuntimeConfig>, disaggregation_mode: DisaggregationMode, health_check_payload: Option<PyObject>, structural_tag_mode: String, structural_tag_scope: String, structural_tag_schema: String, route_to_encoder: bool, + default_thinking_mode: Option<String>, ) -> PyResult<Self> {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/bindings/python/rust/backend.rs` around lines 297 - 345, The backend.WorkerConfig constructor signature changed the positional argument order by inserting a new parameter in the middle, which breaks existing callers. Update the PyO3 constructor in backend.rs so default_thinking_mode is appended at the end of the signature and parameter list after route_to_encoder, and mirror the same ordering in the backend.WorkerConfig type stub in lib/bindings/python/src/dynamo/_core.pyi to keep the Python API contract stable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/src/dynamo/common/backend/worker.py`:
- Line 120: Move the new WorkerConfig field default_thinking_mode to the end of
the class so the dataclass stays append-only and existing positional callers
keep binding correctly. Update the WorkerConfig definition by appending
default_thinking_mode after the current optional fields, rather than inserting
it before exclude_tools_when_tool_choice_none or any existing parameters.
---
Outside diff comments:
In `@lib/bindings/python/rust/backend.rs`:
- Around line 297-345: The backend.WorkerConfig constructor signature changed
the positional argument order by inserting a new parameter in the middle, which
breaks existing callers. Update the PyO3 constructor in backend.rs so
default_thinking_mode is appended at the end of the signature and parameter list
after route_to_encoder, and mirror the same ordering in the backend.WorkerConfig
type stub in lib/bindings/python/src/dynamo/_core.pyi to keep the Python API
contract stable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b0055b16-3d34-4118-ad8d-6ab9157ae694
📒 Files selected for processing (17)
components/src/dynamo/common/backend/tests/test_backend_bindings.pycomponents/src/dynamo/common/backend/worker.pycomponents/src/dynamo/common/configuration/groups/runtime_args.pycomponents/src/dynamo/frontend/prepost.pycomponents/src/dynamo/frontend/sglang_prepost.pycomponents/src/dynamo/frontend/sglang_processor.pycomponents/src/dynamo/frontend/tests/test_sglang_processor_unit.pycomponents/src/dynamo/frontend/tests/test_vllm_processor_unit.pycomponents/src/dynamo/frontend/thinking.pycomponents/src/dynamo/frontend/vllm_processor.pycomponents/src/dynamo/sglang/register.pycomponents/src/dynamo/trtllm/workers/llm_worker.pycomponents/src/dynamo/vllm/main.pylib/backend-common/src/worker.rslib/bindings/python/rust/backend.rslib/bindings/python/src/dynamo/_core.pyilib/llm/src/preprocessor.rs
|
🎯 Code Coverage (details) 🔗 Commit SHA: a839dd0 | Docs | Datadog PR Page | Give us feedback! |
…namo#11047) Adapt and apply ai-dynamo/dynamo PR ai-dynamo#11047 ("Support deployment-level thinking defaults for chat templates") onto feat/glm51-v12-20260610. Adds --dyn-default-thinking-mode enabled|disabled / DYN_DEFAULT_THINKING_MODE, published through model runtime metadata (runtime_data.default_thinking_mode) and applied in the SGLang/vLLM Python frontends and the Rust OpenAI preprocessor when a request carries no thinking control of its own. Request-level thinking/chat_template_args/chat_template_kwargs always win. Adaptations for this branch's older base (~855 commits behind the PR base): - preprocessor.rs: this branch's NvCreateChatCompletionRequest has no first-class `thinking` field, so request-level precedence checks `unsupported_fields` (where a root-level `thinking` lands) instead. - worker.rs: this branch's EngineConfig has no `runtime_data`, so the default-thinking runtime_data map is built fresh rather than cloned. - sglang_prepost.py: this branch lacks the newer-main `_normalize_openai_thinking_template_kwargs` helper; the deployment default is injected at the top of preprocess_chat_request (before reasoning gating and rendering) so both observe the same thinking state. - vllm/main.py: add missing `import json` for the set_engine_specific call. - The PR's frontend Python unit tests were not ported (written against newer-main fixtures); the Rust preprocessor unit tests are included. Original-author: Indrajit Bhosale <iamindrajitb@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Indrajit Bhosale <iamindrajitb@gmail.com>
Signed-off-by: Indrajit Bhosale <iamindrajitb@gmail.com>
Signed-off-by: Indrajit Bhosale <iamindrajitb@gmail.com>
…hinking-toggle Signed-off-by: Indrajit Bhosale <iamindrajitb@gmail.com>
hhzhang16
left a comment
There was a problem hiding this comment.
Approving as a gms-codeowner
|
Pulling in 0e1d1d8 to see if it helps with build timeouts |
|
Pulling in the grove deploy fix be9dd5a |
|
#12203 (just merged) should help with TRTLLM CI test failures |
Summary
Adds a deployment-level default thinking mode for chat-template rendering. This lets operators choose whether requests should default to thinking enabled or disabled while preserving request-level override priority.
The setting is applied at the prompt-preprocessing layer.
It does not directly toggle reasoning inside vLLM, SGLang, or TensorRT-LLM. Instead, Dynamo publishes the deployment setting through model runtime metadata and converts it into compatible chat-template arguments before rendering the model prompt.
Overview
Precedence
Thinking controls are resolved in the following order:
The following request controls suppress the deployment default when present:
thinkingchat_template_args.thinkingchat_template_args.enable_thinkingchat_template_args.thinking_modechat_template_args.reasoning_effortchat_template_kwargsaliasreasoning_effortthinking=falseenabledenable_thinking=truedisabledreasoning_effort=highdisableddisabledenabledDetails
--dyn-default-thinking-mode enabled|disabledand the equivalentDYN_DEFAULT_THINKING_MODEenvironment variable.default_thinking_mode.registration paths.
Rust OpenAI preprocessor.
contain a thinking control:
thinkingenable_thinkingthinking_modethinking,reasoning_effort,chat_template_args, andchat_template_kwargs.The setting remains best-effort at the model level. A model may still generate reasoning
if it ignores the corresponding chat-template control.
Validation
Unit coverage includes Rust preprocessor default injection and Python processor coverage
for the vLLM and SGLang paths. Runtime validation used the ARM64 vLLM image with
Qwen/Qwen3-0.6B; earlier E2E coverage also exercised Qwen3 and SmolLM3 deploymentdefaults and request overrides.
disabledis injected as all compatible template argumentsenabledis injected as all compatible template argumentsdisabledsurvives Kimi's implicit enabled normalizationthinkingnormalization suitedisabled, no request overrideenabled, no request overrideDYN_DEFAULT_THINKING_MODE=disabledchat_template_kwargs.enable_thinkingTracking