Skip to content

[Frontend] Support min_p in the Responses API - #48084

Merged
sfeng33 merged 4 commits into
vllm-project:mainfrom
sungbin1015:frontend/responses-min-p
Oct 8, 2026
Merged

sfeng33 merged 4 commits into
vllm-project:mainfrom
sungbin1015:frontend/responses-min-p

Conversation

@sungbin1015

@sungbin1015 sungbin1015 commented Jul 9, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

The Responses API (/v1/responses) already exposes top_k as a request field
and forwards it to SamplingParams, but its natural companion min_p is
missing — even though SamplingParams (and SamplingParams.from_optional)
fully support it, and the Completions/Chat endpoints both expose it.

This adds the min_p field to ResponsesRequest, resolves its default
alongside top_k in to_sampling_params, and plumbs it through to
SamplingParams.from_optional, mirroring the existing top_k handling exactly.
Users of /v1/responses can now use min-p sampling like every other generation
endpoint.

Not a duplicate: searched open PRs/issues from several angles
(responses min_p, ResponsesRequest min_p in:body,
responses/protocol.py min_p in:body, min_p in:title, responses API sampling parameters). min_p is entirely absent from
responses/protocol.py (no matches in the file), and no open PR adds sampling
parameters to the Responses API. PR #45839 adds sampling params to the separate
Translation API only; this change is orthogonal.

Test Plan

.venv/bin/python -m pytest \
  tests/entrypoints/openai/responses/test_sampling_params.py -v
pre-commit run --files \
  vllm/entrypoints/openai/responses/protocol.py \
  tests/entrypoints/openai/responses/test_sampling_params.py \
  docs/serving/online_serving/openai_compatible_server.md

Added a min_p assertion to test_basic_sampling_params and a new
test_min_p_default verifying the neutral default (0.0).

Test Result

8 passed in 1.07s

ruff check, ruff format, and mypy all pass via pre-commit.

Related work


AI assistance disclosure: this PR was prepared with AI (Claude) assistance. I
have reviewed every changed line and ran the tests above myself.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify

mergify Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Documentation preview: https://vllm--48084.org.readthedocs.build/en/48084/

@mergify mergify Bot added documentation Improvements or additions to documentation frontend labels Jul 9, 2026
@mergify

mergify Bot commented Sep 16, 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, @sungbin1015.

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 Sep 16, 2026
The Responses API already exposes `top_k` and forwards it to
`SamplingParams`, but its natural companion `min_p` is missing even though
`SamplingParams` fully supports it. Add the `min_p` field to
`ResponsesRequest`, resolve its default alongside `top_k`, and plumb it
through `to_sampling_params`, mirroring the existing `top_k` handling so
`/v1/responses` users get min-p sampling like the other endpoints.

Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: sungbin1015 <sbin@solbox.com>
@sungbin1015
sungbin1015 force-pushed the frontend/responses-min-p branch from e49aa01 to 0b9e493 Compare October 6, 2026 06:19
@sungbin1015

Copy link
Copy Markdown
Contributor Author

Rebased onto the latest main and resolved the docs conflict with #55912 (kept the /v1/responses/render paragraph and moved the "Sampling parameters" section below it). No code changes since the original commit. pre-run-check fails only because I have fewer than 4 merged PRs — could a maintainer add the ready label and take a look? Thanks!

@mergify mergify Bot removed the needs-rebase label Oct 6, 2026

@sfeng33 sfeng33 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 contribution.

@sfeng33 sfeng33 added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 8, 2026
@sfeng33

sfeng33 commented Oct 8, 2026

Copy link
Copy Markdown
Member

/ci run

@sfeng33
sfeng33 enabled auto-merge (squash) October 8, 2026 16:37
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

✅ @sungbin1015, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Your branch must contain every commit currently on its upstream target branch. Merge or rebase onto the latest target branch, then rerun the command. Append --allow-stale to a run command to test an outdated branch at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #93648 for commit 4500391db11a.

Signed-off-by: sfeng33 <4florafeng@gmail.com>
@mergify mergify Bot added the performance Performance-related issues label Oct 8, 2026
@sfeng33

sfeng33 commented Oct 8, 2026

Copy link
Copy Markdown
Member

/ci run --allow-stale

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #93685 for commit 459b6f2261aa.

⚠️ This PR is 11 commits behind upstream main. Running CI at your own risk because --allow-stale was requested; outdated CI configuration may cause failures. Before merging, merge or rebase onto the latest main, then rerun /ci run on the latest PR commit.

@sfeng33
sfeng33 merged commit 2ddde19 into vllm-project:main Oct 8, 2026
106 of 107 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation frontend performance Performance-related issues 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