Skip to content

fix: 30MB 超限防护 — 文档去重 + resume 自动重试 - #210

Merged
lishuceo merged 2 commits into
mainfrom
fix/document-dedup-30mb-limit
Apr 8, 2026
Merged

fix: 30MB 超限防护 — 文档去重 + resume 自动重试#210
lishuceo merged 2 commits into
mainfrom
fix/document-dedup-30mb-limit

Conversation

@lishuceo

@lishuceo lishuceo commented Apr 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • 话题内多条消息引用同一 PDF 时,文档重复累积超过 API 30MB 限制
  • deduplicateDocuments() 按 fileName 去重 + 20MB 总量上限,应用于两个历史合并点
  • downloadHistoryFiles 按 fileKey 去重,避免重复下载
  • 新增:executor 检测到 "message size exceeds limit" + resume 模式时,自动丢弃旧 session 开新会话重试

Test plan

  • 6 个 deduplicateDocuments 单元测试通过
  • 全量测试 71/72 files (1 pre-existing flaky)
  • 生产验证:session 清除后话题恢复正常

🤖 Generated with Claude Code

@claude

claude Bot commented Apr 8, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 1m 28s —— View job


PR Review: fix: 30MB 超限防护 — 文档去重 + resume 自动重试

  • Check for previous review comments
  • Read changed files and understand context
  • Review code for bugs, security, and architecture issues
  • Post review summary

⚠️ Issues Found

Overall this is a well-structured fix for a real production issue. The dedup logic is clean, tests are solid, and the approach is sound. Two items worth discussing:

  • 🟡 base64 size accounting (confidence: 80)doc.data.length measures base64 string length, not decoded bytes. The 20MB cap is actually ~15MB of raw data. This is conservative (safe), but the constant name/comment says "base64 字节数" which is slightly misleading. See inline comment.

  • 🟡 Recursive retry side effects (confidence: 85) — The execute() retry without resume is safe against infinite loops (good!), but the recursive call re-runs the full method including onProgress callbacks, which may send duplicate progress cards to the user. See inline comment.

What looks good

  • deduplicateDocuments() is clean, well-tested (6 cases), and correctly exported for testing
  • Dedup applied consistently in both executeClaudeTask and executeDirectTask
  • downloadHistoryFiles fileKey dedup prevents redundant downloads upstream
  • Current-message docs prioritized over history docs (correct merge order)
  • The recursive retry guard (effectiveResumeId check) naturally prevents infinite loops
  • Images are not deduplicated, but they're typically smaller — acceptable for now

Comment thread src/feishu/event-handler.ts
Comment thread src/claude/executor.ts
lishuceo and others added 2 commits April 8, 2026 11:52
会话累积多轮带 PDF 的 turn 后,resume 时 SDK 重放全部历史导致
API payload 超过 30MB 限制。检测到 "message size exceeds limit"
错误且处于 resume 模式时,自动丢弃旧 session 重试。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
根因:每条话题消息都会从引用的父消息下载 PDF,并从聊天历史中
再次下载。即使经过 dedup 去重,每个 turn 仍附带一份 PDF 副本。
SDK resume 会重放所有前序 turn,N turns × 5.9MB = 超 30MB 限制。

修复策略:
- resume 时(canResume/activeConversationId)跳过合并历史文档和图片
- resume 时若当前消息非文件上传(messageType !== 'file'),清除从
  引用父消息继承的 documents(已在前序 turn 中发送过)
- 新增 messageType 字段贯穿 ParsedMessage → QueueTask → executor,
  用于区分"用户上传新文件"与"引用父消息的旧文件"

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@lishuceo
lishuceo force-pushed the fix/document-dedup-30mb-limit branch from 8b0c86f to 88c8b08 Compare April 8, 2026 03:52
@claude

claude Bot commented Apr 8, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 1m 44s —— View job


PR Review: fix: 30MB 超限防护 — 文档去重 + resume 自动重试 (re-review after 88c8b08)

  • Check previous review comments against new commits
  • Read changed files and review new changes
  • Post review summary

✅ Approved

2 previous issues resolved — both inline comment threads have been addressed and resolved.

The new commit (88c8b08) takes the right approach by fixing the root cause: on resume, history file attachments are now skipped entirely since the SDK replays all prior turns (files are already in the conversation). This makes the deduplicateDocuments size cap and the executor retry (from 1d84184) defense-in-depth fallbacks rather than primary mitigations.

What's new in 88c8b08

  • Resume skip logic in both executeClaudeTask and executeDirectTask: when resuming (activeConversationId / canResume), history images and documents are not merged into the current request
  • messageType discrimination: current-message documents are cleared on resume only when messageType !== 'file', correctly distinguishing "user uploaded a new file" from "message quotes a parent that had a file"
  • Consistent implementation: the same logic is applied identically in both code paths, with clear comments explaining the rationale

Assessment

The three-layer defense is sound:

  1. Root cause fix (88c8b08): skip duplicate file attachments on resume
  2. Dedup fallback: deduplicateDocuments() caps non-resume merges at 20MB
  3. Last-resort retry (1d84184): executor detects 30MB error + resume → retries without resume

No new issues found with confidence ≥ 75.

@lishuceo
lishuceo merged commit c2f22de into main Apr 8, 2026
8 checks passed
@lishuceo
lishuceo deleted the fix/document-dedup-30mb-limit branch April 8, 2026 03:56
lishuceo added a commit that referenced this pull request Apr 8, 2026
* fix: resume 时 30MB 超限自动重试(不带 resume 开新会话)

会话累积多轮带 PDF 的 turn 后,resume 时 SDK 重放全部历史导致
API payload 超过 30MB 限制。检测到 "message size exceeds limit"
错误且处于 resume 模式时,自动丢弃旧 session 重试。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: resume 时跳过已在对话中的文件附件,从根源防止 PDF 膨胀

根因:每条话题消息都会从引用的父消息下载 PDF,并从聊天历史中
再次下载。即使经过 dedup 去重,每个 turn 仍附带一份 PDF 副本。
SDK resume 会重放所有前序 turn,N turns × 5.9MB = 超 30MB 限制。

修复策略:
- resume 时(canResume/activeConversationId)跳过合并历史文档和图片
- resume 时若当前消息非文件上传(messageType !== 'file'),清除从
  引用父消息继承的 documents(已在前序 turn 中发送过)
- 新增 messageType 字段贯穿 ParsedMessage → QueueTask → executor,
  用于区分"用户上传新文件"与"引用父消息的旧文件"

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant