Skip to content

Conversation

@njhill
Copy link
Member

@njhill njhill commented Oct 1, 2025

Follow-on to #26005.

Make the logic clearer and less fragile.

Signed-off-by: Nick Hill <nhill@redhat.com>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the logic for handling multiple sampling sequences (n > 1) in add_request. The changes make the code clearer and more robust by correctly using the updated SamplingParams after processing inputs. Specifically, it fixes a bug where the loop range and a condition for handling the last child request were using stale or incorrect parameter objects due to variable shadowing. The new implementation correctly fetches the processed sampling parameters and uses a different variable name to avoid shadowing, which resolves the issues and improves readability. The changes look good and correctly address the fragility of the previous implementation.

Signed-off-by: Nick Hill <nhill@redhat.com>
@njhill njhill added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 1, 2025
@vllm-bot vllm-bot merged commit 169313b into vllm-project:main Oct 2, 2025
44 of 47 checks passed
pdasigi pushed a commit to pdasigi/vllm that referenced this pull request Oct 2, 2025
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: yewentao256 <zhyanwentao@126.com>
tomeras91 pushed a commit to tomeras91/vllm that referenced this pull request Oct 6, 2025
…ect#26032)

Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: Tomer Asida <57313761+tomeras91@users.noreply.github.com>
southfreebird pushed a commit to southfreebird/vllm that referenced this pull request Oct 7, 2025
@njhill njhill deleted the clearer-params-handling branch October 10, 2025 04:25
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 10, 2025
…ect#26032)

Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
alhridoy pushed a commit to alhridoy/vllm that referenced this pull request Oct 24, 2025
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
…ect#26032)

Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants