Repository navigation
Conversation
|
👋 Thanks for the contribution! LibreChat merges all changes into Nothing is needed from you; your commits, reviews and discussion are unchanged. If the diff now shows files you did not touch, rebase onto git remote add upstream https://github.com/LibreChat-AI/LibreChat.git
git fetch upstream dev
git rebase upstream/dev
git push --force-with-leaseMaintainers: apply the |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused fix preserves existing accounting behavior, includes regression coverage, and has no unresolved findings.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes omitted provider-native media accounting so precomputed token budgets include attachments.
Changes:
- Reuses SDK media estimates in both counters without double-counting message framing.
- Adds media-format coverage and an oversized-PDF regression test.
| File | Description |
|---|---|
| packages/api/src/agents/client.ts | Counts native media in both token estimators. |
| packages/api/src/agents/client.spec.ts | Tests SDK consistency and oversized-request rejection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
Count provider-native
type: 'media'blocks in bothcountFormattedMessageTokensandestimateMediaTokensForMessage, using the existing agents SDK counter and subtracting message framing so it is not counted twice.Problem
Google/Vertex document encoding emits
{ type: 'media', mimeType: 'application/pdf', data: ... }. The application-level formatted-message counter handlesdocumentandfile, but skipsmedia. Its fallback readsblock[block.type], which is absent for this shape.As a result, PDF bytes contribute zero tokens to the precomputed
indexTokenCountMap. The runtime trusts those supplied counts, so a PDF-heavy request can bypass the configured context budget and reach the provider, which then rejects it for exceeding its context window. This still reproduces on current main.Changes
These remain local estimates, not provider-exact token counts. This fixes omitted media accounting; it does not guarantee that every PDF estimate matches provider usage. No configuration, persisted data, or resendFiles policy changes are included.
Verification
npx jest src/agents/client.spec.ts --runInBand --coverage=falsefrompackages/api: 35 tests passed.estimateMediaTokensForMessage.spec.tsandhitl/answers.spec.ts: 95 tests passed (130 total).packages/apiTypeScript check: passed (the latter required a 6 GB Node heap).All regression inputs are synthetic. No user conversations or attachments are included.