feat: add glm45 chat template#136
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 370315a6a8
ℹ️ 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".
| assistant_header="<|assistant|>", | ||
| user_header="<|user|>", | ||
| system_prompt=None, | ||
| end_of_turn_token="<|user|>", |
There was a problem hiding this comment.
Do not use the next user header as an end token
When GLM data is tokenized offline (defer_tokenization=False, the default), GeneralParser.parse builds an assistant regex that includes end_of_turn_token inside the captured assistant span (torchspec/data/parse.py:247-251) and preprocess_conversations packs that mask directly. For every multi-turn GLM sample, this line makes the next turn’s <|user|> role header part of the assistant loss mask, training on a user token as assistant output; use a GLM-specific parser or normalization that excludes role headers rather than using <|user|> as an in-span terminator.
Useful? React with 👍 / 👎.
Register a glm45 template for the GLM-4.5 / GLM-5.x chat format. It uses the default GeneralParser; end_of_turn_token is <|user|> because GLM has no dedicated assistant end-of-turn token, so an assistant turn ends at the next role header. Testing (verified locally, not committed): - The parser render is byte-identical to tokenizer.apply_chat_template (the renderer vLLM serves with) on the real GLM-4.5 and GLM-5.2 tokenizers. - The rendered assistant span has exactly one <think> block. parser_type "thinking" would double it, because GLM-5.2's generation prompt already opens <think>; the default parser does not. - The offline loss mask matches the runtime compute_assistant_loss_mask, cross-validated on single-turn, multi-turn, and last-turn-only for both GLM-4.5 and GLM-5.2. Signed-off-by: Aaron Batilo <AaronBatilo@gmail.com>
The glm45 template (added in the previous commit) is a plain GeneralParser and had no test coverage. Add it to the loss-mask cross-validation CI gate plus a focused parser test. - test_loss_mask_cross_validation.py: register glm45 -> zai-org/GLM-4.5 in REFERENCE_MODELS so the parametrized parser-vs-runtime-mask checks cover it. GLM-4.5 loads cleanly via AutoTokenizer; GLM-5.2's tokenizer_config declares the transformers-v5 TokenizersBackend class (would skip), and 4.5 is representative for the shared 4.5->5.2 glm45 grammar. - tests/test_glm45_parser.py: assert glm45 dispatches to GeneralParser (not ThinkingParser, whose last-turn reconstruction would double GLM's already-open <think>); that the rendered assistant turn keeps a single <think>; and that the last assistant turn -- reasoning included -- is fully loss-masked while earlier turns are excluded under last_turn_only.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcf39ad705
ℹ️ 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".
| assistant_header="<|assistant|>", | ||
| user_header="<|user|>", | ||
| system_prompt=None, | ||
| end_of_turn_token="<|user|>", |
There was a problem hiding this comment.
Terminate GLM assistant spans on tool headers
When GLM-formatted data contains tool calls, tool messages are rendered as an <|observation|> role rather than a <|user|> role. Because this registers <|user|> as the only assistant terminator, both GeneralParser and the dynamic loss-mask scan treat a user -> assistant(tool_call) -> tool -> assistant sequence as one assistant span until EOF (or the next user), so tool responses and the following assistant header/content are supervised as assistant output; this also defeats last_turn_loss_only for tool-use samples. The GLM template needs a parser/terminator rule that stops at all GLM role headers, not just user.
Useful? React with 👍 / 👎.
Register a glm45 template for the GLM-4.5 / GLM-5.x chat format. It uses the
default GeneralParser; end_of_turn_token is <|user|> because GLM has no
dedicated assistant end-of-turn token, so an assistant turn ends at the next
role header.
Testing (verified locally, not committed):
renderer vLLM serves with) on the real GLM-4.5 and GLM-5.2 tokenizers.
"thinking" would double it, because GLM-5.2's generation prompt already
opens ; the default parser does not.
cross-validated on single-turn, multi-turn, and last-turn-only for both
GLM-4.5 and GLM-5.2.