Skip to content

fix(sglang): split Kimi K3 reasoning and strip terminal tokens in SGLang chat processor - #12395

Merged
furionw merged 1 commit into
mainfrom
qiwa/sglang-k3-multimodal
Aug 1, 2026
Merged

fix(sglang): split Kimi K3 reasoning and strip terminal tokens in SGLang chat processor#12395
furionw merged 1 commit into
mainfrom
qiwa/sglang-k3-multimodal

Conversation

@furionw

@furionw furionw commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Why

With --dyn-chat-processor sglang, Kimi-K3 responses come back with raw XTML protocol markers inside content and reasoning_content never populated. Two independent gaps cause it. First, kimi_k3 is missing from the thinking-by-default parser family, so force_reasoning resolves False and SGLang's streaming detector waits for an opening <|open|>think<|sep|> marker that K3 never emits — the chat template places it in the prompt, so generation begins already inside the think channel. Second, this mode detokenizes engine token IDs in the frontend and so bypasses SGLang's own trim_matched_stop, letting K3's terminal <|end_of_msg|> survive into the text; that ID is declared only in generation_config.json and differs from the tokenizer's EOS, so neither source alone is sufficient. The Rust chat processor resolves both centrally and is unaffected.

Effect

Request: What is 17 + 25? One word.

before:  content           = 'The user is asking...One word.<|close|>think<|sep|>42<|end_of_msg|>'
         reasoning_content = None

after:   content           = '42'
         reasoning_content = 'The user is asking a simple arithmetic question: 17 + 25...'

What Change

  • Register kimi_k3 in the thinking-by-default parser family and aliases.
  • Merge tokenizer and generation-config EOS IDs before the trailing-token strip.
  • Add unit coverage for EOS merge, thinking state, and media URL forwarding.

Test Plan

  • pytest components/src/dynamo/frontend/tests/test_sglang_processor_unit.py — 193 passed.
  • Verified serving Kimi-K3 at TP=16 on 16x GB200 via the SGLang chat processor.

@github-actions github-actions Bot added feat frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` labels Jul 29, 2026
@datadog-official

This comment has been minimized.

@furionw
furionw deleted the branch main July 30, 2026 02:39
@furionw furionw closed this Jul 30, 2026
@furionw furionw reopened this Jul 30, 2026
@furionw
furionw changed the base branch from qiwa/sglang-server-args-override to main July 30, 2026 02:43
@github-actions github-actions Bot added backend::sglang Relates to the sglang backend and removed size/XL labels Jul 30, 2026
@furionw
furionw force-pushed the qiwa/sglang-k3-multimodal branch from fd31d9c to 4c60564 Compare July 30, 2026 02:43
@copy-pr-bot

copy-pr-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

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.

@furionw
furionw marked this pull request as ready for review July 30, 2026 02:50
@furionw
furionw requested a review from a team as a code owner July 30, 2026 02:50

@devin-ai-integration devin-ai-integration Bot 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

Kimi-K3 reasoning preprocessing

Layer / File(s) Summary
Kimi-K3 parser and template flow
components/src/dynamo/frontend/sglang_prepost.py, components/src/dynamo/frontend/tests/test_sglang_processor_unit.py
Adds the kimi-k3 alias, enables Kimi thinking defaults, aligns thinking with force_reasoning, applies template defaults during preprocessing, and tests multimodal handling.

EOS token handling

Layer / File(s) Summary
Model-configured EOS token resolution
components/src/dynamo/frontend/sglang_processor.py, components/src/dynamo/frontend/tests/test_sglang_processor_unit.py
Merges tokenizer EOS IDs with values from generation_config.json, falls back when unavailable, and uses the merged IDs during engine creation with unit coverage.

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

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the problem, changes, impact, and tests, but it omits the required Related Issues section and reviewer-start guidance. Add the required Related Issues section with an issue link or no-issue confirmation, and identify the files or areas where reviewers should start.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the Kimi K3 reasoning fix and terminal-token stripping in the SGLang chat processor.

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]

This comment was marked as resolved.

@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.

LGTM, but needs merge conflicts addressed.

Is this validated e2e?

@furionw

furionw commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

pasting results

1. Rust path working correctly for SGLang — YES

content           : '42'
reasoning_content : 'The user is asking a simple arithmetic question: 17 + 25.
                     17 + 25 = 42 ... they said "one word"...'

Clean split, no channel markers, no protocol-token leak. The Rust preprocessor/parser (--dyn-chat-processor dynamo, the default) handles K3 correctly.

2. Your PR supports image requests — YES

[red]  prompt_tokens=132  content: 'Red'   reasoning: '...solid red square...'
[blue] prompt_tokens=132  content: 'Blue'  reasoning: '...solid blue color...'
┌─────────────┬───────────────────────────────────────────────────────────────────────────────┐
│             │                                                                               │
├─────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ Image       │ qiwa-dev-sglang-arm64-07-31-k3, official lmsysorg/sglang:kimi-k3-…-arm64 base │
├─────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ Unit tests  │ 189 passed                                                                    │
├─────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ Topology    │ TP=16, 4× GB200 nodes, one MNNVL domain                                       │
├─────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ Weight load │ 635.86 s (~10.6 min)                                                          │
├─────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ Verdict     │ Text ✅ · Image ✅ · reasoning split ✅ · no leak ✅                          │
└─────────────┴───────────────────────────────────────────────────────────────────────────────┘

@furionw
furionw enabled auto-merge (squash) July 31, 2026 22:07
@furionw
furionw force-pushed the qiwa/sglang-k3-multimodal branch from 4c60564 to c08da74 Compare July 31, 2026 22:10
@furionw
furionw requested review from a team as code owners July 31, 2026 22:10
devin-ai-integration[bot]

This comment was marked as resolved.

@furionw
furionw force-pushed the qiwa/sglang-k3-multimodal branch 2 times, most recently from 8f79d88 to 625ce2d Compare July 31, 2026 23:21
devin-ai-integration[bot]

This comment was marked as resolved.

@furionw
furionw force-pushed the qiwa/sglang-k3-multimodal branch from 625ce2d to e9bb08b Compare August 1, 2026 01:29

@devin-ai-integration devin-ai-integration Bot 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.

Devin Review found 2 new potential issues.

Open in Devin Review

Comment thread components/src/dynamo/frontend/sglang_prepost.py
Comment thread components/src/dynamo/frontend/sglang_prepost.py
@furionw
furionw force-pushed the qiwa/sglang-k3-multimodal branch from e9bb08b to 1bed051 Compare August 1, 2026 03:21
devin-ai-integration[bot]

This comment was marked as resolved.

@furionw
furionw force-pushed the qiwa/sglang-k3-multimodal branch from 1bed051 to 2c14d6d Compare August 1, 2026 04:20
@furionw furionw changed the title feat(sglang): align Kimi K3 chat and multimodal preprocessing fix(sglang): split Kimi K3 reasoning and strip terminal tokens in SGLang chat processor Aug 1, 2026
@github-actions github-actions Bot added fix and removed feat labels Aug 1, 2026
@furionw
furionw force-pushed the qiwa/sglang-k3-multimodal branch from 2c14d6d to 769aae5 Compare August 1, 2026 04:30
@furionw
furionw force-pushed the qiwa/sglang-k3-multimodal branch from 769aae5 to c97a354 Compare August 1, 2026 04:31

@devin-ai-integration devin-ai-integration Bot 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.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread components/src/dynamo/frontend/sglang_processor.py
@furionw
furionw merged commit 9cddb34 into main Aug 1, 2026
108 checks passed
@furionw
furionw deleted the qiwa/sglang-k3-multimodal branch August 1, 2026 23:53
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 fix frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants