[Bugfix] Fix logprobs token-string collision from SentencePiece space…#48674
Open
aoshen02 wants to merge 3 commits into
Open
[Bugfix] Fix logprobs token-string collision from SentencePiece space…#48674aoshen02 wants to merge 3 commits into
aoshen02 wants to merge 3 commits into
Conversation
… stripping Fixes vllm-project#44319 SentencePiece's decode() strips the leading space from the first token (add_dummy_prefix inverse), causing distinct tokens like "▁true" (id=1565) and "true" (id=3009) to both decode to "true". When the Legacy Completions API builds top_logprobs as dict[str, float], these collide and overwrite each other — making logprob values appear to change with the requested count. Fix: after decode(), check the raw vocab piece via convert_ids_to_tokens() and restore any leading spaces that were stripped. This keeps output human-readable while making dict keys collision-free. Simplified from vllm-project#44504 — uses convert_ids_to_tokens() directly instead of probing for SentencePiece's id_to_piece through multiple getattr chains, since they return identical results. Co-authored-by: Ao Shen <aoshen@inferact.ai> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Allen Shen <aoshen@inferact.ai>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Replace hardcoded _LEADING_SPACE_MARKERS frozenset with dynamic detection via _get_leading_space_marker() which reads the pre_tokenizer config. Only Metaspace pre_tokenizers (Llama/Mistral/T5) need the fix; ByteLevel (GPT-2) and others skip entirely — no convert_ids_to_tokens call needed. Co-authored-by: Ao Shen <aoshen@inferact.ai> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Allen Shen <aoshen@inferact.ai>
aoshen02
force-pushed
the
fix/logprobs-token-string-collision
branch
from
f4e7b8a to
d6c7466
Compare
chaunceyjiang
left a comment
chaunceyjiang
left a comment
Collaborator
There was a problem hiding this comment.
Thanks. I’ll run some tests locally today.
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
3 participants
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.
… stripping
Fixes #44319
SentencePiece's decode() strips the leading space from the first token (add_dummy_prefix inverse), causing distinct tokens like "▁true" (id=1565) and "true" (id=3009) to both decode to "true". When the Legacy Completions API builds top_logprobs as dict[str, float], these collide and overwrite each other — making logprob values appear to change with the requested count.
Fix: after decode(), check the raw vocab piece via convert_ids_to_tokens() and restore any leading spaces that were stripped. This keeps output human-readable while making dict keys collision-free.
Simplified from #44504 — uses convert_ids_to_tokens() directly instead of probing for SentencePiece's id_to_piece through multiple getattr chains, since they return identical results.
Purpose
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.