Fix to_sharegpt optional block rendering "None" for missing extra columns#6827
Open
vineethsaivs wants to merge 2 commits into
Open
Fix to_sharegpt optional block rendering "None" for missing extra columns#6827vineethsaivs wants to merge 2 commits into
vineethsaivs wants to merge 2 commits into
Conversation
An optional [[...]] block in merged_prompt may reference more than one
column, but only the first column gates the block. When the block is
kept, str.format is called with every referenced column, so a later
column whose value is None renders as the literal string "None" in the
emitted training text (for example [[{city}, {country}]] with a missing
country produces "Paris, None"). Single-column blocks are unaffected
because their sole column is also the gate and is emptied first.
Coerce None values to "" when building the format arguments so absent
extra columns render as empty. Add a regression test that extracts the
pure-Python formatter builders via ast (no GPU / no unsloth import).
for more information, see https://pre-commit.ci
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the chat template processing in unsloth/chat_templates.py to coerce missing (None) columns to empty strings ("") within optional blocks, preventing them from rendering as the literal string "None". Additionally, a new test suite tests/python/test_to_sharegpt_optional_none.py has been added to verify this behavior under various scenarios. I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
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.
What
to_sharegpt(..., merged_prompt=...)lets an optional[[...]]block reference more than one column, e.g.merged_prompt="Location: [[{city}, {country}]]". Only the first column in a block gates whether the block is rendered. When the block is kept,str.formatis called with every referenced column, so a later column whose value isNonerenders as the literal string"None"in the emitted training text:Single-column blocks are unaffected, because the sole column is also the gate and is emptied before
formatis ever reached.Fix
Coerce
Noneto""when building the format arguments for a kept block, so an absent extra column renders as empty ("Location: Paris, end"). The first-column gating semantics are left unchanged.Test
tests/python/test_to_sharegpt_optional_none.pyextracts the pure-Python formatter builders (_parse_combined_prompt,_create_formatter) viaast, so it runs on CPU with noimport unsloth(same pattern astests/python/test_change_system_message.py). It fails before the fix ("Location: Paris, None end") and passes after, and asserts the all-present, empty-gate, and single-column cases are unchanged.