Skip to content

[Rust Frontend] Support OpenAI n choices - #43780

Closed
Xunzhuo wants to merge 1 commit into
vllm-project:mainfrom
Xunzhuo:vsr/rust-frontend-openai-n
Closed

Xunzhuo wants to merge 1 commit into
vllm-project:mainfrom
Xunzhuo:vsr/rust-frontend-openai-n

Conversation

@Xunzhuo

@Xunzhuo Xunzhuo commented May 27, 2026 •

Copy link
Copy Markdown
Member

This adds Rust frontend parity for OpenAI n > 1 on /v1/completions and /v1/chat/completions.

This PR was prepared with AI assistance.

Highlights:

  • Removes the previous Only n=1 is supported compatibility rejection.
  • Fan-outs n choices into child frontend requests, using child request IDs only when n > 1 so existing n=1 request-id behavior is preserved.
  • Offsets the seed by choice index to match vLLM/Python parallel sampling behavior.
  • Aggregates non-streaming choices and usage with prompt tokens counted once and completion tokens summed across choices.
  • Merges streaming child streams into one SSE stream with indexed choices, one final usage chunk when requested, and one [DONE] sentinel.
  • Adds route-level fan-out tests that verify child request IDs and seed offsets for chat and completions.

Test Plan

  • Format Rust code.
  • Check whitespace/conflict-marker hygiene.
  • Verify n > 1 validation and route behavior for chat/completions, both streaming and non-streaming.
  • Verify fan-out request IDs and seed offsets reach the mock engine for chat/completions.
  • Run the full vllm-server test suite.

Test Result

  • cargo fmt
  • git diff --check
  • cargo test -p vllm-server (178 passed, 0 failed)
  • cargo test -p vllm-server non_stream_chat_completions_fans_out_choice_request_ids_and_seeds
  • cargo test -p vllm-server non_stream_completions_fans_out_choice_request_ids_and_seeds
  • cargo test -p vllm-server non_stream_chat_completions_support_n_greater_than_one
  • cargo test -p vllm-server stream_chat_completions_support_n_greater_than_one
  • cargo test -p vllm-server non_stream_completions_support_n_greater_than_one
  • cargo test -p vllm-server stream_completions_support_n_greater_than_one

Not run: cargo nextest because cargo-nextest is not installed in this environment.


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR.
  • User-facing changes.
  • How the implementation works at a high level.
  • How the PR was tested.

Copilot AI review requested due to automatic review settings May 27, 2026 10:43
@Xunzhuo
Xunzhuo force-pushed the vsr/rust-frontend-openai-n branch from bf4335f to d0c8367 Compare May 27, 2026 10:45
@Xunzhuo Xunzhuo closed this May 27, 2026
@Xunzhuo Xunzhuo reopened this May 27, 2026
@Xunzhuo
Xunzhuo marked this pull request as draft May 27, 2026 11:13
@njhill njhill added the rust label May 27, 2026
@njhill
njhill requested a review from BugenZhao May 28, 2026 14:14
@mergify

mergify Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

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

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 Jun 3, 2026
Signed-off-by: xunzhuo <xunzhuo@vllm-semantic-router.ai>
@Xunzhuo
Xunzhuo force-pushed the vsr/rust-frontend-openai-n branch from d0c8367 to 86bfbae Compare June 3, 2026 15:54
@Xunzhuo Xunzhuo changed the title [Frontend] Support n choices in Rust OpenAI routes [Rust Frontend] Support OpenAI n choices Jun 3, 2026
@mergify mergify Bot removed the needs-rebase label Jun 3, 2026
@Xunzhuo
Xunzhuo marked this pull request as ready for review June 4, 2026 02:04
@Xunzhuo
Xunzhuo requested a review from njhill as a code owner June 4, 2026 02:04
@mergify

mergify Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

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

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 Jun 5, 2026

@BugenZhao BugenZhao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the work. The overall implementation LGTM


#[try_stream]
async fn chat_completion_parallel_chunk_stream(
stream: impl Stream<Item = Result<ChatCompletionStreamEvent, ApiError>>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about explicitly specify SelectAll<ChatCompletionEventStream> here so the semantics are more clear?

/// Whether the caller asked for the final streamed usage chunk.
pub include_usage: bool,
/// Number of chat completion choices requested by the caller.
pub n: u32,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe renaming variable will make the variable's purpose clearer?

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this pull request should remain open. Thank you!

@github-actions github-actions Bot added the stale Over 90 days of inactivity label Sep 7, 2026
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

This pull request has been automatically closed due to inactivity. Please feel free to reopen if you intend to continue working on it. Thank you!

@github-actions github-actions Bot closed this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase rust stale Over 90 days of inactivity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants