refactor(tokenizer): use strongly typed enum instead of hardcoded sentinel string

[biswapanda]

Overview:

Address a previous comment: #6996 (comment)

Introduce a DecodeResult enum (Complete | Partial) to the Decoder trait, replacing hardcoded U+FFFD string checks in the incremental detokenization path. This makes partial-decode state explicit in the type system and fixes a pre-existing edge case where legitimate U+FFFD vocabulary tokens were incorrectly suppressed.

Details:

DecodeResult enum (lib/llm/src/tokenizers.rs)

• Complete(String) -- fully valid output (may contain interior U+FFFD from mid-stream invalid bytes, but no trailing incomplete sequences)
• Partial(String) -- output ends with incomplete trailing multi-byte bytes that may be completed by subsequent tokens
• Derives strum::EnumIs for auto-generated is_partial() / is_complete() methods
• Bidirectional From conversions: From<String> for DecodeResult (via from_decoded heuristic) and From<DecodeResult> for String
• from_decoded() constructor classifies based on trailing U+FFFD; as_str() for borrowing

Decoder trait returns Result<DecodeResult> instead of Result<String>

• DecodeStream::step() uses is_partial() instead of ends_with("\u{FFFD}")
• Sequence::append_token_id() uses is_partial() instead of ends_with("\u{FFFD}")

TikToken fix (lib/llm/src/tokenizers/tiktoken.rs)

• Tries String::from_utf8() first on raw bytes -- if valid UTF-8, returns Complete directly with zero extra allocation (takes ownership of the Vec<u8>)
• Only falls back to from_utf8_lossy + trailing-FFFD heuristic (from_decoded) when bytes are genuinely invalid UTF-8
• This fixes the legitimate U+FFFD bug: a vocabulary token whose raw bytes are EF BF BD (valid UTF-8 encoding of U+FFFD) was previously misclassified as Partial by from_utf8_lossy + ends_with('\u{FFFD}'), causing the incremental decoder to suppress it
• The happy path (valid UTF-8) is strictly faster than before -- from_utf8 validates and takes ownership vs from_utf8_lossy which always allocates a new String
• Includes a regression test (test_decode_legitimate_replacement_char_token_is_complete) with a U+FFFD vocab token asserting Complete status

HuggingFace / FastTokenizer -- use From<String> for DecodeResult (which delegates to from_decoded, a post-hoc trailing-FFFD heuristic on the output string, since raw bytes are inaccessible through the HF library API)

Where should the reviewer start?

1. lib/llm/src/tokenizers.rs -- DecodeResult enum definition, From impls, and updated DecodeStream::step() / Sequence::append_token_id()
2. lib/llm/src/tokenizers/tiktoken.rs -- String::from_utf8 fast path with lossy fallback, and the new regression test for legitimate U+FFFD tokens

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

• closes DIS-1585
• Addresses fix: Kimi K2.5 - tiktoken incomplete multi-byte sequence handling with regression tests #6996 (comment)

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

Walkthrough

This pull request introduces a new DecodeResult enum to the tokenizer trait system that represents whether decoded text is complete or partial based on the presence of the Unicode replacement character. The Decoder trait's decode method signature is updated to return Result<DecodeResult> instead of Result<String>, with all implementations and call sites adjusted accordingly.

Changes

Cohort / File(s)	Summary
Decoder Trait Definition lib/llm/src/tokenizers.rs	Added DecodeResult enum with Complete(String) and Partial(String) variants. Updated Decoder trait method to return Result<DecodeResult>. Modified DecodeStream and Sequence methods to work with the new return type, replacing manual replacement-character checks with DecodeResult::is_partial().
Decoder Implementations lib/llm/src/tokenizers/fastokens.rs, lib/llm/src/tokenizers/hf.rs, lib/llm/src/tokenizers/tiktoken.rs	Updated all three tokenizer Decoder implementations to return Result<DecodeResult> instead of Result<String>. Implementations now classify decoded text via DecodeResult::classify(text) before returning.
Test Decoders lib/llm/src/backend.rs, lib/llm/tests/test_stop_behavior.rs	Updated mock and test decoder implementations to return Result<DecodeResult>. Both now construct the decoded string and wrap it in DecodeResult::classify().
Test Assertions lib/llm/tests/tokenizers.rs	Updated decode assertions to call .into_string() on DecodeResult before performing string comparisons and operations in roundtrip and skip-special-tokens tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title accurately describes the main change: introducing a strongly typed enum (DecodeResult) to replace hardcoded sentinel string checks (U+FFFD) in the tokenizer's decoder logic.
Description check	✅ Passed	The pull request description provides comprehensive coverage of all template sections with detailed technical context and implementation decisions.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[rmccorm4]

fixes a pre-existing edge case where legitimate U+FFFD vocabulary tokens were incorrectly suppressed

Can you add a test case that would've failed and caught this before this fix?