Tokenizer panics on non-char-boundary offsets where the ds4 original degrades gracefully #16

New Issue

hugo · 2026-07-25T09:42:00Z

hugo commented

2026-07-25 09:42:00 +00:00

Symptom

Three tokenizer helpers slice a &str at a byte offset and unwrap() the first character. If the offset is ever not on a UTF-8 character boundary, or is at end-of-string, this panics. The tokenizer runs on the model runtime thread, so a panic here does not just fail one request — it permanently disables generation for the whole process (see the runtime-panic issue).

The C original these were ported from is a total function: it degrades gracefully on malformed input instead of faulting. The port turned a total function into a partial one.

Evidence

src/engine/tokenizer.rs:

fn next_char(text: &str, position: usize) -> usize {
    if position >= text.len() { return text.len(); }
    position + text[position..].chars().next().unwrap().len_utf8()   // :569
}

fn cjk_at(text: &str, position: usize) -> bool {
    let character = text[position..].chars().next().unwrap() as u32; // :573  no bounds check at all
    ...
}

fn char_info(text: &str, position: usize) -> CharInfo {
    if position >= text.len() { return CharInfo { ... }; }
    let ch = text[position..].chars().next().unwrap();               // :620

Two distinct failure modes: text[position..] panics if position is not a char boundary, and .unwrap() panics if the slice is empty (cjk_at has no position >= len guard, unlike the other two).

The boundary invariant is maintained by convention across several functions rather than by construction. In particular letter_like (src/engine/tokenizer.rs:579-582) indexes text.as_bytes()[position] directly and treats any byte ≥ 128 as a letter, and consume_letters (:584-589) then advances via next_char. So correctness depends on every producer of a position value agreeing — including the pre-tokenizer loop around src/engine/tokenizer.rs:490-514, which computes positions from last_newline, last_start, and scan.

How ds4 handles this

../ds4/ds4.c decodes UTF-8 by hand and is explicitly tolerant of truncation:

ds4.c:35677 utf8_peek_one(const char *s, uint64_t len, uint64_t pos, uint64_t *next):

const uint8_t c0 = (uint8_t)s[pos];
int n = utf8_len_from_first_byte(c0);
if (pos + (uint64_t)n > len) n = 1;      /* truncated sequence -> treat as one raw byte */
*next = pos + (uint64_t)n;

A malformed or truncated sequence yields a single byte and the scan continues. There is no error path because there is no error.

ds4.c:35722 joyai_cjk_at() short-circuits on if ((uint8_t)s[pos] < 128) return false; before decoding — the DS4Server port (cjk_at, :572) dropped that guard and decodes unconditionally.
ds4.c:35699-35711 joyai_letter_like_at() — the source of DS4Server's letter_like, with a comment explaining exactly why non-ASCII bytes are treated as letters, and noting that CJK/kana are isolated by the pre-tokenizer before the generic letter rule. That comment did not survive the port and is worth carrying over, since it is the rationale for the whole byte-oriented scheme.

The C version cannot panic on any input, valid or not. That property should be preserved.

Suggested fix

Make the three helpers total, matching the C semantics:

fn next_char(text: &str, position: usize) -> usize {
    match text.get(position..).and_then(|rest| rest.chars().next()) {
        Some(ch) => position + ch.len_utf8(),
        None => (position + 1).min(text.len()),   // ponytail: mirrors ds4's `n = 1` truncation fallback
    }
}

str::get(position..) returns None instead of panicking on a non-boundary offset, which is the direct Rust equivalent of ds4's if (pos + n > len) n = 1;.
Add the missing position >= text.len() guard to cjk_at, plus the < 128 ASCII short-circuit from ds4.c:35722 — it is both a correctness guard and a fast path.
char_info already guards the length; switch it to get() for the boundary case.

Also port the explanatory comment from ds4.c:35699-35711 onto letter_like.

Acceptance criteria

A round-trip test over deliberately awkward input — lone continuation bytes (\x80), a truncated multi-byte sequence at end of string, mixed CJK/ASCII/emoji, an empty string — encodes without panicking.
Token output for ordinary ASCII and normal UTF-8 text is byte-identical to before the change (compare against the existing tokenizer tests in src/engine/tokenizer.rs).
cargo clippy --all-targets --all-features -- -D warnings stays clean.

## Symptom Three tokenizer helpers slice a `&str` at a byte offset and `unwrap()` the first character. If the offset is ever not on a UTF-8 character boundary, or is at end-of-string, this panics. The tokenizer runs on the model runtime thread, so a panic here does not just fail one request — it permanently disables generation for the whole process (see the runtime-panic issue). The C original these were ported from is a **total** function: it degrades gracefully on malformed input instead of faulting. The port turned a total function into a partial one. ## Evidence `src/engine/tokenizer.rs`: ```rust fn next_char(text: &str, position: usize) -> usize { if position >= text.len() { return text.len(); } position + text[position..].chars().next().unwrap().len_utf8() // :569 } fn cjk_at(text: &str, position: usize) -> bool { let character = text[position..].chars().next().unwrap() as u32; // :573 no bounds check at all ... } fn char_info(text: &str, position: usize) -> CharInfo { if position >= text.len() { return CharInfo { ... }; } let ch = text[position..].chars().next().unwrap(); // :620 ``` Two distinct failure modes: `text[position..]` panics if `position` is not a char boundary, and `.unwrap()` panics if the slice is empty (`cjk_at` has no `position >= len` guard, unlike the other two). The boundary invariant is maintained by convention across several functions rather than by construction. In particular `letter_like` (`src/engine/tokenizer.rs:579-582`) indexes `text.as_bytes()[position]` directly and treats *any* byte ≥ 128 as a letter, and `consume_letters` (`:584-589`) then advances via `next_char`. So correctness depends on every producer of a `position` value agreeing — including the pre-tokenizer loop around `src/engine/tokenizer.rs:490-514`, which computes positions from `last_newline`, `last_start`, and `scan`. ## How ds4 handles this `../ds4/ds4.c` decodes UTF-8 by hand and is explicitly tolerant of truncation: - `ds4.c:35677` `utf8_peek_one(const char *s, uint64_t len, uint64_t pos, uint64_t *next)`: ```c const uint8_t c0 = (uint8_t)s[pos]; int n = utf8_len_from_first_byte(c0); if (pos + (uint64_t)n > len) n = 1; /* truncated sequence -> treat as one raw byte */ *next = pos + (uint64_t)n; ``` A malformed or truncated sequence yields a single byte and the scan continues. There is no error path because there is no error. - `ds4.c:35722` `joyai_cjk_at()` short-circuits on `if ((uint8_t)s[pos] < 128) return false;` *before* decoding — the DS4Server port (`cjk_at`, `:572`) dropped that guard and decodes unconditionally. - `ds4.c:35699-35711` `joyai_letter_like_at()` — the source of DS4Server's `letter_like`, with a comment explaining exactly why non-ASCII bytes are treated as letters, and noting that CJK/kana are isolated by the pre-tokenizer *before* the generic letter rule. That comment did not survive the port and is worth carrying over, since it is the rationale for the whole byte-oriented scheme. The C version cannot panic on any input, valid or not. That property should be preserved. ## Suggested fix Make the three helpers total, matching the C semantics: ```rust fn next_char(text: &str, position: usize) -> usize { match text.get(position..).and_then(|rest| rest.chars().next()) { Some(ch) => position + ch.len_utf8(), None => (position + 1).min(text.len()), // ponytail: mirrors ds4's `n = 1` truncation fallback } } ``` - `str::get(position..)` returns `None` instead of panicking on a non-boundary offset, which is the direct Rust equivalent of ds4's `if (pos + n > len) n = 1;`. - Add the missing `position >= text.len()` guard to `cjk_at`, plus the `< 128` ASCII short-circuit from `ds4.c:35722` — it is both a correctness guard and a fast path. - `char_info` already guards the length; switch it to `get()` for the boundary case. Also port the explanatory comment from `ds4.c:35699-35711` onto `letter_like`. ## Acceptance criteria - A round-trip test over deliberately awkward input — lone continuation bytes (`\x80`), a truncated multi-byte sequence at end of string, mixed CJK/ASCII/emoji, an empty string — encodes without panicking. - Token output for ordinary ASCII and normal UTF-8 text is byte-identical to before the change (compare against the existing tokenizer tests in `src/engine/tokenizer.rs`). - `cargo clippy --all-targets --all-features -- -D warnings` stays clean.

hugo added the bug label 2026-07-25 09:42:00 +00:00

hugo commented

2026-07-25 11:09:28 +00:00

Fixed in 292ee82. next_char, cjk_at, and char_info now use checked string access and the ds4-compatible one-byte fallback for non-boundary offsets; cjk_at also restores the ASCII fast path, and letter_like documents the byte-oriented rule. Added a regression test covering every byte offset in mixed ASCII/accented/CJK/emoji text plus empty and beyond-end positions. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.

Fixed in 292ee82. next_char, cjk_at, and char_info now use checked string access and the ds4-compatible one-byte fallback for non-boundary offsets; cjk_at also restores the ASCII fast path, and letter_like documents the byte-oriented rule. Added a regression test covering every byte offset in mixed ASCII/accented/CJK/emoji text plus empty and beyond-end positions. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.

hugo closed this issue

2026-07-25 11:09:31 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: hugo/DS4Server#16