Skip to content

fix(data): restore the deepseek-r1-distill chat template - #914

Open
yushengsu-thu wants to merge 2 commits into
sgl-project:mainfrom
yushengsu-thu:fix/deepseek-r1-distill-template
Open

yushengsu-thu wants to merge 2 commits into
sgl-project:mainfrom
yushengsu-thu:fix/deepseek-r1-distill-template

Conversation

@yushengsu-thu

@yushengsu-thu yushengsu-thu commented Oct 1, 2026 •

Copy link
Copy Markdown

Motivation

--chat-template deepseek-r1-distill crashes before any data gets processed:

  File "specforge/data/parse.py", line 197, in set_assistant_pattern
    + re.escape(self.chat_template.end_of_turn_token)
TypeError: decoding to str: need a bytes-like object, NoneType found

The template was added with end_of_turn_token=None in #281, which worked at the time, but after the parser rewrite in #381 that value goes straight into re.escape. It's the only registered template that hits this.

Modifications

  • Set end_of_turn_token to <|end▁of▁sentence|> for deepseek-r1-distill. R1-Distill closes every answer with it, same as deepseek-v3.
  • GeneralParser now raises a readable ValueError when a template that needs end_of_turn_token leaves it unset, instead of the TypeError above.
  • Tests: the R1-Distill loss mask covers each answer plus its EOS, every registered template can build its parser, and the new error path. Also a test_parsers regression case with the real DeepSeek-R1-Distill-Qwen-1.5B tokenizer, same pattern as the other templates.

Note: the unit-test check currently fails in the live server-capture gate before any test runs (runner lost NO_PROXY, see #937); that's unrelated to this change.

deepseek-r1-distill was registered with end_of_turn_token=None. Since sgl-project#381
the general parser passes that token straight to re.escape, so building the
parser raised TypeError before a single sample was tokenized.

Set it to <|end▁of▁sentence|>, which R1-Distill emits after every answer
(same as deepseek-v3), and make the parser raise a clear ValueError when a
template that needs end_of_turn_token leaves it unset.
Copilot AI balanced review requested due to automatic review settings October 1, 2026 04:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@FrankLeeeee FrankLeeeee left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Same pattern as the other templates in test_parsers: render the standard
conversation with DeepSeek-R1-Distill-Qwen-1.5B and compare input_ids and
loss_mask against a saved reference. The mask covers each answer plus its
<|end▁of▁sentence|>.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants