Skip to content

fix: 文档去重防止 API 30MB 限制报错 - #209

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

fix: 文档去重防止 API 30MB 限制报错#209
lishuceo merged 3 commits into
mainfrom
fix/document-dedup-30mb-limit

Conversation

@lishuceo

@lishuceo lishuceo commented Apr 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • 话题内多条消息引用同一 PDF 时,每条消息独立下载并合并,导致相同文档重复累积超过 Anthropic API 30MB message size 限制(实际案例:同一 4.4MB PDF 被下载 8+ 次 → 36MB)
  • 新增 deduplicateDocuments() 按 fileName 去重 + 20MB 总量上限,应用于 executeClaudeTaskexecuteDirectTask 两个历史合并点
  • downloadHistoryFiles 增加按 fileKey 去重,避免重复下载同一文件

Test plan

  • 6 个单元测试覆盖:去重、顺序保持、大小上限、空输入、bug 场景复现
  • 全量测试通过(69/70 files, 1253/1254 tests — 1 个 pre-existing flaky memory test)

🤖 Generated with Claude Code

@claude

claude Bot commented Apr 8, 2026

Copy link
Copy Markdown

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


PR Review: fix: 文档去重防止 API 30MB 限制报错

  • Check previous review comments — none found
  • Read changed files and full context
  • Analyze code for bugs, security, and architecture issues
  • Post review summary

✅ Approved — solid fix with minor suggestions

Overall assessment: This PR correctly addresses a real production issue where duplicate PDFs in thread history could exceed the 30MB API limit. The fix is well-scoped — dedup by fileName at merge time, dedup by fileKey at download time, and a 20MB size cap as a safety net. The logic is sound and applied consistently to both executeClaudeTask and executeDirectTask.

Issues found (2 minor):

  • 🟡 Dead fallback code (confidence: 90) — doc.fileName ?? doc.data.slice(0, 64) in deduplicateDocuments: fileName is required in the DocumentAttachment type, so the ?? branch is unreachable. See inline comment

  • 🟡 Test copies production code (confidence: 80) — The test file re-implements deduplicateDocuments() locally instead of importing the real function. If production logic changes, tests could silently diverge. Consider extracting the function to a shared utility. See inline comment

What looks good:

  • Current-message docs correctly prioritized over history docs (first-wins dedup)
  • downloadHistoryFiles fileKey dedup prevents redundant downloads before they happen
  • 20MB cap is conservative and leaves headroom for system prompt + images
  • Test coverage is thorough with 6 cases including the exact bug scenario reproduction

Comment thread src/__tests__/document-dedup.test.ts Outdated
Comment thread src/feishu/event-handler.ts Outdated
lishuceo and others added 2 commits April 8, 2026 11:13
话题内多条消息引用同一个 PDF 文件时,每条都独立下载并合并,
导致相同文档重复累积超过 Anthropic API 30MB message size 限制。

- 新增 deduplicateDocuments():按 fileName 去重 + 20MB 总量上限
- 两个历史合并点(executeClaudeTask/executeDirectTask)使用去重
- downloadHistoryFiles 按 fileKey 去重,避免重复下载

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
覆盖去重、顺序保持、大小上限、空输入等场景,
包含触发 30MB 报错的实际 bug 场景复现。

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

claude Bot commented Apr 8, 2026

Copy link
Copy Markdown

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


PR Review: fix: 文档去重防止 API 30MB 限制报错 (re-review)

  • Check previous review comments and resolve fixed issues
  • Read updated source files
  • Analyze changes for bugs, security, and architecture issues
  • Post review summary

✅ Approved — no new issues, 2 previous suggestions still open

The diff is unchanged from the previous review. The core fix is solid and correctly addresses the production 30MB limit issue.

Previous issues (2, still open):

  • 🟡 Dead fallback code (confidence: 90) — doc.fileName ?? doc.data.slice(0, 64) at event-handler.ts:1256: fileName is required in DocumentAttachment, making the ?? branch unreachable. Fix this →

  • 🟡 Test copies production code (confidence: 80) — document-dedup.test.ts re-implements deduplicateDocuments() locally instead of importing the real function. If production logic changes, tests could silently diverge. Consider exporting it from a shared utility. Fix this →

Both are minor and non-blocking — the fix works correctly as-is.

- deduplicateDocuments: 移除 fileName ?? data.slice() fallback(fileName 是必填字段)
- 导出 deduplicateDocuments 供测试直接 import,避免逻辑副本漂移
- 测试文件改为从 event-handler.ts 导入生产函数

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

claude Bot commented Apr 8, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 51s —— View job


PR Review: fix: 文档去重防止 API 30MB 限制报错 (re-review after sync)

  • Check previous review comments
  • Read updated files and verify fixes
  • Analyze for new issues
  • Post review summary

✅ Approved — all previous issues fixed, no new issues

2 previous issues fixed:

  • Dead fallback ?? doc.data.slice(0, 64) → now const key = doc.fileName directly
  • Test re-implements production code → now imports deduplicateDocuments from event-handler.js

No new issues found. The implementation is clean:

  • deduplicateDocuments() correctly exported and tested with 6 cases
  • fileKey dedup in downloadHistoryFiles prevents redundant downloads
  • Current-message docs correctly prioritized over history docs at both call sites
  • 20MB cap provides safe headroom below the 30MB API limit

@lishuceo
lishuceo merged commit b70fd0a into main Apr 8, 2026
12 checks passed
@lishuceo
lishuceo deleted the fix/document-dedup-30mb-limit branch April 8, 2026 03:21
lishuceo added a commit that referenced this pull request Apr 8, 2026
* fix: 文档去重 + 大小上限,防止 API 30MB 限制报错

话题内多条消息引用同一个 PDF 文件时,每条都独立下载并合并,
导致相同文档重复累积超过 Anthropic API 30MB message size 限制。

- 新增 deduplicateDocuments():按 fileName 去重 + 20MB 总量上限
- 两个历史合并点(executeClaudeTask/executeDirectTask)使用去重
- downloadHistoryFiles 按 fileKey 去重,避免重复下载

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

* test: 添加 deduplicateDocuments 单元测试

覆盖去重、顺序保持、大小上限、空输入等场景,
包含触发 30MB 报错的实际 bug 场景复现。

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

* fix: address PR review — 移除死代码 fallback,测试改用生产代码 import

- deduplicateDocuments: 移除 fileName ?? data.slice() fallback(fileName 是必填字段)
- 导出 deduplicateDocuments 供测试直接 import,避免逻辑副本漂移
- 测试文件改为从 event-handler.ts 导入生产函数

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