Repository navigation
[rollout] fix: render mapping tool-call arguments through DeepSeek chat templates - #7768
Closed
ruiling-smartbear wants to merge 1 commit into
Closed
ruiling-smartbear wants to merge 1 commit into
ruiling-smartbear wants to merge 1 commit into
Conversation
…at templates
verl stores tool-call arguments as a mapping, which chat templates render with
| tojson. The DeepSeek-R1, V3 and V3.1 templates concatenate
tool['function']['arguments'] into the prompt instead, so every full-history
render of a conversation with a tool-call turn raised
TypeError: can only concatenate str (not "dict") to str
from the initial-prompt render in ContinuousTokenBuilder, the multi-turn SFT
dataset and scripts/chat_template_checker.py. The checker could not validate
the DeepSeek family at all, and the dummy-user retry in apply_chat_template
failed on the same conversation with its own error, hiding the first one.
apply_chat_template now retries a TypeError once with the arguments serialized
as JSON strings, the form these models emit and the DeepSeek API returns for
the same call. Templates that render mappings themselves never raise, so they
keep seeing the original messages; a TypeError with nothing to convert
propagates as before. The history is copied, not mutated.
DeepSeekContinuousTokenBuilder no longer needs to stringify its synthetic tool
call for the append render to work, but keeps doing so. The base builder now
renders the same append through the concatenating template, and the existing
test reflects that instead of expecting the TypeError.
Tests cover the wrapper on a concatenating and a tojson-style template, the
V3.1 tool-call branch under a sandboxed Jinja environment, and the DeepSeek-R1
template rendered through transformers' render_jinja_template. That template is
vendored under tests/utils; its tool-call branch only runs for assistant
messages whose content is None.
Contributor
Author
|
Closing this one: it only affects the DeepSeek-R1 / V3 / V3.1 Jinja templates. DeepSeek-V4 renders through its own encoder ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes #7664.
verl keeps tool-call
argumentsas a mapping through the message history (theOpenAIFunctionCallSchemashape), which templates render with| tojson. The chat templates shipped with DeepSeek-R1, DeepSeek-V3 and DeepSeek-V3.1 concatenatetool['function']['arguments']into the prompt instead, so every full-history render of a conversation with a tool-call turn raisedTypeError: can only concatenate str (not "dict") to str: the initial-prompt render inContinuousTokenBuilder,MultiTurnSFTDataset, andscripts/chat_template_checker.py, which therefore could not validate the DeepSeek family at all. The dummy-user retry inapply_chat_templatethen failed on the same conversation with its own error and hid the first one.apply_chat_templatenow retries aTypeErroronce with the arguments serialized as JSON strings (json.dumps(..., ensure_ascii=False)), the form these models emit and the DeepSeek API returns for the same call. Templates that render mappings themselves never raise, so they keep seeing the original messages; aTypeErrorwith nothing to convert propagates as before; the history is copied, never mutated.Two things this PR deliberately does not do:
contentisNone. Withcontent: "", which is whatToolAgentLoop._build_assistant_messagestores, those two templates render an empty assistant turn and skip the tool calls without raising. That is a property of the templates rather than of the argument type, and the rollout path never re-renders assistant turns (it keeps the generated tokens), so it is left alone here and noted in the test. V3.1 keys its branch ontool_callsand is fully covered.RLHFDataset.filter_overlong_promptscallsprocessor.apply_chat_templatedirectly (thedeepseek.pymodule docstring already records this) and itsexceptbranch counts a failed render as overlong. Not touched here.Checklist Before Starting
[{modules}] {type}: {description}Test
tests/utils/test_chat_template_tool_arguments_on_cpu.py(new, 10 tests): the helper copies rather than mutates and leaves untouched messages as the same objects; a concatenating fake template renders{"city": "Seattle"}with exactly one retry (tokenize on and off); atojson-style fake receives the mapping in a single call; the V3.1 tool-call branch underjinja2.sandbox.ImmutableSandboxedEnvironmentraisescan only concatenate stron its own and renders through the wrapper; the DeepSeek-R1 chat template (vendored verbatim from the R1 distill checkpoints'tokenizer_config.json) rendered throughtransformers.utils.chat_template_utils.render_jinja_template; aTypeErrorunrelated to the arguments still propagates.tests/utils/test_continuous_token_on_cpu.py: one new test rendersbuild_initial_tokensfor a history that already contains a tool-call turn with mapping arguments; the existing append test now checks that the default builder renders the same append asDeepSeekContinuousTokenBuilderinstead of expecting theTypeError.pytest tests/utils/test_chat_template_tool_arguments_on_cpu.py tests/utils/test_continuous_token_on_cpu.py tests/utils/test_turn_separator_on_cpu.py: 142 passed; the agent-loop CPU tests (test_tool_call_id_on_cpu.py,test_call_tool_on_cpu.py,test_agent_loop_extra_fields_schema_on_cpu.py): 18 passed.ruff check,ruff format --checkandmypypass on the touched files.tokenizer_config.jsonofdeepseek-ai/DeepSeek-V3,deepseek-ai/DeepSeek-R1anddeepseek-ai/DeepSeek-V3.1on the Hub (concatenated arguments in all three;content is nonegating in R1 and V3;tool_callsgating in V3.1). I have not rerun the real-tokenizer harness from [rollout] fix: DeepSeek continuous token builder cannot render tool appends #7630 on this branch.API and Usage Example
No API change.
verl.utils.tokenizer.chat_template.tool_call_arguments_as_json(messages)is new and returns(messages, changed).Design & Code Changes
verl/utils/tokenizer/chat_template.py:tool_call_arguments_as_jsonand_apply_chat_template_with_json_arguments; the wrapper's first render goes through the latter. The dummy-user fallback is untouched, so this is independent of [misc] fix: handle strict system-message chat templates #7726, which reworks that block.DeepSeekContinuousTokenBuilderkeeps stringifying its synthetic tool call; it no longer depends on it.Checklist Before Submitting
ruff check/ruff formatpass on the touched files.tests/utils/, covered by the CPU unit-test workflow.ci-requestonce ready.