From 940ea28756db2b41dbc93a3ed6539639f42d158b Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Wed, 10 Jun 2026 15:30:47 -0700 Subject: [PATCH] fix(codex): land function_call_output, schema sanitization, and reasoning effort mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - responses.rs: add ToolResult→function_call_output conversion for user-role blocks (previously silently dropped, breaking multi-turn tool calling) - responses.rs: extract codex_responses_reasoning_effort helper — maps 'maximum'→xhigh, off/none→omit, unknown→medium - responses.rs: tool_to_responses_function now clones and sanitizes schema via schema_sanitize::sanitize_for_responses before sending parameters - schema_sanitize.rs: add sanitize_for_responses — forces root type:object, merges root composition properties (oneOf/anyOf/allOf), removes root enum/not, ensures properties object, prunes dangling required - schema_sanitize.rs: add merge_root_composition_properties helper - schema_sanitize.rs: add 3 responses_sanitize_* tests Refs: #2984 #2955 --- crates/tui/src/client/responses.rs | 146 ++++++++++++++++++++++-- crates/tui/src/tools/schema_sanitize.rs | 143 +++++++++++++++++++++++ 2 files changed, 279 insertions(+), 10 deletions(-) diff --git a/crates/tui/src/client/responses.rs b/crates/tui/src/client/responses.rs index 7a01435f..a79f5648 100644 --- a/crates/tui/src/client/responses.rs +++ b/crates/tui/src/client/responses.rs @@ -16,6 +16,7 @@ use crate::models::{ ContentBlock, ContentBlockStart, Delta, MessageDelta, MessageRequest, MessageResponse, StreamEvent, Tool, Usage, }; +use crate::tools::schema_sanitize; use super::{DeepSeekClient, ERROR_BODY_MAX_BYTES, bounded_error_text, system_to_instructions}; @@ -61,15 +62,7 @@ impl DeepSeekClient { // when it is disabled. CodeWhale's "auto" has no Codex equivalent and // falls back to "medium". if let Some(raw) = request.reasoning_effort.as_deref() { - let effort = match raw.trim().to_ascii_lowercase().as_str() { - "off" | "disabled" | "none" | "false" => None, - "minimal" => Some("minimal"), - "low" => Some("low"), - "high" => Some("high"), - "xhigh" | "max" => Some("xhigh"), - _ => Some("medium"), - }; - if let Some(effort) = effort { + if let Some(effort) = codex_responses_reasoning_effort(raw) { body["reasoning"] = json!({ "effort": effort, "summary": "auto", @@ -508,6 +501,26 @@ fn convert_messages_to_responses_input(request: &MessageRequest) -> Vec { "image_url": image_url.url, })); } + ContentBlock::ToolResult { + tool_use_id, + content, + .. + } => { + if !content_items.is_empty() { + items.push(json!({ + "type": "message", + "role": "user", + "content": content_items, + })); + content_items = Vec::new(); + } + let (call_id, _item_id) = parse_tool_use_id(tool_use_id); + items.push(json!({ + "type": "function_call_output", + "call_id": call_id, + "output": content, + })); + } _ => {} } } @@ -582,15 +595,28 @@ fn convert_messages_to_responses_input(request: &MessageRequest) -> Vec { /// Convert a CodeWhale tool definition to a Responses API function tool. fn tool_to_responses_function(tool: &Tool) -> Value { + let mut parameters = tool.input_schema.clone(); + schema_sanitize::sanitize_for_responses(&mut parameters); json!({ "type": "function", "name": tool.name, "description": tool.description, - "parameters": tool.input_schema, + "parameters": parameters, "strict": false, }) } +fn codex_responses_reasoning_effort(raw: &str) -> Option<&'static str> { + match raw.trim().to_ascii_lowercase().as_str() { + "off" | "disabled" | "none" | "false" => None, + "minimal" => Some("minimal"), + "low" => Some("low"), + "high" => Some("high"), + "xhigh" | "max" | "maximum" => Some("xhigh"), + _ => Some("medium"), + } +} + /// Parse a composite tool_use_id back to (call_id, item_id). /// Composite format: "call_id|item_id" fn parse_tool_use_id(id: &str) -> (String, String) { @@ -626,3 +652,103 @@ fn parse_responses_usage(val: &Value) -> Usage { server_tool_use: None, } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::Message; + + #[test] + fn codex_reasoning_effort_uses_responses_labels() { + assert_eq!(codex_responses_reasoning_effort("max"), Some("xhigh")); + assert_eq!(codex_responses_reasoning_effort("maximum"), Some("xhigh")); + assert_eq!(codex_responses_reasoning_effort("xhigh"), Some("xhigh")); + assert_eq!(codex_responses_reasoning_effort("high"), Some("high")); + assert_eq!(codex_responses_reasoning_effort("medium"), Some("medium")); + assert_eq!(codex_responses_reasoning_effort("auto"), Some("medium")); + assert_eq!(codex_responses_reasoning_effort("off"), None); + } + + #[test] + fn responses_input_includes_user_role_tool_results() { + let request = MessageRequest { + model: "gpt-5.5".to_string(), + messages: vec![ + Message { + role: "assistant".to_string(), + content: vec![ContentBlock::ToolUse { + id: "call_abc|fc_123".to_string(), + name: "checklist_write".to_string(), + input: json!({"items": []}), + caller: None, + }], + }, + Message { + role: "user".to_string(), + content: vec![ContentBlock::ToolResult { + tool_use_id: "call_abc|fc_123".to_string(), + content: "<6 items>".to_string(), + is_error: None, + content_blocks: None, + }], + }, + ], + max_tokens: 128, + system: None, + tools: None, + tool_choice: None, + metadata: None, + thinking: None, + reasoning_effort: None, + stream: None, + temperature: None, + top_p: None, + }; + + let input = convert_messages_to_responses_input(&request); + + assert_eq!(input[0]["type"], "function_call"); + assert_eq!(input[0]["call_id"], "call_abc"); + assert_eq!(input[1]["type"], "function_call_output"); + assert_eq!(input[1]["call_id"], "call_abc"); + assert_eq!(input[1]["output"], "<6 items>"); + } + + #[test] + fn responses_function_tool_sanitizes_root_composition_schema() { + let tool = Tool { + tool_type: None, + name: "apply_patch".to_string(), + description: "Apply patch".to_string(), + input_schema: json!({ + "type": "object", + "properties": { + "patch": {"type": "string"}, + "changes": {"type": "array"} + }, + "oneOf": [ + {"required": ["patch"]}, + {"required": ["changes"]} + ] + }), + allowed_callers: None, + defer_loading: None, + input_examples: None, + strict: None, + cache_control: None, + }; + + let payload = tool_to_responses_function(&tool); + let parameters = &payload["parameters"]; + + assert_eq!(parameters["type"], "object"); + assert!(parameters.get("oneOf").is_none()); + assert!(parameters.get("anyOf").is_none()); + assert!(parameters.get("allOf").is_none()); + assert!(parameters.get("enum").is_none()); + assert!(parameters.get("not").is_none()); + assert!(parameters["properties"].get("patch").is_some()); + assert!(parameters["properties"].get("changes").is_some()); + assert!(tool.input_schema.get("oneOf").is_some()); + } +} diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index 61798d22..a55bb969 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -75,6 +75,35 @@ pub fn sanitize_for_strict(schema: &mut Value) { enforce_strict_subset(schema); } +/// Sanitize a schema for OpenAI Responses function tools. +/// +/// The Responses API requires the top-level `parameters` schema to be an object +/// and rejects top-level `oneOf` / `anyOf` / `allOf` / `enum` / `not`. Keep the +/// schema permissive rather than changing tool semantics: merge any root +/// alternative properties we can see, then remove the root-only composition +/// keywords while preserving nested schemas. +pub fn sanitize_for_responses(schema: &mut Value) { + sanitize(schema); + + if !schema.is_object() { + *schema = Value::Object(Map::new()); + } + + let Some(obj) = schema.as_object_mut() else { + return; + }; + + merge_root_composition_properties(obj); + obj.insert("type".into(), Value::String("object".to_string())); + obj.remove("oneOf"); + obj.remove("anyOf"); + obj.remove("allOf"); + obj.remove("enum"); + obj.remove("not"); + ensure_properties_object(obj); + prune_dangling_required(schema); +} + fn strict_schema_supported(schema: &Value) -> bool { let mut normalized = schema.clone(); sanitize(&mut normalized); @@ -250,6 +279,32 @@ fn ensure_properties_object(obj: &mut Map) -> &mut Map) { + let mut merged = Map::new(); + for key in ["oneOf", "anyOf", "allOf"] { + let Some(items) = obj.get(key).and_then(Value::as_array) else { + continue; + }; + for item in items { + let Some(properties) = item.get("properties").and_then(Value::as_object) else { + continue; + }; + for (name, schema) in properties { + merged.entry(name.clone()).or_insert_with(|| schema.clone()); + } + } + } + + if merged.is_empty() { + return; + } + + let properties = ensure_properties_object(obj); + for (name, schema) in merged { + properties.entry(name).or_insert(schema); + } +} + #[cfg(test)] mod tests { use super::*; @@ -603,6 +658,94 @@ mod tests { assert_eq!(tools[0].input_schema["required"], json!(["query"])); assert_eq!(tools[0].input_schema["additionalProperties"], false); } + + #[test] + fn responses_sanitize_removes_root_composition_from_apply_patch_shape() { + let mut schema = json!({ + "type": "object", + "properties": { + "path": {"type": "string"}, + "patch": {"type": "string"}, + "changes": { + "type": "array", + "items": { + "type": "object", + "properties": { + "path": {"type": "string"}, + "content": {"type": "string"} + }, + "required": ["path", "content"] + } + } + }, + "oneOf": [ + {"required": ["patch"]}, + {"required": ["changes"]} + ] + }); + + sanitize_for_responses(&mut schema); + + assert_eq!(schema["type"], "object"); + assert!(schema.get("oneOf").is_none()); + assert!(schema.get("anyOf").is_none()); + assert!(schema.get("allOf").is_none()); + assert!(schema.get("enum").is_none()); + assert!(schema.get("not").is_none()); + assert!(schema["properties"].get("patch").is_some()); + assert!(schema["properties"].get("changes").is_some()); + } + + #[test] + fn responses_sanitize_merges_root_alternative_properties() { + let mut schema = json!({ + "anyOf": [ + { + "type": "object", + "properties": { + "path": {"type": "string"} + }, + "required": ["path"] + }, + { + "type": "object", + "properties": { + "url": {"type": "string"} + }, + "required": ["url"] + } + ] + }); + + sanitize_for_responses(&mut schema); + + assert_eq!(schema["type"], "object"); + assert!(schema.get("anyOf").is_none()); + assert!(schema["properties"].get("path").is_some()); + assert!(schema["properties"].get("url").is_some()); + assert!(schema.get("required").is_none()); + } + + #[test] + fn responses_sanitize_preserves_nested_alternatives() { + let mut schema = json!({ + "type": "object", + "properties": { + "value": { + "anyOf": [ + {"type": "string"}, + {"type": "integer"} + ] + } + } + }); + + sanitize_for_responses(&mut schema); + + assert_eq!(schema["type"], "object"); + assert!(schema.get("anyOf").is_none()); + assert!(schema["properties"]["value"].get("anyOf").is_some()); + } } /// Normalize a tool's function schema for Kimi / Moonshot API compatibility.