feat(sft): opt-in final stop token in build_training_sample#101
Conversation
Templates like GLM close assistant turns with the next message's role marker, leaving a final assistant turn with no stop supervision; ensure_final_stop appends the canonical stop as a trainable target while remaining a no-op for in-message closes. Co-authored-by: Cursor <cursoragent@cursor.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| ) | ||
| if last_trainable is None or token_ids[last_trainable] not in stop_ids: | ||
| token_ids.append(renderer.get_stop_token_ids()[0]) | ||
| loss_mask.append(True) |
There was a problem hiding this comment.
loss mask True means training or not training. We should train on eos
There was a problem hiding this comment.
it means we train on it
ApprovabilityVerdict: Approved Adds an opt-in parameter ( You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e036812. Configure here.
Co-authored-by: Cursor <cursoragent@cursor.com>
Build the modality flags from the post-append token count so multimodal samples keep aligned per-token arrays. Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc04fbaa8f
ℹ️ 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".
Opaque templates give no reliable way to locate the assistant close, so ensure_final_stop is a no-op for DefaultRenderer instead of corrupting already-terminated turns. Co-authored-by: Cursor <cursoragent@cursor.com>
A filter that excludes the final assistant message must also exclude its stop token. Co-authored-by: Cursor <cursoragent@cursor.com>
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec1ea8ba35
ℹ️ 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".
| last_trainable = next( | ||
| (k for k in range(len(loss_mask) - 1, -1, -1) if loss_mask[k]), None | ||
| ) | ||
| if last_trainable is None or token_ids[last_trainable] not in stop_ids: |
There was a problem hiding this comment.
With ensure_final_stop=True and no role_to_mask, GLM-style renderers can leave the last assistant unterminated: in a user, assistant, user, empty assistant sample, the second user's role marker is a trainable sampled stop for the previous assistant turn, so this global last_trainable check sees a stop token and skips appending one for the final assistant. The stop check should be scoped to tokens attributed to messages[-1] (or append when that final assistant has no trainable tokens).
Useful? React with 👍 / 👎.
2 participants
Summary
ensure_final_stoptobuild_training_sample: when a sample ends with an assistant message whose last trainable token is not a renderer stop token, append the canonical stop as a trainable target<|im_end|>, Llama<|eot_id|>); restores final-turn stop supervision for GLM-style templates where the turn terminator is the next message's role marker or a sampled<|endoftext|>the template never re-rendersValidation
uv run pytest tests/test_build_helpers.py -q— 101 passed, 10 skippedMade with Cursor
Note
Add
ensure_final_stopparameter tobuild_training_sampleto append a trainable stop tokenensure_final_stop: bool = Falsekeyword argument tobuild_training_samplein renderers/base.py.ensure_final_stop=Falseor when the final trainable token is already a stop token.mm_token_type_idsare sized to the original rendered length and do not include the appended stop token, which may cause a length mismatch in multi-modal samples using this flag.Changes since #101 opened
ensure_final_stopparameter inbuild_training_samplefunction [e036812]build_training_sampleutility function clarifying stop token training behavior [7d362e4]_build_mm_token_type_idscall withinbuild_training_samplefunction fromlen(rendered.token_ids)tolen(token_ids)[807f292]build_training_samplefunction inrenderers.baseto requirehas_sampled_infoin addition toensure_final_stopand the last message being from the assistant before appending a stop token, and updated the docstring to clarify thatensure_final_stopis ignored for renderers withoutsampled_masksuch asDefaultRenderer, with an added inline comment explaining the tightened conditional logic [165a941]build_training_samplefunction to conditionally append and train the final stop token based on role masking filter [ec1ea8b]Macroscope summarized fc04fba.