Skip to content

Conversation

@GuyStone
Copy link
Contributor

@GuyStone GuyStone commented Sep 10, 2025

Purpose

  • Use a local variable.
  • Remove unnecessary output variable assignment.

Test Plan

  • No functional change expected - rely on existing Build/CI tests.

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

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 introduces two minor refactorings in the beam_search method. It caches tokenizer.eos_token_id in a local variable and removes an unnecessary intermediate variable for the RequestOutput. These changes improve code clarity and are functionally correct. I've added one suggestion to handle cases where a tokenizer might not have an EOS token defined, which would prevent incorrect behavior during beam search.

Signed-off-by: Guy Stone <guys@spotify.com>
@njhill njhill added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 10, 2025
@vllm-bot vllm-bot merged commit 8a89408 into vllm-project:main Sep 11, 2025
38 of 40 checks passed
skyloevil pushed a commit to skyloevil/vllm that referenced this pull request Sep 13, 2025
dsxsteven pushed a commit to dsxsteven/vllm_splitPR that referenced this pull request Sep 15, 2025
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 10, 2025
…llm-project#24554)

Signed-off-by: Guy Stone <guys@spotify.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
…llm-project#24554)

Signed-off-by: Guy Stone <guys@spotify.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants