180 lines
8.1 KiB
Markdown
180 lines
8.1 KiB
Markdown
# PicoBot 项目优化计划（修订版）
|
||
|
||
> 重新评估日期：2026-08-06 | 项目版本：v0.3.2
|
||
|
||
---
|
||
|
||
## 重新评估说明
|
||
|
||
初版计划存在三个问题：
|
||
|
||
1. **混淆了"优化"与"重构美化"**：迁移 8 个错误类型到 thiserror、拆分文件、重组目录等纯属代码美化，零功能价值，却带来大量 churn 和引入 Bug 的风险
|
||
2. **混入了新功能**：Docker 支持、指标导出、WebSocket 速率限制等是功能需求，不是优化
|
||
3. **投机性优化**：在没有 profiling 数据的情况下假设 `.clone()` 是瓶颈、假设 tokio features 影响编译时间，这些都违背了"不过度工程化"原则
|
||
|
||
本修订版仅保留**有明确证据支撑、高 ROI、低风险**的改进项。
|
||
|
||
---
|
||
|
||
## Tier 1：值得做（高 ROI，低风险）
|
||
|
||
### 1.1 消除函数重复定义
|
||
|
||
**证据**：完全相同的函数在多个文件中重复定义，且错误处理策略不一致。
|
||
|
||
| 函数 | 重复次数 | 位置 |
|
||
|------|----------|------|
|
||
| `current_timestamp()` | 6 处 | `bus/message.rs`、`storage/mod.rs`、`gateway/ws.rs`、`tools/scheduler_manage.rs`、`tools/task/types.rs`、`tools/task/repository.rs` |
|
||
| `dirs::home_dir().unwrap_or_else(\|\| PathBuf::from("."))` | 7 处 | `config/mod.rs`(×3)、`cli/init.rs`(×3)、`logging.rs` |
|
||
| `format_error_chain()` | 3 处 | `agent_loop.rs`、`openai.rs`、`anthropic.rs` |
|
||
|
||
**方案**：提取到 `src/utils.rs` 公共模块，统一错误处理策略。
|
||
|
||
**风险**：极低——纯机械操作，函数体完全相同。
|
||
|
||
---
|
||
|
||
### 1.2 修复飞书正则表达式重复编译
|
||
|
||
**证据**：`channels/feishu.rs` 中 `MdPatterns::new()` 在每条出站消息处理时调用（[L2537](file:///e:/code_project/PicoBot/src/channels/feishu.rs#L2537)），每次编译 9 个正则表达式。正则编译是 CPU 密集型操作，在消息热路径上是不必要的开销。
|
||
|
||
**方案**：将 `MdPatterns` 改为 `LazyLock<MdPatterns>` 全局静态实例，仅编译一次。
|
||
|
||
```rust
|
||
use std::sync::LazyLock;
|
||
|
||
static MD_PATTERNS: LazyLock<MdPatterns> = LazyLock::new(MdPatterns::new);
|
||
```
|
||
|
||
**风险**：低——`MdPatterns` 是无状态的纯数据结构，全局共享安全。
|
||
|
||
---
|
||
|
||
### 1.3 CI 添加 `cargo audit`
|
||
|
||
**证据**：项目有 ~30 个 Rust 依赖和数十个 npm 依赖，但 CI 中无任何安全扫描。依赖漏洞是真实的安全风险。
|
||
|
||
**方案**：在 `.github/workflows/ci.yml` 的 rust-checks job 中添加一步：
|
||
|
||
```yaml
|
||
- name: Security audit
|
||
run: cargo install cargo-audit --locked && cargo audit
|
||
```
|
||
|
||
前端部分添加：
|
||
|
||
```yaml
|
||
- name: npm audit
|
||
run: npm audit --audit-level=high
|
||
```
|
||
|
||
**风险**：零——只读检查，不改变构建产物。
|
||
|
||
---
|
||
|
||
## Tier 2：可以做（中 ROI，中风险）
|
||
|
||
### 2.1 为 LLMProvider 引入结构化错误类型
|
||
|
||
**证据**：`LLMProvider::chat()` 返回 `Box<dyn std::error::Error + Send + Sync>`，导致 `agent_loop.rs` 中的 `is_recoverable_llm_error()` 只能通过字符串子串匹配判断错误类型：
|
||
|
||
```rust
|
||
// agent_loop.rs L527-541 — 脆弱的字符串匹配
|
||
fn is_recoverable_llm_error(error: &str) -> bool {
|
||
let normalized = error.to_ascii_lowercase();
|
||
normalized.contains("429")
|
||
|| normalized.contains("502")
|
||
|| normalized.contains("503")
|
||
// ...
|
||
}
|
||
```
|
||
|
||
这种模式有真实风险：Provider 修改错误消息格式后匹配静默失效，且无法实现"429 读取 Retry-After 头"等精细化策略。
|
||
|
||
**方案**：定义 `ProviderCallError` 枚举替代 `Box<dyn Error>`：
|
||
|
||
```rust
|
||
#[derive(Debug, thiserror::Error)]
|
||
pub enum ProviderCallError {
|
||
#[error("HTTP {status}: {body}")]
|
||
Http { status: u16, body: String },
|
||
#[error("rate limited")]
|
||
RateLimited { retry_after: Option<Duration> },
|
||
#[error("network error: {0}")]
|
||
Network(#[from] reqwest::Error),
|
||
#[error("parse error: {0}")]
|
||
Parse(#[from] serde_json::Error),
|
||
#[error("{0}")]
|
||
Other(String),
|
||
}
|
||
```
|
||
|
||
更新 `LLMProvider` trait 签名、OpenAI/Anthropic Provider 实现、以及 `agent_loop.rs` 的重试逻辑。
|
||
|
||
**风险**：中——涉及 trait 签名变更，影响所有 Provider 实现和调用方。但项目已有 `StorageError` 作为 thiserror 范例，且 `LLMProvider` 是内部 trait（非公开 API），影响面可控。
|
||
|
||
**工作量**：约 3-5 个文件需要改动，现有测试需要调整断言。
|
||
|
||
---
|
||
|
||
### 2.2 WebSocket 媒体文件写入改用 spawn_blocking
|
||
|
||
**证据**：`gateway/ws.rs` [L81-96](file:///e:/code_project/PicoBot/src/gateway/ws.rs#L81-L96) 在 async handler 中直接调用 `std::fs::create_dir_all` 和 `std::fs::write` 保存用户上传的图片/文件。对于较大的图片（几 MB），阻塞时间可能达到几十毫秒。
|
||
|
||
**方案**：用 `tokio::task::spawn_blocking` 包裹文件写入操作。
|
||
|
||
**风险**：低——项目其他地方（`file_read.rs`、`agent_loop.rs` 图片编码）已使用相同模式。
|
||
|
||
**注意**：`http.rs` L162 的 `std::fs::write`（配置保存）**不需要改**——这是一次几 KB 文件的罕见操作，阻塞时间在微秒级，不值得增加代码复杂性。
|
||
|
||
---
|
||
|
||
## Tier 3：明确不做
|
||
|
||
以下项目经重新评估后决定**不做**：
|
||
|
||
| 项目 | 原计划编号 | 不做理由 |
|
||
|------|-----------|----------|
|
||
| 迁移 8 个手工错误类型到 thiserror | 1.1 阶段 1 | **纯美化**。手工实现的 `Display + Error` 工作正常，样板代码已写完。迁移零功能价值，却有引入 Bug 的风险。 |
|
||
| 拆分 `agent_loop.rs` | 2.2 | **过度工程化**。3000 行虽大但结构清晰，函数边界明确。拆分引入 import 变更和合并冲突，收益仅是"文件短了"。 |
|
||
| 重构 `gateway/` 目录结构 | 2.3 | **过度工程化**。30 个文件的平铺结构工作正常，无人报告导航困难。移动文件是大规模 churn，零功能价值。 |
|
||
| 统一锁策略文档 | 2.4 | **现有做法已正确**。tokio::sync 用于 async、parking_lot 用于 sync 是正确的选型模式，不需要额外文档。 |
|
||
| 减少 `.clone()` 调用 | 3.1 | **投机性优化**。无 profiling 数据表明 clone 是瓶颈。Rust 中大部分 clone 是所有权必需。盲目减少 clone 可能引入生命周期问题。应等 profiling 数据支撑后再做。 |
|
||
| 精简 tokio features | 3.2 | **低收益有风险**。`["full"]` 方便且安全，精简后可能遗漏 feature 导致跨平台构建失败，排查成本远超节省的编译时间。 |
|
||
| 评估 SQLite 连接池 | 3.3 | **投机性**。无数据表明连接池是问题。r2d2 提供超时和错误恢复能力，保留更安全。 |
|
||
| 添加 `#[instrument]` | 4.1 | **锦上添花**。项目已有 916 处 tracing 调用，日志覆盖充分。`#[instrument]` 是增量改进，非必需。可在排查具体问题时按需添加。 |
|
||
| 生产指标导出 | 4.2 | **这是新功能，不是优化**。 |
|
||
| 共享测试工具模块 | 5.1 | **低优先级**。仅 MockTodoRepository 重复 2 处，影响极小。 |
|
||
| 可离线集成测试 | 5.2 | **低优先级**。已有 570 个单元测试 + MockProvider，覆盖率足够。 |
|
||
| WebSocket 速率限制 | 6.2 | **新功能**。 |
|
||
| 工具执行超时 | 6.2 | **新功能**。 |
|
||
| Docker 支持 | 7.2 | **新功能**。 |
|
||
| 代码覆盖率报告 | 7.1 | **低优先级**。无证据表明测试覆盖不足是当前瓶颈。 |
|
||
| Clippy `-D warnings` | 7.1 | **需先清理存量**。当前存量警告太多，直接启用会阻断 CI。 |
|
||
|
||
---
|
||
|
||
## 实施顺序
|
||
|
||
```
|
||
Step 1: 函数去重（1.1） ← 30 分钟，零风险
|
||
Step 2: feishu 正则 LazyLock（1.2）← 15 分钟，低风险
|
||
Step 3: CI cargo audit（1.3） ← 10 分钟，零风险
|
||
Step 4: ProviderCallError（2.1） ← 2-3 小时，中风险
|
||
Step 5: ws.rs spawn_blocking（2.2）← 20 分钟，低风险
|
||
```
|
||
|
||
Step 1-3 可以立即执行，互不依赖。Step 4 是独立的大型重构，可单独安排。Step 5 可随时做。
|
||
|
||
---
|
||
|
||
## 量化指标
|
||
|
||
| 指标 | 当前值 | 目标值 |
|
||
|------|--------|--------|
|
||
| 重复函数定义 | 16 处 | 0 |
|
||
| 每条飞书消息正则编译次数 | 9 | 0（编译一次，全局复用） |
|
||
| CI 安全扫描 | 无 | cargo audit + npm audit |
|
||
| `is_recoverable_llm_error` 字符串匹配 | 10 个子串 | 0（改为类型匹配） |
|
||
| `Box<dyn Error>` 在 Provider trait | 2 处 | 0 |
|