Skip to content

[Pooling] Validate non-negative rerank top_n#46119

Merged
yewentao256 merged 2 commits into
vllm-project:mainfrom
taneem-ibrahim:rerank_request_fix
Jun 22, 2026
Merged

[Pooling] Validate non-negative rerank top_n#46119
yewentao256 merged 2 commits into
vllm-project:mainfrom
taneem-ibrahim:rerank_request_fix

Conversation

@taneem-ibrahim

@taneem-ibrahim taneem-ibrahim commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Purpose

top_n represents a result count for rerank requests, so negative values are invalid input. Before this change, top_n=-1 was silently accepted and treated the same as top_n=0, returning all rerank results. This could hide client-side bugs and make request behavior harder to reason about.

This PR adds request-level validation so top_n must be non-negative, while preserving the existing behavior that top_n=0 means “return all results” and values larger than the document count are accepted. It also adds protocol-level tests for the default, valid, oversized, and negative cases.

Test Plan

Run the standard pre-commit hook

Test Result

Tests pass

@taneem-ibrahim taneem-ibrahim requested a review from noooop as a code owner June 19, 2026 01:40
@mergify mergify Bot added the frontend label Jun 19, 2026

@yewentao256 yewentao256 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!

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.

We don't need to have this unit test spcially for this small update

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed the unit test commit.

Comment thread vllm/entrypoints/pooling/scoring/protocol.py Outdated
Signed-off-by: Taneem Ibrahim <taneem.ibrahim@gmail.com>

@yewentao256 yewentao256 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.

LGTM, thanks for the work!

@yewentao256 yewentao256 added the ready ONLY add when PR is ready to merge/full CI is needed label Jun 20, 2026
@yewentao256 yewentao256 merged commit f2069b0 into vllm-project:main Jun 22, 2026
53 checks passed
@taneem-ibrahim taneem-ibrahim deleted the rerank_request_fix branch June 22, 2026 15:50
nkzhenhua pushed a commit to nkzhenhua/vllm that referenced this pull request Jun 24, 2026
Signed-off-by: Taneem Ibrahim <taneem.ibrahim@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
qli88 pushed a commit to qli88/vllm that referenced this pull request Jun 26, 2026
Signed-off-by: Taneem Ibrahim <taneem.ibrahim@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Signed-off-by: Qiang Li <qiang.li2@amd.com>
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

2 participants