From c18f6730f8b04ac79b15a0c9c611f2263fb84282 Mon Sep 17 00:00:00 2001 From: Mohamed Date: Thu, 19 Feb 2026 02:23:46 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20OpenAI=20tool=20calling=20=E2=80=94=20sc?= =?UTF-8?q?hema=20normalization,=20missing=20types,=20and=20Responses=20AP?= =?UTF-8?q?I=20panic=20(#132)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: add missing type key to http tool body schema The body property in HttpTool::parameters_schema() was missing the required \"type\" key, causing OpenAI to reject all tool calls with: Invalid schema for function 'http' Fixes #131 * fix: add missing type key to json tool data schema Same class of bug as http tool body — the data property in JsonTool::parameters_schema() was missing the required "type" key, causing OpenAI to reject all tool calls. Fixes #131 * fix: use Chat Completions API to avoid rig-core Responses API panic The default openai::Client routes through rig-core's Responses API, which panics at "The tool call ID should exist!" because ironclaw doesn't thread call_id through its ToolCall type. Switch to openai::CompletionsClient which uses the Chat Completions API and works correctly with the existing code. * fix: normalize tool schemas for OpenAI strict mode compliance GPT-5/5.2 enforce strict function calling by default. Add normalize_schema_strict() that recursively transforms tool parameter schemas at the provider boundary: - Forces additionalProperties: false on all objects - Makes required list ALL property keys - Converts optional fields to nullable types - Handles nested objects, array items, and combinators Original schemas remain unchanged for other providers. Closes #131 --------- Co-authored-by: Illia Polosukhin --- src/llm/mod.rs | 16 ++-- src/llm/rig_adapter.rs | 171 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 181 insertions(+), 6 deletions(-) diff --git a/src/llm/mod.rs b/src/llm/mod.rs index 21d841f2..dbb27b96 100644 --- a/src/llm/mod.rs +++ b/src/llm/mod.rs @@ -95,11 +95,17 @@ fn create_openai_provider(config: &LlmConfig) -> Result, Ll use rig::providers::openai; - let client: openai::Client = - openai::Client::new(oai.api_key.expose_secret()).map_err(|e| LlmError::RequestFailed { - provider: "openai".to_string(), - reason: format!("Failed to create OpenAI client: {}", e), - })?; + // Use CompletionsClient (Chat Completions API) instead of the default Client + // (Responses API). The Responses API path in rig-core panics when tool results + // are sent back because ironclaw doesn't thread `call_id` through its ToolCall + // type. The Chat Completions API works correctly with the existing code. + let client: openai::CompletionsClient = + openai::Client::new(oai.api_key.expose_secret()) + .map_err(|e| LlmError::RequestFailed { + provider: "openai".to_string(), + reason: format!("Failed to create OpenAI client: {}", e), + })? + .completions_api(); let model = client.completion_model(&oai.model); tracing::info!("Using OpenAI direct API (model: {})", oai.model); diff --git a/src/llm/rig_adapter.rs b/src/llm/rig_adapter.rs index 642cc895..20bdbc72 100644 --- a/src/llm/rig_adapter.rs +++ b/src/llm/rig_adapter.rs @@ -16,6 +16,7 @@ use rig::message::{ use rust_decimal::Decimal; use serde::Serialize; use serde::de::DeserializeOwned; +use serde_json::Value as JsonValue; use crate::error::LlmError; use crate::llm::costs; @@ -50,6 +51,171 @@ impl RigAdapter { // -- Type conversion helpers -- +/// Normalize a JSON Schema for OpenAI strict mode compliance. +/// +/// OpenAI strict function calling requires: +/// - Every object must have `"additionalProperties": false` +/// - `"required"` must list ALL property keys +/// - Optional fields use `"type": ["", "null"]` instead of being omitted from `required` +/// - Nested objects and array items are recursively normalized +/// +/// This is applied as a clone-and-transform at the provider boundary so the +/// original tool definitions remain unchanged for other providers. +fn normalize_schema_strict(schema: &JsonValue) -> JsonValue { + let mut schema = schema.clone(); + normalize_schema_recursive(&mut schema); + schema +} + +fn normalize_schema_recursive(schema: &mut JsonValue) { + let obj = match schema.as_object_mut() { + Some(o) => o, + None => return, + }; + + // Recurse into combinators: anyOf, oneOf, allOf + for key in &["anyOf", "oneOf", "allOf"] { + if let Some(JsonValue::Array(variants)) = obj.get_mut(*key) { + for variant in variants.iter_mut() { + normalize_schema_recursive(variant); + } + } + } + + // Recurse into array items + if let Some(items) = obj.get_mut("items") { + normalize_schema_recursive(items); + } + + // Recurse into `not`, `if`, `then`, `else` + for key in &["not", "if", "then", "else"] { + if let Some(sub) = obj.get_mut(*key) { + normalize_schema_recursive(sub); + } + } + + // Only apply object-level normalization if this schema has "properties" + // (explicit object schema) or type == "object" + let is_object = obj + .get("type") + .and_then(|t| t.as_str()) + .map(|t| t == "object") + .unwrap_or(false); + let has_properties = obj.contains_key("properties"); + + if !is_object && !has_properties { + return; + } + + // Ensure "type": "object" is present + if !obj.contains_key("type") && has_properties { + obj.insert("type".to_string(), JsonValue::String("object".to_string())); + } + + // Force additionalProperties: false (overwrite any existing value) + obj.insert( + "additionalProperties".to_string(), + JsonValue::Bool(false), + ); + + // Ensure "properties" exists + if !obj.contains_key("properties") { + obj.insert( + "properties".to_string(), + JsonValue::Object(serde_json::Map::new()), + ); + } + + // Collect current required set + let current_required: std::collections::HashSet = obj + .get("required") + .and_then(|r| r.as_array()) + .map(|arr| { + arr.iter() + .filter_map(|v| v.as_str().map(String::from)) + .collect() + }) + .unwrap_or_default(); + + // Get all property keys (sorted for deterministic output) + let all_keys: Vec = obj + .get("properties") + .and_then(|p| p.as_object()) + .map(|props| { + let mut keys: Vec = props.keys().cloned().collect(); + keys.sort(); + keys + }) + .unwrap_or_default(); + + // For properties NOT in the original required list, make them nullable + if let Some(JsonValue::Object(props)) = obj.get_mut("properties") { + for key in &all_keys { + // Recurse into each property's schema FIRST (before make_nullable, + // which may change the type to an array and prevent object detection) + if let Some(prop_schema) = props.get_mut(key) { + normalize_schema_recursive(prop_schema); + } + // Then make originally-optional properties nullable + if !current_required.contains(key) { + if let Some(prop_schema) = props.get_mut(key) { + make_nullable(prop_schema); + } + } + } + } + + // Set required to ALL property keys + let required_value: Vec = all_keys + .into_iter() + .map(JsonValue::String) + .collect(); + obj.insert("required".to_string(), JsonValue::Array(required_value)); +} + +/// Make a property schema nullable for OpenAI strict mode. +/// +/// If it has a simple `"type": ""`, converts to `"type": ["", "null"]`. +/// If it already has an array type, adds "null" if not present. +/// Otherwise, wraps with `anyOf: [, {"type": "null"}]`. +fn make_nullable(schema: &mut JsonValue) { + let obj = match schema.as_object_mut() { + Some(o) => o, + None => return, + }; + + if let Some(type_val) = obj.get("type").cloned() { + match type_val { + // "type": "string" → "type": ["string", "null"] + JsonValue::String(ref t) if t != "null" => { + obj.insert( + "type".to_string(), + serde_json::json!([t, "null"]), + ); + } + // "type": ["string", "integer"] → add "null" if missing + JsonValue::Array(ref arr) => { + let has_null = arr.iter().any(|v| v.as_str() == Some("null")); + if !has_null { + let mut new_arr = arr.clone(); + new_arr.push(JsonValue::String("null".to_string())); + obj.insert("type".to_string(), JsonValue::Array(new_arr)); + } + } + _ => {} + } + } else { + // No "type" key — wrap with anyOf including null + // (handles enum-only, $ref, or combinator schemas) + let existing = JsonValue::Object(obj.clone()); + obj.clear(); + obj.insert( + "anyOf".to_string(), + serde_json::json!([existing, {"type": "null"}]), + ); + } +} + /// Convert IronClaw messages to rig-core format. /// /// Returns `(preamble, chat_history)` where preamble is extracted from @@ -117,13 +283,16 @@ fn convert_messages(messages: &[ChatMessage]) -> (Option, Vec Vec { tools .iter() .map(|t| RigToolDefinition { name: t.name.clone(), description: t.description.clone(), - parameters: t.parameters.clone(), + parameters: normalize_schema_strict(&t.parameters), }) .collect() }