Skip to content

Avoid panic when byte_pair_encode is given an empty piece - #583

Open
devYRPauli wants to merge 1 commit into
openai:mainfrom
devYRPauli:fix/empty-piece-panic
Open

Avoid panic when byte_pair_encode is given an empty piece#583
devYRPauli wants to merge 1 commit into
openai:mainfrom
devYRPauli:fix/empty-piece-panic

Conversation

@devYRPauli

Copy link
Copy Markdown

byte_pair_encode special-cases a piece of length one but not length zero. An empty slice falls through to the piece_len < 100 branch into _byte_pair_merge, where for i in 0..piece.len() - 1 underflows usize and the following piece[i..i + 2] slice index panics.

This is reachable from Python:

>>> enc = tiktoken.get_encoding("cl100k_base")
>>> enc.encode("")
[]
>>> enc._encode_single_piece(b"")
pyo3_runtime.PanicException: range end index 2 out of range for slice of length 0

encode("") already returns an empty list, so the panic is an inconsistency rather than intended behaviour. byte_pair_split handles the same situation explicitly with assert!(piece.len() > 1).

The fix returns an empty vector for an empty piece, before the existing length-one check.

Added a regression test alongside the existing ones in src/lib.rs. Without the guard it panics at src/lib.rs:149; with it, cargo test passes 3/3.

Note: #559 proposed the same one-line guard. It was closed by its author without any maintainer response, so I do not believe the change was ever considered and rejected. Happy to close this if that is not the case.

byte_pair_encode special-cases a piece of length one but not length
zero. An empty slice falls through to the piece_len < 100 branch and
calls _byte_pair_merge, where `for i in 0..piece.len() - 1` underflows
usize and the following `piece[i..i + 2]` slice index panics.

This is reachable from Python: enc._encode_single_piece(b"") raises
PanicException("range end index 2 out of range for slice of length 0"),
even though enc.encode("") already returns an empty list.

Return an empty vector for an empty piece, matching that behaviour.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@devYRPauli

Copy link
Copy Markdown
Author

@hauntsaninja this has had no review since July 20. It is 1 commit, src/lib.rs only, +11/-1, and it merges clean onto main.

byte_pair_encode special-cases a piece of length one but not length zero, so an empty piece underflows usize and panics at src/lib.rs:149. encode("") already returns an empty list, so the two paths disagree today. The fix returns an empty vector before the existing length-one check.

One question before you spend time on it. #559 proposed the same guard, and its author closed it with no maintainer reply, so I read it as never considered. If it was considered and rejected, tell me and I will close this.

The head f0a04cdb has zero check runs and zero commit statuses, so no workflow has started on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant