Skip to content

[Bugfix] Reject empty logprob_token_ids and fix falsy validation in OpenAI request protocols - #57394

Open
nandan2003 wants to merge 1 commit into
vllm-project:mainfrom
nandan2003:fix-empty-logprob-token-ids
Open

nandan2003 wants to merge 1 commit into
vllm-project:mainfrom
nandan2003:fix-empty-logprob-token-ids

Conversation

@nandan2003

@nandan2003 nandan2003 commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Fixes #49319. Supersedes stalled PR #49320 by addressing review feedback from @NolenLiang.

In ChatCompletionRequest, CompletionRequest, and BatchChatCompletionRequest:

  • Explicitly check is not None and reject logprob_token_ids=[] with VLLMValidationError.
  • Skip length checks if input isn't a list, letting Pydantic handle type errors cleanly without crashing on TypeError.
  • Add min_length=1 to Field(...) so the OpenAPI schema reflects minItems: 1.
  • Stop coercing logprob_token_ids or None in to_sampling_params().

Test Plan

Added standalone unit tests covering empty lists, non-list inputs, and missing logprobs:

pytest tests/entrypoints/openai/chat_completion/test_logprob_token_ids_validation.py
ruff check
ruff format --check

Test Result

  • Before: logprob_token_ids: [] returned 200 OK (silently dropped to null), and non-list inputs like 1 caused a TypeError.
  • After: Both return 400 Bad Request with clear error messages (must not be an empty list and Input should be a valid list).

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

@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 added frontend bug Something isn't working labels Sep 17, 2026
@nandan2003
nandan2003 force-pushed the fix-empty-logprob-token-ids branch from ddcc9a2 to 7bfaace Compare September 17, 2026 14:44
@nandan2003

Copy link
Copy Markdown
Contributor Author

Hi @DarkLight1337 , @aarnphm

Could you please check on this PR when you get a chance? I believe its a clean merge.

Thanks

@mergify

mergify Bot commented Sep 29, 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, @nandan2003.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify

mergify Bot commented Oct 10, 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, @nandan2003.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

…cols

When logprob_token_ids was provided as an empty list [], it bypassed
validation due to a boolean truthiness check (if data.get(...)). This
allowed invalid requests with logprob_token_ids=[] and logprobs=False
to pass validation, and coerced [] to None in to_sampling_params().

- Explicitly check data.get("logprob_token_ids") is not None.
- Reject empty lists with VLLMValidationError attributing parameter="logprob_token_ids".
- Defer non-list inputs immediately to let Pydantic handle type checking.
- Set min_length=1 on Field(...) to expose minItems: 1 in OpenAPI schemas.
- Add minimal, fast standalone unit tests.

Fixes vllm-project#49319

Signed-off-by: Nandan Vallamdasu <nandan.vallamdasu@outlook.com>
@nandan2003
nandan2003 force-pushed the fix-empty-logprob-token-ids branch from 05f1e3f to 14724e5 Compare October 21, 2026 02:48
@mergify mergify Bot removed the needs-rebase label Oct 21, 2026
@DarkLight1337
DarkLight1337 requested a review from njhill October 21, 2026 03:42
@mergify

mergify Bot commented Oct 22, 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, @nandan2003.

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 Oct 22, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working frontend needs-rebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: logprob_token_ids: [] silently bypasses validation and is discarded, on both Chat Completions and Completions

1 participant