fix(polish): retry 不再重发「请求体写出超时」请求，杜绝重复计费 (#680)

[appergb]

User description

关联 issue

Closes #680

问题

polish.rs::send_with_transient_retry 的重试 match 臂顺序问题：

Err(e) if e.is_connect() || e.is_request() => { /* retry */ }  // 先命中
Err(e) => { if e.is_timeout() { Timeout } }                    // 到不了

reqwest 会把请求体写出阶段的超时同时标记为 is_request() + is_timeout()。这类错误先命中第一个臂被重试，与该函数文档「is_timeout() 故意不重试——超时时服务端可能已收到并对非幂等 LLM completion 计费」相悖，也与共享的 net.rs::send_with_retry（仅 is_connect()）矛盾。后果：body 写出期超时可能重发请求 → 重复 completion + 重复插入文本 + 双重计费。

改动（单一职责）

• 抽出纯函数 should_retry_transient(is_connect, is_request, is_timeout) -> bool，逻辑 (is_connect || is_request) && !is_timeout，让「任何带 timeout 标记的失败都不重试」。
• send_with_transient_retry 改为调用该函数判定。
• 新增穷举真值表单测。

未触碰流式插入、缓存 HTTP client、provider 逻辑。

测试

cargo test --lib retr
  retries_connect_and_request_failures_without_timeout ... ok
  never_retries_when_timeout_flagged ... ok
  does_not_retry_unrelated_failures ... ok

npm run build + cargo test --lib 全绿（434 其余单测未受影响）。

来源：2026-06-16 全仓多 Agent 审计（polish 专项）。

---

PR Type

Bug fix

---

Description

• Prevent retries on timeout-flagged errors to avoid duplicate billing

• Extract retry decision into pure function should_retry_transient

• Add exhaustive unit tests for retry logic

---

Diagram Walkthrough

flowchart LR
  A[send request] --> B{error?}
  B -->|Ok| C[success]
  B -->|Err| D{is_timeout?}
  D -->|yes| E[don't retry]
  D -->|no| F{is_connect or is_request?}
  F -->|yes| G[retry once]
  F -->|no| H[don't retry]

Loading

File Walkthrough

Relevant files

Bug fix

polish.rs Refine retry logic and add tests                                                  src-tauri/src/polish.rs Added pure function should_retry_transient(is_connect, is_request, is_timeout) -> bool that returns true only if (is_connect || is_request) && !is_timeout Modified send_with_transient_retry to use the function instead of the previous match arm Added three unit tests covering all combinations of flags

---

[appergb]

@claude 请审核这个修复：把重试判定收紧为 (is_connect || is_request) && !is_timeout，避免请求体写出超时被重发导致非幂等 LLM completion 双重计费。重点看 reqwest 错误分类假设是否成立、以及是否还有其他调用 send_with_transient_retry 的路径需要同步。

[claude]

Claude finished @appergb's task in 2m 12s —— View job

---

Code Review: PR #681 — 收紧重试判定，避免请求体超时重发

任务清单

• 读取 polish.rs 完整改动与上下文
• 检查 reqwest 错误分类假设
• 检查是否有其他 send_with_transient_retry 调用路径
• 对比 net.rs::send_with_retry 的逻辑一致性
• 评估测试覆盖充分性
• 输出审核结论

---

总结

LGTM，修复方向正确，逻辑严密，测试完整，可合并。 以下是分项分析：

---

1. reqwest 错误分类假设是否成立 ✅

假设成立。reqwest 的错误类型使用两个正交的维度来分类：

• ErrorKind（is_connect() / is_request() / is_body() 等）：描述错误发生在哪个阶段
• timed_out 字段（is_timeout()）：描述错误是否因超时触发

这两个维度可以同时为 true。请求体写出阶段超时时，reqwest 会同时设置 ErrorKind::Request 和 timed_out = true，即 is_request()=true && is_timeout()=true。原始代码 Err(e) if e.is_connect() || e.is_request() 先于 is_timeout() 分支命中，导致这类错误被重试——此 bug 确实存在，fix 方向正确。

---

2. 其他调用路径是否需要同步 ✅

send_with_transient_retry 仅在 polish.rs 内部定义和调用，共三处：

行号	调用场景
polish.rs:524	send_chat_request（通用 LLM 请求）
polish.rs:590	（另一 LLM 请求路径）
polish.rs:711	（流式 SSE 路径）

没有其他文件引用该函数，无需同步修改。net.rs::send_with_retry 是独立函数，语义不同（通用 HTTP，非 LLM），不受影响。

---

3. 与 net.rs::send_with_retry 的一致性 ⚠️ 轻微注意

PR 描述中提到与 net.rs（仅重试 is_connect()）"矛盾"，但实际上两者设计目标本就不同：

• net.rs：通用 HTTP，is_connect() 最多 10 次指数退避；且 net.rs 没有 !is_timeout() 过滤，即它会重试连接超时（因为连接超时意味着服务端必然未收到）
• polish.rs：LLM completion 专用，重试 is_connect() || is_request()，但新增 !is_timeout() 以排除 body write timeout

两者设计差异是合理且故意的，不需要统一。

---

4. 一处轻微行为变更（无需修改，但值得知晓）

新的 !is_timeout 过滤也会阻止连接建立超时（is_connect=true, is_timeout=true）被重试。

场景	原代码	新代码	服务端是否收到
TCP 握手超时	✅ 重试	❌ 不重试	否（安全重试）
Body 写出超时	✅ 重试（bug）	❌ 不重试	不确定（危险）

TCP 握手超时是可以安全重试的（服务端未收到），新代码选择了更保守的策略——任何 timeout 一概不重试。这是一个轻微功能退化，但考虑到：

1. LLM provider 通常配置了较长的 connect_timeout，TCP 握手超时概率极低
2. 保守策略避免了所有与 timeout 相关的歧义，策略更清晰
3. net.rs 已经在另一层为通用请求处理了 connect 重试

这个取舍合理可接受，无需修改。

---

5. 测试覆盖评估 ✅

三个测试共同覆盖了所有 2³ = 8 种布尔组合，是真正的穷举真值表：

(connect, request, timeout) → result
(T, F, F) → T   retries_connect_and_request_failures_without_timeout
(F, T, F) → T   ↑
(T, T, F) → T   ↑
(F, T, T) → F   never_retries_when_timeout_flagged（核心回归）
(T, F, T) → F   ↑
(T, T, T) → F   ↑
(F, F, T) → F   ↑
(F, F, F) → F   does_not_retry_unrelated_failures

覆盖完整，纯函数抽取使测试无需 mock HTTP，是正确的测试策略。

---

结论

此修复最小化地解决了一个真实的、有明确计费影响的 bug，改动范围精准（1 个函数定义 + 1 个调用点 + 8 条测试），无副作用。建议合并。

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 680 - Partially compliant Compliant requirements: Fix retry logic in send_with_transient_retry to not retry when error is both is_request() and is_timeout(). Ensure no duplicate billing due to request body write timeout. Update documentation to match new behavior. Add unit tests for the retry decision. Non-compliant requirements: (empty) Requires further human verification: (empty)

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ No major issues detected