fix(protocols): accept min_tokens=0 in OpenAI requests#1851
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughValidation for ChangesZero-value sampling validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @Azure99, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
There was a problem hiding this comment.
Code Review
This pull request updates the validation of min_tokens in both chat and completion requests to allow a minimum value of 0 instead of 1, and adds a corresponding test to verify this behavior. The reviewer pointed out that since min_tokens is an unsigned 32-bit integer (u32), the #[validate(range(min = 0))] attribute is redundant and can be safely removed to simplify the code.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| #[validate(range(min = 0))] | ||
| pub min_tokens: Option<u32>, |
There was a problem hiding this comment.
Since min_tokens is of type Option<u32>, the underlying value is an unsigned 32-bit integer, which is guaranteed to be non-negative by definition. Therefore, the #[validate(range(min = 0))] attribute is redundant and can be safely removed to avoid unnecessary validation overhead and simplify the code.
| #[validate(range(min = 0))] | |
| pub min_tokens: Option<u32>, | |
| pub min_tokens: Option<u32>, |
| #[validate(range(min = 0))] | ||
| pub min_tokens: Option<u32>, |
There was a problem hiding this comment.
Since min_tokens is of type Option<u32>, the underlying value is an unsigned 32-bit integer, which is guaranteed to be non-negative by definition. Therefore, the #[validate(range(min = 0))] attribute is redundant and can be safely removed to avoid unnecessary validation overhead and simplify the code.
| #[validate(range(min = 0))] | |
| pub min_tokens: Option<u32>, | |
| pub min_tokens: Option<u32>, |
Signed-off-by: Azure99 <961523404@qq.com>
Description
Problem
OpenAI-compatible Chat Completions/Completions requests currently reject
min_tokens: 0at protocol validation time because both request types use#[validate(range(min = 1))].min_tokens=0is commonly used as the no-minimum-generation value. vLLM acceptsmin_tokens >= 0, SGLang documentsmin_new_tokens: int = 0, and TensorRT-LLM documents thatmin_tokens < 1has no effect:Solution
Allow
min_tokens=0in Chat Completions and legacy Completions protocol validation.Changes
ChatCompletionRequest.min_tokensvalidation frommin=1tomin=0.CompletionRequest.min_tokensvalidation frommin=1tomin=0.min_tokens=0.Test Plan
cargo +nightly fmt --all --check- passcargo test -p smg --test spec_test- pass (96 passed; 0 failed; 1 ignored)cargo clippy --all-targets --all-features -- -D warnings- passChecklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit
0for sampling-related minimum settings in both chat and completion requests, instead of requiring values to be at least1.min_tokens = 0edge case and confirm such requests validate successfully.