Repository navigation
Conversation
FlushBytes took a sequence length from the lead byte alone, so BpeTokenizer::Decode returned invalid UTF-8 for byte-fallback runs that are not well formed: a continuation byte (0x80-0xBF) or 0xF8-0xFF on its own was appended raw, a lead byte was appended together with whatever bytes followed it (no continuation, overlong, surrogate or range check), and a truncated sequence gave one U+FFFD followed by its raw continuation bytes. For example [<0xA0>] decoded to "\xA0" and [<0xE2>, <0x80>] to "\xEF\xBF\xBD\x80", where SentencePiece gives one U+FFFD per byte. Such runs reach Decode when the model samples a malformed byte sequence, and when a multi-byte byte-fallback character is decoded one token at a time, as run.cc and api_server.cc do while streaming. Validate each sequence fully (lead byte, continuation bytes, overlong, surrogate and > U+10FFFF), keep well-formed ones and emit U+FFFD for every other byte, as SentencePiece does. Decode on this path now always returns valid UTF-8 and matches SentencePiece. It does not recover a character whose bytes arrive in separate Decode calls; that still needs buffering in the caller. The byte-level path (DecodeByteLevel) is unchanged. Add DecodeMalformedByteFallbackMatchesSentencePiece, which checks a list of malformed runs against fixed expectations and compares Decode with SentencePiece on every run of one or two byte-fallback tokens.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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.
BpeTokenizer::Decodecan return invalid UTF-8 for runs of byte-fallback tokens (<0xHH>).FlushBytestakes the sequence length from the lead byte alone, although its comment says invalid sequences become U+FFFD:[<0xA0>]decodes to"\xA0";[<0xC2>, <0xC2>]decodes to"\xC2\xC2";[<0xE2>, <0x80>]decodes to"\xEF\xBF\xBD\x80".SentencePiece decodes each of these to one U+FFFD per byte. Such runs reach
Decodewhen the model samples a malformed byte sequence, and when a byte-fallback character is decoded one token at a time, asrun.ccandapi_server.ccdo while streaming.api_server.ccthen passes the text tojson::dump(), which throws on invalid UTF-8 by default.Change
ValidUtf8Len()returns the length of the well-formed UTF-8 sequence at a position (lead C2-DF, E0-EF or F0-F4, continuation bytes 10xxxxxx, no overlong, surrogate or above-U+10FFFF code points), or 0.FlushBytes()keeps well-formed sequences and emits U+FFFD for every other byte; its comment now says so. The encode-sideUtf8Len()/DecodeUtf8()are untouched.Scope:
Decodeon the byte-fallback path now always returns valid UTF-8, matching SentencePiece.Decodecalls: "€" (<0xE2>,<0x82>,<0xAC>) decoded one token at a time was lost before (U+FFFD, raw\x82, raw\xAC) and still is (three U+FFFD). That needs buffering in the caller, asdeepseek/run_dsv4.ccdoes withCompleteUtf8Prefix().DecodeByteLevel(Qwen3-styletokenizer.json) is unchanged, because its raw bytes let per-token output concatenate back into a character split across byte-level tokens, and HF-style U+FFFD would break that. Streamingapi_server.ccneeds caller-side buffering there, which is a separate change.Question: SentencePiece or HF rule?
The old comment said "matching HuggingFace ByteFallback". HF replaces the whole run with U+FFFD if any part is invalid; SentencePiece keeps the well-formed sequences. So
[<0xC3>, <0xA9>, <0xA9>]gives three U+FFFD in HF tokenizers 0.23.3, but"é"+ U+FFFD in SentencePiece and here. This PR follows SentencePiece becausebpe_tokenizer_test.ccalready uses it as this backend's reference, and a Gemma checkpoint can be converted from eithertokenizer.modelortokenizer.json, soDecodeoutput stays the same for both. Switching to the HF rule is a small change if you prefer it.Testing
New test
BpeTokenizerTest.DecodeMalformedByteFallbackMatchesSentencePiece, next toDecodeRoundTrip, checks valid, lone continuation, invalid lead, missing continuation, truncated, overlong, surrogate, above-U+10FFFF and mixed runs against fixed expectations and SentencePiece, then compares the two on all 65,792 runs of one or two byte tokens. Ondevit fails (10 listed cases, 35,720 runs differ); with the fix, all 8 tests inbpe_tokenizer_testpass.bazel test //:bpe_tokenizer_testcannot be analyzed in the OSS tree because:packed_tokenizerneeds the undeclared//python:convert_from_safetensors, so the test was built with CMake against the pinned highway/sentencepiece:Also checked: on 50,000 random 1-8 token sequences, this code and the
sentencepiece0.2.2 Python package differ 38,665 times ondev(all invalid UTF-8) and 0 times with this change, also under ASan/UBSan.This change was prepared with an AI coding agent (Claude Code), which also ran the checks above. The submitter is responsible for it and will answer review.
🤖 Generated with Claude Code