mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix: OpenAI tool calling — schema normalization, missing types, and Responses API panic (#132)
* 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 <[email protected]>
This commit is contained in:
co-authored by
Illia Polosukhin
parent
479ca888a2
commit
c18f6730f8
+11
-5
@@ -95,11 +95,17 @@ fn create_openai_provider(config: &LlmConfig) -> Result<Arc<dyn LlmProvider>, 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);
|
||||
|
||||
+170
-1
@@ -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<M: CompletionModel> RigAdapter<M> {
|
||||
|
||||
// -- 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": ["<original>", "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<String> = 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<String> = obj
|
||||
.get("properties")
|
||||
.and_then(|p| p.as_object())
|
||||
.map(|props| {
|
||||
let mut keys: Vec<String> = 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<JsonValue> = 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": "<T>"`, converts to `"type": ["<T>", "null"]`.
|
||||
/// If it already has an array type, adds "null" if not present.
|
||||
/// Otherwise, wraps with `anyOf: [<existing>, {"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<String>, Vec<RigMessage
|
||||
}
|
||||
|
||||
/// Convert IronClaw tool definitions to rig-core format.
|
||||
///
|
||||
/// Applies OpenAI strict-mode schema normalization to ensure all tool
|
||||
/// parameter schemas comply with OpenAI's function calling requirements.
|
||||
fn convert_tools(tools: &[IronToolDefinition]) -> Vec<RigToolDefinition> {
|
||||
tools
|
||||
.iter()
|
||||
.map(|t| RigToolDefinition {
|
||||
name: t.name.clone(),
|
||||
description: t.description.clone(),
|
||||
parameters: t.parameters.clone(),
|
||||
parameters: normalize_schema_strict(&t.parameters),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user