fix: UTF-8 char boundary panic in repetition detector - #195
Closed
LopezNuance wants to merge 1 commit into
Closed
Conversation
…ndary) The repetition detector splits the output buffer into two halves using byte arithmetic (len/2, len-600) which panics when the offset lands inside a multi-byte UTF-8 character (math symbols like ᵢ, ×, σ). This poisons the Mutex<LlamaContext> and every subsequent request returns 502 forever. Also included: sliding-window KV eviction to allow generation beyond the context window, char-trigram cosine repetition detection, model caching, and penalty forwarding. Adds floor_char_boundary() — O(1) walk-back to nearest valid char boundary. Applied at the two slice sites in the repetition detection window splitting. Includes 7 unit tests. cargo fmt applied. Fixes Michael-A-Kuykendall#183 (residual from the from_utf8_lossy fix which addressed token-level UTF-8 but not the repetition detector string slicing). Signed-off-by: Scott Johnson <rsjohnnson@gmail.com> Signed-off-by: scott <you@example.com>
LopezNuance
force-pushed
the
fix/utf8-char-boundary-v2
branch
from
April 10, 2026 20:00
ff47ae8 to
4784103
Compare
Contributor
Author
|
Closing in favor of a comprehensive fix PR that addresses the root causes of degenerate output: penalty parameter order swap, sampler chain ordering, KV eviction destroying system prompt, and more. Sorry for the noise — the previous PRs were iterating on symptoms while the root causes were being diagnosed. |
4 tasks
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.
Summary
The repetition detector splits the output buffer into two halves using byte arithmetic (
len/2,len-600) which panics when the offset lands inside a multi-byte UTF-8 character (math symbols likeᵢ,×,σ, CJK). This poisons theMutex<LlamaContext>and every subsequent request to that Shimmy instance returns 502 forever until restart.This PR also includes the sliding-window KV eviction, char-trigram cosine repetition detection, model caching, and penalty forwarding that were pending in prior PRs on this branch.
Fix
Add
floor_char_boundary()— walks back at most 3 bytes to a valid UTF-8 char boundary. O(1), matches the not-yet-stablestr::floor_char_boundary()API. Applied at the two panicking slice sites in the repetition detection window splitting.Note on prior PRs
PRs #187, #192, #193, and #194 all addressed aspects of the same UTF-8 handling issue. #187 fixed
from_utf8in token emission. #192–#194 attempted to fix the repetition detector but usedchar_indices()on an already-invalid slice (which itself panics). This PR replaces that with the correct approach. Sorry for the noise — squashed to a single signed commit to keep the history clean.Test plan
cargo fmt -- --checkpassescargo clippy --no-default-features --features huggingface,llama -- -D warnings— zero warningscargo test --lib --features llama— 334 passedcargo test --lib --features llama ppt— 15 PPT contracts passedcargo test --test regression --features llama— 123 passed (1 pre-existing: issue_129 GPU workflow)