徐涛
40e4b3d8fe
修复当 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
269 lines
11 KiB
Markdown
269 lines
11 KiB
Markdown
# 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<ResponseTool> │ │
|
||
│ │ 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<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
|
||
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<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
|
||
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<ResponseTool>
|
||
if let Some(builtin_tools) {
|
||
for v in extra {
|
||
items.push(from_value(v)) // ← 只有进了 else 才执行
|
||
}
|
||
}
|
||
Some(items)
|
||
}
|
||
```
|
||
|
||
**改后**（伪代码）：
|
||
```
|
||
let mut items: Vec<ResponseTool> = 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 注入问题分析结论
|