fix(search): stop the rerank tokenizer panicking on trailing multi-byte uppercase runes - #569
Merged
Merged
Conversation
…te uppercase runes
tokenize's SCREAMING->Camel lookahead guarded with i+1 < len(s), a byte
comparison, before reading []rune(s[i:])[1]. When the current rune is a
multi-byte uppercase letter at the end of the string ("ТЕКСТ", "CAFÉ"),
the byte guard passes but the suffix decodes to a single rune, so the
index panics with "index out of range [1] with length 1". The panic
surfaced as an explore internal error for any query whose retrieved
candidates contained all-caps non-ASCII words.
Decode the lookahead rune with utf8.DecodeRuneInString instead, which
also drops the O(n) rune-slice allocation per boundary check.
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
1 participant
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.
Problem
A community user reported
explorefailing with:for the plain-text query
простой текстon v0.63.2, and read it as a corrupted index. The index was fine — this is a per-request panic in the rerank tokenizer, caught by the tool firewall.Root cause
tokenize's SCREAMING→Camel lookahead ininternal/search/rerank/tokens.goguarded withi+1 < len(s)— a byte comparison (icomes fromrange s) — before reading[]rune(s[i:])[1]. When a word ends in consecutive uppercase letters whose last rune is multi-byte (ТЕКСТ,ПРОСТОЙ,CAFÉ), the byte guard passes but the suffix decodes to a single rune, so the index panics with exactly the reported message.The query itself was lowercase and harmless:
rerank.Tokenizealso runs over retrieved candidate text, so any repo containing all-caps non-ASCII words (Russian docs/strings in this case) trips it. Corpus-dependent, which is why it never reproduced on ASCII-only codebases.Fix
Decode the lookahead rune with
utf8.DecodeRuneInString(s[i+utf8.RuneLen(r):])instead of materialising the suffix as a rune slice. This also drops the O(n) allocation per boundary check.Tests
New
tokens_test.go: the three panic inputs, the split-across-multibyte case (ЖКХStatus→жкх,status), and the existing ASCII behaviors (ParseHTTPHeader,validate_user_token,HTTPHeader).go test -race ./internal/search/rerank/andgolangci-lintare green.