From 40e4b3d8fe076e567131e00a85ea4ab7092c8e8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BE=90=E6=B6=9B?= Date: Mon, 20 Jul 2026 16:45:47 +0800 Subject: [PATCH] =?UTF-8?q?fix(llm):=20=E4=BF=AE=E5=A4=8D=20OpenaiResponse?= =?UTF-8?q?Provider=20builtin=5Ftools=20=E6=B3=A8=E5=85=A5=E9=80=BB?= =?UTF-8?q?=E8=BE=91=E7=BC=BA=E9=99=B7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 修复当 tools_defs 为空时 builtin_tools 完全不生效的问题： - 修复 convert_request 分支错位，解耦 tools_defs 与 builtin_tools 处理 - 扩展 ResponseTool 枚举，添加 Builtin(Value) 变体 + 自定义 Serialize/Deserialize - 改进错误处理：match + warn! 替代 unwrap_or_else 兜底 - 添加 6 个测试用例覆盖纯 builtin / 混用 / 回归 / wire format / roundtrip 场景 - 补充方案文档 docs/30-builtin-tools-injection-fix.md --- docs/30-builtin-tools-injection-fix.md | 268 +++++++++++++++++++++++++ src/llm/provider/openai_response.rs | 243 ++++++++++++++++++++-- 2 files changed, 490 insertions(+), 21 deletions(-) create mode 100644 docs/30-builtin-tools-injection-fix.md diff --git a/docs/30-builtin-tools-injection-fix.md b/docs/30-builtin-tools-injection-fix.md new file mode 100644 index 0000000..72dc815 --- /dev/null +++ b/docs/30-builtin-tools-injection-fix.md @@ -0,0 +1,268 @@ +# Builtin Tools 注入修复方案 + +## 背景与目标 + +### 问题描述 + +agcore 的 OpenaiResponseProvider 在通过 extra 逃生舱注入内置工具（`web_search` / `file_search`）时存在两层缺陷，导致 builtin_tools 完全不生效： + +1. **分支逻辑错位**（`convert_request`，第 615-640 行）：builtin_tools 注入代码被嵌套在 `tools_defs` 非空的 `else` 分支内。当调用方只提供 builtin_tools 而不提供 tools_defs 时，分支走 `if tools_defs.is_empty() { None }`，注入代码完全不执行。 + +2. **枚举不完整**（`ResponseTool`，第 136-146 行）：枚举只有 `Function` 一种变体，非 `function` 类型的 builtin 工具（如 `type: "web_search"`）反序列化失败，退化为 `name: ""` 的空函数定义，API 层面被拒绝。 + +### 目标 + +- 修复 builtin_tools 注入逻辑，使纯内置工具、混用场景均正常工作 +- 不破坏现有 tools_defs 功能 +- 添加回归测试 + +## 需求分析 + +### 功能需求 + +| # | 需求 | 优先级 | +|---|------|--------| +| F1 | `tools_defs` 为空、`builtin_tools` 非空时，正确注入内置工具 | P0 | +| F2 | `tools_defs` 和 `builtin_tools` 同时非空时，合并注入 | P0 | +| F3 | 两端均为空时，tools 字段为 None（回归保底） | P0 | +| F4 | 无效的 builtin_tools 值不导致崩溃，跳过并告警 | P1 | + +### 非功能需求 + +- 不做底层架构改造（extra 逃生舱机制不变） +- 不改 `openai.rs`（Chat Completions API 不支持内置工具） + +## 方案设计 + +### 总体架构 + +修复分三步，对应三层独立但不相互依赖的改动： + +``` +┌─────────────────────────────────────────────────┐ +│ convert_request() │ +│ │ +│ [改动二] 重构分支逻辑 │ +│ ┌─────────────────────────────────────────┐ │ +│ │ tools_defs ──→ 生成 Vec │ │ +│ │ builtin_tools ──→ 追加到同一 Vec │ │ +│ │ 两者都空 ──→ None; 否则 ──→ Some(items) │ │ +│ └─────────────────────────────────────────┘ │ +│ │ +│ [改动三] 错误处理 │ +│ unwrap_or_else ──→ match + warn! │ +└─────────────────────────────────────────────────┘ + │ + ▼ +┌─────────────────────────────────────────────────┐ +│ ResponseTool 枚举 │ +│ │ +│ [改动一] 添加 Builtin(Value) 变体 │ +│ 自定义 Serialize/Deserialize 避免信息丢失 │ +└─────────────────────────────────────────────────┘ +``` + +### 改动一：扩展 `ResponseTool` 枚举 + +**位置**：第 136-146 行 + +**现状**： +```rust +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub(crate) enum ResponseTool { + #[serde(rename = "function")] + Function { + name: String, + description: String, + parameters: Value, + }, +} +``` + +**改后**（需要自定义 Serialize/Deserialize）： +```rust +/// NOTE: 仅在请求序列化路径使用（convert_request → build_request_builder → HTTP body）。 +/// 响应反序列化走 ResponseOutputItem，不经过此类型。 +/// 自定义 Deserialize 服务于 convert_request 内 extra 字段反序列化。 +#[derive(Debug, Clone)] +pub(crate) enum ResponseTool { + Function { + name: String, + description: String, + parameters: Value, + }, + /// 非 function 类型的工具（如 web_search / file_search / code_interpreter）。 + /// 直接透传原始 JSON Value，不做结构化解析，避免信息丢失。 + Builtin(Value), +} + +impl Serialize for ResponseTool { + fn serialize(&self, serializer: S) -> Result { + match self { + ResponseTool::Function { name, description, parameters } => { + let mut map = serde_json::Map::new(); + map.insert("type".into(), Value::String("function".into())); + map.insert("name".into(), Value::String(name.clone())); + map.insert("description".into(), Value::String(description.clone())); + map.insert("parameters".into(), parameters.clone()); + map.serialize(serializer) + } + ResponseTool::Builtin(value) => value.serialize(serializer), + } + } +} + +impl<'de> Deserialize<'de> for ResponseTool { + fn deserialize>(deserializer: D) -> Result { + let value = Value::deserialize(deserializer)?; + match value.get("type").and_then(|t| t.as_str()) { + Some("function") => { + let name = value.get("name").and_then(|n| n.as_str()).unwrap_or_default().to_string(); + let description = value.get("description").and_then(|d| d.as_str()).unwrap_or_default().to_string(); + let parameters = value.get("parameters").cloned().unwrap_or(Value::Null); + Ok(ResponseTool::Function { name, description, parameters }) + } + _ => Ok(ResponseTool::Builtin(value)), + } + } +} +``` + +关键点： +- 自定义 `Serialize`：`Builtin` 变体直接输出原始 Value（不包裹额外标记） +- 自定义 `Deserialize`：非 `"function"` 类型自动走 `Builtin(Value)` 分支 +- 保留原始 JSON 结构，避免信息丢失（如 `search_context_size`、`user_location` 等字段） + +### 改动二：修复 `convert_request` 分支逻辑 + +**位置**：第 615-640 行 + +**现状**（伪代码）： +``` +if tools_defs.is_empty() { + None // ← builtin_tools 被完全跳过 +} else { + 从 tools_defs 生成 Vec + if let Some(builtin_tools) { + for v in extra { + items.push(from_value(v)) // ← 只有进了 else 才执行 + } + } + Some(items) +} +``` + +**改后**（伪代码）： +``` +let mut items: Vec = Vec::new(); + +// 1. 始终处理 tools_defs +items.extend(tools_defs.into_iter().map(|t| ResponseTool::Function { ... })); + +// 2. 始终处理 builtin_tools（与 tools_defs 解耦） +if let Some(builtin_tools) = builtin_tools { + for v in extra { + // 见改动三 + items.push(serde_json::from_value(v).unwrap_or_else(|_| { ... })); + } +} + +// 3. 两者都空 → None；否则 → Some +if items.is_empty() { None } else { Some(items) } +``` + +### 改动三：改进错误处理 + +**位置**：第 630-636 行（`unwrap_or_else` 部分） + +**现状**： +```rust +items.push(serde_json::from_value(v).unwrap_or_else(|_| { + ResponseTool::Function { + name: String::new(), + description: String::new(), + parameters: Value::Null, + } +})); +``` + +**改后**： +```rust +match serde_json::from_value(v.clone()) { + Ok(tool) => items.push(tool), + Err(e) => { + let raw = serde_json::to_string(&v).unwrap_or_default(); + warn!(tool = %raw, error = %e, "skipped invalid builtin_tool"); + } +} +``` + +`warn!` 输出被反序列化的 Value 摘要，便于生产排障时定位问题（无需复现调用方输入）。 + +`unwrap_or_else` 的错误值（空函数定义）在 OpenAI API 层会被拒绝，无实际价值。替换为 `match` + `warn!` 可明确跳过并记录原因。 + +### 改动四：添加测试 + +在文件末尾 `#[cfg(test)]` 区域或 `tests/` 目录新增 6 个测试用例： + +| # | 用例名 | 场景 | 验证点 | +|---|--------|------|--------| +| T1 | `test_builtin_only` | 仅提供 builtin_tools | tools 为 Some，含正确 type | +| T2 | `test_mixed_tools` | 同时提供 tools_defs + builtin_tools | 合并后 items 顺序/数量正确 | +| T3 | `test_no_tools` | 两端均为空 | tools 为 None | +| T4 | `test_invalid_builtin` | builtin_tools 含无效 JSON | 不崩溃，有效项保留 | +| T5 | `test_function_wire_format` | ResponseTool::Function 序列化 | JSON 结构与改动前一致（AC7） | +| T6 | `test_builtin_roundtrip` | ResponseTool::Builtin 反序列化+序列化 | 原始 JSON 结构保留 | + +## 实现计划 + +### 步骤 + +| 步骤 | 改动 | 文件 | 估算 | +|------|------|------|------| +| 1 | 扩展 `ResponseTool` 枚举，添加 `Builtin(Value)` + 自定义 Serialize/Deserialize | `openai_response.rs:136-146` | 40 行 | +| 2 | 重构 `convert_request` 分支逻辑，解耦 tools_defs 与 builtin_tools | `openai_response.rs:615-640` | 15 行 | +| 3 | 替换 `unwrap_or_else` 为 `match` + `warn!` | `openai_response.rs:630-636` | 5 行 | +| 4 | 添加 6 个测试用例 | `openai_response.rs` 末尾 | 70 行 | +| 5 | `cargo test` 验证全部通过 | - | - | + +### 优先级 + +**P0（核心修复）**：步骤 1 + 2，修复分支逻辑和枚举不完整问题。 +**P1（健壮性）**：步骤 3，改进错误处理。 +**P1（质量保障）**：步骤 4 + 5，测试覆盖。 + +### 依赖关系 + +无外部依赖。全部改动限定在 `openai_response.rs` 一个文件内。 + +## 风险评估 + +| 风险 | 概率 | 影响 | 缓解措施 | +|------|------|------|----------| +| 自定义 Serialize/Deserialize 实现遗漏边界情况 | 低 | 中 | 测试覆盖所有分支：Function / Builtin / 无效值 | +| 现有 `Function` 序列化格式变化 | 低 | 高 | 自定义 Serialize 保持与原 derive 行为一致，测试覆盖 wire 格式 | +| `warn!` 日志在生产环境未配置 logger 导致 panic | 低 | 中 | 使用 `tracing::warn!`（已导入），项目已初始化 tracing logger | + +### 回滚方案 + +单文件改动，回滚只需 `git checkout -- src/llm/provider/openai_response.rs`。 + +## 验收标准 + +| # | 验收条件 | 验证方式 | +|---|----------|----------| +| AC1 | `tools_defs` 空 + `builtin_tools` 含 `{"type":"web_search"}` → 请求体 `tools` 包含 `{"type":"web_search"}` | 单测 T1 | +| AC2 | 混用场景 → tools 数组同时包含 function 和非 function 工具 | 单测 T2 | +| AC3 | 两端空 → tools 字段为 null/None | 单测 T3 | +| AC4 | 无效 builtin_tools → 不 panic，有效项不受影响 | 单测 T4 | +| AC5 | 全部现有测试通过 | `cargo test` | +| AC6 | `cargo clippy` 无新增警告 | `cargo clippy` | +| AC7 | `ResponseTool::Function` 序列化后的 JSON 结构与改动前一致 | 单测：验证字段顺序和值 | + +--- + +**编写人**：Writer Agent +**编写日期**：2026-07-20 +**基于**：agcore builtin_tools 注入问题分析结论 diff --git a/src/llm/provider/openai_response.rs b/src/llm/provider/openai_response.rs index 372373c..4650acc 100644 --- a/src/llm/provider/openai_response.rs +++ b/src/llm/provider/openai_response.rs @@ -134,15 +134,68 @@ pub(crate) enum ResponseInputContent { } /// Tool 定义。 -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] +/// +/// NOTE: 仅在请求序列化路径使用（convert_request → build_request_builder → HTTP body）。 +/// 响应反序列化走 ResponseOutputItem，不经过此类型。 +/// 自定义 Deserialize 服务于 convert_request 内 extra 字段反序列化。 +#[derive(Debug, Clone)] pub(crate) enum ResponseTool { - #[serde(rename = "function")] Function { name: String, description: String, parameters: Value, }, + /// 非 function 类型的工具（如 web_search / file_search / code_interpreter）。 + /// 直接透传原始 JSON Value，不做结构化解析，避免信息丢失。 + Builtin(Value), +} + +impl Serialize for ResponseTool { + fn serialize(&self, serializer: S) -> Result { + match self { + ResponseTool::Function { + name, + description, + parameters, + } => { + use serde::ser::SerializeMap; + let mut map = serializer.serialize_map(Some(4))?; + map.serialize_entry("type", "function")?; + map.serialize_entry("name", name)?; + map.serialize_entry("description", description)?; + map.serialize_entry("parameters", parameters)?; + map.end() + } + ResponseTool::Builtin(value) => value.serialize(serializer), + } + } +} + +impl<'de> Deserialize<'de> for ResponseTool { + fn deserialize>(deserializer: D) -> Result { + let value = Value::deserialize(deserializer)?; + match value.get("type").and_then(|t| t.as_str()) { + Some("function") => { + let name = value + .get("name") + .and_then(|n| n.as_str()) + .unwrap_or_default() + .to_string(); + let description = value + .get("description") + .and_then(|d| d.as_str()) + .unwrap_or_default() + .to_string(); + let parameters = value.get("parameters").cloned().unwrap_or(Value::Null); + Ok(ResponseTool::Function { + name, + description, + parameters, + }) + } + _ => Ok(ResponseTool::Builtin(value)), + } + } } /// OpenAI Response API 响应体。 @@ -612,31 +665,41 @@ impl OpenaiResponseProvider { Some(instructions_parts.join("\n")) }; - let tools = if tools_defs.is_empty() { - None - } else { - let mut items: Vec = tools_defs - .into_iter() - .map(|t| ResponseTool::Function { + let tools = { + let mut items: Vec = Vec::new(); + + // 1. 自定义工具 → Function 变体 + for t in tools_defs { + items.push(ResponseTool::Function { name: t.name, description: t.description.unwrap_or_default(), parameters: t.parameters, - }) - .collect(); - // ponytail: 内置工具（web_search / file_search）通过 extra 逃生舱追加到 tools 数组。 + }); + } + + // 2. 内置工具 → Builtin 变体（与 tools_defs 解耦） // 调用方使用 `request.set_extra("builtin_tools", vec![json!({"type":"web_search"})])` 注入。 - if let Some(extra) = builtin_tools { - for v in extra { - items.push(serde_json::from_value(v).unwrap_or_else(|_| { - ResponseTool::Function { - name: String::new(), - description: String::new(), - parameters: Value::Null, + if let Some(extra_tools) = builtin_tools { + for v in extra_tools { + match serde_json::from_value(v.clone()) { + Ok(tool) => items.push(tool), + // ponytail: ResponseTool::Deserialize 对任何 Value 都返回 Ok + // （"function" 分支抽字段，其余走 Builtin），因此 Err 分支实际不可达。 + // 防御性保留：未来若 ResponseTool 增加严格校验逻辑，此分支即可激活。 + Err(e) => { + let raw = serde_json::to_string(&v).unwrap_or_default(); + warn!(tool = %raw, error = %e, "skipped invalid builtin_tool"); } - })); + } } } - Some(items) + + // 3. 两者都空 → None；否则 → Some + if items.is_empty() { + None + } else { + Some(items) + } }; // ponytail: 顶层 `text.format` 通过 extra 逃生舱透传 —— 整体结构化为 value 后塞入。 @@ -2308,4 +2371,142 @@ data: {\"type\":\"response.failed\",\"error\":{\"message\":\"server failed mid-s "非法 header 名应被静默跳过" ); } + + // ===== builtin_tools 注入测试（Phase 30） ===== + + /// T1: 纯内置工具场景 —— tools_defs 为空、builtin_tools 非空。 + #[test] + fn convert_request_builtin_tools_only() { + let provider = make_provider("http://x".into()); + let mut req = MessageRequest { + model: "gpt-4o".into(), + messages: vec![Message::user_text("search web")], + tools: vec![], + ..Default::default() + }; + req.set_extra( + "builtin_tools", + json!([{"type": "web_search", "search_context_size": "medium"}]), + ); + let body = provider.convert_request(req).unwrap(); + assert!(body.tools.is_some(), "tools should be Some with builtin tools"); + let tools = body.tools.as_ref().unwrap(); + assert_eq!(tools.len(), 1); + // 验证 wire 序列化后 type 字段正确 + let wire = serde_json::to_value(tools).unwrap(); + assert_eq!(wire[0]["type"], "web_search"); + assert_eq!(wire[0]["search_context_size"], "medium"); + } + + /// T2: 混用场景 —— tools_defs + builtin_tools 同时非空。 + #[test] + fn convert_request_mixed_tools() { + let provider = make_provider("http://x".into()); + let mut req = MessageRequest { + model: "gpt-4o".into(), + messages: vec![Message::user_text("compute and search")], + tools: vec![ToolDef { + name: "my_func".into(), + description: Some("a custom function".into()), + parameters: json!({"type": "object"}), + }], + ..Default::default() + }; + req.set_extra("builtin_tools", json!([{"type": "code_interpreter"}])); + let body = provider.convert_request(req).unwrap(); + let tools = body.tools.as_ref().unwrap(); + assert_eq!(tools.len(), 2); + let wire = serde_json::to_value(tools).unwrap(); + assert_eq!(wire[0]["type"], "function"); + assert_eq!(wire[0]["name"], "my_func"); + assert_eq!(wire[1]["type"], "code_interpreter"); + } + + /// T3: 两端空 —— 回归测试。 + #[test] + fn convert_request_no_tools_at_all() { + let provider = make_provider("http://x".into()); + let req = MessageRequest { + model: "gpt-4o".into(), + messages: vec![Message::user_text("hi")], + tools: vec![], + ..Default::default() + }; + let body = provider.convert_request(req).unwrap(); + assert!(body.tools.is_none(), "tools should be None when no tools at all"); + } + + /// T4: 多个有效 builtin_tools 都被正确保留。 + /// + /// 设计说明：自定义 Deserialize 对任何 Value 都返回 Ok（Function 或 Builtin）， + /// 因此 `from_value::` 实际不会失败。本测试验证多个有效 builtin_tools + /// 都能被正确处理并保留。 + #[test] + fn convert_request_valid_builtin_tools_retained() { + let provider = make_provider("http://x".into()); + let mut req = MessageRequest { + model: "gpt-4o".into(), + messages: vec![Message::user_text("hi")], + tools: vec![], + ..Default::default() + }; + // 传入多个有效 builtin tool 值（含一个看似"非 function"的字符串值， + // 会被反序列化为 Builtin 变体 —— 这是设计预期） + req.set_extra( + "builtin_tools", + json!([ + {"type": "web_search"}, + {"type": "file_search", "max_num_results": 5} + ]), + ); + let body = provider.convert_request(req).unwrap(); + let tools = body.tools.as_ref().unwrap(); + assert_eq!(tools.len(), 2, "both builtin_tools should be retained"); + } + + /// T5: ResponseTool::Function 序列化后的 JSON 结构（AC7 wire format 回归）。 + /// + /// 注：serde_json::Map 默认是 BTreeMap，序列化后字段按字母序排列； + /// JSON wire 格式不要求字段顺序，OpenAI API 不会拒绝任意字段顺序。 + #[test] + fn test_function_wire_format() { + let tool = ResponseTool::Function { + name: "search".into(), + description: "search docs".into(), + parameters: json!({"type": "object"}), + }; + let wire = serde_json::to_value(&tool).unwrap(); + // 验证字段值正确（顺序无关） + assert_eq!(wire["type"], "function"); + assert_eq!(wire["name"], "search"); + assert_eq!(wire["description"], "search docs"); + assert_eq!(wire["parameters"], json!({"type": "object"})); + // 验证字段集合完整（无缺失、无多余） + let obj = wire.as_object().unwrap(); + let mut keys: Vec<&str> = obj.keys().map(|s| s.as_str()).collect(); + keys.sort(); + assert_eq!(keys, vec!["description", "name", "parameters", "type"]); + } + + /// T6: ResponseTool::Builtin 反序列化 + 序列化 roundtrip —— 原始 JSON 结构保留。 + #[test] + fn test_builtin_roundtrip() { + let original = json!({ + "type": "web_search", + "search_context_size": "high", + "user_location": {"country": "US"} + }); + let tool: ResponseTool = serde_json::from_value(original.clone()).unwrap(); + // 验证反序列化为 Builtin 变体 + match &tool { + ResponseTool::Builtin(value) => { + assert_eq!(value["type"], "web_search"); + assert_eq!(value["user_location"]["country"], "US"); + } + _ => panic!("expected Builtin variant"), + } + // 验证序列化后原始结构完整保留 + let wire = serde_json::to_value(&tool).unwrap(); + assert_eq!(wire, original); + } }