[router] Add comprehensive validation to Responses API (#13127)

This commit is contained in:
Keyang Ru
2025-11-12 11:19:27 -08:00
committed by GitHub
parent 4ef4390540
commit 401ed0c594
10 changed files with 1330 additions and 182 deletions
@@ -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()
@@ -171,9 +171,6 @@ class FunctionCallingBaseTest(ResponseAPIBaseTest):
"input": input_list,
},
)
self.assertEqual(resp2.status_code, 200)
data2 = resp2.json()
self.assertEqual(data2["status"], "completed")
@@ -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"]
+6 -29
View File
@@ -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() {
+30
View File
@@ -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)
// ============================================================================
+290 -64
View File
@@ -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<Vec<IncludeField>>,
/// 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<u32>,
/// Maximum number of tool calls
#[serde(skip_serializing_if = "Option::is_none")]
#[validate(range(min = 1))]
pub max_tool_calls: Option<u32>,
/// 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<f32>,
/// 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<Vec<ResponseTool>>,
/// Number of top logprobs to return
#[serde(skip_serializing_if = "Option::is_none")]
#[validate(range(min = 0, max = 20))]
pub top_logprobs: Option<u32>,
/// 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<f32>,
/// 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<TextConfig>,
/// 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<f32>,
/// Presence penalty
#[serde(skip_serializing_if = "Option::is_none")]
#[validate(range(min = -2.0, max = 2.0))]
pub presence_penalty: Option<f32>,
/// Stop sequences
#[serde(skip_serializing_if = "Option::is_none")]
#[validate(custom(function = "validate_stop"))]
pub stop: Option<StringOrArray>,
/// 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(())
}
@@ -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<http::HeaderMap>,
model_id: Option<String>,
) -> 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
-17
View File
@@ -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 {
+1 -1
View File
@@ -191,7 +191,7 @@ async fn v1_rerank(
async fn v1_responses(
State(state): State<Arc<AppState>>,
headers: http::HeaderMap,
Json(body): Json<ResponsesRequest>,
ValidatedJson(body): ValidatedJson<ResponsesRequest>,
) -> Response {
state
.router
File diff suppressed because it is too large Load Diff