Repository navigation
Conversation
…as SGLang does SGLang v0.5.23 removed its encoding_dsv4 / encoding_dsv41 encoders and renders DeepSeek-V4 / V4.1 through deepseek-recipe. #4548 kept miles on vendored copies of the old encoders, so miles and the server each own a renderer and every DeepSeek format change needs a manual copy. V4 / V4.1 now render through SGLang's DeepSeekRecipe (convert plus encode_conversation), so a prompt equals the chat endpoint's render of the same request. The one deliberate difference is the V4 effort profile: the V4-Flash / V4-Pro checkpoints ship the "preview" profile, so high renders no effort line and max renders the line recipe calls high. Text renders decode the ids. recipe rejects a history whose last assistant turn awaits tool results, so such a history renders with placeholder results that are then cut off. TITO's V4 delta diffs recipe's ids; it falls back to the text diff only at a plain-text junction, such as a user turn after a tool result, where tokens merge across the boundary. Encoder-only kwargs (drop_thinking, context, add_default_bos_token) are now rejected. V4 / V4.1 prompts change where recipe differs from the vendored encoders: tool JSON keeps the client's key order without an injected strict, a developer turn renders as system, a V4 mid-conversation system turn moves to the front, special-token spellings in content stay text, and without tools earlier reasoning is not rendered. deepseek-recipe is now installed for the hosted CPU suite and in the ROCm image, whose v0.5.22 base predates it. Needs the DeepSeekRecipe API from sgl-project/sglang branch jiajun/deepseek-recipe-convert-api.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
@claude review always |
- Import SGLang's recipe renderer on first use, so images whose SGLang lacks it (Rubin, TPU) still import miles for every model. - Cut a pending-call history after its trailing assistant turn: recipe drops an empty system turn, which left a placeholder tool result in the prefix and broke the next TITO append. - Resolve "none" and numeric-string efforts as SGLang's chat endpoint does. - At a TITO plain-text junction, re-encode only the text up to the next special token with the BPE alone, so special-token spellings in content stay text as in recipe's render. - Reject an unknown thinking_mode; surface recipe's error for a tool call without an id instead of a KeyError. - Install deepseek-recipe in the ROCm CI job, and fix prose the deleted encoders left stale.
There was a problem hiding this comment.
I re-reviewed after the latest commit and found no new bugs. This change replaces the V4/V4.1 render path with an external renderer, so a human look would still be worthwhile.
What was reviewed:
- The follow-up commit moves the renderer import into
_renderer(), sodeepseek.pyno longer needs the module when it loads. TheDeepSeekRecipeannotation stays underTYPE_CHECKING, together withfrom __future__ import annotations. _placeholder_tool_resultsnow readscall.get("id"), so a pending tool call with no id no longer raises.DeepSeekRecipeFamily._encode, the TITO ids diff with its plain-text fallback, and the deepseek-recipe pins in CI and Docker.
Extended reasoning...
The PR deletes about 1,600 lines of vendored V4/V4.1 encoders. In their place, V4/V4.1 rendering and the V4 TITO incremental tokenization now go through sglang's deepseek-recipe renderer, and deepseek-recipe==0.2.0 is pinned in the ROCm CI workflow, the ROCm Dockerfile and the CPU CI requirements. It touches no security-sensitive surface such as auth, crypto or untrusted input handling. The latest commit addresses both earlier inline points: the renderer is imported lazily, and pending tool calls without an id no longer fail. The change is large and alters rendered prompts by design (tool JSON key order, developer turns, dropped earlier reasoning). The PR says the e2e and ROCm image runs have not been done yet, which argues for a human reviewer.
The check sat in DeepSeekRecipeFamily._encode, which sessions never reach with a bad value: the TITO tokenizer resolves thinking at construction and rewrites the kwargs to a boolean, so a typo such as "think" silently ran in chat mode. Validate in DeepSeekFamily._build_encode_config, which both paths share; a null thinking_mode keeps meaning chat.
guapisolo
added
the
run-ci-rollout
|
/rerun-test tests/e2e/sglang/test_session_server_multi_role/test_deepseekv4.py |
|
/rerun-test tests/e2e/megatron/model_scripts/test_deepseek_v4_1_4layer_ci.py |
|
/rerun-test tests/e2e/megatron/model_scripts/test_deepseek_v4_flash_4layer_ci.py |
|
✅ |
|
✅ |
|
❌ The execution job failed; inspect the workflow run for the failing step. |
…swer DeepSeek V4 / V4.1 now render through deepseek-recipe, which rejects an assistant tool call that no tool message answers. After a ROLLBACK step re-sampled a turn that called a tool, the verifier appended FORCE_FINAL to it, so 12 of 50 V4 session e2e trajectories failed with a 500. USER_FOLLOWUP, SYSTEM_REMINDER and ASSISTANT_INPUT could do the same. Before any step other than TOOL_RESULT and ROLLBACK, roll such a call back the way the tool-call ROLLBACK fallback does, then run the step.
|
/rerun-test tests/e2e/sglang/test_session_server_multi_role/test_deepseekv4.py |
|
❌ The execution job failed; inspect the workflow run for the failing step. |
…t back A harness that receives a tool call runs it before asking for anything else, and FORCE_FINAL asks the model to summarize all results. Answer the pending call with the next mock tool result, as TOOL_RESULT does, before a step that appends another turn, rather than dropping the call.
…e fails The per-sample sidecar already records the first expected/actual pair of each assistant_text mismatch, but CI logs only carried the ratio, so a failed gate could not be read without the preserved sidecar file. Quote the first three examples in the assertion message.
|
/rerun-test tests/e2e/sglang/test_session_server_multi_role/test_deepseekv4.py |
|
✅ |
|
/rerun-test tests/e2e/sglang/test_session_server_multi_role/test_glm47.py |
|
/rerun-test tests/e2e/sglang/test_session_server_multi_role/test_inkling.py |
|
/rerun-test tests/e2e/sglang/test_session_server_multi_role/test_nemotron3.py |
|
/rerun-test tests/e2e/sglang/test_session_server_multi_role/test_qwen3.py |
|
/rerun-test tests/e2e/sglang/test_session_server_multi_role/test_qwen35.py |
|
/rerun-test tests/e2e/sglang/test_session_server_multi_role/test_qwen36.py |
|
/rerun-test tests/e2e/sglang/test_session_server_multi_role/test_qwen38small.py |
|
✅ |
|
✅ |
|
✅ |
|
✅ |
|
✅ |
|
✅ |
|
✅ |
deepseek-recipe's output parser strips the whitespace a V4 turn emits before its content, such as a blank line after </think>. A re-render of the stored turn then drops it, and the session comparator reports an assistant_text mismatch on every sample that sampled one. Store that whitespace on the server-side message, restore it when a client replays the turn, and render it back before the content; the client's message is unchanged. Inkling's content_blocks restore now uses the same SERVER_MESSAGE_KEYS mechanism.
A real V4 parse failure returns empty content, while the step's reply keeps the call text in content. The blank line after </think> is not the reason: the recipe parser strips it in both modes, which #4596 now handles.
Summary
Render DeepSeek-V4 / V4.1 through SGLang's
DeepSeekRecipe, as its chat endpoint does.Motivation
SGLang v0.5.23 renders V4 / V4.1 through deepseek-recipe and removed
encoding_dsv4/encoding_dsv41. #4548 kept miles on vendored copies, so every DeepSeek format change needed a manual copy.Usage
No new flag;
apply_chat_templateand the V4 / V4.1 TITO tokenizers keep their interface.Design Notes
DeepSeekRecipeFamilybuilds the chat endpoint's request body, and SGLang'sDeepSeekRecipe.convert/encode_conversationturn it into prompt ids. Other families are unchanged.sglang-milesanddevrebuilds; then drop theci-sglang-prline._DSV4_PREVIEW_EFFORT.strict; developer renders as system; a V4 mid-conversation system turn moves first.id, emptytool_calls, and unanswered tool calls, as at the endpoint, except a trailing turn's calls.content_leading_whitespaceand renders it back. The client's message is unchanged.preserve_server_message_staterestores that key on a strict match, through theSERVER_MESSAGE_KEYSpath that Inkling'scontent_blocksnow shares.Verification
test_render_matches_the_chat_endpoint: 9 scenarios × thinking × V4 / V4.1 equal the endpoint's ids at SGLang83e38f97c7.chat_template_utilsCPU tests ate49a15b51: 1653 passed locally.test_template.pyis left tostage-a-cpu./rerun-testate7f0b08b4: all eighttest_session_server_multi_role/files passed./rerun-testat551e5e303: both V4 4-layer CI files passed.test_deepseekv4.pyate49a15b51, and the ROCm image build.Review Focus
DeepSeekRecipeFamily._encode: request body, placeholder cut, header strip.DeepSeekV4TITOTokenizer.postprocess_completion: which whitespace it stores.assistant_texton every sample, since recipe renders no reasoning there; training tokens are unchanged.ci-sglang-pr: jiajun/deepseek-recipe-convert-api