diff --git a/sgl-router/src/routers/grpc/pipeline.rs b/sgl-router/src/routers/grpc/pipeline.rs index 1c23bf64a..94f55789d 100644 --- a/sgl-router/src/routers/grpc/pipeline.rs +++ b/sgl-router/src/routers/grpc/pipeline.rs @@ -106,9 +106,13 @@ impl PreparationStage { let token_ids = encoding.token_ids().to_vec(); // Step 4: Build tool constraints if needed - let tool_call_constraint = body_ref.tools.as_ref().and_then(|tools| { - utils::generate_tool_constraints(tools, &request.tool_choice, &request.model) - }); + let tool_call_constraint = if let Some(tools) = body_ref.tools.as_ref() { + utils::generate_tool_constraints(tools, &request.tool_choice, &request.model).map_err( + |e| utils::bad_request_error(format!("Invalid tool configuration: {}", e)), + )? + } else { + None + }; // Step 5: Create stop sequence decoder (build once, reuse in non-stream) let stop_decoder = utils::create_stop_decoder( diff --git a/sgl-router/src/routers/grpc/utils.rs b/sgl-router/src/routers/grpc/utils.rs index 2b9aa1ac4..709678685 100644 --- a/sgl-router/src/routers/grpc/utils.rs +++ b/sgl-router/src/routers/grpc/utils.rs @@ -158,51 +158,54 @@ pub fn generate_tool_constraints( tools: &[Tool], tool_choice: &Option, _model: &str, -) -> Option<(String, String)> { - let choice = tool_choice.as_ref()?; +) -> Result, String> { + let Some(choice) = tool_choice.as_ref() else { + return Ok(None); + }; match choice { // Specific function: Return parameters schema directly // tools should already be filtered to contain only the specific function ToolChoice::Function { .. } => { if tools.is_empty() { - return None; + return Ok(None); } let tool = &tools[0]; // Return the tool's parameters schema directly (not wrapped in array) - let params_schema = serde_json::to_string(&tool.function.parameters).ok()?; - Some(("json_schema".to_string(), params_schema)) + let params_schema = serde_json::to_string(&tool.function.parameters) + .map_err(|e| format!("Failed to serialize tool parameters: {}", e))?; + Ok(Some(("json_schema".to_string(), params_schema))) } // Required: Array of tool calls with minItems: 1 ToolChoice::Value(ToolChoiceValue::Required) => { let schema = build_required_array_schema(tools)?; - Some(("json_schema".to_string(), schema)) + Ok(Some(("json_schema".to_string(), schema))) } // AllowedTools with required mode: tools are already filtered ToolChoice::AllowedTools { mode, .. } => { if mode == "required" { if tools.is_empty() { - return None; + return Ok(None); } let schema = build_required_array_schema(tools)?; - Some(("json_schema".to_string(), schema)) + Ok(Some(("json_schema".to_string(), schema))) } else { // "auto" mode - no constraint needed - None + Ok(None) } } // "auto" or "none" - no constraint - _ => None, + _ => Ok(None), } } /// Build JSON schema for required tool calls (array with minItems: 1) /// Includes $defs consolidation from all tools (matching Python's behavior) -pub fn build_required_array_schema(tools: &[Tool]) -> Option { +pub fn build_required_array_schema(tools: &[Tool]) -> Result { // Build anyOf schemas for each tool let mut any_of_schemas = Vec::new(); for tool in tools { @@ -228,11 +231,12 @@ pub fn build_required_array_schema(tools: &[Tool]) -> Option { if let Some(existing) = all_defs.get(def_name) { // Check for conflicts if existing != def_schema { - error!( - "Tool definition '{}' has multiple schemas, which is not supported", + let error_msg = format!( + "Tool definition '{}' has multiple conflicting schemas, which is not supported", def_name ); - return None; + error!("{}", error_msg); + return Err(error_msg); } } else { all_defs.insert(def_name.clone(), def_schema.clone()); @@ -260,7 +264,8 @@ pub fn build_required_array_schema(tools: &[Tool]) -> Option { } } - serde_json::to_string(&array_schema).ok() + serde_json::to_string(&array_schema) + .map_err(|e| format!("Failed to serialize tool schema: {}", e)) } /// Filter tools based on tool_choice (shared by both routers)