Repository navigation
Conversation
serve_tokens set output_kind only for streaming, so non-streaming requests kept the SamplingParams default (CUMULATIVE). serve_tokens_full_generator keeps only the last streamed RequestOutput, and under CUMULATIVE each sequence is pruned from the stream once it finishes, so with n > 1 the sequences that finished before the final step never reach `choices` and the response silently returns fewer than n completions. The OpenAI completion path already uses FINAL_ONLY when not streaming; do the same here. Root cause and repro from @qgallouedec in vllm-project#52398. Fixes vllm-project#52398 Signed-off-by: Chenglun Hu <chenglunhu@gmail.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 954d971f8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| payload = { | ||
| "model": MODEL_NAME, | ||
| "token_ids": [9707, 1879, 11], | ||
| "sampling_params": {"max_tokens": 5, "temperature": 1.0, "n": n}, |
There was a problem hiding this comment.
With max_tokens=5, all choices normally hit the length limit in the same engine iteration; the RequestOutputCollector then merges those same-step cumulative outputs, so the pre-fix implementation can still return all n choices and pass this test. An unseeded, temperature-1 sample only exposes the regression if a choice happens to emit EOS early, making coverage probabilistic and potentially flaky. Use a deterministic setup or mocked generator that finishes choices on different steps so reverting the production fix reliably fails.
AGENTS.md reference: AGENTS.md:L82-L93
Useful? React with 👍 / 👎.
|
There is already an open PR to fix the issue: #52399 |
|
closing — #52399 already covers this (didn't surface in my search, my bad for the dup). thanks @DarkLight1337. |
Non-streaming
/inference/v1/generatewithn > 1returns fewer thannchoices, no error.serve_tokensonly setsoutput_kindfor the streaming case, so non-streaming keeps the SamplingParams default (CUMULATIVE);serve_tokens_full_generatorthen keeps only the last streamedRequestOutput, and under CUMULATIVE each sequence is pruned from the stream once it finishes, so sequences that finished before the final step never make it intochoices. The OpenAI completion path already uses FINAL_ONLY when not streaming — this does the same.Root cause + repro are @qgallouedec's in #52398; I offered there to put up the fix. Happy to hand it back if you'd rather carry it yourself.
Added an
n=2/4regression test totest_serving_tokens.py(the existing scale-out tests are alln=1).Fixes #52398