Switch vllm_rollout endpoint /v1/completions to /inference/v1/generate - #16
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request migrates the vLLM rollout implementation from the /v1/completions endpoint to the /inference/v1/generate endpoint. The changes include updating request payload construction, simplifying response parsing for tokens and logprobs, and renaming internal functions to reflect the new API. Additionally, a validation check was added to prevent the use of rollout routing replay, as it is currently unsupported. Feedback suggests refining the error message for this validation check to be more descriptive and professional.
Comment on lines
+1825
to
+1827
| raise ValueError( | ||
| "TODO: add https://github.com/vllm-project/vllm/pull/39568 to support rollout routing replay in /inference/v1/generate. " | ||
| ) |
Contributor
There was a problem hiding this comment.
The error message contains a 'TODO' and a raw URL, which is a bit informal for a user-facing ValueError. It would be better to state clearly that the feature is currently unsupported and provide the reference as a note.
Suggested change
| raise ValueError( | |
| "TODO: add https://github.com/vllm-project/vllm/pull/39568 to support rollout routing replay in /inference/v1/generate. " | |
| ) | |
| raise ValueError( | |
| "Rollout routing replay is not yet supported for vLLM when using the /inference/v1/generate endpoint. " | |
| "Support is planned (see https://github.com/vllm-project/vllm/pull/39568)." | |
| ) |
aoshen02
reviewed
May 22, 2026
Collaborator
|
Please add some tests. |
Collaborator
Author
|
Added a new test covered: Qwen3-0.6B, Qwen3-30B-A3B, Qwen3-30B-A3B-R3. |
Collaborator
Author
|
Note that currently vllm v0.21.0 will fail at the |
aoshen02
added a commit
that referenced
this pull request
May 26, 2026
Aligns all 5 example rollout functions to the same /inference/v1/generate (token-in/token-out) path that slime/rollout/vllm_rollout.py:387 has used since PR #16. Previously each example had its own ad-hoc shape: 4 of them on /v1/completions (OpenAI completions, with `prompt` + token-id field and parallel `logprobs.token_logprobs` / `token_ids` arrays), and the VLM example on /v1/chat/completions (image_url message content). Migration pattern (text token-in/out): - POST /inference/v1/generate with {model, token_ids, sampling_params} - Map rollout sampling params via _build_inference_sampling_params - Parse choices[0] via _inference_generate_tokens_and_logprobs - finish_reason is the scalar choices[0].finish_reason (no more meta_info.finish_reason.type wrapper) VLM (geo3k_vlm_multi_turn): switched to canonical two-stage flow — POST /v1/chat/completions/render to get token_ids+features, forward to /inference/v1/generate. The example now maintains a messages list across turns (env.format_observation already returns chat-style content with image objects) and re-renders each turn; train-side multimodal features are still produced locally by the HF processor. Side effects: - retool / search-r1: postprocess_responses (string truncation at </tool_call> / </code> / </answer>) was a workaround for SGLang's return_logprob=False path that re-tokenized truncated strings. It's unsound under token-precise tracking, so it's removed; both branches (logprobs on/off) now share the engine's token output verbatim. - tau-bench: call_to_action_sglang stays for backward compat; docstring clarified that it's engine-agnostic. Also finishes the consistent_hash / x-session-id rename inside docs/{en,zh}/advanced/vllm-config.md — the file was renamed in 8b3d1e3 but its body still pointed at `consistent_hashing` / `X-SMG-Routing-Key` from the SGLang-router era. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
momo609
pushed a commit
that referenced
this pull request
May 26, 2026
#16) * change vllm_rollout endpoint /v1/completions to /inference/v1/generate * update with vllm latest main commit: 4f940896a32c9e2a0eba7f50d521bf5f6b4de458 * test vllm inference generate endpoint * test config aligns with the latest main
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR switches the vLLM text-only rollout path from the OpenAI-compatible /v1/completions endpoint to the SkyRL style token-only /inference/v1/generate endpoint.
The new path tokenizes prompts on the rollout client side, sends token_ids to vLLM, and parses generated token ids and per-token logprobs directly from the
/inference/v1/generateresponse. This also aligns the vLLM rollout behavior with the existing SGLang text-only rollout style, where the rollout client owns tokenization and the server receives token ids.More reasons about why use
inference/v1/generateinstead of/v1/completions:/inference/v1/generateis designed for this, see [RFC]: Disaggregated Everything - Token In <> Token Out API Server vllm#22817 .Changes
/v1/completions.Note:
This PR only targets the text-only vLLM rollout path. Multimodal rollout is not part of this change. The existing multimodal flow still uses: /v1/chat/completions/render -> /inference/v1/generate in SkyRL.
Routed experts support is currently not supported in
/inference/v1/generatein v0.20.2. So please don't merge this pr currently. It is in vllm-project/vllm#39568. Once it is released in next version. I will keep doing this pr.