Skip to content

[Misc] Remove the duplicate code#28111

Merged
aarnphm merged 1 commit intovllm-project:mainfrom
chaunceyjiang:misc
Nov 6, 2025
Merged

[Misc] Remove the duplicate code#28111
aarnphm merged 1 commit intovllm-project:mainfrom
chaunceyjiang:misc

Conversation

@chaunceyjiang
Copy link
Collaborator

@chaunceyjiang chaunceyjiang commented Nov 5, 2025

Purpose

When implementing #24317, #26706 had not yet been merged. Now that #26706 is merged, this piece of code is no longer needed.

Test Plan

A complete end-to-end test was written in #24317, which can verify that the functionality remains correct after removing this code.

Test Result


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.

Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
@mergify mergify bot added the frontend label Nov 5, 2025
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 correctly removes a redundant block of code from _construct_input_messages_with_harmony. The removed code, which converted a dict to a ResponseFunctionToolCall object, is now handled by the function_call_parsing model validator in the ResponsesRequest class. This change is a good cleanup that improves maintainability by eliminating duplicated logic. The change is correct and I have no further suggestions.

@chaunceyjiang chaunceyjiang added the ready ONLY add when PR is ready to merge/full CI is needed label Nov 5, 2025
@aarnphm aarnphm merged commit 07d6145 into vllm-project:main Nov 6, 2025
52 checks passed
ZhengHongming888 pushed a commit to ZhengHongming888/vllm that referenced this pull request Nov 8, 2025
Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend 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.

2 participants