Skip to content

feat(data): mask Muse Spark assistant spans - #1000

Merged
jiapingW merged 3 commits into
sgl-project:mainfrom
yudigege86:feat/muse-chat-template
Oct 28, 2026
Merged

jiapingW merged 3 commits into
sgl-project:mainfrom
yudigege86:feat/muse-chat-template

Conversation

@yudigege86

Copy link
Copy Markdown

Motivation

Muse Spark renders ShareGPT rows through its own tokenizer as Harmony messages (<|start|>assistant to=self<|message|>...<|eom|> and to=user closed by <|eot|>). data.chat_template: qwen3.5 only selects a loss-mask parser, and that parser searches for <|im_start|>assistant markers this template never emits. Every mask is empty and the rows are dropped.

This adds a muse profile that keeps rendering on tokenizer.apply_chat_template and masks the assistant spans that template actually produces.

Modifications

  • Register muse in specforge/data/template.py.
  • Add MuseParser and dispatch parser_type="muse". One assistant header and one end token cannot describe both to=self and to=user.
  • Supervise the interior and closing token of to=self, to=user, to=commentary, and to=<tool name> messages. Leave the <|start|>assistant to=…<|message|> prefix, and system, user, and tool-result turns, unsupervised.
  • Do not pass enable_thinking. Do not reuse the qwen3.5 or gpt-oss Harmony parsers.
  • Add a tokenizer-free unit test for a reasoning-plus-answer turn and for commentary plus a tool call.

Related Issues

Independent of the Muse capture patch in #999.

Accuracy Test

tests.test_data.test_muse_parser and tests.test_data.test_template_registry pass. The reasoning-plus-answer example has loss_mask.sum() >= 2 and covers both bodies. No GPU training run was performed.

Benchmark & Profiling

Not run.

Checklist

yudigege86 and others added 2 commits October 20, 2026 16:46
The qwen3.5 parser looks for ChatML markers that Muse's Harmony template never emits, so every loss mask is empty.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

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

The overall design looks reasonable.

One correctness issue should be fixed before merging: train_only_last_turn=True currently keeps only the last regex match. Since Muse renders reasoning and the final answer as separate spans, this excludes the last turn’s reasoning from supervision. It should select all assistant spans belonging to the last turn, while keeping earlier turns as context only.

Please add a multi-turn regression test covering this behavior, and a test against a pinned Muse template to verify the assumptions currently encoded in the mock tokenizer.

train_only_last_turn kept only the final regex match, so the last turn's reasoning was left unsupervised.

Co-authored-by: Cursor <cursoragent@cursor.com>
@yudigege86

Copy link
Copy Markdown
Author

train_only_last_turn now keeps every assistant span in the last Muse turn. A turn is the trailing run of spans, and a user, tool, or system message ends the previous one, so earlier turns stay context. The last turn's reasoning is supervised along with its answer.

The multi-turn regression covers that case. A pinned Muse Spark chat template checks the mock tokenizer's span layout, including tool-argument bodies and the unsupervised tool schema in the system prompt.

@yudigege86
yudigege86 requested a review from jiapingW October 21, 2026 20:37
@yudigege86

Copy link
Copy Markdown
Author

@jiapingW could you take another look?

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

@yudigege86

Copy link
Copy Markdown
Author

@jiapingW could you merge it pls? I don't have the permission

@jiapingW
jiapingW merged commit 8b45bab into sgl-project:main Oct 28, 2026
3 checks passed
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.

2 participants