From 912d6b619de0f2a44df74704973cff90fa0fb4e9 Mon Sep 17 00:00:00 2001 From: Reid <61492567+reidliu41@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:02:00 +0800 Subject: [PATCH] [Rust Frontend] Align sampling validation with Python (#47494) Signed-off-by: reidliu41 --- rust/src/server/src/error.rs | 16 ++++ rust/src/server/src/grpc/tests.rs | 29 +++++++ rust/src/text/src/error.rs | 4 + rust/src/text/src/lib.rs | 2 +- rust/src/text/src/lower.rs | 119 +++++++++++++++++++++++++++- rust/src/text/src/lower/sampling.rs | 92 +++++++++++++++++++++ 6 files changed, 260 insertions(+), 2 deletions(-) create mode 100644 rust/src/text/src/lower/sampling.rs diff --git a/rust/src/server/src/error.rs b/rust/src/server/src/error.rs index fbd13b77ba..3a8c0ae5c9 100644 --- a/rust/src/server/src/error.rs +++ b/rust/src/server/src/error.rs @@ -141,6 +141,22 @@ mod tests { assert!(response.error.message.contains("max_tokens=4")); } + #[test] + fn sampling_params_validation_maps_to_invalid_request() { + let api_error = text_submit_error( + "failed to submit completion request", + vllm_text::Error::SamplingParams(vllm_text::SamplingParamsError::OutOfRange { + parameter: "top_p", + value: 0.0, + expected: "(0, 1]", + }), + ); + assert_eq!(api_error.status_code(), StatusCode::BAD_REQUEST); + let response = api_error.to_error_response(); + assert_eq!(response.error.error_type, "invalid_request_error"); + assert!(response.error.message.contains("top_p")); + } + #[test] fn chat_wrapped_prompt_too_long_maps_to_invalid_request() { let error = vllm_chat::Error::Text(vllm_text::Error::PromptTooLong { diff --git a/rust/src/server/src/grpc/tests.rs b/rust/src/server/src/grpc/tests.rs index 75da670ec3..808f342275 100644 --- a/rust/src/server/src/grpc/tests.rs +++ b/rust/src/server/src/grpc/tests.rs @@ -648,6 +648,35 @@ async fn unary_generate_min_tokens_above_max_tokens_returns_invalid_argument() { server_task.abort(); } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[serial] +async fn unary_generate_invalid_sampling_params_returns_invalid_argument() { + let (mut client, server_task, _engine_task) = grpc_test_server( + b"engine-grpc-invalid-sampling", + default_stream_output_specs(), + ) + .await; + + let status = client + .generate(pb::GenerateRequest { + request_id: "test-invalid-sampling".to_string(), + model: "test-model".to_string(), + prompt: Some(pb::generate_request::Prompt::Text("hi".to_string())), + sampling: Some(pb::RandomSampling { + top_p: 2.0, + ..Default::default() + }), + ..Default::default() + }) + .await + .expect_err("should fail when top_p is out of range"); + + assert_eq!(status.code(), tonic::Code::InvalidArgument); + assert!(status.message().contains("top_p")); + + server_task.abort(); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[serial] async fn streaming_generate_yields_incremental_responses() { diff --git a/rust/src/text/src/error.rs b/rust/src/text/src/error.rs index e72e68196c..6b385fad68 100644 --- a/rust/src/text/src/error.rs +++ b/rust/src/text/src/error.rs @@ -6,6 +6,7 @@ use vllm_engine_core_client::Error as EngineCoreError; use vllm_llm::Error as LlmError; pub use crate::lower::logprobs::LogprobsError; +pub use crate::lower::sampling::SamplingParamsError; pub use crate::lower::token_ids::TokenIdsError; #[derive(Debug, Error)] @@ -23,6 +24,8 @@ pub enum Error { Logprobs(#[from] LogprobsError), #[error(transparent)] TokenIds(#[from] TokenIdsError), + #[error(transparent)] + SamplingParams(#[from] SamplingParamsError), #[error( "`min_tokens` must be less than or equal to `max_tokens`, \ got min_tokens={min_tokens}, max_tokens={max_tokens}" @@ -50,6 +53,7 @@ impl Error { | Self::EmptyPromptTokenIds { .. } | Self::Logprobs(_) | Self::TokenIds(_) + | Self::SamplingParams(_) | Self::MinTokensExceedsMaxTokens { .. } | Self::InvalidThinkingTokenBudget | Self::InvalidRepetitionDetection { .. } diff --git a/rust/src/text/src/lib.rs b/rust/src/text/src/lib.rs index b00155999e..646e4bac9f 100644 --- a/rust/src/text/src/lib.rs +++ b/rust/src/text/src/lib.rs @@ -10,7 +10,7 @@ use std::mem::take; pub use backend::{DynTextBackend, SamplingHints, SamplingLimits, TextBackend}; -pub use error::{Error, LogprobsError, Result, TokenIdsError}; +pub use error::{Error, LogprobsError, Result, SamplingParamsError, TokenIdsError}; use futures::Stream; pub use lower::{ PreparedTextRequest, lower_sampling_params, lower_text_request, resolve_max_tokens, diff --git a/rust/src/text/src/lower.rs b/rust/src/text/src/lower.rs index bd43a1d141..aa54595acd 100644 --- a/rust/src/text/src/lower.rs +++ b/rust/src/text/src/lower.rs @@ -4,9 +4,11 @@ use std::collections::BTreeSet; pub(crate) mod logprobs; +pub(crate) mod sampling; pub(crate) mod token_ids; use logprobs::validate_logprobs; +use sampling::validate_resolved_sampling_params; use token_ids::{validate_prompt_token_ids, validate_vocab_range}; use vllm_engine_core_client::protocol::sampling::{ EngineCoreSamplingParams, RepetitionDetectionParams, @@ -186,6 +188,7 @@ pub fn lower_sampling_params( skip_reading_prefix_cache, extra_args: vllm_xargs, }; + validate_resolved_sampling_params(¶ms)?; validate_vocab_range(¶ms, &sampling_limits)?; Ok(params) } @@ -319,7 +322,7 @@ mod tests { use super::*; use crate::backend::hf::HfTextBackend; use crate::backend::{SamplingHints, TextBackend as _}; - use crate::error::{LogprobsError, TokenIdsError}; + use crate::error::{LogprobsError, SamplingParamsError, TokenIdsError}; use crate::request::{Prompt, TextRequest}; fn stub_tokenizer() -> TestTokenizer { @@ -482,6 +485,120 @@ mod tests { assert!(message.contains("min_count=1")); } + #[test] + fn lower_sampling_params_rejects_invalid_sampling_ranges() { + let cases = [ + ( + "temperature", + SamplingParams { + temperature: Some(5.0), + ..SamplingParams::default() + }, + ), + ( + "top_p", + SamplingParams { + top_p: Some(0.0), + ..SamplingParams::default() + }, + ), + ( + "min_p", + SamplingParams { + min_p: Some(2.0), + ..SamplingParams::default() + }, + ), + ( + "repetition_penalty", + SamplingParams { + repetition_penalty: Some(0.0), + ..SamplingParams::default() + }, + ), + ( + "frequency_penalty", + SamplingParams { + frequency_penalty: Some(100.0), + ..SamplingParams::default() + }, + ), + ( + "presence_penalty", + SamplingParams { + presence_penalty: Some(100.0), + ..SamplingParams::default() + }, + ), + ]; + + for (expected_parameter, sampling_params) in cases { + let error = + lower_sampling_params_with_limits(sampling_params, sample_sampling_limits()) + .unwrap_err(); + + assert!( + matches!( + error, + Error::SamplingParams(SamplingParamsError::OutOfRange { + parameter, + .. + }) if parameter == expected_parameter + ), + "{expected_parameter} should be rejected" + ); + } + } + + #[test] + fn lower_sampling_params_rejects_non_finite_sampling_values() { + for (expected_parameter, sampling_params) in [ + ( + "temperature", + SamplingParams { + temperature: Some(f32::INFINITY), + ..SamplingParams::default() + }, + ), + ( + "repetition_penalty", + SamplingParams { + repetition_penalty: Some(f32::NAN), + ..SamplingParams::default() + }, + ), + ] { + let error = + lower_sampling_params_with_limits(sampling_params, sample_sampling_limits()) + .unwrap_err(); + + assert!( + matches!( + error, + Error::SamplingParams(SamplingParamsError::NotFinite { + parameter, + .. + }) if parameter == expected_parameter + ), + "{expected_parameter} should reject non-finite values" + ); + } + } + + #[test] + fn lower_sampling_params_accepts_python_compatible_repetition_penalty_above_two() { + let params = lower_sampling_params_with_limits( + SamplingParams { + repetition_penalty: Some(2.5), + ..SamplingParams::default() + }, + sample_sampling_limits(), + ) + .unwrap(); + + assert_eq!(params.repetition_penalty, 2.5); + } + #[test] fn lower_text_request_applies_python_style_eos_hints() { let prepared = lower_text_request( diff --git a/rust/src/text/src/lower/sampling.rs b/rust/src/text/src/lower/sampling.rs new file mode 100644 index 0000000000..edcdc4b049 --- /dev/null +++ b/rust/src/text/src/lower/sampling.rs @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright contributors to the vLLM project + +use thiserror::Error; +use vllm_engine_core_client::protocol::sampling::EngineCoreSamplingParams; + +#[derive(Debug, Error, PartialEq)] +pub enum SamplingParamsError { + #[error("{parameter} must be a finite number, got {value}")] + NotFinite { parameter: &'static str, value: f32 }, + #[error("{parameter} must be in {expected}, got {value}")] + OutOfRange { + parameter: &'static str, + value: f32, + expected: &'static str, + }, +} + +fn validate_frequency_penalty(value: f32) -> Result<(), SamplingParamsError> { + validate_closed_range("frequency_penalty", value, -2.0, 2.0, "[-2, 2]") +} + +fn validate_presence_penalty(value: f32) -> Result<(), SamplingParamsError> { + validate_closed_range("presence_penalty", value, -2.0, 2.0, "[-2, 2]") +} + +fn validate_temperature(value: f32) -> Result<(), SamplingParamsError> { + validate_finite("temperature", value)?; + validate_closed_range("temperature", value, 0.0, 2.0, "[0, 2]") +} + +fn validate_top_p(value: f32) -> Result<(), SamplingParamsError> { + if value > 0.0 && value <= 1.0 { + return Ok(()); + } + Err(SamplingParamsError::OutOfRange { + parameter: "top_p", + value, + expected: "(0, 1]", + }) +} + +fn validate_min_p(value: f32) -> Result<(), SamplingParamsError> { + validate_closed_range("min_p", value, 0.0, 1.0, "[0, 1]") +} + +fn validate_repetition_penalty(value: f32) -> Result<(), SamplingParamsError> { + validate_finite("repetition_penalty", value)?; + if value > 0.0 { + return Ok(()); + } + Err(SamplingParamsError::OutOfRange { + parameter: "repetition_penalty", + value, + expected: "(0, inf)", + }) +} + +pub(crate) fn validate_resolved_sampling_params( + params: &EngineCoreSamplingParams, +) -> Result<(), SamplingParamsError> { + validate_temperature(params.temperature)?; + validate_top_p(params.top_p)?; + validate_min_p(params.min_p)?; + validate_frequency_penalty(params.frequency_penalty)?; + validate_presence_penalty(params.presence_penalty)?; + validate_repetition_penalty(params.repetition_penalty) +} + +fn validate_finite(parameter: &'static str, value: f32) -> Result<(), SamplingParamsError> { + if value.is_finite() { + return Ok(()); + } + Err(SamplingParamsError::NotFinite { parameter, value }) +} + +fn validate_closed_range( + parameter: &'static str, + value: f32, + min: f32, + max: f32, + expected: &'static str, +) -> Result<(), SamplingParamsError> { + if value >= min && value <= max { + return Ok(()); + } + Err(SamplingParamsError::OutOfRange { + parameter, + value, + expected, + }) +}