Fix CodeLlama tokenizer stripping real leading spaces on decode (#46491) - #46574
Closed
kpal002 wants to merge 7 commits into
Closed
Fix CodeLlama tokenizer stripping real leading spaces on decode (#46491)#46574kpal002 wants to merge 7 commits into
kpal002 wants to merge 7 commits into
Conversation
…huggingface#46491) `decoders.Strip(content=" ", left=1)` in the decoder sequence was unconditionally removing one leading space on every decode call. This correctly removed the synthetic ▁ prefix introduced by the Metaspace normalizer for inputs like "hello world", but silently corrupted inputs that began with real user-provided spaces (e.g. indented Python code), since the synthetic and user-provided spaces were indistinguishable after the Replace(▁→space) step. Fix: remove Strip from the decoder sequence and override `_decode` to perform the strip conditionally. The first real (non-special) token is inspected: if its text consists entirely of ▁ characters (e.g. ▁=29871, ▁▁=259, ▁▁▁=1678) it represents real user-provided leading spaces and must not be stripped; otherwise the leading space is the synthetic Metaspace prefix and should be stripped. This correctly handles 2, 3, 4, and more leading spaces, as well as the BOS-prepended case.
Closed
4 tasks
…written by tokenizer.json load
… is authoritative
Member
Contributor
|
CI Dashboard: View test results in Grafana |
Contributor
|
[For maintainers] Suggested jobs to run (before merge) run-slow: code_llama |
4 tasks
Member
|
closing as superseded, see #47487 (comment), thanks! |
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 this fixes
Closes #46491.
decoders.Strip(content=" ", left=1)inCodeLlamaTokenizer.__init__was unconditionally removing one leading space on everydecodecall. For inputs that begin with real spaces (e.g. indented Python code), this silently corrupted the output —decode(encode(" if x:\n"))returned" if x:\n"instead of" if x:\n".The stripping was intended to remove the synthetic
▁prefix thatMetaspace(prepend_scheme="first")prepends before the first content token. The bug: afterReplace(▁ → " "), the synthetic prefix and real user-provided leading spaces are indistinguishable in the decoded string, soStrip(left=1)ate one real space whenever the input started with one or more spaces.Root cause
Fix
Remove
Stripfrom the decoder sequence and override_decodeto perform the strip conditionally, based on the first real (non-special) token:▁characters (▁=29871,▁▁=259,▁▁▁=1678, …), it was produced by real user-provided leading spaces → do not strip.This handles 2, 3, 4, and N leading spaces, and is correct in the presence of a prepended BOS token (BOS is in
all_special_idsand is skipped when looking for the first real token).Note: a single leading space (
" hello"vs"hello") is inherently ambiguous — both encode to the same token IDs — and cannot round-trip. This was true before this fix and remains true; it is documented in the new test.Before / after
Tests
Added
test_leading_space_roundtriptotests/models/code_llama/test_tokenization_code_llama.pycovering 2-, 3-, and 4-space-prefixed strings viaencode(s, add_special_tokens=False)→decode(…, skip_special_tokens=True).Ran locally on this PR's branch (
pytest tests/models/code_llama/test_tokenization_code_llama.py, Python 3.12.13, transformers 5.10.0.dev0 editable install):including
CodeLlamaTokenizationTest::test_leading_space_roundtrip. Also green on CI: https://circleci.com/gh/huggingface/transformers/2354143Disclosure
This fix was drafted with AI-agent assistance (diagnosis, patch, and test). I reviewed every changed line, confirmed the root cause by tracing the
super().__init__()overwrite ofself._tokenizerand the BPE merge behavior for multi-▁ tokens, and ran the test suite locally (output above) before opening this PR.