Skip to content

[Frontend] Delegate preprocessing to OpenAIServingRender#36483

Merged
vllm-bot merged 20 commits intovllm-project:mainfrom
sagearc:delegate-openai-renderer
Mar 13, 2026
Merged

[Frontend] Delegate preprocessing to OpenAIServingRender#36483
vllm-bot merged 20 commits intovllm-project:mainfrom
sagearc:delegate-openai-renderer

Conversation

@sagearc
Copy link
Copy Markdown
Contributor

@sagearc sagearc commented Mar 9, 2026

Purpose

OpenAIServingRender (#36166) owns the authoritative preprocessing logic for chat and completion requests, but OpenAIServingChat and OpenAIServingCompletion were duplicating it rather than delegating.

Changes:

  • Split render_chat_request / render_completion_request into a public entry point (with model check) and a private helper (render_chat / render_completion) with no model/engine checks.
  • OpenAIServingChat and OpenAIServingCompletion now accept openai_serving_render and delegate their preprocessing to those helpers after doing their own engine-aware checks.
  • OpenAIServingRender is created first in init_generate_state so it can be passed to all serving classes.

Test Plan

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: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
@mergify mergify bot added the v1 label Mar 9, 2026
Copy link
Copy Markdown
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 preprocessing logic for chat and completion requests by centralizing it into OpenAIServingRender. This is a good architectural improvement that reduces code duplication.

However, I've identified a critical issue. The warmup() method in vllm/entrypoints/openai/chat_completion/serving.py was not updated to reflect this refactoring. It still calls self._preprocess_chat(), which has been moved to OpenAIServingRender. This will cause an AttributeError and prevent the server from starting.

Since this code is not in the diff, I cannot add a specific comment. The fix is to update the call in warmup() to delegate to self.openai_serving_render._preprocess_chat(...). This is a critical fix.

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 9, 2026

Hi @sagearc, the pre-commit checks have failed. Please run:

uv pip install pre-commit
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy failing?
mypy is run differently in CI. If the failure is related to this check, please use the following command to run it locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
@mergify
Copy link
Copy Markdown

mergify bot commented Mar 9, 2026

Hi @sagearc, the pre-commit checks have failed. Please run:

uv pip install pre-commit
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy failing?
mypy is run differently in CI. If the failure is related to this check, please use the following command to run it locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
@sagearc sagearc requested a review from mgoin as a code owner March 9, 2026 11:12
sagearc added 2 commits March 9, 2026 13:16
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
@mergify
Copy link
Copy Markdown

mergify bot commented Mar 11, 2026

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

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

@mergify mergify bot added the needs-rebase label Mar 11, 2026
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
@mergify mergify bot removed the needs-rebase label Mar 12, 2026
sagearc and others added 4 commits March 12, 2026 13:12
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
@sagearc sagearc requested a review from DarkLight1337 March 12, 2026 12:10
@sagearc
Copy link
Copy Markdown
Contributor Author

sagearc commented Mar 12, 2026

@DarkLight1337 Updated OpenAIRenderer with the new model registry, let me know what do you think

sagearc and others added 2 commits March 12, 2026 14:17
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
@mergify
Copy link
Copy Markdown

mergify bot commented Mar 12, 2026

Hi @sagearc, the pre-commit checks have failed. Please run:

uv pip install pre-commit
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy failing?
mypy is run differently in CI. If the failure is related to this check, please use the following command to run it locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10

Copy link
Copy Markdown
Member

@DarkLight1337 DarkLight1337 left a comment

Choose a reason for hiding this comment

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

Thanks, let's see if tests pass now

@DarkLight1337 DarkLight1337 enabled auto-merge (squash) March 12, 2026 14:57
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Mar 12, 2026
@mergify
Copy link
Copy Markdown

mergify bot commented Mar 12, 2026

Hi @sagearc, the pre-commit checks have failed. Please run:

uv pip install pre-commit
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy failing?
mypy is run differently in CI. If the failure is related to this check, please use the following command to run it locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 12, 2026

Hi @sagearc, the pre-commit checks have failed. Please run:

uv pip install pre-commit
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy failing?
mypy is run differently in CI. If the failure is related to this check, please use the following command to run it locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10

@sagearc sagearc requested a review from DarkLight1337 March 13, 2026 07:29
@vllm-bot vllm-bot merged commit a226861 into vllm-project:main Mar 13, 2026
43 of 45 checks passed
@sagearc sagearc deleted the delegate-openai-renderer branch March 13, 2026 07:41
whycoming pushed a commit to whycoming/vllm that referenced this pull request Mar 13, 2026
…ect#36483)

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: whycoming <120623296@qq.com>
Lucaskabela pushed a commit to Lucaskabela/vllm that referenced this pull request Mar 17, 2026
wendyliu235 pushed a commit to wendyliu235/vllm-public that referenced this pull request Mar 18, 2026
fxdawnn pushed a commit to fxdawnn/vllm that referenced this pull request Mar 19, 2026
QiuMike pushed a commit to QiuMike/vllm-omni that referenced this pull request Mar 25, 2026
with vllm top commit 1b6cb920e6ebcac57154e6154578c39d4892a16c
has some diffs with vllm-omni,

vllm-project/vllm#32104
vllm-project/vllm#32951
vllm-project/vllm#37287
vllm-project/vllm#36483

just modify vllm-omni to work

Signed-off-by: Michael Qiu <qiudayu.qdy@antgroup.com>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm that referenced this pull request Mar 27, 2026
Monishver11 pushed a commit to Monishver11/vllm that referenced this pull request Mar 27, 2026
…ect#36483)

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Monishver Chandrasekaran <monishverchandrasekaran@gmail.com>
vrdn-23 pushed a commit to vrdn-23/vllm that referenced this pull request Mar 30, 2026
…ect#36483)

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Vinay Damodaran <vrdn@hey.com>
EricccYang pushed a commit to EricccYang/vllm that referenced this pull request Apr 1, 2026
…ect#36483)

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: EricccYang <yangyang4991@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 v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants