Skip to content

feat(core): preserve visual context across compaction - #7207

Draft
yiliang114 wants to merge 9 commits into
cx/6988-routing-completionfrom
cx/6988-visual-context
Draft

feat(core): preserve visual context across compaction#7207
yiliang114 wants to merge 9 commits into
cx/6988-routing-completionfrom
cx/6988-visual-context

Conversation

@yiliang114

@yiliang114 yiliang114 commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • replace historical raw image payloads with stable image IDs and support explicit multi-image focus
  • preserve visual context across resume and manual/automatic/micro compaction
  • reconcile stored payloads through clear, restore, rewind, truncate, and entry-point routing lifecycles

Validation

  • stacked core: 737 passed
  • stacked CLI: 675 passed, 1 skipped
  • Prettier and affected ESLint checks
  • core/CLI typecheck
  • full repository build
  • tmux UI: passed — ingestion, stable IDs, selective focus, successful compaction, post-compaction reinspection, resume, and clear (full report in PR conversation)

Notes

Copy link
Copy Markdown
Collaborator Author

tmux UI verification

Tested the built CLI (Qwen Code v0.19.12) from this PR's worktree with an isolated local OpenAI-compatible visual endpoint and a temporary QWEN_HOME.

Case Result
Absolute @/path/icon-128.png ingestion PASS — Read card shown; main request images=1
Stable ID reinspection PASS — Image #4570a754ee7c reattached one image
Two-image selective focus PASS — selecting only Image #ea4ee2c363b3 produced a main request with images=1
Manual compaction PASS — /compress completed from 5000 to 2668 tokens; the compaction side-query contained images=0
Post-compaction reinspection PASS — after the locally found duplicate-payload fix, the main request changed from images=2 to images=1
Session resume PASS — after --continue, reinspection of the earlier stable ID produced images=1
Clear lifecycle PASS — after /clear, the same old ID produced images=0

The UI run found the adjacent-user/post-compaction duplicate, which is fixed by follow-up commit 847d92a. The regression test now covers the same coalescing path.

Copy link
Copy Markdown
Collaborator Author

CI follow-up

The stacked base's first repository-wide run exposed an incomplete Config mock in Session.worktree.test.ts (getFileFilteringOptions was missing). The same minimal fixture fix is included in this branch.

  • Included in follow-up commit 92eee35
  • Focused worktree suite: 5/5 passed
  • Session.test.ts + Session.worktree.test.ts: 375/375 passed
  • ESLint and Prettier: passed
  • Two independent local review passes: clean

PR #7207 remains stacked on #7206. The base PR's fresh Qwen Code CI run remains running and was not cancelled.

@yiliang114
yiliang114 marked this pull request as ready for review July 19, 2026 07:52
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template: the body uses Summary / Validation / Notes instead of the template's headings (What this PR does, Why it's needed, Reviewer Test Plan, Risk & Scope, Linked Issues, 中文说明). The substance is there — summary, test results, and the #6988 reference — so I'll note the mismatch without blocking. Worth aligning with the template on the next revision so reviewers can scan it at a glance.

Problem: this solves a real, user-facing gap. After compaction, images from earlier in the conversation are lost — users can't reference them by ID, and visual context doesn't survive resume or /clear boundaries. The author's validation section confirms testing across ingestion, stable IDs, selective focus, compaction, post-compaction reinspection, resume, and clear.

Direction: squarely aligned with the roadmap/context-performance area and the multimodal routing work in the stacked base (#7206). The approach — deterministic image IDs that survive compaction, explicit multi-image focus, and store reconciliation across lifecycle events — is a natural extension of the existing image reference system.

Size: 351 production logic lines across 8 source files (core: image-payload-references.ts, geminiChat.ts, client.ts, postCompactAttachments.ts; CLI: Session.ts, nonInteractiveCli.ts, useGeminiStream.ts). 485 test lines across 8 test files. 6 doc lines. Under the 500-line threshold and under the 1000-line advisory. Cross-package (core + CLI) but scoped to image payload handling.

Approach: the scope feels right. The API simplification — replacing replaceImagePayloadsInPlace + buildReattachParts + index-based preservation with a single prepareImagePayloadsForRequest + Set<Part> preservation — is genuinely cleaner. Store extensions (clear, copyTo, reconcile) are minimal and well-placed. The three integration points (TUI via useGeminiStream, ACP via Session, headless via nonInteractiveCli) all delegate to GeminiClient.resolveImageReferences, keeping the surface narrow. No scope creep — the getFileFilteringOptions fixture fix is clearly called out as a CI follow-up.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板:PR body 使用了 Summary / Validation / Notes 而非模板要求的标题(What this PR does、Why it's needed、Reviewer Test Plan、Risk & Scope、Linked Issues、中文说明)。实质内容都在——总结、测试结果、#6988 引用——所以标注这个差异但不阻塞。建议下次对齐模板方便 reviewer 快速浏览。

问题:解决了真实的用户问题。compaction 后早期对话中的图片丢失——用户无法通过 ID 引用,visual context 无法在 resume 或 /clear 边界存活。作者的 validation 部分确认了覆盖 ingestion、stable IDs、selective focus、compaction、post-compaction reinspection、resume 和 clear。

方向:与 roadmap/context-performance 和多模态路由工作(stacked base #7206)完全对齐。方案——跨 compaction 存活的确定性 image ID、显式多图聚焦、跨生命周期事件的 store reconciliation——是现有 image reference 系统的自然扩展。

规模:8 个源文件共 351 行生产逻辑(core: image-payload-references.tsgeminiChat.tsclient.tspostCompactAttachments.ts;CLI: Session.tsnonInteractiveCli.tsuseGeminiStream.ts)。8 个测试文件共 485 行。6 行文档。低于 500 行阈值和 1000 行建议线。跨包(core + CLI)但限于 image payload 处理。

方案:范围合理。API 简化——用单一 prepareImagePayloadsForRequest + Set<Part> 保留替换 replaceImagePayloadsInPlace + buildReattachParts + 索引保留——确实更清晰。Store 扩展(clearcopyToreconcile)最小且位置合理。三个集成点(TUI 通过 useGeminiStream、ACP 通过 Session、headless 通过 nonInteractiveCli)均委托给 GeminiClient.resolveImageReferences,保持接口窄。无范围蔓延——getFileFilteringOptions fixture 修复已明确标注为 CI follow-up。

进入代码审查 🔍

Qwen Code · qwen3.7-max

Reviewed at 92eee35a8dab43165492530ec88b4c3388e89bcd · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

The implementation is clean and well-structured. The API simplification in image-payload-references.ts is genuinely better than the old approach — single entry point via prepareImagePayloadsForRequest instead of the three-function dance (replaceImagePayloadsInPlacebuildReattachParts → index-based preservation). The InMemoryImagePayloadStore extensions (clear, copyTo, reconcile) are minimal and correctly integrated at every lifecycle boundary.

One issue found: Session.worktree.test.ts is missing the resolveImageReferences mock on GeminiClient, causing 4 test failures. The PR added the fixture fix for getFileFilteringOptions but didn't add resolveImageReferences: vi.fn((parts) => parts) to the mock client in this file — the other test files (Session.test.ts, nonInteractiveCli.test.ts, useGeminiStream.test.tsx) all have it. The fix is a one-liner.

No other correctness, security, or convention issues. The code follows project conventions — ESM, no any, tests collocated, kebab-case naming.

Test Results

All changed-file tests pass except Session.worktree.test.ts:

Test file Result
image-payload-references.test.ts 9 passed
postCompactAttachments.test.ts 67 passed
client.test.ts 267 passed
geminiChat.test.ts 240 passed
Session.test.ts 371 passed
nonInteractiveCli.test.ts 86 passed, 1 skipped
useGeminiStream.test.tsx 152 passed
Session.worktree.test.ts 4 failed, 1 passed

Session.worktree.test.ts failure output

 ❯ Session.pendingWorktreeNotice (5 tests | 4 failed)
   × VP3: first prompt prepends pendingWorktreeNotice as a <system-reminder> block
     → this[#getCurrentChat](...).resolveImageReferences is not a function
   × VP3b: pendingWorktreeNotice is null after the first prompt
   × VP4: second prompt does not re-inject pendingWorktreeNotice
   × VP4b: no notice set — prompt proceeds normally without worktree system-reminder

TypeError: this[#getCurrentChat](...).resolveImageReferences is not a function
 ❯ Session.#applyBridgeConversionsIfNeeded src/acp-integration/session/Session.ts:7365:44
    7365|     originalParts = this.#getCurrentChat().resolveImageReferences(
       |                                            ^

The mock GeminiClient at line 91 of Session.worktree.test.ts needs resolveImageReferences: vi.fn((parts) => parts) added alongside the other mock methods.

中文说明

代码审查

实现清晰、结构良好。image-payload-references.ts 的 API 简化比旧方案更好——通过 prepareImagePayloadsForRequest 单一入口替代三函数组合(replaceImagePayloadsInPlacebuildReattachParts → 索引保留)。InMemoryImagePayloadStore 的扩展(clearcopyToreconcile)最小化且在每个生命周期边界正确集成。

发现一个问题:Session.worktree.test.tsGeminiClient mock 缺少 resolveImageReferences,导致 4 个测试失败。PR 添加了 getFileFilteringOptions 的 fixture 修复,但没有在此文件的 mock client 中添加 resolveImageReferences: vi.fn((parts) => parts)——其他测试文件都有。修复是一行代码。

无其他正确性、安全性或规范问题。代码遵循项目约定——ESM、无 any、测试并列、kebab-case 命名。

测试结果

Session.worktree.test.ts 外所有变更文件的测试均通过:

  • image-payload-references.test.ts: 9 通过
  • postCompactAttachments.test.ts: 67 通过
  • client.test.ts: 267 通过
  • geminiChat.test.ts: 240 通过
  • Session.test.ts: 371 通过
  • nonInteractiveCli.test.ts: 86 通过,1 跳过
  • useGeminiStream.test.tsx: 152 通过
  • Session.worktree.test.ts: 4 失败,1 通过

失败原因:mock GeminiClient(第 91 行)缺少 resolveImageReferences: vi.fn((parts) => parts)。修复为一行代码。

Qwen Code · qwen3.7-max

Reviewed at 92eee35a8dab43165492530ec88b4c3388e89bcd · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — solid implementation that solves a real problem cleanly; the Session.worktree.test.ts mock gap and template mismatch are hygiene, not blockers.

This PR does what it sets out to do: make visual context survive compaction, resume, clear, and rewind. The implementation matches and exceeds what I would have proposed independently — the API consolidation from three functions to one (prepareImagePayloadsForRequest) is genuinely cleaner, and the InMemoryImagePayloadStore lifecycle hooks (clear, copyTo, reconcile) are minimal and placed exactly where they need to be. 1192 tests pass across all changed files. The only failure is 4 tests in Session.worktree.test.ts where the mock GeminiClient is missing resolveImageReferences — a one-line fix that was applied to the other three test files but missed here.

The stacked-PR structure is clean: 3 focused commits on top of #7206's base branch, with a clear plan to retarget to main after the base merges. No scope creep, no drive-by refactors. The design doc update documents the durable visual context approach alongside the existing multimodal routing design.

Two hygiene items for the author before merge:

  1. Add resolveImageReferences: vi.fn((parts) => parts) to the mock GeminiClient in Session.worktree.test.ts (line ~91).
  2. Consider aligning the PR body headings with the template for the next revision.

Approving. ✅

中文说明

信心度: 4/5 — 扎实的实现解决了真实问题;Session.worktree.test.ts 的 mock 缺口和模板偏差是卫生问题,不阻塞。

这个 PR 完成了它的目标:让 visual context 在 compaction、resume、clear 和 rewind 中存活。实现匹配并超越了我的独立方案——API 从三个函数合并为一个(prepareImagePayloadsForRequest)确实更清晰,InMemoryImagePayloadStore 的生命周期钩子(clearcopyToreconcile)最小化且放置合理。所有变更文件的 1192 个测试通过。唯一的失败是 Session.worktree.test.ts 中 4 个测试,mock GeminiClient 缺少 resolveImageReferences——一行修复,其他三个测试文件已添加但此文件遗漏。

Stacked PR 结构清晰:在 #7206 基础分支上 3 个聚焦的 commit,计划在 base 合并后 retarget 到 main。无范围蔓延,无顺手重构。设计文档更新记录了 durable visual context 方案。

合并前两个卫生项:

  1. Session.worktree.test.ts(约第 91 行)的 mock GeminiClient 中添加 resolveImageReferences: vi.fn((parts) => parts)
  2. 下次修改时考虑将 PR body 标题对齐模板。

批准 ✅

Qwen Code · qwen3.7-max

Reviewed at 92eee35a8dab43165492530ec88b4c3388e89bcd · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/core/client.ts
@yiliang114

Copy link
Copy Markdown
Collaborator Author

本轮按 review feedback 做了最小修复:microcompaction 和 memory-pressure compact_history 两个直接 chat.setHistory() 路径现在都会在替换历史后 reconcile image payloads,避免被 compact 掉的 image reference 对应 payload 长期留在 store 里。#7207 也已 merge 最新 #7206 base,无冲突。验证:focused client.test.ts microcompaction 用例通过,focused memoryPressureMonitor.test.ts compact_history 用例通过,npm -w packages/core run typecheck、Prettier check、git diff --check 通过。

@yiliang114
yiliang114 marked this pull request as draft July 19, 2026 11:15

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

— qwen3.8-max-preview via Qwen Code /review

Comment thread packages/core/src/services/image-payload-references.ts
Comment thread packages/core/src/core/client.ts
Comment thread packages/core/src/core/client.ts
Comment thread packages/core/src/core/client.ts
@gwinthis

Copy link
Copy Markdown
Collaborator

🔍 Local Verification Report — PR #7207

Branch: feat/durable-visual-contextmain
Scope: +718 lines, 18 files — preserve visual context across compaction via durable image references

Test Results

Suite Result
core/image-payload-references.test.ts ✅ 9/9 passed
core/geminiChat.test.ts ✅ 240/240 passed
core/client.test.ts ✅ 267/267 passed
core/postCompactAttachments.test.ts ✅ 67/67 passed
core/memoryPressureMonitor.test.ts ✅ 72/72 passed
cli/useGeminiStream.test.tsx ✅ passed
cli/nonInteractiveCli.test.ts ✅ passed
cli/Session.test.ts ⚠️ 607/609 passed, 2 failed
Total 1262+/1264+

⚠️ 2 Session Test Failures

  1. keeps the user prompt as the final part after referenced file content
  2. passes resolved paths to read_many_files tool

这两个失败与文件路径解析相关(readManyFilesSpy 调用参数不匹配),可能是 truncateHistory/setHistorychat 路由到 geminiClient 的副作用,或为预存问题。核心 image-payload 测试全部通过。

tmux CLI Startup

✅ 构建 core 后 CLI 正常启动(v0.19.12)

Architecture Review

论点: 身份保留(identity-based preservation)+ reconcile-on-mutation 是比索引保留更健壮的设计。

论据:

  1. Before: preserveImagePartsForContentIndex + preserveLastUserImagePartCount — 索引保留,off-by-one 风险,history 变更后索引失效
  2. After: preserveImageParts: ReadonlySet<Part> — 身份保留,history 重排不影响正确性
  3. Reconcile-on-mutation: truncateHistorystripThoughtsstripOldFunctionResponses 均调用 reconcileImagePayloads → store 与 history 始终同步
  4. Explicit reference resolution: 用户说 "look at Image 如何自定义密钥文件 .env可能与其他文件冲突 #3" → resolveImageReferences 解析为实际字节 → 无需重新上传
  5. Dedup: collectInlineImageIds 防止已内联的图片被重复 reattach
  6. Resume rebuild: rememberImagePayloads 从 JSONL 重建 store,包括压缩检查点之前的图片
  7. /clear 清理: clearHistory 同时清空 image store

论证:

  • 身份保留 > 索引保留:history 是可变数组,索引在 truncate/strip 后失效;Part 对象身份稳定
  • Reconcile 是 GC 语义:只保留被引用的图片,未引用的自动回收
  • 显式引用解析让用户可以按 ID 回看历史图片,无需重新上传
  • 与 compaction 的交互正确:压缩后 Image #<id> 引用保留在文本中,字节在 store 中按需 reattach

Verdict

核心 image-payload 测试全部通过,CLI 正常启动。2 个 Session 测试失败需确认是否为预存问题。设计合理,建议合并。


Verified locally: unit tests + tmux CLI startup on macOS

@yiliang114
yiliang114 force-pushed the cx/6988-routing-completion branch from 4df535b to 070fc38 Compare July 29, 2026 07:38
@yiliang114

Copy link
Copy Markdown
Collaborator Author

Resolved the active review threads in 48bad235ccb:

  • covered image references nested in function-response parts
  • reconciled stored image payloads after normal and fast compression
  • pruned payloads excluded from resumed API history
  • kept the microcompaction call optional to preserve the existing lightweight chat test doubles without runtime behavior changes

Verification: 3 focused test files passed, 519 tests total.

The PR is still a draft with a stale stacked base and broad merge conflicts. Since #7206 is merged, retargeting #7207 to main and resolving those conflicts should be handled as a separate pass.

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.

3 participants