[Responses][CI] Filter negative token IDs in schema fuzz test to avoid 500 errors#35231
Conversation
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
…d 500 errors Signed-off-by: Andreas Karatzas <akaratza@amd.com>
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
There was a problem hiding this comment.
Code Review
This pull request adds server-side validation to reject negative token IDs in completion requests, which previously caused 500 errors. The change is implemented via a model_validator in CompletionRequest and is accompanied by new tests. The implementation is correct, but I've suggested a simplification to the validation logic to improve readability and maintainability by removing a redundant check and a nested loop.
| if isinstance(prompt, list): | ||
| for item in prompt: | ||
| if isinstance(item, str): | ||
| continue | ||
| if isinstance(item, int): | ||
| if item < 0: | ||
| raise VLLMValidationError( | ||
| "Token IDs in `prompt` must be non-negative.", | ||
| parameter="prompt", | ||
| ) | ||
| elif isinstance(item, list): | ||
| for token_id in item: | ||
| if isinstance(token_id, int) and token_id < 0: | ||
| raise VLLMValidationError( | ||
| "Token IDs in `prompt` must be non-negative.", | ||
| parameter="prompt", | ||
| ) |
There was a problem hiding this comment.
The logic for validating prompt token IDs can be simplified to improve readability and maintainability. The current implementation has a redundant check for string types and a nested loop that can be replaced with a more concise any() expression. This also removes code duplication.
if isinstance(prompt, list):
for item in prompt:
if isinstance(item, int):
if item < 0:
raise VLLMValidationError(
"Token IDs in `prompt` must be non-negative.",
parameter="prompt",
)
elif isinstance(item, list):
if any(isinstance(x, int) and x < 0 for x in item):
raise VLLMValidationError(
"Token IDs in `prompt` must be non-negative.",
parameter="prompt",
)There was a problem hiding this comment.
Replaced the custom validator with a Pydantic-native Field(ge=0) constraint on the prompt type annotation. No custom validation code needed. Also added explicit test cases for both flat and nested negative token ID prompts in test_completion_error.py.
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
…d 500 errors (vllm-project#35231) Signed-off-by: Andreas Karatzas <akaratza@amd.com>
…d 500 errors (vllm-project#35231) Signed-off-by: Andreas Karatzas <akaratza@amd.com>
…d 500 errors (vllm-project#35231) Signed-off-by: Andreas Karatzas <akaratza@amd.com> Signed-off-by: xjx <493337577@qq.com>
…d 500 errors (vllm-project#35231) Signed-off-by: Andreas Karatzas <akaratza@amd.com>
…d 500 errors (vllm-project#35231) Signed-off-by: Andreas Karatzas <akaratza@amd.com>
…d 500 errors (vllm-project#35231) Signed-off-by: Andreas Karatzas <akaratza@amd.com> Signed-off-by: Andrii Skliar <askliar@nvidia.com>
…d 500 errors (vllm-project#35231) Signed-off-by: Andreas Karatzas <akaratza@amd.com>
Adds server-side validation to reject negative token IDs in
POST /v1/completions, which previously caused an unhandled 500 Internal Server Error.Problem
When the
promptfield contains negative token IDs (e.g.{"prompt": [[-1]]}), the server crashes with a 500 instead of returning a proper 400 validation error. This was discovered via schemathesis fuzz testing intest_openapi_stateless.Fix
Added a
model_validatorinCompletionRequest(vllm/entrypoints/openai/completion/protocol.py) that checks all token IDs inpromptare non-negative, consistent with how other fields likelogprobs,prompt_logprobs, andcache_saltare already validated on the same class.Testing
test_openapi_statelessschemathesis fuzz test now passes forPOST /v1/completionswithout needing a test-side filter (the server returns a 400 which schemathesis treats as valid behavior).test_completion_error.pyfor both flat ([-1]) and nested ([[-1]]) negative token ID prompts, verifying the validator returns a proper validation error.