Repository navigation
chat : DeepSeek-V4.1 chat template, detection and DSML tool calls - #294
danielhanchen wants to merge 7 commits into
Conversation
…calls Add the V4.1 chat template and detect it in common_chat. V4.1 keeps the V4 DSML tool call format but its tag names carry a leading space, so the DeepSeek parser handles both. The converter embeds the V4.1 template in place of the V4 one. Add V4.1 parser cases to test-chat.
…parators The V4.1 template reads keys the OpenAI message struct drops (task, per-message tools, tool call namespace) and joins content blocks with blank lines around images. Keep unknown message keys on common_chat_msg and restore them, with media markers as separate text blocks, only for templates using the V4.1 DSML tags. Fixes encoding.py golden cases 3-5 on the common_chat apply path.
…em-message tools The V4.1 template only accepts low/high/max or 1-100 and raises otherwise, so map minimal -> low, xhigh -> max and medium -> unset (template default) instead of failing the request. Tools declared only on a system message now also feed the tool call parser and grammar. Both are V4.1 only.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ec84b857b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| foreach_function(tools, [&](size_t tool_index, const json & tool) { | ||
| const auto & function = tool.at("function"); | ||
| std::string name = function.at("name"); |
There was a problem hiding this comment.
When a V4.1 system-message tool supplies namespace (or function.namespace), the template advertises and renders the tool as namespace::name, but common_chat_params_init_deepseek_v3_2() builds the PEG rule from only function.name. Consequently, the grammar blocks the qualified call and the response parser rejects it, so namespaced tools supported by the new template cannot actually be invoked. Build the parser's tool name using the same qualification logic as the template.
Useful? React with 👍 / 👎.
| json tools = inputs.tools; | ||
| if (is_v41 && !(tools.is_array() && !tools.empty()) && inputs.messages.is_array()) { | ||
| for (const auto & msg : inputs.messages) { | ||
| if (msg.value("role", "") == "system" && msg.contains("tools") && msg.at("tools").is_array()) { |
There was a problem hiding this comment.
When both top-level tools and a leading system message's tools field are present, the template keeps the system-message list (not first.get("tools") prevents attaching the top-level list), while this condition prevents the parser from reading any system-message tools because inputs.tools is nonempty. The prompt therefore advertises one set of tools while the grammar/parser accepts a different set, breaking calls to the advertised tools. Select tools according to the same precedence used by the template.
Useful? React with 👍 / 👎.
| {%- if thinking_mode is not defined or thinking_mode is none -%} | ||
| {%- if enable_thinking is defined and enable_thinking is not none -%} | ||
| {%- set thinking_mode = "thinking" if enable_thinking else "chat" -%} |
There was a problem hiding this comment.
The documented thinking_mode kwarg takes precedence over enable_thinking here, but the specialized parser still branches solely on inputs.enable_thinking. For example, with a server default of enable_thinking=false and request-level thinking_mode="thinking", the template emits <think>..., while the parser only looks for a bare </think> and returns the reasoning tags and trace as ordinary content (or rejects a constrained tool response). Derive the parser mode from the same effective thinking_mode resolution as the template.
Useful? React with 👍 / 👎.
| {%- set task = m.get("task") -%} | ||
| {%- if task is not none -%} | ||
| {%- set task_tokens = {"action": "<｜action｜>", "query": "<｜query｜>", "authority": "<｜authority｜>", "domain": "<｜domain｜>", "title": "<｜title｜>", "read_url": "<｜read_url｜>"} -%} | ||
| {%- if task not in task_tokens -%}{{- raise_exception("Invalid task: '" ~ task ~ "'") -}}{%- endif -%} | ||
| {%- if task == "action" -%}{{- "<｜Assistant｜>" ~ ("<think>" if is_thinking else "</think>") -}}{%- endif -%} | ||
| {{- task_tokens[task] -}} |
There was a problem hiding this comment.
For a final message with task, this branch emits the transition token even when add_generation_prompt is false. Therefore the renders used by common_chat_template_generation_prompt_impl() with the flag off and on are identical, its computed generation prompt is empty, and common_chat_session prepends nothing to model output even though the specialized PEG parser requires <｜Assistant｜> at the start. Every task completion, including the tested action case, consequently fails response parsing; make the task transition participate in generation-prompt detection or teach the parser about these task prefixes.
Useful? React with 👍 / 👎.
| if (has_tool_calls) { | ||
| foreach_function(inputs.tools, [&](size_t tool_index, const json & tool) { | ||
| foreach_function(tools, [&](size_t tool_index, const json & tool) { | ||
| const auto & function = tool.at("function"); | ||
| std::string name = function.at("name"); |
There was a problem hiding this comment.
The template deliberately accepts both OpenAI-wrapped tools and bare function schemas via tool.function if tool.function is defined else tool, but the specialized parser sends system-message tools through foreach_function(), which skips every entry lacking type: "function" and a function member. A bare tool is thus advertised in the prompt while the resulting grammar contains no rule capable of accepting its call. Normalize bare entries before building the PEG rules or extend the iteration logic to match the template.
Useful? React with 👍 / 👎.
| {%- set first = mns.msgs[0] -%} | ||
| {%- if tools and not first.get("tools") -%}{%- set first = dict(first, tools=tools) -%}{%- endif -%} | ||
| {%- if response_format and not first.get("response_format") -%}{%- set first = dict(first, response_format=response_format) -%}{%- endif -%} |
There was a problem hiding this comment.
This template preserves an existing system-message response_format instead of replacing it with the top-level one, but the specialized parser still builds structured-output grammar exclusively from inputs.json_schema. If both schemas are supplied, the prompt instructs the model to follow the system-message schema while sampling enforces the different top-level schema; if only the message field is supplied, no schema grammar is installed at all. Resolve response-format precedence once and use the same effective schema for rendering and grammar construction.
Useful? React with 👍 / 👎.
| {%- if prev is not none and prev.role == "user" and prev.content_blocks is defined and prev.get("task") is none -%} | ||
| {%- set mns.msgs = mns.msgs[:-1] + [dict(prev, content_blocks=prev.content_blocks + blocks)] -%} |
There was a problem hiding this comment.
When an ordinary user message is followed by another user message carrying task, this merge condition checks only the previous message. It then keeps prev and appends only the new content blocks, silently discarding the second message's task, so the requested task transition is never emitted. Either avoid merging when the current message has a task or explicitly transfer its task metadata to the merged message.
Useful? React with 👍 / 👎.
| if (k != "role" && k != "content" && k != "tool_calls" && k != "reasoning_content" && | ||
| k != "name" && k != "tool_call_id") { | ||
| extra[k] = v; |
There was a problem hiding this comment.
An assistant message that uses the newly supported thinking alias stores that value only in extra_json; it never populates reasoning_content. For a trailing assistant prefill containing only thinking, common_chat_msg::empty() consequently reports the continuation message as empty and the specialized continuation path is skipped entirely; with other content present it still chooses the no-reasoning branch and loses the reasoning prefix. Parse this alias into reasoning_content (or make continuation handling consult it) so the advertised alias works for continue_final_message and automatic assistant prefills.
Useful? React with 👍 / 👎.
|
Pushed 29b3b4b: namespaced DeepSeek V4.1 tool calls now parse. The template writes the tool name qualified with its namespace, but the parser built the expected tool-name literal from the bare |
Adds the DeepSeek-V4.1 chat template, its detection in
common_chat, and DSML tool-call parsing. These were moved out of the minimal PR (#289).Stacked on the runtime PR (#293,
dsv41-runtime). The diff is only this change: 6 files, +585/-18, of which 275 lines are the template file.What it adds
models/templates/deepseek-ai-DeepSeek-V4.1-Flash-1017.jinja): the template shipped with the model. The converter embeds it in place of the inherited DeepSeek-V4 template.common/chat.cpp,common/parsers/deepseek.cpp): V4.1 keeps the V4 DSML tool-call format, but its tag names carry a leading space, so the DeepSeek parser handles both.task, per-messagetools, the tool call namespace) and joins content blocks with blank lines around images. Unknown message keys are kept oncommon_chat_msgand restored, with media markers as separate text blocks, only for templates using the V4.1 DSML tags.reasoning_effort: the template only acceptslow/high/maxor 1-100 and raises otherwise.minimalmaps tolow,xhightomax, andmediumto unset (the template default), instead of failing the request.test-chat.Verification
test-chatpasses on this branch and at the top of the stack.encoding.pygolden cases 3-5 now match on thecommon_chatapply path.Known gaps