[Fix] Add validation for served model name to reserve : for LoRA adapter syntax#12912
Conversation
Summary of ChangesHello @neelabhsinha, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a crucial validation step for the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a validation to prevent the use of colons in served_model_name, which is a good fix to avoid conflicts with LoRA adapter syntax. The implementation is correct and the error message is clear. The addition of unit tests is also appreciated. However, the new tests do not correctly exercise the validation logic as they don't call the method where the validation is implemented. I've provided suggestions to fix the tests to ensure the new logic is properly covered.
…apter syntax (sgl-project#12912) Co-authored-by: neelabhsinha <neelabhsinha97@gmail.com>
Motivation
Fixes #12745
The --served-model-name argument currently allows colon (:) characters, which conflicts with the model:adapter syntax used for LoRA adapter specification in OpenAI-compatible APIs. When a served model name contains a colon,
it gets incorrectly parsed as if it includes a LoRA adapter name, causing confusion and unexpected behavior.
This PR adds validation to prevent colons in --served-model-name, ensuring clear separation between the served model name and the LoRA adapter syntax.
Modifications
- Added assertion in check_server_args() method to raise an AssertionError if served_model_name contains a colon
- Provides a clear error message explaining why colons are not allowed and shows the invalid value
- test_served_model_name_with_colon_raises_error: Verifies that a colon in the served model name raises the expected error with appropriate message
- test_served_model_name_without_colon_succeeds: Ensures that valid model names (without colons) work correctly
Accuracy Tests
Not applicable - this is a validation change that prevents invalid configurations. It does not affect model outputs or inference behavior.
Benchmarking and Profiling
Not applicable - this is a validation check that runs only during server initialization. No impact on inference speed.