fix(dataprep): smart_chunk_text single-chunk path leaks internal tensor type when eos_token_id is None#7151
Conversation
…or type when eos_token_id is None RawTextDataLoader.smart_chunk_text()'s single-chunk branch only converts `tokens` to a plain Python list inside the `if eos_token_id is not None:` guard. When a tokenizer has no eos_token_id configured, that conversion is skipped entirely and the function returns whatever internal tensor-like object came out of the tokenizer normalization step (e.g. a torch.Tensor) as "input_ids", instead of a list of ints. The sibling multi-chunk branch a few lines below does the conversion unconditionally, before checking eos_token_id -- the two branches of the same method disagree on output type depending purely on whether the tokenizer has an EOS token. Downstream, create_causal_dataset() does `labels = [list(ids) for ids in input_ids]`; list()'ing a tensor produces a list of 0-d tensor elements rather than plain ints, inconsistent with every multi-chunk sample and liable to break type inference in Dataset.from_dict()/downstream collation. Fix: move the list conversion out of the eos_token_id guard, matching the multi-chunk branch's existing pattern. Added test_smart_chunk_text_single_chunk_no_eos_returns_plain_list to tests/test_raw_text.py, confirmed red against unfixed code (assertion failure: input_ids was a MockTensor, not a list) and green after the fix. Full tests/test_raw_text.py (both test functions) passes. ruff check + the repo's ruff-format-with-kwargs script: clean. Note: tests/test_raw_text.py does not appear to be wired into any .github/workflows/*.yml CI job (a pre-existing repo characteristic, not something introduced by this change) -- verified locally via `python3 tests/test_raw_text.py`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Code Review
This pull request updates the smart_chunk_text method in unsloth/dataprep/raw_text.py to ensure that tokens are unconditionally converted to a plain list when returning tokenized single-chunk text, even if the tokenizer does not have an eos_token_id. Additionally, a unit test has been added to tests/test_raw_text.py to verify this behavior. There are no review comments to address, and we have no further 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.
|
Thanks for the PR! |
Problem
RawTextDataLoader.smart_chunk_text()(unsloth/dataprep/raw_text.py) hastwo branches for building the
input_idsoutput, and they disagree onoutput type:
Single-chunk branch (when the whole text fits in
chunk_size):The
tolist()/list()conversion only happens inside theif eos_token_id is not None:guard. When the tokenizer has no EOS tokenconfigured, that branch never runs, and
tokens— whatevertensor-like object came out of the tokenizer normalization step a few
lines up — is returned as
input_idsverbatim.Multi-chunk branch (a few lines below), for comparison:
Here the conversion runs unconditionally, before checking
eos_token_id— the correct pattern.Impact
create_causal_dataset()doeslabels = [list(ids) for ids in input_ids].list()-ing a tensor produces a list of 0-d tensor elements rather thanplain Python ints, inconsistent with every multi-chunk sample and liable to
break type inference in
Dataset.from_dict()/ downstreamtokenization/collation code that expects plain ints.
Repro (bare, before fix — via a minimal mock tokenizer, no torch/GPU needed)
Fix
Move the list conversion out of the
eos_token_idguard, matching themulti-chunk branch's existing pattern (2-line diff, single-chunk branch
only).
Testing
Added
test_smart_chunk_text_single_chunk_no_eos_returns_plain_listtotests/test_raw_text.py(reusing the file's existingMockTensor/mock-tokenizer pattern, no heavy deps). Confirmed:
input_idswas the mocktensor type, not a list)
tests/test_raw_text.py(both test functions) passes:python3 tests/test_raw_text.py→✅ All tests passed!/✅ test_smart_chunk_text_single_chunk_no_eos_returns_plain_list passed!ruff checkand the repo'sscripts/run_ruff_format.py(kwarg-spacingformatter): clean.
Note:
tests/test_raw_text.pydoesn't appear to be wired into any.github/workflows/*.ymljob as far as I could find — a pre-existing repocharacteristic, not something this PR changes. Verified locally instead.
Duplicate-work check
Searched merged/closed PRs and issues for
smart_chunk_text/raw_text.py— found #7126 ("guard smart_chunk_text against stride >=chunk_size", merged), a different bug in the same function (stride
validation, not the EOS-branch type leak this PR fixes). No open PR
touches this file.
Disclosure: I used AI assistance (Claude) to find this bug and draft the
fix and test. I independently traced both branches, reproduced the type
mismatch against unfixed code with a bare mock-tokenizer repro, verified
the fix red→green, and ran the full local test file + ruff before
submitting.