fix(core): Reduce multimodal history payload size - #6045
Conversation
E2E Test ReportScopeValidated the fix against the original failing multimodal session, using a real interactive tmux run and the real Original failure case
Test setup
ResultThe resumed request completed successfully. There was no screenshot-compaction notice in the TUI and no API log summary for the main request: {
"model": "qwen3.7-plus",
"messages": 141,
"dataImageUrls": 3,
"imageRefs": 33,
"recentTailBlocks": 1,
"compressionNotice": false,
"hasEconnreset": false,
"promptTokens": 90481,
"completionTokens": 162,
"cachedTokens": 0
}InterpretationThis confirms the fix addresses the observed failure mode: the model request stays below the provider transport/body pressure point by replacing historical inline image bytes with stable image references and reattaching only the bounded recent image tail. The run also confirms the fix does not depend on lowering the default screenshot compaction threshold; the original 33-image session did not trigger compaction with the threshold at 50. Supporting checks
|
|
Thanks for the PR! (Re-run after 10 new commits addressing review feedback.) Template looks good ✓ — all required sections present with bilingual description. On direction: this solves a real production problem. Long multimodal sessions with accumulated screenshots were hitting transport-layer failures ( On approach: the scope feels right. The new Moving on to code review. 🔍 中文说明感谢贡献!(在作者根据审查反馈提交了 10 个新 commit 后重新运行。) 模板完整 ✓ — 所有章节齐全,包含双语说明。 方向:解决了真实的生产问题。长多模态会话中积累的截图导致在模型上下文窗口内也会触发传输层错误( 方案:范围合理。新增 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
|
Code review — read the diff against my own proposal (replace historical inline images with text markers, reattach recent + referenced images). The PR's approach matches what I would have done, with one nice design choice I didn't anticipate: the No critical blockers. Two observations from the previous review round:
The Unit tests — all 8 new/updated tests pass (6 in Build & typecheck — both pass clean: Smoke test — CLI starts and responds normally with the PR code: Note: a full before/after tmux test for this PR requires a real multimodal session with many historical screenshots (the original failure had 33), which isn't practical in this environment. The unit tests and the author's own E2E run (documented in the PR body) cover the core behavior. 中文说明代码审查 — 对比我自己的方案(将历史 inline 图片替换为文本标记,重新附带最近和被引用的图片),PR 的方案与我一致,且有一个我没预料到的好设计: 无阻断性问题。两个观察:
单元测试 — 全部 8 个新增/更新测试通过。 完整的前后对比 tmux 测试需要包含大量历史截图的真实多模态会话(原始失败场景有 33 张截图),在此环境中不可行。单元测试和作者自己的 E2E 运行已覆盖核心行为。 — Qwen Code · qwen3.7-max |
|
Stepping back to look at the whole picture: this is a clean, well-scoped fix for a real production problem. Long multimodal sessions with accumulated screenshots were hitting transport-layer failures (ECONNRESET) even within the model's context window. The fix replaces historical inline image bytes with stable text references and only reattaches a bounded tail of recent images — exactly the right approach for this problem. The implementation is straightforward and well-tested. The The 10 commits since the first triage show the author actively responding to review feedback — addressing the My independent proposal would have been essentially the same: replace inline bytes with text markers, keep a bounded tail. The PR's version is slightly better thanks to the content-addressable deduplication and the explicit reference-by-id reattachment. Approving. ✅ 中文说明退后看全局:这是一个针对真实生产问题的干净、范围合理的修复。长多模态会话中积累的截图导致在模型上下文窗口内也会触发传输层错误(ECONNRESET)。修复方案将历史 inline 图片字节替换为稳定的文本引用,只重新附带有限的最近图片——完全正确的解决思路。 实现清晰,测试充分。 首次 triage 以来的 10 个 commit 表明作者在积极响应审查反馈。之前的低置信度建议(纵深防御清洗、压缩后冗余)已记录但不阻断。 我的独立方案基本相同:用文本标记替换 inline 字节,保留有限尾部。PR 版本因为内容寻址去重和显式按 id 引用而略胜一筹。 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Review follow-up summary for c94b3b1:
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
f40fbdf to
fb90b66
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution Summary — PR #6045Branch info
Conflicting file
ConflictPR side (HEAD)Commit Main side (origin/main)A separate change added ResolutionKept both flags. They disable two different implicit sub-linters within actionlint and are complementary, not mutually exclusive: actionlint \
-color \
-pyflakes= \
-shellcheck= \
-ignore 'SC2002:' \
...This preserves both intentions:
Files modified
Verification
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No high-confidence blockers. Two minor hygiene suggestions below.
Also noting: CI is currently red on Test (ubuntu-latest, Node 22.x) — worth checking whether the failure is related to the latest push before merging.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No high-confidence review findings. The approach is sound and the test coverage covers the core behavior changes well.
Downgraded from Approve to Comment: CI still running (13 checks pending).
Three low-confidence items for the author's consideration (terminal-only, not posted as inline comments):
currentUserPartCountingeminiChat.ts:2006is captured once but reused after reactive compaction — produces benign outcomes in practice (clamps to 0, preserving all parts) but is semantically stale.InMemoryImagePayloadStorehas no eviction policy — session-scoped and saves memory overall by replacing inline payloads, but a long session with many unique screenshots could grow unboundedly.- Post-compaction double-processing:
composePostCompactHistoryrestores images inline, thenprepareImagePayloadsForRequeststrips and re-attaches them — correct result but redundant work.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
No high-confidence review findings. The approach is sound and the test coverage covers the core behavior changes well. Two low-confidence items for the author's consideration:
storedImageToPartpasses rawdisplayNameandmimeTypein the reattachment path while the reference text path sanitizes both — consider sanitizing for defense-in-depth.currentUserPartCountis stale after reactive compression (tryCompress+setHistoryat line ~2309) — distinct from the recovery-path issue already noted at line 2555.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
| return extractCuratedHistory(this.history).map(copyContentContainer); | ||
| private getRequestHistory(currentUserContent?: Content): Content[] { | ||
| const curatedHistory = extractCuratedHistory(this.history); | ||
| const preserveImagePartsForContentIndex = currentUserContent |
There was a problem hiding this comment.
[Critical] currentUserContent is looked up by object identity after extractCuratedHistory(), but curation can merge adjacent user turns into a new object. For example, post-compact history can end with a restored-attachments user turn; the next image prompt is merged with it, so findIndex(content === currentUserContent) returns -1. During output recovery a synthetic recovery user turn is appended, and the fallback tail-count preservation applies to that synthetic turn instead of the original multimodal turn. With maxRecentImagesToRetain: 0, the recovery request can replace the user's current image bytes with a text ref and continue without the image.
Please preserve the current image parts by part identity or part range through curation rather than by the Content object identity alone, and add a regression test with adjacent user turns before output recovery.
— gpt-5 via Qwen Code /review
| const referencedIds = collectReferencedImageIds(contents.at(-1)); | ||
| const collected: CollectedImage[] = []; | ||
| const transformed = contents.map((content, index) => { | ||
| if (index === options.preserveImagePartsForContentIndex) { |
There was a problem hiding this comment.
[Suggestion] The preserve branch returns the original content object, unlike the other branches that create a copied container. If that preserved entry is also the trailing user content and historical images are reattached, the later last.parts = ... append mutates the caller's input Content.
Return a shallow copy here, including a copied parts array, so prepareImagePayloadsForRequest keeps its no-surprise transformation behavior:
| if (index === options.preserveImagePartsForContentIndex) { | |
| if (index === options.preserveImagePartsForContentIndex) { | |
| return { | |
| ...content, | |
| ...(content.parts ? { parts: [...content.parts] } : {}), | |
| }; | |
| } |
— gpt-5 via Qwen Code /review
| popPartialIfPushed(); | ||
| requestContents = self.getRequestHistory(); | ||
| requestContents = | ||
| self.getRequestHistory(currentUserContent); |
There was a problem hiding this comment.
[Suggestion] The latest change threads currentUserContent through the reactive-compression retry path too, but the new regression coverage only exercises output recovery. Reactive compression replaces history via tryCompress() before rebuilding request contents, so a future regression in the fallback or identity behavior could still drop the current image bytes on the retry without a test catching it.
Please add a focused GeminiChat test that sends a current inline image, triggers context-length reactive compression, and asserts the retry request still includes that image with maxRecentImagesToRetain: 0.
— gpt-5 via Qwen Code /review
✅ Independent verification — real tmux E2E + binary A/BVerified as a maintainer against a locally-built binary from this PR's head ( Method
1) Real interactive TUI (tmux) — reproduces the PR's exact evidenceResumed a 33‑screenshot session ( (An identical non‑interactive
A second run at N=8 / default context (well below any threshold) passed all 7 identically: 8 refs, 3 inline, 62.5% reduction — including the same result under a plain non‑interactive 2) Load‑bearing A/B on the real binaryI neutralized
The transform is what produces the bounded payload — not an artifact of the harness. 3) Unit tests
4) Scope note (not a defect — the layering, for reviewers)The reference transform runs in Observed corollary: with the small default 131,072 context, ~33 images (fixed est. ≈ 57k tokens) cross the hard limit and the pre‑existing compaction path intercepts first (I saw 5) Other files
VerdictLGTM. The mechanism works end‑to‑end on the real binary, reproduces the PR's exact stated numbers in the real TUI, is proven load‑bearing by an A/B on distinct binary SHAs, and carries the intended prompt‑injection guard. The compaction‑ordering note above is informational scope, not a blocker. 🇨🇳 中文说明(完整对应)✅ 独立验证 —— 真实 tmux 端到端 + 二进制 A/B 对照我作为维护者,基于本 PR head( 方法
1)真实交互式 TUI(tmux)—— 复现 PR 的原始证据恢复一个 33 截图的会话( (同场景的非交互
另一次 N=8 / 默认上下文(远低于任何阈值)的运行 7 项全过:8 引用、3 inline、62.5% 缩减 —— 纯非交互 2)真实二进制上的承重 A/B 对照我把
有界载荷是由该变换产生的 —— 不是测试脚手架的假象。 3)单元测试
4)范围说明(非缺陷 —— 分层,供 reviewer 参考)引用变换在 观察到的推论:用较小的默认 131,072 上下文时,~33 张图片(固定估值 ≈ 57k tokens)会越过 hard limit,既有的压缩路径会先介入(我看到 5)其它文件
结论LGTM。 该机制在真实二进制上端到端有效,在真实 TUI 里复现了 PR 声明的确切数字,通过不同二进制 SHA 的 A/B 证明承重,并带有预期的防提示注入保护。上面的压缩分层说明属于信息性范围说明,不是阻断项。 方法/脚本:真实二进制 + 伪 OpenAI 端点抓取线上请求 + 变异 A/B;证据取自真正发送的 |
The always-on image payload replacement introduced by PR #6045 replaced ALL historical images with text references on every request, causing users' old screenshots to be reattached and triggering infinite fix loops when the model mistook stale buggy screenshots for current state. Replace the always-on approach with a threshold-gated design: - Below 20 images (configurable): zero transformation, images stay in-place in history - At or above 20: in-place replace historical images with text references, reattach only the most recent 3 unique images - Replacement is persistent (mutates this.history), so the count resets and won't re-trigger until 20 new images accumulate - Current user request images are protected via skipContent Also lower DEFAULT_SCREENSHOT_TRIGGER_THRESHOLD from 50 to 20 to align with the new image payload threshold.
* fix(core): reduce multimodal history payload size * fix(core): use kebab-case image payload filenames * fix(core): address image payload review blockers * fix(core): preserve current request image payloads * ci: disable implicit actionlint pyflakes integration * fix(core): reattach recent unique image payloads * fix(core): preserve referenced image payloads * fix(core): tolerate partial config mocks in MCP discovery * fix(core): preserve current images during recovery * fix(core): gate image payload replacement behind threshold The always-on image payload replacement introduced by PR QwenLM#6045 replaced ALL historical images with text references on every request, causing users' old screenshots to be reattached and triggering infinite fix loops when the model mistook stale buggy screenshots for current state. Replace the always-on approach with a threshold-gated design: - Below 20 images (configurable): zero transformation, images stay in-place in history - At or above 20: in-place replace historical images with text references, reattach only the most recent 3 unique images - Replacement is persistent (mutates this.history), so the count resets and won't re-trigger until 20 new images accumulate - Current user request images are protected via skipContent Also lower DEFAULT_SCREENSHOT_TRIGGER_THRESHOLD from 50 to 20 to align with the new image payload threshold. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
What this PR does
This PR replaces historical inline image payloads in model requests with stable text references and reattaches only the most recent images, plus any explicitly referenced image ids, when sending the next request.
Why it's needed
Long multimodal sessions can accumulate many base64 screenshots while still being far below the model context limit. Those large request bodies can fail at the transport/provider boundary with errors such as
API Error: terminated (cause: read ECONNRESET). This keeps the textual history stable while bounding how many image bytes are resent on each request.Reviewer Test Plan
How to verify
Resume a multimodal session with many historical screenshots and send a text-only follow-up. Confirm the outgoing request contains stable
[Image #...]references for the historical screenshots, only a bounded number of recentinlineDataimages, and no screenshot-triggered compaction when the default threshold has not been reached.Evidence (Before & After)
Before: the original failing session had 33 historical tool screenshots and produced
API Error: terminated (cause: read ECONNRESET)even though context usage was below the 1M window. After: a real tmux E2E run againstqwen3.7-plusresumed that session with the default screenshot threshold restored to 50; the logged request contained 33 image references, 3 inline image payloads, no compaction notice, and no ECONNRESET.Tested on
Environment (optional)
Validated locally with focused vitest coverage,
npm run build && npm run typecheck, and real tmux E2E usingdist/cli.js,--resume 2f739fb1-c80b-4a49-ab8b-7a67e67fee6a,--openai-logging, andqwen3.7-plus.Risk & Scope
Linked Issues
N/A
中文说明
What this PR does
这个 PR 会在发送模型请求时,把历史里的 inline 图片 payload 替换成稳定的文本引用,并且只重新附带最近的图片,以及用户当前消息里显式引用到的图片 id。
Why it's needed
长多模态会话可能积累大量 base64 截图,即使上下文 token 远低于模型 1M 窗口,请求体本身也可能在传输或 provider 边界触发
API Error: terminated (cause: read ECONNRESET)。这个改动保持历史文本结构稳定,同时限制每次请求重新发送的图片字节数。Reviewer Test Plan
How to verify
恢复一个包含大量历史截图的多模态 session,然后发送纯文本 follow-up。确认实际发出的请求中,历史截图变成稳定的
[Image #...]引用,只有有限数量的最近图片以inlineData重新附带,并且在默认截图阈值未达到时不会触发截图压缩。Evidence (Before & After)
Before:原始失败 session 有 33 张历史工具截图,在上下文用量低于 1M 窗口时仍出现
API Error: terminated (cause: read ECONNRESET)。After:使用真实 tmux E2E 和qwen3.7-plus恢复该 session,默认截图阈值保持 50;日志中的请求包含 33 个图片引用、3 个 inline 图片 payload,没有压缩提示,也没有 ECONNRESET。Tested on
Environment (optional)
本地用 focused vitest、
npm run build && npm run typecheck,以及基于dist/cli.js、--resume 2f739fb1-c80b-4a49-ab8b-7a67e67fee6a、--openai-logging、qwen3.7-plus的真实 tmux E2E 验证。Risk & Scope
Linked Issues
N/A