Skip to content

Conversation

@hmellor
Copy link
Member

@hmellor hmellor commented Sep 22, 2025

Reinstates backward compatiblility for the following APIs removed by #22772:

  • Adds GuidedDecodingParams which wraps StructuredOutputsParams (this was just a rename)
  • Add guided_decoding back to SamplingParams/SamplingParams.from_optional and forward the value to structured_outputs_params

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
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 adds backward compatibility for GuidedDecodingParams by reintroducing it as a deprecated alias for StructuredOutputsParams. The changes are logically sound for ensuring compatibility. However, the implementation introduces duplicated code for issuing deprecation warnings, which can impact maintainability and user experience. My review focuses on removing this redundancy.

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@aarnphm
Copy link
Collaborator

aarnphm commented Sep 23, 2025

Fo we need this for 0.11?

im also good with breaking change

@hmellor
Copy link
Member Author

hmellor commented Sep 23, 2025

We've been made aware that having the breaking change this sudden will causee a lot of breakages for RL use cases.

As backwards compatibility goes, this is pretty unobtrusive to vLLM and very loud with warnings to encourage users to switch.

@tlrmchlsmth tlrmchlsmth added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 23, 2025
Copy link
Member

@russellb russellb left a comment

Choose a reason for hiding this comment

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

This seems important and aligned with our deprecation policy: https://docs.vllm.ai/en/latest/contributing/deprecation_policy.html#deprecation-pipeline

It's actually proposing to skip the "Deprected + off by default" step, but I'm OK with that. I think that part is more important for HTTP API breakages, which are harder to inform the client about.

@hmellor hmellor merged commit 875d6de into vllm-project:main Sep 23, 2025
51 of 52 checks passed
@hmellor hmellor deleted the guided-decoding-bc branch September 23, 2025 16:07
@facebook-github-bot
Copy link

@minosfuture has imported this pull request. If you are a Meta employee, you can view this in D83067559.

FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
…5422)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: yewentao256 <zhyanwentao@126.com>
gjc0824 pushed a commit to gjc0824/vllm that referenced this pull request Oct 10, 2025
…5422)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: gaojc <1055866782@qq.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 10, 2025
…5422)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
choprahetarth pushed a commit to Tandemn-Labs/vllm that referenced this pull request Oct 11, 2025
…5422)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
…5422)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
…5422)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed structured-output v1

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants