Skip to content

Conversation

sfeng33
Copy link
Contributor

@sfeng33 sfeng33 commented Sep 1, 2025

Purpose

This PR introduces a basic Renderer component that aims to consolidate vLLM's fragmented input processing pipeline, implementing the design outlined in RFC #22880. The Renderer will serve as a single unified entry point for converting high-level API requests into tokenized formats ready for engine consumption.

Changes

Core Implementation

  • BaseRenderer: Abstract base class defining the unified input processing interface.
  • Renderer.render_prompt: Concrete implementation for completion-style requests handling text/token inputs, truncation, and validation.

Integration Updates

Updated two endpoints to use render_prompt for input processing:

  • Pooling endpoint
  • Tokenize endpoint

Test Plan

  1. Unit test
python -m pytest tests/entrypoints/test_renderer.py -v
python -m pytest tests/entrypoints/openai/test_tokenization.py -v
python -m pytest tests/entrypoints/openai/test_pooling.py -v
  1. Manual test
python -m vllm.entrypoints.openai.api_server \
  --model BAAI/bge-base-en-v1.5 \
  --port 8000

curl -X POST http://localhost:8000/pooling \
-H "Content-Type: application/json" \
-d '{
  "model": "BAAI/bge-base-en-v1.5",
  "input": ["First sentence", "Second sentence", "Third sentence"]
}'

curl -X POST http://localhost:8000/pooling \
-H "Content-Type: application/json" \
-d '{
  "model": "BAAI/bge-base-en-v1.5",
  "input": [[101, 2028, 102], [101, 2048, 102], [101, 2093, 102]],
  "truncate_prompt_tokens": 1
}'

@mergify mergify bot added the frontend label Sep 1, 2025
@sfeng33 sfeng33 changed the title [Refactor] Implement basic Renderer [Refactor] Introduce basic Renderer for completion-style request Sep 1, 2025
@sfeng33 sfeng33 marked this pull request as ready for review September 1, 2025 01:15
@DarkLight1337
Copy link
Member

/gemini review

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 a Renderer class to centralize input processing, which is a good step towards simplifying the codebase. The refactoring of the pooling and tokenization endpoints to use this new component is well-executed. However, I've found a few critical issues in the new Renderer implementation and its integration. One issue can lead to incorrect prompt ordering in batched requests with mixed input types. Another is a regression where handling for a specific parameter value is missing. Additionally, there's a bug in the logging logic that will cause runtime errors. Addressing these issues will ensure the new component is robust and correct.

Copy link

mergify bot commented Sep 1, 2025

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

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

Signed-off-by: sfeng33 <[email protected]>
Signed-off-by: sfeng33 <[email protected]>
Signed-off-by: sfeng33 <[email protected]>
Signed-off-by: sfeng33 <[email protected]>
Signed-off-by: sfeng33 <[email protected]>
Signed-off-by: sfeng33 <[email protected]>
Signed-off-by: sfeng33 <[email protected]>
Signed-off-by: sfeng33 <[email protected]>
@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 4, 2025
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) September 4, 2025 03:33
@DarkLight1337 DarkLight1337 merged commit 712b273 into vllm-project:main Sep 4, 2025
48 checks passed
@sfeng33 sfeng33 deleted the processor branch September 4, 2025 05:25
JasonZhu1313 pushed a commit to JasonZhu1313/vllm that referenced this pull request Sep 7, 2025
eicherseiji pushed a commit to eicherseiji/vllm that referenced this pull request Sep 9, 2025
LopezCastroRoberto pushed a commit to LopezCastroRoberto/vllm that referenced this pull request Sep 11, 2025
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