Repository navigation
fix(added-vocab): fix panic in single_word check - #2666
Merged
SBrandeis merged 5 commits intoNov 10, 2026
Merged
Conversation
…he whole text `is_single_word` read the char before a match from `s[search..match_start]`. `search` is where the scan resumed, which is not a word boundary: - after a rejected candidate it is `match_start + 1`, which can be inside a multi-byte char, and slicing there panics; - it can be inside the same word (`zz` matched at 1..3 in `zzz`); - after an earlier added token it is that token's end (`foo` matched in `xfoo`). Read the char before the match from the whole text, as 0.23 did. Signed-off-by: 0z5a <[email protected]>
SBrandeis
reviewed
Nov 10, 2026
SBrandeis
left a comment
SBrandeis
left a comment
Collaborator
There was a problem hiding this comment.
Nice catch, thanks for fixing it
I left a few cosmetic comments, otherwise lgtm
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Co-authored-by: Simon Brandeis <[email protected]>
Co-authored-by: Simon Brandeis <[email protected]>
Co-authored-by: Simon Brandeis <[email protected]>
Contributor
Author
Done |
SBrandeis
approved these changes
Nov 10, 2026
SBrandeis
changed the title
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.
Fixes #2664
is_single_wordread the char before a match froms[search..match_start].searchis where the scan resumed, and that is not a word boundary:match_start + 1. That can be inside a multi-byte char, so the slice panics.zzmatches at 1..3 inzzz.foomatches inxfoo.This PR reads the char before the match from the whole text, as 0.23 does. The resume position after a rejected candidate is unchanged.
The new test,
single_word_looks_before_where_the_scan_resumed, covers all three cases. Each assert fails on its own against main (theücase panics), and all pass with this change.cargo fmt --checkand clippy on tk-encode with-D warningsare clean.With bert-base-multilingual-cased and the inputs from the issue, the Python results now match 0.23.3 on aarch64 and x86_64 Linux:
ко ##тик и кот111 11Dr Smith ##field and SmithfieldI also checked single_word × lstrip × rstrip × normalized against 0.23.3 on six tokenizers (10 flag sets each). Main panics for 10 tokenizer/flag-set combinations; with this change there are none. The only differences left are the Metaspace
▁from #2512 (T5, Mistral) and spans of tokens withrstrip, which this PR does not touch.#2362 includes the same change inside a larger feature. This PR is just the fix and its test.