Skip to content

[Frontend] Apply each endpoint's top_logprobs cut in derender - #59518

Merged
DarkLight1337 merged 7 commits into
vllm-project:mainfrom
yinli-systems:topk-cut-parity
Oct 15, 2026
Merged

DarkLight1337 merged 7 commits into
vllm-project:mainfrom
yinli-systems:topk-cut-parity

Conversation

@yinli-systems

@yinli-systems yinli-systems commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Fixes #59513. Builds on the generate logprobs shape from #58181 (merged); rebased on main, one commit.

The engine's per-position logprobs hold the sampled token first and then ranks 1..k, so k distinct entries when the sampled token is in the top k and k + 1 when it is not. /v1/chat/completions keeps the first k and /v1/completions the first k + 1 (the OpenAI completions convention). Generate cut to max(k, 1) and derender passed that through, so /v1/completions/derender returned k entries where the coupled endpoint returns k + 1 whenever the sampled token was outside the top k. The greedy parity tests never produce that case. Following @shimib's suggestion on #58181:

  • Generate (Python and Rust, both output_modes) returns every candidate the engine produced, sampled first, with no cut. Rust drops the repeated sampled entry (its engine row lists a sampled top-1 token twice) by skipping later entries equal to the sampled token (linear, no allocation, even for logprobs=-1).
  • Derender applies the coupled endpoint's cut through the same helpers the coupled endpoints now use (chat_top_logprobs_limit / completion_top_logprobs_limit), read from chat_request / completion_request: chat keeps entry i when logprob_token_ids is set, top_logprobs == -1 or i < top_logprobs (_get_top_logprobs); completions when logprob_token_ids is set or logprobs >= i. The cut happens before decoding. Without the request derender returns every candidate (documented).
  • Rust /v1/chat/completions drops the repeated sampled entry, like Python; before, a sampled top-1 token appeared twice and one candidate was lost.
  • Docs: renderer.md (what generate returns) and derenderer.md (the per-endpoint cut).

Test Plan

  • New tests/entrypoints/scale_out/derender/test_logprobs_endpoint_parity.py: engine dicts built with the engine's own append_logprobs_for_next_position and detokenized as the engine does, with the sampled token at rank 1, inside and outside the top k, run through the real coupled chat / completions helpers and through generate + derender; k in 0, 1, 2, 5 (and -1 for chat). Plus: without the request, every candidate is kept.
  • Updated generate unit tests to engine-realistic dicts; Rust tests for the generate dedup (no cut) and the chat dedup.
  • End-to-end on a live server (--enable-scale-out, Qwen3-4B, L20): the same request (temperature=1.5, seeds 7/11/23, k = 1/2/5, 96 tokens) through the coupled endpoint and through render -> /inference/v1/generate -> derender, compared per position (token ids, top_logprobs length, tokens, bytes, logprobs within 1e-4), before and after this change.
  • Benchmark of generate and derender cost at top_logprobs=20.

Test Result

End-to-end, 18 trials (9 chat, 9 completions), 1,632 comparable positions:

before after
positions where generate + derender differs from the coupled endpoint 215 0
of which the sampled token was outside the top k 215 0
chat positions that differ 0 0

Before the change the mismatches were exactly the 215 completions positions with the sampled token outside the top k. One chat trial is excluded on both sides because the coupled endpoint itself is not run-to-run deterministic there (two identical coupled requests diverge at position 73 once the prefix is cached, at temperature=1.5); generate is repeatable.

Benchmark (1,000 positions, top_logprobs=20, 30% of positions with the sampled token outside the top k, Qwen2.5 tokenizer, median of 7):

before after
generate _create_tokens_logprobs 14.8 µs/token 13.5 µs/token
generate response size 1,043 B/token 1,057 B/token
chat derender 66.2 µs/token 65.2 µs/token
completions derender 67.4 µs/token 69.3 µs/token (decodes the entry it used to drop)

Tests on an L20 with live servers (tiny Llama, gpt2, Qwen3-0.6B): test_derender.py 27, test_derender_stream.py 52, test_logprobs_endpoint_parity.py 10, test_generate_logprobs_conversion.py 5, test_serving_tokens.py 13, test_generate_stream.py 31, test_tokens_logprobs.py 7 passed; not run there: the DeepSeek parser and gpt-oss harmony tests (model size) and the LoRA test (adapters not downloaded). CPU-only runs: 414 passed across scale_out, test_serving_chat.py and openai/completion (3 failures that fail identically on main). Rust: vllm-server 440 passed, cargo clippy -D warnings and cargo fmt clean. mypy (tools/pre_commit/mypy.py 3.12) reports only the 2 existing Parser errors in online_derenderer.py that main has too; ruff and typos clean.

AI assistance (Claude Code) was used for this change; the submitter reviewed every changed line and ran the tests above.

@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run for upstream CI or /amd-ci run for AMD CI only whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use the corresponding /ci run, /ci retry, and /ci cancel commands, or their /amd-ci variants. New commits do not start upstream CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: 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.

🚀

@mergify

mergify Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Documentation preview: https://vllm--59518.org.readthedocs.build/en/59518/

@mergify mergify Bot added documentation Improvements or additions to documentation frontend rust labels Sep 30, 2026
@mergify

mergify Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @yinli-systems.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify

mergify Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @yinli-systems.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify

mergify Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @yinli-systems.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@yinli-systems

Copy link
Copy Markdown
Contributor Author

@hickeyma Rebased onto main. Two upstream changes overlapped, so in addition to the rebase:

  • [Rust Frontend] Support logprob_token_ids on OpenAI endpoints #59549 already dedupes the repeated sampled token in Rust chat_top_logprob_entries (unique_by, with a test), so this PR now uses that version and no longer touches that function.
  • [Bugfix][Frontend] Return all logprobs when completions requests logprobs=-1 #55717 made /v1/completions return every candidate for logprobs=-1. completion_top_logprobs_limit now returns None (all) for -1, so completions derender follows it; I checked it against the new coupled predicate for every logprobs in -1..29 × row length 0..39 × logprob_token_ids on/off. Docs and the parity test no longer expect {} for -1 (0e38921). This also corrects what I wrote above: a position with candidates is never {} now; positions without engine logprobs are still None.

@hickeyma

Copy link
Copy Markdown
Contributor

/ci run

@github-actions

Copy link
Copy Markdown

❌ @hickeyma, Only reviewers with write access can use CI commands before CI is delegated to the PR author.

@hickeyma hickeyma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @yinli-systems

@DarkLight1337
DarkLight1337 enabled auto-merge (squash) October 15, 2026 10:59
@DarkLight1337

Copy link
Copy Markdown
Member

/ci run

@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 15, 2026
@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #95111 for commit 55030bd6cdda.

…no logprobs

A chat_request that does not ask for logprobs says nothing about the cut, so
derender now passes through what generate returned (as without a chat_request
and as completions derender already does for logprobs unset) instead of
cutting to top_logprobs=0. Fixes test_text_parity_logprobs, which sends a
chat_request without logprobs fields.

Signed-off-by: Yin Li <kevinli.ai.work@gmail.com>
@yinli-systems

Copy link
Copy Markdown
Contributor Author

@DarkLight1337 @hickeyma Thanks for the approvals and for starting CI. CI found two failures in Entrypoints Integration (API Server):

  • test_derender_parity.py::test_text_parity_logprobs (MI355, shard 2) was caused by this PR. The test sends derender a chat_request with only model and messages, so no logprobs. The chat helper treated the unset top_logprobs as a cut of 0 and dropped every candidate. Fixed in a55673f: a chat_request that doesn't ask for logprobs says nothing about the cut, so derender now passes through what generate returned. That's the same as having no chat_request, and the same as completions derender already does when logprobs is unset. A unit test now covers both endpoints, with no request and with a request that doesn't ask for logprobs.
  • test_derender_parity.py::test_parity_reasoning (H200, shard 1) isn't from this PR. It fails the same way on main: Buildkite #95108 for e10c10a has the identical <|end▁of▁sentence|> content mismatch, and the test doesn't request logprobs.

The three AMD kernel and model jobs that exited with -1 look like infrastructure failures, not test failures.

auto-merge was automatically disabled October 15, 2026 11:52

Head branch was pushed to by a user without write access

@DarkLight1337

Copy link
Copy Markdown
Member

Should be fixed by #61757, let's wait for that and then we could rebase this PR

@DarkLight1337

Copy link
Copy Markdown
Member

/ci retry

@DarkLight1337
DarkLight1337 enabled auto-merge (squash) October 15, 2026 13:52
@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #95139 for commit 88badd2d0ad5, running 2 failed step(s) from Buildkite CI #95111.

@DarkLight1337
DarkLight1337 merged commit 3dcfaeb into vllm-project:main Oct 15, 2026
18 checks passed
shimib added a commit to shimib/vllm that referenced this pull request Oct 15, 2026
Resolves the overlap with vllm-project#59518: the batch path keeps both the
hidden-reasoning gate and the top_logprobs cut, and the parsed streaming
path now passes top_limit=_chat_top_logprobs_limit(chat_request) to
_resolve_logprobs like the plain path, as agreed on the PRs.
hao-aaron added a commit to hao-aaron/vllm that referenced this pull request Oct 19, 2026
Adapt to vllm-project#59518, which replaced ResponseOptions.logprobs with
include_logprobs and dropped the top_logprobs cut: carry the requested k
as ResponseOptions.top_k_logprobs, and feed k + 1 wide rows to the
top-k/default render parity test.

Signed-off-by: hao-aaron <aaronhao@inferact.ai>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hao-aaron added a commit to hao-aaron/vllm that referenced this pull request Oct 19, 2026
Adapt to vllm-project#59518, which dropped the top_logprobs cut from the generate
response: render_tokens_logprobs no longer takes k and emits every
candidate of the row (one fewer when the sampled id repeats in the
top-k), matching the per-entry path.

Signed-off-by: hao-aaron <aaronhao@inferact.ai>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
michael-gpu pushed a commit to practical-inference/vllm-ampere-extended that referenced this pull request Oct 20, 2026
…roject#59518)

Signed-off-by: Yin Li <kevinli.ai.work@gmail.com>
Co-authored-by: Yin Li <kevinli.ai.work@gmail.com>
Co-authored-by: Cyrus Leung <tlleungac@connect.ust.hk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation frontend ready ONLY add when PR is ready to merge/full CI is needed rust

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: top_logprobs cut differs across chat, completions, generate and derender when the sampled token is outside the top-k

3 participants