From 401ed0c594f676ce527d3270237272a321df525c Mon Sep 17 00:00:00 2001 From: Keyang Ru Date: Wed, 12 Nov 2025 11:19:27 -0800 Subject: [PATCH] [router] Add comprehensive validation to Responses API (#13127) --- .../backends/test_grpc_backend.py | 21 +- .../e2e_response_api/mixins/function_call.py | 3 - .../mixins/state_management.py | 5 +- sgl-router/src/protocols/chat.rs | 35 +- sgl-router/src/protocols/common.rs | 30 + sgl-router/src/protocols/responses.rs | 354 +++++-- .../grpc/regular/responses/handlers.rs | 46 +- sgl-router/src/routers/openai/router.rs | 17 - sgl-router/src/server.rs | 2 +- sgl-router/tests/spec/responses.rs | 999 +++++++++++++++++- 10 files changed, 1330 insertions(+), 182 deletions(-) diff --git a/sgl-router/py_test/e2e_response_api/backends/test_grpc_backend.py b/sgl-router/py_test/e2e_response_api/backends/test_grpc_backend.py index c7634da8a..8fa3a8a9b 100644 --- a/sgl-router/py_test/e2e_response_api/backends/test_grpc_backend.py +++ b/sgl-router/py_test/e2e_response_api/backends/test_grpc_backend.py @@ -58,20 +58,10 @@ class TestGrpcBackend(StateManagementTests, MCPTests, StructuredOutputBaseTest): for worker in cls.cluster.get("workers", []): kill_process_tree(worker.pid) - def test_previous_response_id_chaining(self): - super().test_previous_response_id_chaining() - @unittest.skip("TODO: return 501 Not Implemented") def test_conversation_with_multiple_turns(self): super().test_conversation_with_multiple_turns() - @unittest.skip("TODO: decode error message") - def test_mutually_exclusive_parameters(self): - super().test_mutually_exclusive_parameters() - - def test_mcp_basic_tool_call_streaming(self): - return super().test_mcp_basic_tool_call_streaming() - def test_structured_output_json_schema(self): """Override with simpler schema for Llama model (complex schemas not well supported).""" data = { @@ -152,7 +142,7 @@ class TestGrpcHarmonyBackend( cls.base_url_port, timeout=90, num_workers=1, - tp_size=4, + tp_size=2, policy="round_robin", worker_args=[ "--reasoning-parser=gpt-oss", @@ -171,15 +161,6 @@ class TestGrpcHarmonyBackend( for worker in cls.cluster.get("workers", []): kill_process_tree(worker.pid) - def test_previous_response_id_chaining(self): - super().test_previous_response_id_chaining() - - @unittest.skip( - "TODO: fix requests.exceptions.JSONDecodeError: Expecting value: line 1 column 1 (char 0)" - ) - def test_mutually_exclusive_parameters(self): - super().test_mutually_exclusive_parameters() - @unittest.skip("TODO: 501 Not Implemented") def test_conversation_with_multiple_turns(self): super().test_conversation_with_multiple_turns() diff --git a/sgl-router/py_test/e2e_response_api/mixins/function_call.py b/sgl-router/py_test/e2e_response_api/mixins/function_call.py index 3bb92e687..d46a63a94 100644 --- a/sgl-router/py_test/e2e_response_api/mixins/function_call.py +++ b/sgl-router/py_test/e2e_response_api/mixins/function_call.py @@ -171,9 +171,6 @@ class FunctionCallingBaseTest(ResponseAPIBaseTest): "input": input_list, }, ) - - self.assertEqual(resp2.status_code, 200) - data2 = resp2.json() self.assertEqual(data2["status"], "completed") diff --git a/sgl-router/py_test/e2e_response_api/mixins/state_management.py b/sgl-router/py_test/e2e_response_api/mixins/state_management.py index c120dc089..a52c5a1dc 100644 --- a/sgl-router/py_test/e2e_response_api/mixins/state_management.py +++ b/sgl-router/py_test/e2e_response_api/mixins/state_management.py @@ -92,9 +92,8 @@ class StateManagementTests(ResponseAPIBaseTest): def test_mutually_exclusive_parameters(self): """Test that previous_response_id and conversation are mutually exclusive.""" - # Create conversation and response - conv_resp = self.create_conversation() - conversation_id = conv_resp.json()["id"] + # TODO: Remove this once the conversation API is implemented for GRPC backend + conversation_id = "conv_123" resp1 = self.create_response("Test") response1_id = resp1.json()["id"] diff --git a/sgl-router/src/protocols/chat.rs b/sgl-router/src/protocols/chat.rs index 9ebf971d6..6c3f2699a 100644 --- a/sgl-router/src/protocols/chat.rs +++ b/sgl-router/src/protocols/chat.rs @@ -5,7 +5,12 @@ use serde_json::Value; use validator::Validate; use super::{ - common::*, + common::{ + default_model, default_true, validate_stop, ChatLogProbs, ContentPart, Function, + FunctionCall, FunctionChoice, GenerationRequest, ResponseFormat, StreamOptions, + StringOrArray, Tool, ToolCall, ToolCallDelta, ToolChoice, ToolChoiceValue, ToolReference, + Usage, + }, sampling_params::{validate_top_k_value, validate_top_p_value}, }; use crate::protocols::{ @@ -301,34 +306,6 @@ pub struct ChatCompletionRequest { // Validation Functions // ============================================================================ -/// Validates stop sequences (max 4, non-empty strings) -fn validate_stop(stop: &StringOrArray) -> Result<(), validator::ValidationError> { - match stop { - StringOrArray::String(s) => { - if s.is_empty() { - return Err(validator::ValidationError::new( - "stop sequences cannot be empty", - )); - } - } - StringOrArray::Array(arr) => { - if arr.len() > 4 { - return Err(validator::ValidationError::new( - "maximum 4 stop sequences allowed", - )); - } - for s in arr { - if s.is_empty() { - return Err(validator::ValidationError::new( - "stop sequences cannot be empty", - )); - } - } - } - } - Ok(()) -} - /// Validates messages array is not empty and has valid content fn validate_messages(messages: &[ChatMessage]) -> Result<(), validator::ValidationError> { if messages.is_empty() { diff --git a/sgl-router/src/protocols/common.rs b/sgl-router/src/protocols/common.rs index 008d2e425..a5895be14 100644 --- a/sgl-router/src/protocols/common.rs +++ b/sgl-router/src/protocols/common.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; use serde_json::Value; +use validator; // ============================================================================ // Default value helpers @@ -73,6 +74,35 @@ impl StringOrArray { } } +/// Validates stop sequences (max 4, non-empty strings) +/// Used by both ChatCompletionRequest and ResponsesRequest +pub fn validate_stop(stop: &StringOrArray) -> Result<(), validator::ValidationError> { + match stop { + StringOrArray::String(s) => { + if s.is_empty() { + return Err(validator::ValidationError::new( + "stop sequences cannot be empty", + )); + } + } + StringOrArray::Array(arr) => { + if arr.len() > 4 { + return Err(validator::ValidationError::new( + "maximum 4 stop sequences allowed", + )); + } + for s in arr { + if s.is_empty() { + return Err(validator::ValidationError::new( + "stop sequences cannot be empty", + )); + } + } + } + } + Ok(()) +} + // ============================================================================ // Content Parts (for multimodal messages) // ============================================================================ diff --git a/sgl-router/src/protocols/responses.rs b/sgl-router/src/protocols/responses.rs index 16e40dcd9..f2f41d348 100644 --- a/sgl-router/src/protocols/responses.rs +++ b/sgl-router/src/protocols/responses.rs @@ -7,12 +7,14 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use validator::Validate; -// Import shared types from common module -use super::common::{ - default_model, default_true, ChatLogProbs, Function, GenerationRequest, PromptTokenUsageInfo, - StringOrArray, ToolChoice, UsageInfo, +use super::{ + common::{ + default_model, default_true, validate_stop, ChatLogProbs, Function, GenerationRequest, + PromptTokenUsageInfo, StringOrArray, ToolChoice, ToolChoiceValue, ToolReference, UsageInfo, + }, + sampling_params::{validate_top_k_value, validate_top_p_value}, }; -use crate::protocols::builders::ResponsesResponseBuilder; +use crate::protocols::{builders::ResponsesResponseBuilder, validated::Normalizable}; // ============================================================================ // Response Tools (MCP and others) @@ -324,7 +326,7 @@ pub enum TextFormat { }, } -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] #[serde(rename_all = "snake_case")] pub enum IncludeField { #[serde(rename = "code_interpreter_call.outputs")] @@ -483,6 +485,7 @@ pub struct ResponsesRequest { pub include: Option>, /// Input content - can be string or structured items + #[validate(custom(function = "validate_response_input"))] pub input: ResponseInput, /// System instructions for the model @@ -491,10 +494,12 @@ pub struct ResponsesRequest { /// Maximum number of output tokens #[serde(skip_serializing_if = "Option::is_none")] + #[validate(range(min = 1))] pub max_output_tokens: Option, /// Maximum number of tool calls #[serde(skip_serializing_if = "Option::is_none")] + #[validate(range(min = 1))] pub max_tool_calls: Option, /// Additional metadata @@ -539,6 +544,7 @@ pub struct ResponsesRequest { default = "default_temperature", skip_serializing_if = "Option::is_none" )] + #[validate(range(min = 0.0, max = 2.0))] pub temperature: Option, /// Tool choice behavior @@ -547,14 +553,17 @@ pub struct ResponsesRequest { /// Available tools #[serde(skip_serializing_if = "Option::is_none")] + #[validate(custom(function = "validate_response_tools"))] pub tools: Option>, /// Number of top logprobs to return #[serde(skip_serializing_if = "Option::is_none")] + #[validate(range(min = 0, max = 20))] pub top_logprobs: Option, /// Top-p sampling parameter #[serde(default = "default_top_p", skip_serializing_if = "Option::is_none")] + #[validate(custom(function = "validate_top_p_value"))] pub top_p: Option, /// Truncation behavior @@ -563,6 +572,7 @@ pub struct ResponsesRequest { /// Text format for structured outputs (text, json_object, json_schema) #[serde(skip_serializing_if = "Option::is_none")] + #[validate(custom(function = "validate_text_format"))] pub text: Option, /// User identifier @@ -579,26 +589,32 @@ pub struct ResponsesRequest { /// Frequency penalty #[serde(skip_serializing_if = "Option::is_none")] + #[validate(range(min = -2.0, max = 2.0))] pub frequency_penalty: Option, /// Presence penalty #[serde(skip_serializing_if = "Option::is_none")] + #[validate(range(min = -2.0, max = 2.0))] pub presence_penalty: Option, /// Stop sequences #[serde(skip_serializing_if = "Option::is_none")] + #[validate(custom(function = "validate_stop"))] pub stop: Option, /// Top-k sampling parameter (SGLang extension) #[serde(default = "default_top_k")] + #[validate(custom(function = "validate_top_k_value"))] pub top_k: i32, /// Min-p sampling parameter (SGLang extension) #[serde(default)] + #[validate(range(min = 0.0, max = 1.0))] pub min_p: f32, /// Repetition penalty (SGLang extension) #[serde(default = "default_repetition_penalty")] + #[validate(range(min = 0.0, max = 2.0))] pub repetition_penalty: f32, } @@ -647,6 +663,37 @@ impl Default for ResponsesRequest { } } +impl Normalizable for ResponsesRequest { + /// Normalize the request by applying defaults: + /// 1. Apply tool_choice defaults based on tools presence + /// 2. Apply parallel_tool_calls defaults + /// 3. Apply store field defaults + fn normalize(&mut self) { + // 1. Apply tool_choice defaults + if self.tool_choice.is_none() { + if let Some(tools) = &self.tools { + let choice_value = if !tools.is_empty() { + ToolChoiceValue::Auto + } else { + ToolChoiceValue::None + }; + self.tool_choice = Some(ToolChoice::Value(choice_value)); + } + // If tools is None, leave tool_choice as None (don't set it) + } + + // 2. Apply default for parallel_tool_calls if tools are present + if self.parallel_tool_calls.is_none() && self.tools.is_some() { + self.parallel_tool_calls = Some(true); + } + + // 3. Ensure store defaults to true if not specified + if self.store.is_none() { + self.store = Some(true); + } + } +} + impl GenerationRequest for ResponsesRequest { fn is_stream(&self) -> bool { self.stream.unwrap_or(false) @@ -752,80 +799,259 @@ pub fn validate_conversation_id(conv_id: &str) -> Result<(), validator::Validati Ok(()) } +/// Validates tool_choice requires tools and references exist +fn validate_tool_choice_with_tools( + request: &ResponsesRequest, +) -> Result<(), validator::ValidationError> { + let Some(tool_choice) = &request.tool_choice else { + return Ok(()); + }; + + let has_tools = request.tools.as_ref().is_some_and(|t| !t.is_empty()); + let is_some_choice = !matches!(tool_choice, ToolChoice::Value(ToolChoiceValue::None)); + + // Check if tool_choice requires tools but none are provided + if is_some_choice && !has_tools { + let mut e = validator::ValidationError::new("tool_choice_requires_tools"); + e.message = Some("Invalid value for 'tool_choice': 'tool_choice' is only allowed when 'tools' are specified.".into()); + return Err(e); + } + + // Validate tool references exist when tools are present + if !has_tools { + return Ok(()); + } + + // Extract function tool names from ResponseTools + let tools = request.tools.as_ref().unwrap(); + let function_tool_names: Vec<&str> = tools + .iter() + .filter_map(|t| match t.r#type { + ResponseToolType::Function => t.function.as_ref().map(|f| f.name.as_str()), + _ => None, + }) + .collect(); + + // Validate tool references exist + match tool_choice { + ToolChoice::Function { function, .. } => { + if !function_tool_names.contains(&function.name.as_str()) { + let mut e = validator::ValidationError::new("tool_choice_function_not_found"); + e.message = Some( + format!( + "Invalid value for 'tool_choice': function '{}' not found in 'tools'.", + function.name + ) + .into(), + ); + return Err(e); + } + } + ToolChoice::AllowedTools { + mode, + tools: allowed_tools, + .. + } => { + // Validate mode is "auto" or "required" + if mode != "auto" && mode != "required" { + let mut e = validator::ValidationError::new("tool_choice_invalid_mode"); + e.message = Some( + format!( + "Invalid value for 'tool_choice.mode': must be 'auto' or 'required', got '{}'.", + mode + ) + .into(), + ); + return Err(e); + } + + // Validate that all function tool references exist + for tool_ref in allowed_tools { + if let ToolReference::Function { name } = tool_ref { + if !function_tool_names.contains(&name.as_str()) { + let mut e = validator::ValidationError::new("tool_choice_tool_not_found"); + e.message = Some( + format!( + "Invalid value for 'tool_choice.tools': tool '{}' not found in 'tools'.", + name + ) + .into(), + ); + return Err(e); + } + } + // Note: MCP and hosted tools don't need existence validation here + // as they are resolved dynamically at runtime + } + } + _ => {} + } + + Ok(()) +} + /// Schema-level validation for cross-field dependencies fn validate_responses_cross_parameters( request: &ResponsesRequest, ) -> Result<(), validator::ValidationError> { - use super::common::{ToolChoice, ToolReference}; + // 1. Validate tool_choice requires tools (enhanced) + validate_tool_choice_with_tools(request)?; - // Only validate if both tools and tool_choice are present - if let (Some(tools), Some(tool_choice)) = (&request.tools, &request.tool_choice) { - // Extract function tool names from ResponseTools - let function_tool_names: Vec<&str> = tools - .iter() - .filter_map(|t| match t.r#type { - ResponseToolType::Function => t.function.as_ref().map(|f| f.name.as_str()), - _ => None, - }) - .collect(); + // 2. Validate top_logprobs requires include field + if request.top_logprobs.is_some() { + let has_logprobs_include = request + .include + .as_ref() + .is_some_and(|inc| inc.contains(&IncludeField::MessageOutputTextLogprobs)); - match tool_choice { - ToolChoice::Function { function, .. } => { - // Validate the specific function exists - if !function_tool_names.contains(&function.name.as_str()) { - let mut e = validator::ValidationError::new("tool_choice_function_not_found"); - e.message = Some( - format!( - "Invalid value for 'tool_choice': function '{}' not found in 'tools'.", - function.name - ) - .into(), - ); + if !has_logprobs_include { + let mut e = validator::ValidationError::new("top_logprobs_requires_include"); + e.message = Some( + "top_logprobs requires include field with 'message.output_text.logprobs'".into(), + ); + return Err(e); + } + } + + // 3. Validate background/stream conflict + if request.background == Some(true) && request.stream == Some(true) { + let mut e = validator::ValidationError::new("background_conflicts_with_stream"); + e.message = Some("Cannot use background mode with streaming".into()); + return Err(e); + } + + // 4. Validate conversation and previous_response_id are mutually exclusive + if request.conversation.is_some() && request.previous_response_id.is_some() { + let mut e = validator::ValidationError::new("mutually_exclusive_parameters"); + e.message = Some("Mutually exclusive parameters. Ensure you are only providing one of: 'previous_response_id' or 'conversation'.".into()); + return Err(e); + } + + // 5. Validate input items structure + if let ResponseInput::Items(items) = &request.input { + // Check for at least one valid input message + let has_valid_input = items.iter().any(|item| { + matches!( + item, + ResponseInputOutputItem::Message { .. } + | ResponseInputOutputItem::SimpleInputMessage { .. } + ) + }); + + if !has_valid_input { + let mut e = validator::ValidationError::new("input_missing_user_message"); + e.message = Some("Input items must contain at least one message".into()); + return Err(e); + } + } + + // 6. Validate text format conflicts (for future structured output constraints) + // Currently, Responses API doesn't have regex/ebnf like Chat API, + // but this is here for completeness and future-proofing + + Ok(()) +} + +// ============================================================================ +// Field-Level Validation Functions +// ============================================================================ + +/// Validates response input is not empty and has valid content +fn validate_response_input(input: &ResponseInput) -> Result<(), validator::ValidationError> { + match input { + ResponseInput::Text(text) => { + if text.is_empty() { + let mut e = validator::ValidationError::new("input_text_empty"); + e.message = Some("Input text cannot be empty".into()); + return Err(e); + } + } + ResponseInput::Items(items) => { + if items.is_empty() { + let mut e = validator::ValidationError::new("input_items_empty"); + e.message = Some("Input items cannot be empty".into()); + return Err(e); + } + // Validate each item has valid content + for item in items { + validate_input_item(item)?; + } + } + } + Ok(()) +} + +/// Validates individual input items have valid content +fn validate_input_item(item: &ResponseInputOutputItem) -> Result<(), validator::ValidationError> { + match item { + ResponseInputOutputItem::Message { content, .. } => { + if content.is_empty() { + let mut e = validator::ValidationError::new("message_content_empty"); + e.message = Some("Message content cannot be empty".into()); + return Err(e); + } + } + ResponseInputOutputItem::SimpleInputMessage { content, .. } => match content { + StringOrContentParts::String(s) if s.is_empty() => { + let mut e = validator::ValidationError::new("message_content_empty"); + e.message = Some("Message content cannot be empty".into()); + return Err(e); + } + StringOrContentParts::Array(parts) if parts.is_empty() => { + let mut e = validator::ValidationError::new("message_content_empty"); + e.message = Some("Message content parts cannot be empty".into()); + return Err(e); + } + _ => {} + }, + ResponseInputOutputItem::Reasoning { .. } => { + // Reasoning content can be empty - no validation needed + } + ResponseInputOutputItem::FunctionCallOutput { output, .. } => { + if output.is_empty() { + let mut e = validator::ValidationError::new("function_output_empty"); + e.message = Some("Function call output cannot be empty".into()); + return Err(e); + } + } + _ => {} + } + Ok(()) +} + +/// Validates ResponseTool structure based on tool type +fn validate_response_tools(tools: &[ResponseTool]) -> Result<(), validator::ValidationError> { + for tool in tools { + match tool.r#type { + ResponseToolType::Function => { + if tool.function.is_none() { + let mut e = validator::ValidationError::new("function_tool_missing_function"); + e.message = Some("Function tool must have a function definition".into()); return Err(e); } } - ToolChoice::AllowedTools { - mode, - tools: allowed_tools, - .. - } => { - // Validate mode is "auto" or "required" - if mode != "auto" && mode != "required" { - let mut e = validator::ValidationError::new("tool_choice_invalid_mode"); - e.message = Some( - format!( - "Invalid value for 'tool_choice.mode': must be 'auto' or 'required', got '{}'.", - mode - ) - .into(), - ); + ResponseToolType::Mcp => { + if tool.server_url.is_none() { + let mut e = validator::ValidationError::new("mcp_tool_missing_server_url"); + e.message = Some("MCP tool must have a server_url".into()); return Err(e); } - - // Validate that all function tool references exist - for tool_ref in allowed_tools { - if let ToolReference::Function { name } = tool_ref { - if !function_tool_names.contains(&name.as_str()) { - let mut e = - validator::ValidationError::new("tool_choice_tool_not_found"); - e.message = Some( - format!( - "Invalid value for 'tool_choice.tools': tool '{}' not found in 'tools'.", - name - ) - .into(), - ); - return Err(e); - } - } - // Note: MCP and hosted tools don't need existence validation here - // as they are resolved dynamically at runtime - } } _ => {} } } + Ok(()) +} +/// Validates text format configuration (JSON schema name cannot be empty) +fn validate_text_format(text: &TextConfig) -> Result<(), validator::ValidationError> { + if let Some(TextFormat::JsonSchema { name, .. }) = &text.format { + if name.is_empty() { + let mut e = validator::ValidationError::new("json_schema_name_empty"); + e.message = Some("JSON schema name cannot be empty".into()); + return Err(e); + } + } Ok(()) } diff --git a/sgl-router/src/routers/grpc/regular/responses/handlers.rs b/sgl-router/src/routers/grpc/regular/responses/handlers.rs index f90635c0c..97108b104 100644 --- a/sgl-router/src/routers/grpc/regular/responses/handlers.rs +++ b/sgl-router/src/routers/grpc/regular/responses/handlers.rs @@ -45,7 +45,6 @@ use serde_json::json; use tokio::sync::mpsc; use tracing::{debug, error, warn}; use uuid::Uuid; -use validator::Validate; use super::{ conversions, @@ -83,48 +82,7 @@ pub async fn route_responses( headers: Option, model_id: Option, ) -> Response { - // 1. Validate request (includes conversation ID format) - if let Err(validation_errors) = request.validate() { - // Extract the first error message for conversation field - let error_message = validation_errors - .field_errors() - .get("conversation") - .and_then(|errors| errors.first()) - .and_then(|error| error.message.as_ref()) - .map(|msg| msg.to_string()) - .unwrap_or_else(|| "Invalid request parameters".to_string()); - - return ( - StatusCode::BAD_REQUEST, - axum::Json(json!({ - "error": { - "message": error_message, - "type": "invalid_request_error", - "param": "conversation", - "code": "invalid_value" - } - })), - ) - .into_response(); - } - - // 2. Validate mutually exclusive parameters - if request.previous_response_id.is_some() && request.conversation.is_some() { - return ( - StatusCode::BAD_REQUEST, - axum::Json(json!({ - "error": { - "message": "Mutually exclusive parameters. Ensure you are only providing one of: 'previous_response_id' or 'conversation'.", - "type": "invalid_request_error", - "param": serde_json::Value::Null, - "code": "mutually_exclusive_parameters" - } - })), - ) - .into_response(); - } - - // 3. Reject background mode (no longer supported) + // 1. Reject background mode (no longer supported) let is_background = request.background.unwrap_or(false); if is_background { return ( @@ -141,7 +99,7 @@ pub async fn route_responses( .into_response(); } - // 4. Route based on execution mode + // 2. Route based on execution mode let is_streaming = request.stream.unwrap_or(false); if is_streaming { route_responses_streaming(ctx, request, headers, model_id).await diff --git a/sgl-router/src/routers/openai/router.rs b/sgl-router/src/routers/openai/router.rs index ef0ade353..8813117b4 100644 --- a/sgl-router/src/routers/openai/router.rs +++ b/sgl-router/src/routers/openai/router.rs @@ -695,23 +695,6 @@ impl crate::routers::RouterTrait for OpenAIRouter { let url = format!("{}/v1/responses", base_url); - // Validate mutually exclusive params: previous_response_id and conversation - // TODO: this validation logic should move the right place, also we need a proper error message module - if body.previous_response_id.is_some() && body.conversation.is_some() { - return ( - StatusCode::BAD_REQUEST, - Json(json!({ - "error": { - "message": "Mutually exclusive parameters. Ensure you are only providing one of: 'previous_response_id' or 'conversation'.", - "type": "invalid_request_error", - "param": Value::Null, - "code": "mutually_exclusive_parameters" - } - })), - ) - .into_response(); - } - // Clone the body for validation and logic, but we'll build payload differently let mut request_body = body.clone(); if let Some(model) = model_id { diff --git a/sgl-router/src/server.rs b/sgl-router/src/server.rs index 5db429788..7a6c6d3a0 100644 --- a/sgl-router/src/server.rs +++ b/sgl-router/src/server.rs @@ -191,7 +191,7 @@ async fn v1_rerank( async fn v1_responses( State(state): State>, headers: http::HeaderMap, - Json(body): Json, + ValidatedJson(body): ValidatedJson, ) -> Response { state .router diff --git a/sgl-router/tests/spec/responses.rs b/sgl-router/tests/spec/responses.rs index 129b934bc..8814c12e7 100644 --- a/sgl-router/tests/spec/responses.rs +++ b/sgl-router/tests/spec/responses.rs @@ -1,4 +1,11 @@ -use sglang_router_rs::protocols::responses::{ResponseInput, ResponsesRequest}; +use serde_json::json; +use sglang_router_rs::protocols::{ + common::{Function, StringOrArray, ToolChoice, ToolChoiceValue}, + responses::{ + IncludeField, ResponseInput, ResponseInputOutputItem, ResponseTool, ResponseToolType, + ResponsesRequest, StringOrContentParts, TextConfig, TextFormat, + }, +}; use validator::Validate; /// Test that valid conversation IDs pass validation @@ -201,3 +208,993 @@ fn test_validate_conversation_id_missing_prefix() { ); } } + +// ============================================================================ +// Field-Level Validation Tests +// ============================================================================ + +/// Test temperature range validation +#[test] +fn test_validate_temperature_range() { + // Valid temperatures + for temp in [0.0, 1.0, 2.0, 0.5, 1.5] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + temperature: Some(temp), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "Temperature {} should be valid", + temp + ); + } + + // Invalid temperatures + for temp in [-0.1, 2.1, -1.0, 3.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + temperature: Some(temp), + ..Default::default() + }; + assert!( + request.validate().is_err(), + "Temperature {} should be invalid", + temp + ); + } +} + +/// Test frequency_penalty range validation +#[test] +fn test_validate_frequency_penalty_range() { + // Valid penalties + for penalty in [-2.0, -1.0, 0.0, 1.0, 2.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + frequency_penalty: Some(penalty), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "Frequency penalty {} should be valid", + penalty + ); + } + + // Invalid penalties + for penalty in [-2.1, 2.1, -3.0, 3.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + frequency_penalty: Some(penalty), + ..Default::default() + }; + assert!( + request.validate().is_err(), + "Frequency penalty {} should be invalid", + penalty + ); + } +} + +/// Test presence_penalty range validation +#[test] +fn test_validate_presence_penalty_range() { + // Valid penalties + for penalty in [-2.0, -1.0, 0.0, 1.0, 2.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + presence_penalty: Some(penalty), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "Presence penalty {} should be valid", + penalty + ); + } + + // Invalid penalties + for penalty in [-2.1, 2.1, -3.0, 3.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + presence_penalty: Some(penalty), + ..Default::default() + }; + assert!( + request.validate().is_err(), + "Presence penalty {} should be invalid", + penalty + ); + } +} + +/// Test top_logprobs range validation +#[test] +fn test_validate_top_logprobs_range() { + // Valid values + for val in [0, 1, 10, 20] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + top_logprobs: Some(val), + include: Some(vec![IncludeField::MessageOutputTextLogprobs]), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "top_logprobs {} should be valid", + val + ); + } + + // Invalid values + for val in [21, 30, 100] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + top_logprobs: Some(val), + include: Some(vec![IncludeField::MessageOutputTextLogprobs]), + ..Default::default() + }; + assert!( + request.validate().is_err(), + "top_logprobs {} should be invalid", + val + ); + } +} + +/// Test top_p range validation +#[test] +fn test_validate_top_p_range() { + // Valid values (> 0.0 and <= 1.0) + for val in [0.01, 0.5, 1.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + top_p: Some(val), + ..Default::default() + }; + assert!(request.validate().is_ok(), "top_p {} should be valid", val); + } + + // Invalid values (0.0 is invalid because it means no tokens, < 0 or > 1) + for val in [0.0, -0.1, 1.1, 2.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + top_p: Some(val), + ..Default::default() + }; + assert!( + request.validate().is_err(), + "top_p {} should be invalid", + val + ); + } +} + +/// Test top_k validation +#[test] +fn test_validate_top_k() { + // Valid values (-1 means disabled, or >= 1) + for val in [-1, 1, 10, 100] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + top_k: val, + ..Default::default() + }; + assert!(request.validate().is_ok(), "top_k {} should be valid", val); + } + + // Invalid values (0 or < -1) + for val in [0, -2, -10] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + top_k: val, + ..Default::default() + }; + assert!( + request.validate().is_err(), + "top_k {} should be invalid", + val + ); + } +} + +/// Test min_p range validation +#[test] +fn test_validate_min_p_range() { + // Valid values + for val in [0.0, 0.5, 1.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + min_p: val, + ..Default::default() + }; + assert!(request.validate().is_ok(), "min_p {} should be valid", val); + } + + // Invalid values + for val in [-0.1, 1.1, 2.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + min_p: val, + ..Default::default() + }; + assert!( + request.validate().is_err(), + "min_p {} should be invalid", + val + ); + } +} + +/// Test repetition_penalty range validation +#[test] +fn test_validate_repetition_penalty_range() { + // Valid values + for val in [0.0, 1.0, 2.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + repetition_penalty: val, + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "repetition_penalty {} should be valid", + val + ); + } + + // Invalid values + for val in [-0.1, 2.1, 3.0] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + repetition_penalty: val, + ..Default::default() + }; + assert!( + request.validate().is_err(), + "repetition_penalty {} should be invalid", + val + ); + } +} + +/// Test max_output_tokens minimum validation +#[test] +fn test_validate_max_output_tokens() { + // Valid values + for val in [1, 100, 1000] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + max_output_tokens: Some(val), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "max_output_tokens {} should be valid", + val + ); + } + + // Invalid values + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + max_output_tokens: Some(0), + ..Default::default() + }; + assert!( + request.validate().is_err(), + "max_output_tokens 0 should be invalid" + ); +} + +/// Test max_tool_calls minimum validation +#[test] +fn test_validate_max_tool_calls() { + // Valid values + for val in [1, 5, 10] { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + max_tool_calls: Some(val), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "max_tool_calls {} should be valid", + val + ); + } + + // Invalid values + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + max_tool_calls: Some(0), + ..Default::default() + }; + assert!( + request.validate().is_err(), + "max_tool_calls 0 should be invalid" + ); +} + +/// Test input validation (empty text) +#[test] +fn test_validate_input_empty_text() { + let request = ResponsesRequest { + input: ResponseInput::Text("".to_string()), + ..Default::default() + }; + let result = request.validate(); + assert!(result.is_err(), "Empty input text should be invalid"); + + if let Err(errors) = result { + let error_msg = errors.to_string(); + assert!( + error_msg.contains("input") || error_msg.contains("empty"), + "Error should mention input or empty" + ); + } +} + +/// Test input validation (empty items array) +#[test] +fn test_validate_input_empty_items() { + let request = ResponsesRequest { + input: ResponseInput::Items(vec![]), + ..Default::default() + }; + let result = request.validate(); + assert!(result.is_err(), "Empty input items should be invalid"); +} + +/// Test input validation (items with empty content) +#[test] +fn test_validate_input_items_empty_content() { + let request = ResponsesRequest { + input: ResponseInput::Items(vec![ResponseInputOutputItem::SimpleInputMessage { + content: StringOrContentParts::String("".to_string()), + role: "user".to_string(), + r#type: None, + }]), + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "Input item with empty content should be invalid" + ); +} + +/// Test stop sequences validation (max 4) +#[test] +fn test_validate_stop_sequences_max() { + // Valid: 4 or fewer stop sequences + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + stop: Some(StringOrArray::Array(vec![ + "stop1".to_string(), + "stop2".to_string(), + "stop3".to_string(), + "stop4".to_string(), + ])), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "4 stop sequences should be valid" + ); + + // Invalid: more than 4 stop sequences + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + stop: Some(StringOrArray::Array(vec![ + "stop1".to_string(), + "stop2".to_string(), + "stop3".to_string(), + "stop4".to_string(), + "stop5".to_string(), + ])), + ..Default::default() + }; + assert!( + request.validate().is_err(), + "5 stop sequences should be invalid" + ); +} + +/// Test stop sequences validation (non-empty) +#[test] +fn test_validate_stop_sequences_non_empty() { + // Invalid: empty string stop sequence + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + stop: Some(StringOrArray::String("".to_string())), + ..Default::default() + }; + assert!( + request.validate().is_err(), + "Empty stop sequence should be invalid" + ); + + // Invalid: array with empty string + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + stop: Some(StringOrArray::Array(vec!["".to_string()])), + ..Default::default() + }; + assert!( + request.validate().is_err(), + "Array with empty stop sequence should be invalid" + ); +} + +/// Test tools validation (function tool must have function) +#[test] +fn test_validate_tools_function_missing() { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: Some(vec![ResponseTool { + r#type: ResponseToolType::Function, + function: None, // Missing function definition + server_url: None, + authorization: None, + server_label: None, + server_description: None, + require_approval: None, + allowed_tools: None, + }]), + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "Function tool without function definition should be invalid" + ); +} + +/// Test tools validation (MCP tool must have server_url) +#[test] +fn test_validate_tools_mcp_missing_url() { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: Some(vec![ResponseTool { + r#type: ResponseToolType::Mcp, + function: None, + server_url: None, // Missing server_url + authorization: None, + server_label: None, + server_description: None, + require_approval: None, + allowed_tools: None, + }]), + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "MCP tool without server_url should be invalid" + ); +} + +/// Test text format validation (JSON schema name cannot be empty) +#[test] +fn test_validate_text_format_json_schema_empty_name() { + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + text: Some(TextConfig { + format: Some(TextFormat::JsonSchema { + name: "".to_string(), // Empty name + schema: json!({}), + description: None, + strict: None, + }), + }), + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "JSON schema with empty name should be invalid" + ); +} + +// ============================================================================ +// Cross-Field Validation Tests (Schema-Level) +// ============================================================================ + +/// Test tool_choice requires tools +#[test] +fn test_validate_tool_choice_requires_tools() { + // Valid: tool_choice with tools + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: Some(vec![ResponseTool { + r#type: ResponseToolType::Function, + function: Some(Function { + name: "test_func".to_string(), + description: None, + parameters: json!({}), + strict: None, + }), + server_url: None, + authorization: None, + server_label: None, + server_description: None, + require_approval: None, + allowed_tools: None, + }]), + tool_choice: Some(ToolChoice::Value(ToolChoiceValue::Auto)), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "tool_choice with tools should be valid" + ); + + // Valid: tool_choice=none without tools is OK + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: None, + tool_choice: Some(ToolChoice::Value(ToolChoiceValue::None)), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "tool_choice=none without tools should be valid" + ); + + // Invalid: tool_choice=auto without tools + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: None, + tool_choice: Some(ToolChoice::Value(ToolChoiceValue::Auto)), + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "tool_choice=auto without tools should be invalid" + ); + + if let Err(errors) = result { + let error_msg = errors.to_string(); + assert!( + error_msg.contains("tool_choice") && error_msg.contains("tools"), + "Error should mention tool_choice requires tools" + ); + } +} + +/// Test top_logprobs requires include field +#[test] +fn test_validate_top_logprobs_requires_include() { + // Valid: top_logprobs with correct include field + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + top_logprobs: Some(5), + include: Some(vec![IncludeField::MessageOutputTextLogprobs]), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "top_logprobs with include field should be valid" + ); + + // Invalid: top_logprobs without include field + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + top_logprobs: Some(5), + include: None, + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "top_logprobs without include field should be invalid" + ); + + // Invalid: top_logprobs with wrong include field + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + top_logprobs: Some(5), + include: Some(vec![IncludeField::ReasoningEncryptedContent]), + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "top_logprobs with wrong include field should be invalid" + ); +} + +/// Test background/stream conflict +#[test] +fn test_validate_background_stream_conflict() { + // Invalid: both background and stream enabled + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + background: Some(true), + stream: Some(true), + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "background=true with stream=true should be invalid" + ); + + if let Err(errors) = result { + let error_msg = errors.to_string(); + assert!( + error_msg.contains("background") || error_msg.contains("stream"), + "Error should mention background/stream conflict" + ); + } +} + +/// Test previous_response_id format validation +/// NOTE: Format validation removed - previous_response_id format is not validated +/// response_id generated by the grpc router is not necessarily start with 'resp_' +#[test] +#[ignore] +fn test_validate_previous_response_id_format() { + // Valid: starts with "resp_" + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + previous_response_id: Some("resp_123abc".to_string()), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "previous_response_id with resp_ prefix should be valid" + ); + + // Invalid: doesn't start with "resp_" + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + previous_response_id: Some("response_123abc".to_string()), + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "previous_response_id without resp_ prefix should be invalid" + ); + + if let Err(errors) = result { + let error_msg = errors.to_string(); + assert!( + error_msg.contains("previous_response_id") && error_msg.contains("resp_"), + "Error should mention previous_response_id format" + ); + } +} + +/// Test conversation and previous_response_id mutual exclusion +#[test] +fn test_validate_conversation_previous_response_mutual_exclusion() { + // Valid: only conversation + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + conversation: Some("conv_123".to_string()), + previous_response_id: None, + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "Only conversation should be valid" + ); + + // Valid: only previous_response_id + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + conversation: None, + previous_response_id: Some("resp_123".to_string()), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "Only previous_response_id should be valid" + ); + + // Invalid: both conversation and previous_response_id + let request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + conversation: Some("conv_123".to_string()), + previous_response_id: Some("resp_123".to_string()), + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "Both conversation and previous_response_id should be invalid" + ); + + if let Err(errors) = result { + let error_msg = errors.to_string(); + assert!( + error_msg.contains("mutually exclusive") + || (error_msg.contains("conversation") + && error_msg.contains("previous_response_id")), + "Error should mention mutual exclusion, got: {}", + error_msg + ); + } +} + +/// Test input items structure validation +#[test] +fn test_validate_input_items_structure() { + // Valid: items with at least one message + let request = ResponsesRequest { + input: ResponseInput::Items(vec![ResponseInputOutputItem::SimpleInputMessage { + content: StringOrContentParts::String("Hello".to_string()), + role: "user".to_string(), + r#type: None, + }]), + ..Default::default() + }; + assert!( + request.validate().is_ok(), + "Input items with message should be valid" + ); + + // Invalid: items with no messages (only function calls) + let request = ResponsesRequest { + input: ResponseInput::Items(vec![ResponseInputOutputItem::FunctionCallOutput { + id: None, + call_id: "call_123".to_string(), + output: "result".to_string(), + status: None, + }]), + ..Default::default() + }; + let result = request.validate(); + assert!( + result.is_err(), + "Input items without messages should be invalid" + ); +} + +// ============================================================================ +// Normalization Tests (Normalizable Trait) +// ============================================================================ + +/// Test tool_choice defaults to auto when tools are present +#[test] +fn test_normalize_tool_choice_auto() { + use sglang_router_rs::protocols::validated::Normalizable; + + let mut request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: Some(vec![ResponseTool { + r#type: ResponseToolType::Function, + function: Some(Function { + name: "test_func".to_string(), + description: None, + parameters: json!({}), + strict: None, + }), + server_url: None, + authorization: None, + server_label: None, + server_description: None, + require_approval: None, + allowed_tools: None, + }]), + tool_choice: None, + ..Default::default() + }; + + request.normalize(); + + assert!( + request.tool_choice.is_some(), + "tool_choice should be set after normalization" + ); + assert!( + matches!( + request.tool_choice, + Some(ToolChoice::Value(ToolChoiceValue::Auto)) + ), + "tool_choice should default to auto when tools are present" + ); +} + +/// Test tool_choice defaults to none when tools array is empty +#[test] +fn test_normalize_tool_choice_none() { + use sglang_router_rs::protocols::validated::Normalizable; + + let mut request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: Some(vec![]), + tool_choice: None, + ..Default::default() + }; + + request.normalize(); + + assert!( + request.tool_choice.is_some(), + "tool_choice should be set after normalization" + ); + assert!( + matches!( + request.tool_choice, + Some(ToolChoice::Value(ToolChoiceValue::None)) + ), + "tool_choice should default to none when tools array is empty" + ); +} + +/// Test tool_choice is not overridden if already set +#[test] +fn test_normalize_tool_choice_no_override() { + use sglang_router_rs::protocols::validated::Normalizable; + + let mut request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: Some(vec![ResponseTool { + r#type: ResponseToolType::Function, + function: Some(Function { + name: "test_func".to_string(), + description: None, + parameters: json!({}), + strict: None, + }), + server_url: None, + authorization: None, + server_label: None, + server_description: None, + require_approval: None, + allowed_tools: None, + }]), + tool_choice: Some(ToolChoice::Value(ToolChoiceValue::Required)), + ..Default::default() + }; + + request.normalize(); + + assert!( + matches!( + request.tool_choice, + Some(ToolChoice::Value(ToolChoiceValue::Required)) + ), + "tool_choice should not be overridden if already set" + ); +} + +/// Test parallel_tool_calls defaults to true when tools are present +#[test] +fn test_normalize_parallel_tool_calls() { + use sglang_router_rs::protocols::validated::Normalizable; + + let mut request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: Some(vec![ResponseTool { + r#type: ResponseToolType::Function, + function: Some(Function { + name: "test_func".to_string(), + description: None, + parameters: json!({}), + strict: None, + }), + server_url: None, + authorization: None, + server_label: None, + server_description: None, + require_approval: None, + allowed_tools: None, + }]), + parallel_tool_calls: None, + ..Default::default() + }; + + request.normalize(); + + assert!( + request.parallel_tool_calls.is_some(), + "parallel_tool_calls should be set after normalization" + ); + assert_eq!( + request.parallel_tool_calls, + Some(true), + "parallel_tool_calls should default to true when tools are present" + ); +} + +/// Test parallel_tool_calls is not set when tools are absent +#[test] +fn test_normalize_parallel_tool_calls_no_tools() { + use sglang_router_rs::protocols::validated::Normalizable; + + let mut request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: None, + parallel_tool_calls: None, + ..Default::default() + }; + + request.normalize(); + + assert!( + request.parallel_tool_calls.is_none(), + "parallel_tool_calls should remain None when tools are absent" + ); +} + +/// Test parallel_tool_calls is not overridden if already set +#[test] +fn test_normalize_parallel_tool_calls_no_override() { + use sglang_router_rs::protocols::validated::Normalizable; + + let mut request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + tools: Some(vec![ResponseTool { + r#type: ResponseToolType::Function, + function: Some(Function { + name: "test_func".to_string(), + description: None, + parameters: json!({}), + strict: None, + }), + server_url: None, + authorization: None, + server_label: None, + server_description: None, + require_approval: None, + allowed_tools: None, + }]), + parallel_tool_calls: Some(false), + ..Default::default() + }; + + request.normalize(); + + assert_eq!( + request.parallel_tool_calls, + Some(false), + "parallel_tool_calls should not be overridden if already set" + ); +} + +/// Test store defaults to true +#[test] +fn test_normalize_store_default() { + use sglang_router_rs::protocols::validated::Normalizable; + + let mut request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + store: None, + ..Default::default() + }; + + request.normalize(); + + assert!( + request.store.is_some(), + "store should be set after normalization" + ); + assert_eq!(request.store, Some(true), "store should default to true"); +} + +/// Test store is not overridden if already set +#[test] +fn test_normalize_store_no_override() { + use sglang_router_rs::protocols::validated::Normalizable; + + let mut request = ResponsesRequest { + input: ResponseInput::Text("test".to_string()), + store: Some(false), + ..Default::default() + }; + + request.normalize(); + + assert_eq!( + request.store, + Some(false), + "store should not be overridden if already set to false" + ); +}