Repository navigation
[Frontend] Integer token IDs for generate output logprobs (GenerateLogProbs) - #58181
Merged
DarkLight1337 merged 7 commits intoOct 6, 2026
Merged
Conversation
yinli-systems
requested review from
AndreasKaratzas,
BugenZhao,
DarkLight1337,
NickLucche,
aarnphm,
njhill and
robertgshaw2-redhat
as code owners
September 22, 2026 16:21
Contributor
|
Documentation preview: https://vllm--58181.org.readthedocs.build/en/58181/ |
mergify
Bot
added
documentation
1 of 5 tasks
yinli-systems
force-pushed
the
generate-logprobs-shape
branch
from
September 22, 2026 16:27
479dc67 to
3f48845
Compare
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment Once the PR is approved or has the If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
Contributor
|
LGTM — the conversion is correct and the U+FFFD context handling matches the engine's contract. One sequencing heads-up: this rewrites the same |
Contributor
Author
|
Thanks for the review! Agreed on the order: #55029 first. I've already prepared the follow-up locally. It is #55029 merged onto this branch: the streaming paths keep your per-chunk resolution and carried context ( |
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
yinli-systems
force-pushed
the
generate-logprobs-shape
branch
from
September 25, 2026 02:45
3f48845 to
778f3da
Compare
yinli-systems
force-pushed
the
generate-logprobs-shape
branch
from
September 27, 2026 10:30
778f3da to
e15090f
Compare
1 task done
1 task done
hickeyma
reviewed
Sep 29, 2026
hickeyma
left a comment
hickeyma
left a comment
Contributor
There was a problem hiding this comment.
@yinli-systems decode_token_id still has a bug which I raised in #59241. It goes through convert_ids_to_tokens → convert_tokens_to_string which drops the SentencePiece leading space the engine has restored since #48674. So ▁true comes back as 'true' instead of ' true' and top_logprobs keys collide on /v1/completions/derender.
Since you're already touching this function, could you switch it to convert_ids_list_to_tokens(tokenizer, [token_id])[0]? It's just one line.
Contributor
Author
|
@hickeyma Done in 5c83af2: |
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
Contributor
|
@yinli-systems Do you mind resolving the merge conflict? |
…gProbs)
`/inference/v1/generate` is a vLLM-defined token-in/token-out API, but its
output logprobs used the OpenAI shape: the generate server has no tokenizer,
so it wrote every token as a `"token_id:N"` placeholder string that derender
parsed back out. The two frontends also disagreed on `bytes` (Python left it
unset, Rust set it to the bytes of the placeholder itself).
Replace `GenerateResponseChoice.logprobs` and
`GenerateResponseStreamChoice.logprobs` with `GenerateLogProbs`:
GenerateLogProb: token_id, logprob, rank
GenerateLogProbsContent(GenerateLogProb): top_logprobs: list[GenerateLogProb]
GenerateLogProbs: content: list[GenerateLogProbsContent] | None
`top_logprobs` is a list in rank order, not a dict, so the ordering is explicit
and the keys stay integers on the wire. `rank` is kept: the engine already
computes it and the Rust frontend's gRPC `CandidateTokenInfo` carries it.
`content=None` is the normal "no per-token candidates requested" state.
Both frontends emit the new shape, streaming and non-streaming. Derender
converts it to `ChatCompletionLogProbs` / `CompletionLogProbs`, filling `token`
and `bytes` from the tokenizer with the existing U+FFFD byte-fallback
correction; that correction needs the preceding sampled token ids as context,
which the integer shape now carries directly. The placeholder parsing
(`_parse_token_id_placeholder`, `resolve_token_id_placeholder`) drops out of
the generate path; `decode_token_id` replaces it.
`prompt_logprobs` on the same response is unchanged, and
`return_tokens_as_token_ids` on `/v1/chat/completions` and `/v1/completions` is
untouched: it is a user-facing OpenAI option that uses `token_id:N` on purpose.
Breaking change for direct generate consumers, replaced in one release per the
discussion in vllm-project#57574. Code that reads only `content[i].logprob` is unaffected;
code that parsed the placeholder reads `content[i].token_id` instead.
Closes vllm-project#57574
Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Yin Li <kevinli.ai.work@gmail.com>
decode_token_id went through convert_ids_to_tokens -> convert_tokens_to_string, which drops the Metaspace leading space the engine restores since vllm-project#48674 (`▁true` came back as "true"). On /v1/completions/derender that made `▁true` and `true` collide as top_logprobs keys and shifted text_offset by one per spaced token; chat derender token/bytes did not match /v1/chat/completions. Resolve through convert_ids_list_to_tokens, the engine's per-token detokenization. The no-vocab-entry guard stays in front of it. Fixes vllm-project#59241. Signed-off-by: Yin Li <kevinli.ai.work@gmail.com>
- GenerateLogProb.logprob is required again, as in vllm-project#57574 and the Rust frontend; _create_tokens_logprobs passes -9999.0 explicitly in its one fallback branch, so a derender payload missing logprob is rejected instead of silently becoming -9999. - Describe top_logprobs order as the engine's (sampled token first, then rank order) in the Python and Rust docs and renderer.md, and drop the `content: null` note, which is not a state this PR produces. - Decode a position's sampled and top-k ids in one batch (decode_token_ids): 2 + n tokenizer calls per position instead of 3n. - Switch the two oversized-logprobs derender tests to token_id payloads so they reach the bound checks instead of failing schema validation. - Tests: sampled token outside the top k comes first, logprob required, an unknown top id does not shift its neighbours. Signed-off-by: Yin Li <kevinli.ai.work@gmail.com>
The Rust generate route mapped every engine entry at a position, so it returned the sampled token twice when it was also in the top k and never cut to the requested count (k+1 entries where Python returns k). It now drops repeated token ids (keeping the sampled entry first) and cuts to max(logprobs, 1), or all for -1, matching the Python generate server. ResponseOptions carries the requested count instead of a bool. Document that rank is the vocabulary rank on every entry and that the list is not sorted by it. Signed-off-by: Yin Li <kevinli.ai.work@gmail.com>
yinli-systems
force-pushed
the
generate-logprobs-shape
branch
from
October 5, 2026 11:25
de1c0cd to
4a6748f
Compare
Contributor
Author
|
@hickeyma Rebased onto main after #58588. Tokens mode now carries |
Member
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
hickeyma
reviewed
Oct 5, 2026
Contributor
There was a problem hiding this comment.
@yinli-systems Getting there, thanks for your patience. Do you mind addressing the claude bot review comments?
- Rust generate: drop the repeated sampled entry with one HashSet pass instead of rescanning the earlier entries for every entry. With logprobs=-1 the position holds the whole vocabulary, where the rescan was quadratic (~2e10 comparisons per token for a 200k vocab); the dedup of a 200k-entry position now takes ~0.1 s in a debug test build. - Both frontends: the engine reports rank 0 for a sampled token whose logprob is NaN; send it as rank null, the way the NaN logprob itself is clamped, and document it. Signed-off-by: Yin Li <kevinli.ai.work@gmail.com>
Contributor
Author
hickeyma
approved these changes
Oct 5, 2026
hickeyma
left a comment
hickeyma
left a comment
Contributor
There was a problem hiding this comment.
LGTM, thanks @yinli-systems
Contributor
|
/ready |
Member
|
@claude review |
DarkLight1337
added
the
verified
There was a problem hiding this comment.
Beyond the inline NaN-logprob finding, I also checked whether the now-strict GenerateLogProbs/GenerateTokensChoice schema (protocol.py:404) would break /inference/v1/generate parsing during a mixed-version disaggregated rollout (old generate server emitting the old ChatCompletionLogProbs shape against a new derender, or vice versa) — that is exactly the breaking change the PR's release note already documents, not a new defect introduced here, so I'm not flagging it separately.
Extended reasoning...
Verified the confirmed NaN-logprob finding independently: _create_tokens_logprobs (serving.py) computes max(step_token.logprob, -9999.0) with the possibly-NaN value as the first argument, and Python's max() keeps the first operand when a comparison against NaN is involved, so the sentinel never applies and a literal NaN reaches the JSON response; the PR's own new test only asserts rank is None, not the logprob value, so this slipped through. Checked git history: the two findings from my prior review (Rust O(n^2) dedup, rank=0 on NaN) were addressed in commit 871a7cd, whose message claims the logprob was "already clamped" — that belief is what left this new bug unaddressed. The mixed-version 422 candidate was examined and ruled out as the documented breaking-change behavior itself, not a separate bug.
max(nan, -9999.0) is nan in Python, so the clamps in _create_tokens_logprobs and the text-mode builder let a NaN logprob through: JSONResponse rejected it (500, non-streaming) and streaming wrote "logprob": null, which derender then refused as logprob is required. The Rust frontend already gets -9999.0 from f32::max. Add _clamp_logprob (NaN -> -9999.0, else max(x, -9999.0)) and use it at every call site, tokens and text mode. Tests assert -9999.0 for the content and top_logprobs entries and that the result serializes as strict JSON. Signed-off-by: Yin Li <kevinli.ai.work@gmail.com>
Member
|
@claude review |
DarkLight1337
approved these changes
Oct 6, 2026
DarkLight1337
enabled auto-merge (squash)
October 6, 2026 15:48
Member
|
/ci run |
github-actions
Bot
added
the
ready
|
✅ Triggered Buildkite CI #93120 for commit |
Contributor
CI selector (shadow): no narrower answer, every step runs: 193 test steps (269 jobs) instead of 45 (58 jobs)Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works. Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.
Why: rust: rust/src/server/src/routes/inference/generate.rs: root crate bucket; 18 gate-env steps + 12 hardware-image steps; + the PyO3 bridge claim (vllm/tool_parsers/rust_tool_parser.py) Selector would run (193)
Would skip (today's rules run them) (0)none Would add (today's rules do not run them) (148)
AMD mirrors: would skip (0)none AMD mirrors: would add (123)
18 changed files · base |
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
6 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.
Purpose
Fixes #57574
Fixes #59241
Implements the
GenerateLogProbsshape agreed in #57574./inference/v1/generateis a vLLM-defined token-in/token-out API, but its output logprobs used the OpenAI shape: the generate server has no tokenizer, so it wrote every token as a"token_id:N"placeholder string that derender parsed back out, and the two frontends disagreed onbytes(Python left it unset, the Rust frontend set it to the UTF-8 bytes of the placeholder itself).Scope is exactly the split @hickeyma described in #57574 (comment): this PR is
contentonly. #57442 stacksGenerateLogProbs.sampledon top afterwards.Shape
top_logprobsis a list, not a dict: JSON turns dict keys into strings and the order would be implicit. It keeps the engine's order: the sampled token first, then the remaining candidates in rank order. With non-greedy sampling the sampled token can be outside the top k (e.g. ranks[5, 1, 2]); it then takes one of the k slots and rank k is left out, as on the OpenAI endpoints and the main engine path.rankis kept: the engine already computes it and the Rust frontend's gRPCCandidateTokenInfocarries it. It was being dropped on the floor before.bytes, no stringtoken.logprobis required, as in [Feature]: Integer token IDs for logprobs in/inference/v1/generateresponses (GenerateLogProbs) #57574 and the Rust frontend. When the sampled token is absent from the engine's top-k map the server passes-9999.0explicitly, the value the OpenAI shapes already put on the wire for that case, so it is unchanged.What changed
scale_out/token_in_token_out):_create_tokens_logprobsbuilds the new type;GenerateResponseChoice.logprobsandGenerateResponseStreamChoice.logprobsareGenerateLogProbs | None, streaming and non-streaming.routes/inference/generate): same field swap,position_to_generate_logprobs_contentemits ids + rank instead offormat_token_id._resolve_logprobsconvertsGenerateLogProbs→ChatCompletionLogProbs, fillingtoken/bytesfrom the tokenizer, and_convert_chat_logprobs_to_completion_logprobsthen producesCompletionLogProbsas before. The U+FFFD byte-fallback correction (_correct_decoded_token) is untouched — it needs the preceding sampled token ids as context, and the integer shape now carries those directly instead of reconstructing them from placeholder strings._parse_token_id_placeholderandresolve_token_id_placeholderare gone; the id-taking core is nowdecode_token_idinentrypoints/generate/base/serving.py.prompt_logprobs(already integer-keyed) andreturn_tokens_as_token_idson/v1/chat/completions//v1/completions, which is a user-facing OpenAI option that usestoken_id:Non purpose.Release note
Documented in
docs/serving/online_serving/renderer.md(new "Generate Output Logprobs" section, with the breaking-change note) andderenderer.md.Test Plan
tests/entrypoints/scale_out/derender/test_generate_logprobs_conversion.py(new): unit tests over_resolve_logprobswith a stub tokenizer — decoding + bytes + rank-order preservation, the U+FFFD byte-fallback path where the second half of a two-byte character is only decodable with the preceding sampled id as context, andcontent=None.tests/entrypoints/scale_out/token_in_token_out/test_tokens_logprobs.py: sampled entry carriestoken_id/rank, per-candidate ids,logprobs=0still emits the sampled token, sampled token missing from the top-k map keeps the-9999.0sentinel.tests/entrypoints/scale_out/derender/test_derender.py,test_derender_parity.py: derender chat/completion logprob tests and the coupled-vs-derender parity test now feed the generate-shaped payload; parity assertions ontoken/bytesare unchanged.tests/entrypoints/scale_out/token_in_token_out/test_generate_stream.py,test_serving_tokens.py: streaming chunks and the sampling-mask test readtoken_id.routes::tests::non_stream_raw_generate_returns_token_output_envelopeandstream_raw_generate_returns_sse_chunks_and_usageassert integertoken_id+rankand a nulltoken/byteson the wire.Test Result
Rust, locally:
The two updated assertions run through the mock engine and confirm the wire shape end to end (
"token_id": 33, "rank": 1,token/bytesabsent), streaming and non-streaming.Python, locally (CPU-only box, no GPU):
ruff check/ruff format --checkclean on every touched file.Caveat on the local Python run: the box has no GPU, so the interpreter is a vLLM 0.29.0 install with this branch's source on
PYTHONPATH. That is enough for the pure-Python paths above, but the server-backed entrypoints tests do not run in it.tests/entrypoints/scale_out/token_in_token_out/test_generate_stream.pyfails there — identically (29 failed, 1 passed) on unmodifiedmainin the same setup, so those failures are the environment, not this change. CI is still the first real execution of the server-backed tests; happy to fix up promptly if anything falls over.Essential Elements