Skip to content

fix(core): preserve managed memory during microcompaction - #6733

Closed
destire-mio wants to merge 3 commits into
QwenLM:mainfrom
destire-mio:fix/managed-memory-microcompaction
Closed

fix(core): preserve managed memory during microcompaction#6733
destire-mio wants to merge 3 commits into
QwenLM:mainfrom
destire-mio:fix/managed-memory-microcompaction

Conversation

@destire-mio

Copy link
Copy Markdown
Contributor

What this PR does

Prevents managed auto-memory reads from being cleared by microcompaction. Project-level and user-level managed memory remain available after both idle-triggered and size-triggered compaction, while ordinary old tool results continue to be compacted.

Why it's needed

Managed memory can be intentionally loaded into a conversation for durable context. Previously, microcompaction treated these reads like ordinary file output and replaced their content with a placeholder, which made the memory unavailable later in the session.

Reviewer Test Plan

How to verify

  1. Create a history that contains an older read_file response for a managed-memory path, followed by ordinary file reads.
  2. Trigger idle or size-based microcompaction.
  3. Confirm the managed-memory response remains intact and ordinary older output is cleared.

Evidence (Before & After)

Before: managed-memory read_file responses were cleared with ordinary tool output.

After: the focused regression coverage confirms project-level and user-level managed memory is retained for idle compaction, and project-level memory is retained for size compaction.

Tested on

OS Status
🍏 macOS ✅ unit test + core typecheck
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

macOS, Node.js v24.14.0.

Risk & Scope

  • Main risk or tradeoff: managed memory is deliberately excluded from microcompaction, so it can retain a small amount of additional context.
  • Not validated / out of scope: full integration and cross-platform suites.
  • Breaking changes / migration notes: none.

Linked Issues

Fixes #6713

AI-assisted contribution

This contribution was implemented with Codex and independently reviewed before submission.

中文说明

本 PR 做了什么

防止受管理的自动记忆读取结果被微压缩清除。项目级和用户级的受管理记忆在空闲触发和大小触发的压缩后仍可使用,而普通的旧工具结果仍会被压缩。

为什么需要它

受管理记忆会被有意加载到对话中,作为可持续使用的上下文。此前,微压缩会把这些读取结果当作普通文件输出,并用占位符替换其内容,导致记忆在同一会话后续阶段不可用。

审查者测试计划

如何验证

  1. 创建一段历史,其中包含一个较早的、读取受管理记忆路径的 read_file 响应,随后加入普通文件读取。
  2. 触发基于空闲时间或结果大小的微压缩。
  3. 确认受管理记忆响应仍保持完整,而普通较旧输出被清除。

证据(修改前与修改后)

修改前:受管理记忆的 read_file 响应会和普通工具输出一起被清除。

修改后:聚焦的回归测试确认项目级和用户级受管理记忆在空闲压缩中会被保留,项目级受管理记忆在大小压缩中也会被保留。

测试平台

OS 状态
🍏 macOS ✅ 单元测试和 core 类型检查
🪟 Windows ⚠️ 未测试
🐧 Linux ⚠️ 未测试

环境(可选)

macOS,Node.js v24.14.0。

风险与范围

  • 主要风险或取舍:受管理记忆会被有意排除在微压缩之外,因此可能保留少量额外上下文。
  • 未验证或不在范围内:完整集成测试与跨平台测试套件。
  • 破坏性变更或迁移说明:无。

关联 Issue

修复 #6713

AI 辅助贡献

本贡献由 Codex 协助实现,并在提交前进行了独立审查。

@destire-mio
destire-mio marked this pull request as ready for review July 11, 2026 13:44
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: This is a well-documented, observed bug. Linked issue #6713 provides clear reproduction steps (create managed memory → read it → trigger microcompaction → marker disappears), and our own triage confirmed the root cause in collectCompactablePartRefs() — it checks only tool names against COMPACTABLE_TOOLS without inspecting file paths. The parent report #6487 identified this as "Bug 2" from the start.

Direction: Aligned. Managed memory is a durable context mechanism — if its content gets silently cleared by microcompaction, the feature is broken. This fix directly addresses the identified gap. CHANGELOG doesn't have a direct entry for this, but the memory system is a core product feature and preserving it across compaction is clearly necessary.

Size: 53 production lines + 130 test lines across 5 files (client.ts: 2 prod, geminiChat.ts: 2 prod, memoryPressureMonitor.ts: 19 prod, microcompact.ts: 35 prod, microcompact.test.ts: 130 test). Small, focused change. Core paths touched — well within the safe zone.

Approach: The scope feels exactly right. The fix is what our triage suggested as option 1: exempt memory-path reads from microcompaction by checking file paths with the existing isAnyAutoMemPath() predicate. No new abstractions, no drive-by refactors. The projectRoot threading through MicrocompactOptions is the minimal plumbing needed. The paths?.length === 1 guard in isManagedMemoryRead() is a reasonable defensive check for single-file read_file results.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:这是一个有详细记录的已观测 bug。关联 issue #6713 提供了清晰的复现步骤(创建受管理记忆 → 读取 → 触发微压缩 → 标记消失),我们自己的 triage 也确认了 collectCompactablePartRefs() 中的根因——仅检查工具名称是否在 COMPACTABLE_TOOLS 中,不检查文件路径。父报告 #6487 一开始就将此识别为"Bug 2"。

方向:对齐。受管理记忆是可持续上下文机制——如果其内容被微压缩悄悄清除,该功能就失效了。此修复直接解决了已识别的缺口。CHANGELOG 中没有直接对应条目,但记忆系统是核心产品功能,在压缩过程中保留它是明确必要的。

规模:53 行生产代码 + 130 行测试代码,跨 5 个文件(client.ts: 2 行生产, geminiChat.ts: 2 行生产, memoryPressureMonitor.ts: 19 行生产, microcompact.ts: 35 行生产, microcompact.test.ts: 130 行测试)。小而聚焦的改动。触及核心路径——在安全范围内。

方案:范围恰到好处。修复方案与我们 triage 建议的选项 1 一致:通过已有的 isAnyAutoMemPath() 谓词检查文件路径,将记忆路径读取从微压缩中豁免。没有新的抽象,没有顺手重构。通过 MicrocompactOptions 传递 projectRoot 是所需的最小管道。isManagedMemoryRead() 中的 paths?.length === 1 防护对单文件 read_file 结果是合理的防御检查。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

The core logic is clean and correct. The fix threads projectRoot through MicrocompactOptions into collectCompactablePartRefs() and planSizeBasedClearing(), uses the existing isAnyAutoMemPath() predicate and the already-available buildCallIdToFilePath() / getFilePathsForResponse() helpers to exempt managed memory reads from compaction. The !projectRoot short-circuit preserves backward compatibility when the parameter isn't provided. No over-abstraction, no unnecessary code.

However, there's a regression: 3 existing tests in memoryPressureMonitor.test.ts fail. The PR adds this.coreConfig.getTargetDir() to the memoryPressureMonitor.ts call site, but the test's createMockConfig() doesn't mock getTargetDir. This throws at runtime in those 3 tests ("compacts history and clears fileReadCache when meta is non-null", "overrides positive toolResultsThresholdMinutes to 0", "defaults undefined toolResultsThresholdMinutes to 0").

Fix is straightforward — add getTargetDir: () => '/project' (or a sensible default) to createMockConfig(). But it needs to happen before merge.

Test Results

microcompact.test.ts — 59/59 passed ✅

All 3 new managed-memory tests pass:

 ✓ keeps managed memory reads during idle compaction
 ✓ keeps managed memory reads during size compaction
 ✓ keeps user-level managed memory reads during idle compaction

 Test Files  1 passed (1)
      Tests  59 passed (59)

memoryPressureMonitor.test.ts — 3 failed / 69 passed ❌

 × compacts history and clears fileReadCache when meta is non-null
 × overrides positive toolResultsThresholdMinutes to 0
 × defaults undefined toolResultsThresholdMinutes to 0

 Test Files  1 failed (1)
      Tests  3 failed | 69 passed (72)

Root cause: createMockConfig() in memoryPressureMonitor.test.ts (line 135) doesn't include getTargetDir, but the PR's memoryPressureMonitor.ts change calls this.coreConfig.getTargetDir().

TypeCheck — passed ✅

npx tsc --noEmit → clean (exit 0)

Real-scenario testing

Microcompaction of managed memory is inherently a multi-turn behavior — it cannot be meaningfully triggered in -p (single-turn) mode because all tool calls within a single model turn are collected together and the model responds immediately, leaving no aging gap for compaction. The unit tests above are the authoritative verification for this internal behavior, exercising the exact code paths (idle compaction, size compaction, project-level and user-level memory paths) with controlled history data.

中文说明

代码审查

核心逻辑干净且正确。修复通过 MicrocompactOptionsprojectRoot 传递到 collectCompactablePartRefs()planSizeBasedClearing(),使用已有的 isAnyAutoMemPath() 谓词和已有的 buildCallIdToFilePath() / getFilePathsForResponse() 辅助函数来豁免受管理的记忆读取。!projectRoot 的短路保护在参数未提供时保持了向后兼容性。没有过度抽象,没有不必要的代码。

但是存在一个回归:memoryPressureMonitor.test.ts 中的 3 个现有测试失败。 PR 在 memoryPressureMonitor.ts 调用处添加了 this.coreConfig.getTargetDir(),但测试的 createMockConfig() 没有 mock getTargetDir。这导致那 3 个测试运行时抛出异常。

修复很简单——在 createMockConfig() 中添加 getTargetDir: () => '/project'(或合理的默认值)。但这需要在合并前完成。

测试结果

microcompact.test.ts — 59/59 通过 ✅

3 个新的受管理记忆测试全部通过。

memoryPressureMonitor.test.ts — 3 个失败 / 69 个通过 ❌

根因:createMockConfig() 缺少 getTargetDir mock。

类型检查 — 通过 ✅

真实场景测试

受管理记忆的微压缩本质上是多轮行为——-p(单轮)模式无法有意义地触发,因为单模型轮次内的所有工具调用一起收集,模型立即响应,没有老化间隔。上述单元测试是此内部行为的权威验证,使用受控历史数据覆盖了确切的代码路径(空闲压缩、大小压缩、项目级和用户级记忆路径)。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

This is a well-targeted fix for a confirmed bug. The approach is exactly what I'd have done — thread projectRoot through the options, reuse the existing isAnyAutoMemPath() predicate with the already-available buildCallIdToFilePath() helper. The new tests cover all three compaction paths (idle + size for project-level, idle for user-level) and pass cleanly. The diff is minimal — 53 production lines, no scope creep, no drive-by refactors.

The logic is sound and I'm confident in the correctness of the core fix. The one issue blocking merge is the 3 failing tests in memoryPressureMonitor.test.ts — the createMockConfig() helper doesn't include getTargetDir, which the PR's memoryPressureMonitor.ts change now calls. Straightforward fix: add getTargetDir: () => '/some/path' to the mock. Once that's patched, this looks ready.

中文说明

这是一个针对已确认 bug 的精准修复。方案和我会做的完全一致——通过选项传递 projectRoot,复用已有的 isAnyAutoMemPath() 谓词和已有的 buildCallIdToFilePath() 辅助函数。新测试覆盖了所有三种压缩路径(项目级的空闲+大小压缩,用户级的空闲压缩),全部通过。diff 非常精简——53 行生产代码,无范围膨胀,无顺手重构。

逻辑正确,我对核心修复的正确性有信心。阻止合并的唯一问题是 memoryPressureMonitor.test.ts 中 3 个失败的测试——createMockConfig() 辅助函数没有包含 getTargetDir,而 PR 在 memoryPressureMonitor.ts 中现在调用了它。修复很简单:在 mock 中添加 getTargetDir: () => '/some/path'。修复后即可合并。

Qwen Code · qwen3.7-max

@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.

Core logic is solid and well-tested, but 3 existing tests in memoryPressureMonitor.test.ts fail because createMockConfig() doesn't include getTargetDir. Once that mock is patched, this looks ready to ship. 🙏

@destire-mio

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #6714, which has now merged a more comprehensive fix for the same managed-memory microcompaction issue (including project, user, and team memory handling). Thank you for the review and feedback.

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.

Managed memory content is cleared by microcompaction

2 participants