[Fix] Supervise GLM-5.2 assistant stop tokens - #1997
Open
jayhenry wants to merge 2 commits into
Open
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Summary
<|endoftext|>only when no following role token can terminate the assistant turnloss=False, thinking cleanup, the slow prefix-diff oracle, and the fast tokenizer consistentadd-chat-templateagent skill for auditing HF chat templates and stop-token loss masksGLM-5.2 chat-template semantics
The official GLM-5.2 chat template does not append a dedicated end-of-turn token after an intermediate assistant message. Instead, the role token beginning the next message is also a configured generation stop token:
<|endoftext|><|user|><|observation|>All three IDs are listed in the official GLM-5.2
eos_token_idgeneration configuration.Previously, XTuner masked
<|user|>and<|observation|>and did not add<|endoftext|>after the final assistant message. Consequently, an SFT sample could supervise the assistant body without supervising any token that tells the model to stop.This PR assigns the loss of the preceding assistant to the applicable user/observation role token. If there is no such next-role boundary, it appends and supervises
<|endoftext|>. The following user or tool payload remains masked. An assistant withloss=Falsealso keeps its stop token masked.Why not append
<|endoftext|>after every assistant?That simpler approach would change training histories to:
Inference engines such as vLLM normally rebuild chat history with the model's official chat template, which produces:
vLLM can still stop the current generation when the model emits
<|endoftext|>, but subsequent multi-turn and tool-call prompts would omit the intermediate EOS seen during SFT. This creates an unnecessary train/serve format mismatch and may degrade multi-turn or tool-call behavior. Using the official role boundary as the supervised stop target avoids that mismatch.Tests
The tests cover normal multi-turn conversations, reasoning preservation/cleanup, tool calls,
loss=False, generation prompts, default system messages, fast/slow parity, and the label state of all three GLM-5.2 stop tokens.