[Misc] Use VLLMValidationError in batch, pooling, and tokenize protocol validators#36256
Conversation
|
Hi @umut-polat, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
There was a problem hiding this comment.
Code Review
This pull request continues the effort to standardize error handling by replacing ValueError with the custom VLLMValidationError in several protocol validators. The changes are generally good, but I've identified a potential improvement in how validation errors for conflicting parameters are reported. In pooling/base/protocol.py and serve/tokenize/protocol.py, the error points to a single parameter when two are in conflict, which could be misleading. I've suggested removing the specific parameter from the error to make it more accurate, albeit less specific.
| raise VLLMValidationError( | ||
| "Cannot set both `continue_final_message` and " | ||
| "`add_generation_prompt` to True." | ||
| "`add_generation_prompt` to True.", | ||
| parameter="continue_final_message", | ||
| ) |
There was a problem hiding this comment.
The validation error is raised when both continue_final_message and add_generation_prompt are True. However, the parameter argument for VLLMValidationError is set only to "continue_final_message". This can be misleading for API clients that use this field to highlight the source of the error, as the issue stems from the combination of two parameters. To avoid ambiguity, consider omitting the parameter argument for this validation check. This makes the structured error less specific but more accurate.
raise VLLMValidationError(
"Cannot set both `continue_final_message` and "
"`add_generation_prompt` to True."
)d3acfdf to
593576c
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
593576c to
eb4730c
Compare
fd8ce19 to
8a0ff16
Compare
|
Hi @umut-polat, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
…ol validators Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com>
8a0ff16 to
cc1ace9
Compare
|
Sorry for the delay! |
…ol validators (vllm-project#36256) Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com>
…ol validators (vllm-project#36256) Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com>
…ol validators (vllm-project#36256) Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com>
…ol validators (vllm-project#36256) Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com>
…ol validators (vllm-project#36256) Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com>
…ol validators (vllm-project#36256) Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com> Signed-off-by: Monishver Chandrasekaran <monishverchandrasekaran@gmail.com>
…ol validators (vllm-project#36256) Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com>
Replaces
ValueErrorwithVLLMValidationErrorin the remaining protocol model validators across three files:run_batch.py:validate_no_fileinBatchTranscriptionRequestandBatchTranslationRequest→ parameter:filepooling/base/protocol.py:check_generation_prompt→ parameter:continue_final_messagetokenize/protocol.py:check_generation_prompt→ parameter:continue_final_messageThis completes the
VLLMValidationErrormigration across all protocol-level model validators in the OpenAI-compatible endpoints.Previous PRs in this series: #35456, #35510, #35664, #35666, #36254.