-
-
Notifications
You must be signed in to change notification settings - Fork 12k
Strengthen input validation and tests for 'parse_raw_prompts’. #30652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this 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 aims to strengthen input validation for parse_raw_prompts, particularly for list-of-lists inputs. The changes introduce stricter checks and add a new test case. While the intent is good, the new validation logic in vllm/inputs/parse.py is not fully robust. It fails to correctly validate all elements in nested lists and doesn't consistently check for empty sub-lists. I've provided a critical review comment with a suggested code change to fix these validation flaws, ensuring all sub-prompts are correctly and thoroughly validated.
|
This pull request has merge conflicts that must be resolved before it can be |
4c270c1 to
28d588a
Compare
DarkLight1337
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
|
PTAL at the failing test |
Head branch was pushed to by a user without write access
Signed-off-by: Kayvan Mivehnejad <[email protected]>
Signed-off-by: Kayvan Mivehnejad <[email protected]>
Signed-off-by: Kayvan Mivehnejad <[email protected]>
Signed-off-by: Kayvan Mivehnejad <[email protected]>
…ested list inputs Signed-off-by: Kayvan Mivehnejad <[email protected]>
66685ab to
598e47d
Compare
Purpose
The ‘parse_raw_prompts’ can benefit from the stricter input validation in the array-of-arrays input path. This is for when input is a list of lists; however, we verify the nested inner list is a non-null list of integers beyond the first inner list verification.
Test Plan
Added a pytest coverage for invalid and edge-case inputs of mixed-type nested lists for parse_raw_prompts. Updated the array-of-arrays input condition of parse_raw_prompts to strictly check empty prompt lists within list and mixed-type nested lists. Validated these invalid inputs by running “pytest tests/test_inputs.py” to confirm correct exception types are raised for malformed inputs.
Test Result
Before this change, mixed-type nested prompt inputs were not checked for rejection and could lead to ambiguous behavior. After applying strict validation, the new and existing tests pass, and malformed inputs deterministically raise TypeError or ValueError as expected.
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.