Skip to content

fix(frontend): apply --default-chat-template-kwargs for vllm/sglang - #16570

Merged
KrishnanPrash merged 2 commits into
mainfrom
kprashanth/16530-sglang-default-cttk
Nov 4, 2026
Merged

KrishnanPrash merged 2 commits into
mainfrom
kprashanth/16530-sglang-default-cttk

Conversation

@KrishnanPrash

@KrishnanPrash KrishnanPrash commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Dynamo renders the chat template in the frontend, so an engine's --default-chat-template-kwargs never reaches Dynamo traffic. SGLang workers ignore it (#16530), and the vLLM and SGLang sidecars cannot see it. The frontend already took the flag, but only for --dyn-chat-processor vllm. This PR applies it on every backend and every chat processor. The chat, Responses and Anthropic handlers merge the defaults into each request before they read its thinking controls, so a default such as {"enable_thinking": false} acts like the same key sent by the client.

Repro (Qwen3-0.6B). Set the flag on the frontend; either backend gets it:

python -m dynamo.frontend --default-chat-template-kwargs '{"enable_thinking": false}'

# Backend A: SGLang worker
python -m dynamo.sglang --model-path Qwen/Qwen3-0.6B --dyn-reasoning-parser qwen3

# Backend B: SGLang engine plus the Dynamo sidecar
python -m sglang.launch_server --model-path Qwen/Qwen3-0.6B \
  --grpc-port 30001 --incremental-streaming-output
python -m dynamo.sglang.sidecar --grpc-endpoint 127.0.0.1:30001 --dyn-reasoning-parser qwen3

# Ask "What is 2+2? Answer in one word." with max_tokens=256.
Request Before After
no chat_template_kwargs the model thinks no reasoning, answer 2, same as native SGLang
{"enable_thinking": true} thinks thinks

Key Changes

  • Frontend owns template defaults: one flag reaches workers and sidecars, and no engine reports the value over gRPC.
  • Merge before thinking normalization: the handlers decide thinking and the parser mode from the request.
  • Precedence: a request key wins per key, except null and "auto", which count as unset, as in vLLM. A request thinking control replaces the defaults' thinking keys. The defaults rank above --dyn-default-thinking-mode.
  • vLLM spellings fold in: vLLM's underscore and dotted forms still work.
  • No model card change: checksums and worker sets stay the same; per-model card defaults are out of scope.

Validation

Live with SGLang 0.5.22 and vLLM 0.32.0 on SGLang and vLLM workers and sidecars, and on both Python chat processors: the default reached the prompt, and a request key won. Unit tests cover the merge, null and "auto", and the reasoning decision after the merge.

Breaking Changes

python -m dynamo.sglang and python -m dynamo.vllm refuse --default-chat-template-kwargs and name the frontend flag. The frontend flag cannot set chat_template.

Issues and Ongoing Work

Pull Request Checklist
  • I used Dynamo's /pr-checklist skill.

Summary by CodeRabbit

  • New Features
    • SGLang deployments can set default chat-template options that apply to requests across frontend processing paths. Options supplied with an individual request take precedence over deployment defaults.
    • Defaults cannot set thinking controls or chat-template selection; configure those through their dedicated options. Nonempty defaults are not supported with the SGLang tokenizer option.
  • Documentation
    • Updated SGLang guidance to explain default options, precedence, limitations, and worker behavior during configuration changes.

@KrishnanPrash
KrishnanPrash requested review from a team as code owners October 15, 2026 20:59
@github-actions github-actions Bot added fix documentation Improvements or additions to documentation backend::sglang Relates to the sglang backend frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` labels Oct 15, 2026
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: ai-dynamo/dynamo/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 4f1a5b01-c2ed-4d6b-b454-a28585e9d347
📥 Commits

Reviewing files that changed from the base of the PR and between d8be649 and f64a4dd.

📒 Files selected for processing: 11
  • components/src/dynamo/frontend/sglang_prepost.py
  • components/src/dynamo/frontend/sglang_processor.py
  • components/src/dynamo/frontend/tests/test_sglang_processor_unit.py
  • components/src/dynamo/sglang/args.py
  • components/src/dynamo/sglang/register.py
  • components/src/dynamo/sglang/tests/test_runtime_metadata.py
  • components/src/dynamo/sglang/tests/test_sglang_unit.py
  • docs/fern/pages/developer-guide/knowledge-base/modular-components/backends/sglang/reference-guide.md
  • docs/fern/pages/use-cases/tool-calling-and-reasoning/reasoning-parsing.md
  • lib/llm/src/model_card.rs
  • lib/llm/src/preprocessor.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The change publishes SGLang default chat-template kwargs in runtime metadata and applies them in the Rust preprocessor and SGLang frontend. Request-provided values take precedence. SGLang also rejects unsupported defaults and tokenizer-mode combinations.

Changes

Chat-template defaults

Layer / File(s) Summary
Validate and publish defaults
components/src/dynamo/sglang/args.py, components/src/dynamo/sglang/register.py, components/src/dynamo/sglang/tests/*, lib/llm/src/model_card.rs, docs/fern/pages/...
SGLang rejects defaults containing Dynamo-handled keys and rejects nonempty defaults with --use-sglang-tokenizer. It publishes nonempty defaults as runtime metadata. The model-card checksum includes the defaults contract. Tests and documentation cover these behaviors.
Apply defaults in the Rust preprocessor
lib/llm/src/preprocessor.rs
The preprocessor fills missing chat-template arguments from runtime defaults and preserves request-provided values.
Apply defaults in the SGLang frontend
components/src/dynamo/frontend/sglang_prepost.py, components/src/dynamo/frontend/sglang_processor.py, components/src/dynamo/frontend/tests/test_sglang_processor_unit.py
The frontend reads runtime defaults and passes them through worker-pool and inline preprocessing. Request kwargs override matching defaults.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Severity of issue fixed: Medium

Merge Risk: Minimal · up to f64a4

No actionable issue remains from this review; the change is ready to merge after normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Linked Issues check Passed Issue #16530 requires SGLang defaults to reach both frontend paths under request chat_template_kwargs, with request values winning. The PR publishes default_chat_template_kwargs in SGLang runtime …
Out of Scope Changes check Passed The changed argument validation, runtime metadata transport, processor plumbing, Rust preprocessor merge, checksum update, tests, and documentation directly support issue #16530. The checksum change k…
Title check Passed The title identifies the main frontend change and names the relevant --default-chat-template-kwargs setting. It also mentions vLLM, which is outside the stated scope, but the title remains substanti…
Description check Passed The pull request description includes all required sections: Overview, Key Changes, Validation, and Issues and Ongoing Work. It links issue #16530 and includes the repository checklist, with sufficien…
Full details: Linked Issues check

Explanation

Issue #16530 requires SGLang defaults to reach both frontend paths under request chat_template_kwargs, with request values winning. The PR publishes default_chat_template_kwargs in SGLang runtime metadata, passes it through SglangProcessor, and merges it in sglang_prepost.py and the Rust preprocessor. The supplied tests cover metadata publication, both merge paths, precedence, and request immutability. The startup validation also rejects unsupported control keys with guidance, which addresses the issue's warning/rejection alternative and prevents silent misuse.

Full details: Out of Scope Changes check

Explanation

The changed argument validation, runtime metadata transport, processor plumbing, Rust preprocessor merge, checksum update, tests, and documentation directly support issue #16530. The checksum change keeps workers with different defaults in separate worker sets, which is required for the published setting to take effect consistently. No unrelated implementation or documentation change is demonstrated.

Full details: Title check

Explanation

The title identifies the main frontend change and names the relevant --default-chat-template-kwargs setting. It also mentions vLLM, which is outside the stated scope, but the title remains substantially related to the changeset.

Full details: Description check

Explanation

The pull request description includes all required sections: Overview, Key Changes, Validation, and Issues and Ongoing Work. It links issue #16530 and includes the repository checklist, with sufficient implementation details and validation evidence.

Finishing Touches
Fix failing CI checks
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

@devin-ai-integration

devin-ai-integration Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

⏹️ Dynamo PR CI cancelled — run 41728830927 (attempt 1) on 02c7e3853b

Gate checks: ❌ backend-status-check · ✅ deploy-status-check · ✅ dynamo-status-check

Framework Build
vLLM ⏹️ 1 ✅ 5

Failure details

No job failed outright. The only non-passing job, vllm-runtime-efa / Build multi-arch cuda13.0, was cancelled because it hit the job's 1h timeout. It was not a fail-fast or manual cancel. backend-status-check fails only because it mirrors that job.

⏰ vllm-runtime-efa / Build multi-arch cuda13.0: job exceeded maximum execution time of 1h0m0s

Job: vllm-runtime-efa / Build multi-arch cuda13.0 · Cancelled step: Build and Push Test Image · Logs: gh api repos/ai-dynamo/dynamo/actions/jobs/126282944377/logs

#24 exporting manifest list sha256:b51cf01284417b0ca7870c806523e7ba2397947fc4ccb340f1d642976eedab89 done
#24 DONE 13.2s
#1 [internal] load build definition from Dockerfile.test
#28 [linux/arm64 internal] load metadata for ***.dkr.ecr.us-west-2.amazonaws.com/ai-dynamo/dynamo:02c7e3853b19bae78df6b9d8d4a43c42daaf2d85-vllm-runtime-efa
#29 [linux/arm64 internal] load metadata for docker.io/library/debian:bookworm-slim
#30 [internal] load .dockerignore
##[error]The operation was canceled.
Annotation: The job has exceeded the maximum execution time of 1h0m0s

The multi-arch runtime image built and was pushed (job started 23:42 UTC). The build of the follow-on test image started at 00:40 UTC, and GitHub cancelled the job at the 60-minute limit (00:42 UTC). The log has no build or compile error. The build ran out of time, so rerunning the failed jobs is likely enough, especially with the build cache now warm.

For agents
{"pr": 16570, "run_id": 41728830927, "run_attempt": 1, "head_sha": "02c7e3853b19bae78df6b9d8d4a43c42daaf2d85",
 "failures": [{"job": "vllm-runtime-efa / Build multi-arch cuda13.0", "job_id": 126282944377, "failed_step": "Build and Push Test Image", "conclusion": "cancelled", "signature": "job exceeded maximum execution time of 1h0m0s", "tests": [], "log_cmd": "gh api repos/ai-dynamo/dynamo/actions/jobs/126282944377/logs"}]}

Posted automatically by Devin for run 41728830927. Updated on every full-CI run of this PR.

@biswapanda biswapanda left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and independently reproduced #16530 against head f64a4ddc45e89f781750fc9dee933edbb6f98416.

The scoped source review found no concrete correctness defect in this fix: the worker publishes defaults in engine-specific runtime metadata; the Rust and SGLang frontend paths merge them per key below explicit request values; the process-pool initializer carries them into workers; and model-card identity includes the defaults. Explicit false survives the merge.

Reproduction used the actual cached GLM-5 tokenizer/chat template (revision c183ef8c61faee82855eca1ed9bb3a9a7ce3b0b2) and a multi-turn conversation containing earlier assistant reasoning:

  • Python pre-fix production module: deployment clear_thinking=false is ignored; earlier reasoning disappears (24 prompt tokens). Exact-head production module: reasoning is preserved (33 tokens). Five cases passed, including explicit true/false override, no defaults, and per-key merging.
  • Rust full OpenAIPreprocessor::generate with captured backend-bound token IDs: actual parent preprocessing/model-card modules reproduce the missing reasoning; restored exact-head modules pass all four cases (33/24/33/24 tokens). Other supporting source/dependencies stayed at head for these parent-module comparisons.
  • Real spawned ProcessPoolExecutor using production initializer/preprocessing worker: three cases passed. Existing focused argument guards (2), frontend merge (1), and Rust merge/checksum-isolation tests (2) passed.

These are real template/tokenization and operator-path checks, not GPU generation or a model-quality benchmark. Python used SGLang 0.5.19 and Transformers 5.12.1; the repository's supported SGLang version integration and live HTTP/discovery deployment were not exercised.

The remaining metadata test also passes with a freshly built, unmodified exact-head native extension loaded through a temporary overlay: get_runtime_config publishes {"clear_thinking": false} into real ModelRuntimeConfig runtime data (1 passed); an independent native engine-specific JSON roundtrip passes. The complete runtime-metadata test file also passes (62 tests) with those fresh bindings. The fixture mocks topology/bootstrap/capacity helpers, so this verifies publication through PyO3, not live worker discovery. The isolated build used the supported NIXL SDK-absent stub/dlopen mode; no transport validation is implied. The shared virtualenv was not changed.

Current GitHub checks still contain failures; these scoped local results do not establish CI health or merge readiness.

Additional supported-version validation (2026-10-16), unchanged head f64a4dd: reran with isolated repository-pinned SGLang 0.5.21 and exact-head native bindings. Original GLM-5 CPU preprocessing baseline remains RED; all five fixed cases, three spawned-process cases, runtime-metadata 62/62, CLI guards 2/2, and frontend merge 1/1 pass. Shared virtualenv unchanged. This closes the earlier installed-SGLang-version limitation for these paths; GPU inference, live HTTP serving, and CI health remain unverified.

@peilii peilii left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@KrishnanPrash
KrishnanPrash requested a review from a team as a code owner October 28, 2026 01:08
@KrishnanPrash KrishnanPrash changed the title fix(sglang): apply --default-chat-template-kwargs in the frontend fix(frontend): apply --default-chat-template-kwargs for every backend Oct 28, 2026

@akshatha-k akshatha-k left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc changes lgtm

@KrishnanPrash
KrishnanPrash requested a review from a team as a code owner October 29, 2026 18:50
@github-actions github-actions Bot added the backend::vllm Relates to the vllm backend label Oct 29, 2026

@rmccorm4 rmccorm4 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two reproduced P2 findings in the frontend defaults implementation.

Comment thread components/src/dynamo/frontend/main.py Resolved
Comment thread lib/llm/src/http/service/anthropic.rs Resolved

@biswapanda biswapanda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A design consideration - Currently the frontend flag sets the same defaults for all models. Models that need different defaults require request overrides or separate frontends.

Can each deployment publish its defaults through its Model Deployment Card (MDC), as it does for default_thinking_mode? With this design, one frontend can serve models with different settings. Where an engine cannot expose its native defaults, we could accept an explicit Dynamo-owned worker/sidecar option and publish it through MDC.

@KrishnanPrash what do you think?

@biswapanda biswapanda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline location for the unset-value issue described in #16570 (comment).

Comment thread lib/llm/src/frontend_config.rs Resolved

@biswapanda biswapanda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@KrishnanPrash KrishnanPrash changed the title fix(frontend): apply --default-chat-template-kwargs for every backend fix(frontend): apply --default-chat-template-kwargs for vllm/sglang Nov 2, 2026
@KrishnanPrash

Copy link
Copy Markdown
Contributor Author

A design consideration - Currently the frontend flag sets the same defaults for all models. Models that need different defaults require request overrides or separate frontends.

Can each deployment publish its defaults through its Model Deployment Card (MDC), as it does for default_thinking_mode? With this design, one frontend can serve models with different settings. Where an engine cannot expose its native defaults, we could accept an explicit Dynamo-owned worker/sidecar option and publish it through MDC.

@KrishnanPrash what do you think?

@biswapanda agree with the high-level feedback. For the upcoming v1.7 release, will unblock by merging this PR as-is to provide a frontend lever. And then draft a more comprehensive MDC proposal for future release.

@KrishnanPrash
KrishnanPrash force-pushed the kprashanth/16530-sglang-default-cttk branch from 087f8bf to 148325d Compare November 2, 2026 20:28
@biswapanda

Copy link
Copy Markdown
Contributor

A design consideration - Currently the frontend flag sets the same defaults for all models. Models that need different defaults require request overrides or separate frontends.
Can each deployment publish its defaults through its Model Deployment Card (MDC), as it does for default_thinking_mode? With this design, one frontend can serve models with different settings. Where an engine cannot expose its native defaults, we could accept an explicit Dynamo-owned worker/sidecar option and publish it through MDC.
@KrishnanPrash what do you think?

@biswapanda agree with the high-level feedback. For the upcoming v1.7 release, will unblock by merging this PR as-is to provide a frontend lever. And then draft a more comprehensive MDC proposal for future release.

Sounds good @KrishnanPrash.

@jthomson04 jthomson04 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One must-fix (the vLLM CPU-only test) and a few nits.

Comment thread lib/llm/src/frontend_config.rs Resolved
Comment thread lib/llm/src/frontend_config.rs Resolved
@KrishnanPrash
KrishnanPrash enabled auto-merge (squash) November 3, 2026 23:20
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
@KrishnanPrash
KrishnanPrash force-pushed the kprashanth/16530-sglang-default-cttk branch from 02c7e38 to 0c5a76c Compare November 4, 2026 17:19
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
@KrishnanPrash
KrishnanPrash merged commit e4aeea8 into main Nov 4, 2026
147 of 149 checks passed
@KrishnanPrash
KrishnanPrash deleted the kprashanth/16530-sglang-default-cttk branch November 4, 2026 19:52
jain-ria added a commit that referenced this pull request Nov 5, 2026
- Engine loss (#17972): api.v1 has no state stream, so poll GetModelInfo
  every 1 s, each poll bounded by min(connect_attempt_timeout, 5 s), and
  report "SGLang engine lost: ..." through main's engine_lost hook after 3
  failed polls in a row. HealthCheck is not polled: it runs a probe
  generation and fails while the scheduler is paused.
- Media (#16203): not ported. Requests with multi_modal_data keep the
  stack's "multimodal payloads are not supported" refusal; MediaSupport,
  media_support, require_media_support and the runtime.v1 media wire-tag
  test are dropped. Main's mm_processor_kwargs, media-uuid and
  encoder_result refusals stay.
- Native HTTP (#16983): keep the per-dispatch rid and inspect(). Abort
  goes through the api.v1 Abort RPC with the dispatch rid, falling back to
  ctx.id(); main's HTTP /abort_request call is dropped, and its
  malformed-output abort cases now check the Abort RPC.
- #16570: drop the default-chat-template-kwargs warning and its test;
  api.v1 GetServerInfo has no such field.
- Keep #17710's migration mapping and #16303's logit_bias refusal.
- Remove the runtime.v1 sglang.proto and headless.rs. proto/README.md now
  names sglang#45859 and #46637 as service.proto's sources.
- .pre-commit-config.yaml: union of both exclude patterns.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: jain-ria <riajain@NVIDIA.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::sglang Relates to the sglang backend backend::vllm Relates to the vllm backend documentation Improvements or additions to documentation fix frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: dynamo.sglang accepts SGLang's --default-chat-template-kwargs but silently ignores it

6 participants