Skip to content

fix(memory): recall relevant topics beyond scan cap - #8803

Merged
yiliang114 merged 6 commits into
codex/7040-memory-recallfrom
codex/7040-memory-recall-candidates
Aug 11, 2026
Merged

fix(memory): recall relevant topics beyond scan cap#8803
yiliang114 merged 6 commits into
codex/7040-memory-recallfrom
codex/7040-memory-recall-candidates

Conversation

@yiliang114

Copy link
Copy Markdown
Collaborator

What this PR does

This PR lets native memory recall consider relevant project and user topics beyond the shared 200-document scan window, while keeping model selection bounded. Recall ranks the complete parsed pool, sends at most 200 candidates with a lexical/recent balance, and limits the selector manifest to 25,000 UTF-8 bytes. Forget, Indexer, Status, Extraction, and team-memory scanning keep their existing limits.

Why it's needed

Before this change, the scanner parsed every topic and then returned only the 200 most recent documents. Recall consumed that capped result, so an older but directly relevant memory could never reach either the heuristic fallback or the model selector. This preserves the existing safety bounds at the model boundary without discarding relevant topics before ranking.

Reviewer Test Plan

How to verify

  1. Create more than 200 project memory topics and make the relevant topic older than every filler topic. Confirm the regular scanner still returns 200 documents and omits the old target.
  2. Ask a query containing the target's unique terms. Confirm heuristic recall can surface the old target from the complete pool.
  3. Run model-backed selection over more than 200 topics. Confirm no more than 200 candidates reach the selector, including the old lexical target and recent fallback candidates.
  4. Use multibyte descriptions large enough to exceed the prompt budget. Confirm the selector manifest remains at or below 25,000 UTF-8 bytes and returned paths are accepted only when they were included in that manifest.

Local verification completed:

  • npx vitest run src/memory/recall.test.ts src/memory/relevanceSelector.test.ts src/memory/memoryLifecycle.integration.test.ts src/memory/scan.test.ts — 61/61 passed
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm run bundle
  • deterministic headless CLI E2E using a local bundle, an isolated memory home, 201 real topic files, and an OpenAI-compatible mock endpoint

Evidence (Before & After)

Before: the regular 200-document scan excludes the deliberately old target topic.

After, the bundled CLI receives the recalled marker at the next safe delivery point after the bounded initial wait:

tool=run_shell_command
assistant=MEMORY_PRESENT
result=MEMORY_PRESENT
selector manifest: 120 candidates, 24987 bytes

The E2E is headless and has no visual UI surface, so there is no meaningful screenshot to attach.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Local bundled CLI with an isolated memory home and an OpenAI-compatible mock endpoint.

Risk & Scope

  • Main risk or tradeoff: recall performs lexical ranking over the complete already-parsed topic pool; a persistent catalog remains out of scope until topic counts show that this local pass is too slow.
  • Not validated / out of scope: Windows and Linux E2E, persistent indexing, provider integration, and changes to non-recall scanner consumers.
  • Breaking changes / migration notes: none.

Linked Issues

Refs #7040

Depends on #8716. This PR is stacked on codex/7040-memory-recall and should be retargeted to main after #8716 merges.

中文说明

这个 PR 做了什么

这个 PR 让原生记忆召回可以考虑共享 200 文档扫描窗口之外的相关项目级和用户级主题,同时保持模型选择有界。召回会对完整的已解析候选池排序,以词法相关性和近期文档相结合的方式最多向模型选择器发送 200 个候选,并把选择器 manifest 限制在 25,000 UTF-8 字节以内。Forget、Indexer、Status、Extraction 和团队记忆扫描继续保留原有上限。

为什么需要它

改动前,扫描器会解析全部主题,但最终只返回最近的 200 个文档。召回直接使用这个有上限的结果,因此较旧但明确相关的记忆无法进入启发式回退或模型选择器。这个改动在模型边界继续保留已有安全上限,同时避免在排序前丢弃相关主题。

Reviewer 测试计划

如何验证

  1. 创建超过 200 个项目记忆主题,并让相关主题比所有填充主题都旧。确认常规扫描器仍只返回 200 个文档,并排除这个旧目标。
  2. 使用包含目标唯一关键词的查询。确认启发式召回可以从完整候选池中找到旧目标。
  3. 对超过 200 个主题运行模型选择。确认到达选择器的候选不超过 200 个,并同时包含旧的词法相关目标和近期回退候选。
  4. 使用足以超过提示预算的多字节描述。确认选择器 manifest 不超过 25,000 UTF-8 字节,并且只有实际包含在 manifest 中的路径才会被接受。

已完成本地验证:

  • npx vitest run src/memory/recall.test.ts src/memory/relevanceSelector.test.ts src/memory/memoryLifecycle.integration.test.ts src/memory/scan.test.ts — 61/61 通过
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm run bundle
  • 使用本地 bundle、隔离的记忆目录、201 个真实主题文件和 OpenAI 兼容 mock 端点完成确定性的 headless CLI E2E

Before & After 证据

改动前:常规 200 文档扫描会排除刻意设为较旧的目标主题。

改动后:在有界初始等待结束后的下一个安全注入点,bundle CLI 收到了召回标记:

tool=run_shell_command
assistant=MEMORY_PRESENT
result=MEMORY_PRESENT
selector manifest: 120 candidates, 24987 bytes

这个 E2E 是 headless 流程,没有可视 UI 界面,因此没有有意义的截图可附加。

测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

本地 bundle CLI、隔离的记忆目录和 OpenAI 兼容 mock 端点。

风险与范围

  • 主要风险或取舍:召回会在完整的已解析主题池上执行词法排序;在主题数量证明本地扫描过慢之前,持久化目录仍不在本次范围内。
  • 未验证 / 范围外:Windows 和 Linux E2E、持久化索引、Provider 集成,以及对非召回扫描消费者的改动。
  • 破坏性改动 / 迁移说明:无。

关联 Issue

Refs #7040

依赖 #8716。这个 PR 堆叠在 codex/7040-memory-recall 上,应在 #8716 合并后把 base 改回 main

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@yiliang114

yiliang114 commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

Scope and verification report

This PR addresses one specific problem from #7040: topic documents were capped to the 200 most recent files before relevance ranking, so an older relevant memory could never reach either the heuristic path or the model selector even though the scanner had already read and parsed it.

The new recall flow is:

complete parsed project + user topic pool
  → local relevance ranking
  → up to 180 lexical candidates + recent candidates, capped at 200 total
  → selector manifest capped at 25,000 UTF-8 bytes
  → existing final recall limit of at most 5 documents

This does not close the whole RFC. It does not change delivery telemetry, the bounded initial-wait lifecycle, multilingual evaluation, provider integration, persistent indexing, or the final five-document recall limit.

Impact on other memory modules

Area Impact
Project/user recall Intentionally considers the complete already-parsed topic pool before local ranking.
Model selector Still receives at most 200 candidates; its manifest is additionally capped at 25,000 UTF-8 bytes. Only paths actually included in the manifest can be returned.
Heuristic fallback Scores the complete recall pool but still returns at most 5 documents.
Forget No behavior change; it continues to use the regular capped project/user scanners.
Indexer No behavior change; project, user, and team scans remain capped.
Status No behavior change; it continues to use the regular capped project scanner.
Extraction No behavior change; it continues to use the regular capped project/user scanners.
Team memory No behavior change; scanning remains deterministic and capped at 200.
Storage, writes, schema, config, providers, UI No changes.

The two uncapped scanner entry points have one production consumer: recall. The existing capped scanner APIs remain unchanged for every other consumer. The scanner already read and parsed all topic files before applying the old cap, so this change does not introduce additional file I/O; it adds relevance ranking over the complete parsed pool. A persistent catalog remains out of scope unless measured topic counts show that ranking becomes too slow.

E2E verification

Verified the current head (62ab4a9) with the bundled CLI, an isolated memory home/runtime, 201 real topic files, and a deterministic OpenAI-compatible mock endpoint.

The fixture contains 200 recent unrelated topics plus one deliberately old relevant topic containing OVERFLOW-ZEPHYR-7040. The regular capped scanner returned 200 documents and omitted the old target. Recall still placed that target in the bounded selector manifest and delivered it at the ToolResult safe point after the initial wait expired.

tool=run_shell_command
selector manifest: 120 candidates, 24987 bytes
assistant=MEMORY_PRESENT
result=MEMORY_PRESENT

The mock fails the run if the old target is absent, the candidate count exceeds 200, the manifest exceeds 25,000 UTF-8 bytes, or the recalled body does not reach the main-model request.

PR #8803 bounded memory recall E2E verification

Regression checks

  • 4 targeted test files: 61/61 passed
  • full repository typecheck: passed
  • ESLint on all changed TypeScript files: 0 errors, 0 warnings
  • repository lint, build, and bundle: passed
  • local correctness and security reviews: no findings
  • Ponytail simplification review: Lean already. Ship.

The image above is a rendering of the recorded headless CLI results. The test used the real local CLI execution path with a deterministic mock provider; it verifies scan, ranking, selector bounds, and delivery, but does not claim real-model semantic quality.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

  • Problem: real and observable, not theoretical. The 200-document scanner cap is deterministic: the scanner parses every topic, then slices to the 200 most recent, so an older-but-relevant topic can never reach the heuristic fallback or the model selector. The new integration test reproduces exactly this. Refs RFC: Reliable auto-memory recall — timing, quality, and telemetry #7040, the recall RFC tracking this work.
  • Direction: aligned. RFC: Reliable auto-memory recall — timing, quality, and telemetry #7040 is on the roadmap (context-performance), and the change widens what recall can rank while keeping the existing model-boundary bounds (200 candidates, 25 KB manifest). One workflow note: this is stacked on codex/7040-memory-recall (fix(memory): improve recall reliability and candidate coverage #8716, still open), and ci.yml only triggers for bases main / release/** — so the standard CI suite does not run on this PR until it is retargeted after fix(memory): improve recall reliability and candidate coverage #8716 merges.
  • Size: core paths touched (packages/core/src/memory/). Production logic ≈ 99 lines (recall.ts 30, relevanceSelector.ts 47, scan.ts 22); tests ≈ 144 lines; design doc 56 lines. Well below every escalation threshold, and the author is a maintainer.
  • Approach: scope feels right. Dedicated uncapped scan variants for recall only, with Forget / Indexer / Status / Extraction staying on the capped APIs — I verified those consumers are untouched. Notably, the scanner already parsed every topic before slicing, so this adds ranking over the pool but no new parse cost. Deferring a persistent catalog until measurements demand it (per the design doc) is the right call.
  • Risk: no elevated risk signals — no high-risk paths matched.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

  • 问题: 真实且可观测,不是理论性问题。扫描器的 200 文档上限是确定性的:先解析全部主题,再截取最近 200 条,因此较旧但相关的主题永远无法进入启发式回退或模型选择器。新增的集成测试精确复现了这一点。关联 RFC: Reliable auto-memory recall — timing, quality, and telemetry #7040(记忆召回 RFC)。
  • 方向: 对齐。RFC: Reliable auto-memory recall — timing, quality, and telemetry #7040 在路线图上(context-performance),此改动扩大了召回可排序的范围,同时保留模型侧的既有边界(200 候选、25 KB manifest)。一个流程提示:本 PR 堆叠在 codex/7040-memory-recallfix(memory): improve recall reliability and candidate coverage #8716,尚未合并)之上,而 ci.yml 只对 main / release/** 基分支触发——在 fix(memory): improve recall reliability and candidate coverage #8716 合并并重新指向 main 之前,标准 CI 不会在本 PR 上运行。
  • 规模: 触及核心路径(packages/core/src/memory/)。生产逻辑约 99 行(recall.ts 30、relevanceSelector.ts 47、scan.ts 22);测试约 144 行;设计文档 56 行。远低于所有升级阈值,且作者是维护者。
  • 方案: 范围合理。为召回单独新增无上限扫描变体,Forget / Indexer / Status / Extraction 继续使用有上限的 API——已核实这些消费方未被改动。值得注意的是,扫描器在截取前本来就解析了全部主题,因此本改动只增加对候选池的排序开销,没有新的解析成本。按设计文档所述、等测量数据证明需要后再引入持久化目录,是合理的取舍。
  • 风险: 无升级风险信号——未命中任何高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at 62ab4a918d60a75bbb7254b49dc2c49fc833e364 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review

My independent take on "recall should see past the 200-doc cap without unbounding model cost" was exactly what this PR does: uncapped scan variant scoped to recall, a bounded candidate shortlist, and a byte-bounded manifest. The implementation matches, and I found no correctness issues:

  • Candidate bounding (selectModelCandidateDocuments) — up to 180 lexical matches by the existing scorer, filled to 200 by recency. When fewer than 180 lexical matches exist, the recency share grows, matching the design doc's "at least 20 recent reserve". Lexical-first ordering also means the byte budget below prioritizes relevant entries.
  • Manifest byte cap (formatMemoryManifest) — UTF-8 accounting is correct (newline separators counted, first line exempt), and overflow continues rather than breaks, so a later smaller entry can still fit. validFilePaths / byFilePath are now keyed on the bounded includedDocs, so the selector cannot return a path that fell out of the manifest, and an empty manifest short-circuits before the side query.
  • Consumers — I grepped the other scan consumers (forget.ts, indexer.ts, status.ts, extractionAgentPlanner.ts): all still use the capped APIs, as claimed. The heuristic fallback now scores the complete pool but still returns at most 5 docs — consistent with the design doc.
  • Tests pin the change — the lifecycle integration test fails without this change (capped scan provably omits the old target), and the new unit tests bound candidates at 200 and the manifest at ≤ 25,000 UTF-8 bytes using multibyte () content.
  • Non-blocking notes: the docs_scanned telemetry field now reports the uncapped pool size — a small metric-distribution shift worth knowing about, no change needed. The two new scan wrappers are covered indirectly (recall unit tests + lifecycle integration) rather than with direct scan.test.ts cases — fine at this size.

No critical findings.

Testing evidence

This run is unattended CI, so per policy I did not build or run any PR code. The evidence below is the PR's own CI, read via the API — and the headline is that the standard test suite does not run on this PR: ci.yml triggers only for bases main / release/**, and this PR targets codex/7040-memory-recall. The only check-runs on the reviewed commit are bot orchestration:

Check-runs on 62ab4a91 (bot orchestration only — no unit/build/lint suite):

Check Conclusion
authorize success
label success
review-config success
precheck-pr skipped
review-pr skipped
ack-review-request skipped
delay-automatic-review skipped
resolve-pr skipped
verify skipped
tmux-testing skipped
publish-verify skipped
publish-tmux skipped
triage (this run) in progress

Additionally, a push-event run of qwen-code-pr-review.yml on the head branch completed as failure but started zero jobs — a workflow-level evaluation failure, not a test failure; it carries no test signal either way.

The PR description lists local verification (vitest 61/61, lint, typecheck, build, bundle, and a headless E2E with 201 real topic files and a mock endpoint) — that is the author's claim, not independently re-run evidence. Nothing above has executed this commit's tests in shared CI.

Sandboxed verification would settle this: @qwen-code /verify — with the standard suite absent on this stacked base, the claims that an old lexical target survives recall beyond the 200-doc cap and that the selector manifest stays ≤ 25,000 UTF-8 bytes currently rest on the author's local runs alone.

中文说明

代码审查

我对"召回应能越过 200 文档上限、同时不给模型成本解绑"的独立方案与本 PR 完全一致:仅供召回使用的无上限扫描变体、有界的候选短名单、按字节设限的 manifest。实现与之一致,未发现正确性问题:

  • 候选上界(selectModelCandidateDocuments——按既有打分器最多取 180 个词法匹配,再按近期补足至 200。词法匹配不足 180 时近期份额自动增大,符合设计文档"至少保留 20 个近期名额"。词法优先的排序也让下面的字节预算优先分配给相关条目。
  • manifest 字节上限(formatMemoryManifest——UTF-8 计数正确(计入换行分隔符、首行豁免),超限用 continue 而非 break,后面较小的条目仍可入选。validFilePaths / byFilePath 现在以有界的 includedDocs 为键,选择器无法返回已落出 manifest 的路径;manifest 为空时会在发起 side query 前提前返回。
  • 消费方——已 grep 其余扫描消费方(forget.tsindexer.tsstatus.tsextractionAgentPlanner.ts):全部仍使用有上限的 API,与 PR 声明一致。启发式回退现在对完整池打分,但仍最多返回 5 篇——与设计文档一致。
  • 测试能钉住该行为——生命周期集成测试在没有本改动时会失败(有上限的扫描必然漏掉旧目标),新单测将候选数限定在 200、manifest 限定在 ≤ 25,000 UTF-8 字节,且使用了多字节()内容。
  • 非阻塞提示: docs_scanned 遥测字段现在报告未截断的池大小——指标分布会有小幅变化,知悉即可,无需改动。两个新扫描封装通过间接覆盖(recall 单测 + 生命周期集成测试)而非 scan.test.ts 直接用例覆盖——在此规模下可接受。

无关键阻塞项。

测试证据

本次为无人值守 CI 运行,按策略不构建、不运行任何 PR 代码。以下证据来自对该 PR 自身 CI 的 API 读取——结论是标准测试套件没有在本 PR 上运行ci.yml 仅对 main / release/** 基分支触发,而本 PR 指向 codex/7040-memory-recall。被审 commit 上仅有的 check-run 都是机器人编排任务(见上方英文表格)。

此外,head 分支上有一次 push 触发的 qwen-code-pr-review.yml 运行以 failure 结束,但一个 job 都未启动——属于工作流层面的评估失败,不是测试失败,不提供任何测试信号。

PR 描述中列出的本地验证(vitest 61/61、lint、typecheck、build、bundle,以及用 201 个真实主题文件和 mock endpoint 的 headless E2E)——那是作者的自述,不是独立复核的证据。目前没有任何共享 CI 执行过该 commit 的测试。

沙箱验证可以补齐这一点:@qwen-code /verify——由于堆叠基分支上没有标准套件,"旧的词法目标能越过 200 文档上限被召回"以及"选择器 manifest ≤ 25,000 UTF-8 字节"这两个行为声明目前仅依赖作者的本地运行。

Qwen Code · qwen3.8-max

Reviewed at 62ab4a918d60a75bbb7254b49dc2c49fc833e364 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 3/5 — clean code review, but this commit has never been through the standard suite and its base PR is still open; a human should decide when it lands, not the gate.

Stepping back: the approach matches what I would have written — smaller than I feared going in (~99 production lines), every edit is in scope, no drive-by changes, and the tests genuinely pin the behavior rather than decorating it. Six months from now this reads as a careful, bounded change with a design doc that says what it deliberately did not do.

My reservation is not the code; it is the evidence:

  • No CI coverage exists for this commit. ci.yml only triggers on main / release/** bases, and this PR targets codex/7040-memory-recall. The check-run list on the head SHA is bot orchestration only, and the one push run failed without starting a single job. So there is no shared, reproducible confirmation that the suite — including the three new test files — passes on this head. The author's local results are plausible but are a claim, not evidence.
  • The base is a moving target. fix(memory): improve recall reliability and candidate coverage #8716 is still open. When it merges and this PR gets retargeted to main, the diff context shifts and CI finally runs — that is the point at which an approval means something. The approve-on-green finalize path cannot cover this PR as-is, because it only fires on PR CI workflow completions, and none will fire on this base.
  • I am therefore not approving now, even though nothing in the review argues against the change. Approving a core-module change that has never run the standard suite once would attest to more than I actually verified.

Suggested path to merge: land #8716 → retarget this PR to main → CI green → re-run @qwen-code /triage (or a maintainer triggers @qwen-code /verify against the current head for earlier sandboxed evidence).

No change requests — nothing to fix in the code itself.

中文说明

置信度:3/5 —— 代码审查干净,但该 commit 从未跑过标准套件,且其基 PR 仍未合并;何时合入应由人来决定,而不是由门禁决定。

退一步看:方案与我独立设想的一致——比我预期的更小(约 99 行生产代码),每处改动都在范围内,没有顺手夹带的改动,测试真正钉住了行为而不是装饰。六个月后再看,这会是一次有界、克制、附带设计文档(并明确说明了刻意不做什么)的改动。

我的保留意见不在代码,而在证据:

  • 该 commit 没有任何 CI 覆盖。 ci.yml 只对 main / release/** 基分支触发,而本 PR 指向 codex/7040-memory-recall。head SHA 上的 check-run 全部是机器人编排任务,唯一一次 push 运行一个 job 都未启动就失败了。因此没有任何共享、可复现的证据表明套件(包括三个新测试文件)在该 head 上通过。作者的本地结果可信,但只是自述,不是证据。
  • 基分支仍在变化。 fix(memory): improve recall reliability and candidate coverage #8716 尚未合并。等它合并、本 PR 重新指向 main 后,diff 上下文会变化,CI 才会真正运行——到那时批准才有意义。approve-on-green 兜底流程无法覆盖当前形态的本 PR:它只在 PR CI 工作流完成时触发,而这个基分支上不会有任何 PR CI 触发。
  • 因此我在现在批准,尽管审查中没有任何反对该改动的理由。对一个从未跑过标准套件的核心模块改动投批准票,等于为我并未验证的东西背书。

建议的合入路径:合并 #8716 → 将本 PR 重新指向 main → CI 全绿 → 重新运行 @qwen-code /triage(或由维护者对当前 head 触发 @qwen-code /verify 提前获得沙箱证据)。

不请求修改——代码本身没有需要修的问题。

Qwen Code · qwen3.8-max

Reviewed at 62ab4a918d60a75bbb7254b49dc2c49fc833e364 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

⏸️ Deferring to @yiliang114 — no shared test evidence exists for this commit: the standard CI suite does not trigger on the stacked base (ci.yml is gated to main / release/**), and base PR #8716 is still open. The code review itself is clean (see above) — this is a verification/workflow call, not a code objection. Needs a human call: either trigger @qwen-code /verify for sandboxed evidence now, or retarget to main after #8716 merges and let CI + a /triage re-run settle it.

⏸️ 转交 @yiliang114 —— 该 commit 目前没有共享测试证据:标准 CI 套件不会在堆叠基分支上触发(ci.yml 仅限 main / release/**),且基 PR #8716 尚未合并。代码审查本身是干净的(见上文)——这是验证与流程层面的判断,不是代码问题。需要人来决定:现在触发 @qwen-code /verify 获取沙箱证据,或在 #8716 合并后重新指向 main,由 CI 和重新运行的 /triage 来收尾。

@yiliang114

Copy link
Copy Markdown
Collaborator Author

Closeout for 62ab4a918d60:

  • Changed: no code or PR-body changes; the review found no correctness issue.
  • Verified: the PR records focused local tests, typecheck, lint, build, bundle, and a bounded recall E2E. Shared PR CI has not run because this stacked PR targets codex/7040-memory-recall.
  • Intentionally not changed: the zero-job push workflow failure is orchestration-only and provides no code-test signal, so it does not justify unrelated changes.
  • Pending: wait for fix(memory): improve recall reliability and candidate coverage #8716 to merge, then retarget this PR to main and use the resulting shared CI plus re-review as the merge gate.
中文摘要

本轮不修改代码或 PR body;现有 review 未发现正确性问题。PR 已记录聚焦本地测试、typecheck、lint、build、bundle 和有界 recall E2E,但由于当前堆叠在 codex/7040-memory-recall 上,共享 PR CI 尚未运行。零 job 的 push workflow failure 属于编排失败,不是代码测试信号。等待 #8716 合并后,将本 PR retarget 到 main,再以共享 CI 和重新 review 作为合并门禁。

@wenshao wenshao 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. Test Plan (not a blocker): src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory.

中文说明

已审查——无阻断问题。 建议见行内评论。 Test Plan(非阻断):src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory

— glm-5.2 via Qwen Code /review (v0.21.8)

Comment thread packages/core/src/memory/relevanceSelector.ts
Comment thread packages/core/src/memory/relevanceSelector.ts
Comment thread packages/core/src/memory/recall.ts
@yiliang114
yiliang114 requested a review from wenshao August 9, 2026 20:11
@yiliang114

Copy link
Copy Markdown
Collaborator Author

Changed: strengthened the existing bounded-selector tests so truncated-out paths are rejected and lexical matches keep first claim on the manifest budget.

Verified: focused relevance-selector and recall tests passed 55/55, Prettier passed, and git diff --check passed. New telemetry or logging was intentionally not added because it is a separate responsibility. Automatic review is running.

中文摘要

已加强现有 selector 测试,覆盖截断路径拒绝与词法候选优先。聚焦测试 55/55、Prettier 和 diff 检查通过。未新增独立 telemetry/logging 职责;自动 review 正在运行。

@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. Suggestions are inline. Not explored to full depth (tool budget reached): This PR (fix(memory): recall relevant topics beyond scan ...: did not execute the new tests — the review worktree has no node_modules and installing was out of budget; all analysis is static.. Test Plan (not a blocker): src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory; 61 passed — this review observed 19386, 1101, 18349, 1466, 481, 2941, 454 passed.

中文说明

已审查。 建议见行内评论。 未探索到全部深度(达到工具调用预算):This PR (fix(memory): recall relevant topics beyond scan ...:did not execute the new tests — the review worktree has no node_modules and installing was out of budget; all analysis is static.。 Test Plan(非阻断):src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory; 61 passed — this review observed 19386, 1101, 18349, 1466, 481, 2941, 454 passed

— qwen3.8-max via Qwen Code /review (v0.21.8)

Comment thread packages/core/src/memory/recall.ts
Comment thread packages/core/src/memory/relevanceSelector.ts
Comment thread packages/core/src/memory/scan.ts
Comment thread docs/design/2026-08-09-bounded-memory-recall-candidates.md
Comment thread packages/core/src/memory/recall.ts
Comment thread packages/core/src/memory/relevanceSelector.test.ts
Comment thread packages/core/src/memory/relevanceSelector.ts
Comment thread packages/core/src/memory/relevanceSelector.ts
Comment thread packages/core/src/memory/recall.ts Outdated
@yiliang114

Copy link
Copy Markdown
Collaborator Author

Closeout for this feedback pass:

  • Changed: filtered active-tool usage noise before model candidate bounding and reserved 20 recent documents ahead of lexical candidates without crowding out lexical matches. Pushed commit 59c7711.
  • Verified: 61 focused Core tests, Core typecheck/build, targeted downstream coverage, Prettier, and diff check pass.
  • Intentionally not changed: forget semantics, a new tracked CLI E2E harness, and mutation-only/doc expansions outside the committed recall scope.
  • Pending: post-push CI and automatic review are running.
中文摘要

已在候选截断前过滤活跃工具噪音,并将 20 个近期候选稳定放在词法候选前,同时避免近期填充挤掉词法命中。提交 59c7711 已推送;61 个聚焦测试、Core typecheck/build 和下游验证通过。远端 CI 与自动 review 仍在运行。

@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. Suggestions are inline. Not explored to full depth (tool budget reached): This PR (fix(memory): recall relevant topics beyond scan ...: could not execute the mutation checks — no node_modules in the worktree or parent checkout; all mutation analysis above is static reasoning from the source (t…; This PR (fix(memory): recall relevant topics beyond scan ...: Since node_modules isn't installed in the worktree, the changed unit tests couldn't be run.; This PR (fix(memory): recall relevant topics beyond scan ...: could not execute the changed unit tests — the review worktree has no node_modules and npx vitest fails to resolve vitest/config ; a full monorepo install …. Test Plan (not a blocker): src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory; 61 passed — this review observed 19390, 1101, 18350, 1466, 481, 2941, 454 passed.

中文说明

已审查。 建议见行内评论。 未探索到全部深度(达到工具调用预算):This PR (fix(memory): recall relevant topics beyond scan ...:could not execute the mutation checks — no node_modules in the worktree or parent checkout; all mutation analysis above is static reasoning from the source (t…;This PR (fix(memory): recall relevant topics beyond scan ...:Since node_modules isn't installed in the worktree, the changed unit tests couldn't be run.;This PR (fix(memory): recall relevant topics beyond scan ...:could not execute the changed unit tests — the review worktree has no node_modules and npx vitest fails to resolve vitest/config ; a full monorepo install …。 Test Plan(非阻断):src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory; 61 passed — this review observed 19390, 1101, 18350, 1466, 481, 2941, 454 passed

— qwen3.8-max via Qwen Code /review (v0.21.8)

Comment thread packages/core/src/memory/recall.test.ts Outdated
Comment thread packages/core/src/memory/recall.ts
Comment thread packages/core/src/memory/relevanceSelector.ts
@yiliang114

yiliang114 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Changed: no code or PR body changes. One outdated thread was resolved, and two late mutation-only test-hardening requests were declined with scope evidence and resolved.

Verified: the exact production code dynamically fills remaining candidate capacity and deduplicates by file path. The PR is mergeable, active review threads are zero, and applicable product checks are green.

Intentionally not changed: no additional test matrix was added after five review rounds without a current behavior defect.

Pending: none. The cancelled route job was reply-dispatch noise; product checks completed successfully.

中文摘要

未修改代码或 PR body;关闭一条过时 thread,并按晚轮 mutation-only 测试硬化处理两条建议。当前生产逻辑正确、可合并、active thread 为 0、产品检查全绿。

@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. Test Plan (not a blocker): src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory; 61 passed — this review observed 19391, 1101, 18350, 1466, 481, 2941 passed.

中文说明

已审查——无阻断问题。 建议见行内评论。 Test Plan(非阻断):src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory; 61 passed — this review observed 19391, 1101, 18350, 1466, 481, 2941 passed

— qwen3.8-max via Qwen Code /review (v0.21.8)

Comment thread packages/core/src/memory/recall.ts Outdated
Comment thread packages/core/src/memory/recall.ts
Comment thread packages/core/src/memory/relevanceSelector.test.ts Outdated
Comment thread packages/core/src/memory/recall.ts

@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. Test Plan (not a blocker): src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory; 61 passed — this review observed 1101, 18350, 1466, 481, 2941, 454 passed.

中文说明

已审查——无阻断问题。 建议见行内评论。 Test Plan(非阻断):src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory; 61 passed — this review observed 1101, 18350, 1466, 481, 2941, 454 passed

— qwen3.8-max via Qwen Code /review (v0.21.8)

Comment thread packages/core/src/memory/relevanceSelector.ts
Comment thread packages/core/src/memory/recall.ts
Comment thread packages/core/src/memory/relevanceSelector.test.ts
Comment thread packages/core/src/memory/recall.ts

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

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed. Test Plan (not a blocker): src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory.

中文说明

⚠️ 已从批准降级为评论:CI still running。 已审查。 Test Plan(非阻断):src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory

— deepseek-v4-flash via Qwen Code /review (v0.21.8)

@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. Test Plan (not a blocker): src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory; 61 passed — this review observed 1101, 18350, 1466, 481, 2941, 454 passed.

中文说明

已审查——无阻断问题。 建议见行内评论。 Test Plan(非阻断):src/memory/recall.test.tsno such file or directory; src/memory/relevanceSelector.test.tsno such file or directory; src/memory/memoryLifecycle.integration.test.tsno such file or directory; src/memory/scan.test.tsno such file or directory; 61 passed — this review observed 1101, 18350, 1466, 481, 2941, 454 passed

— qwen3.8-max via Qwen Code /review (v0.21.8)

Comment thread packages/core/src/memory/recall.test.ts
Comment thread packages/core/src/memory/recall.test.ts
@yiliang114
yiliang114 merged commit bbd040d into codex/7040-memory-recall Aug 11, 2026
74 of 75 checks passed
samuelhsin pushed a commit to samuelhsin/qwen-code that referenced this pull request Aug 18, 2026
…M#8716)

* fix(memory): improve recall delivery and multilingual fallback

* fix(memory): bound heuristic recall scoring

* test(memory): pin initial recall budget with fake timers

Rewrite the slow-recall test to assert with fake timers that the main
request is still held 1 ms inside the 100 ms initial budget and proceeds
without memory at expiry, so budget changes can no longer pass unnoticed.

* test(memory): pin recall budget and scoring contracts

Address review findings with mutation-verified pins:
- settle-early: bounded wait ends when recall settles, not at full budget
- Cron and ToolResult consume points stay zero-wait
- post-wait replacement guard refuses stale handles
- type boost flips the winner (tie-break no longer masks its removal)
- hiragana-only coverage for the CJK tokenizer
- design doc: RFC QwenLM#7040 sets no numeric overhead target; fix attribution

* fix(memory): preserve recall field weighting

* fix(memory): recall relevant topics beyond scan cap (QwenLM#8803)

* fix(memory): bound recall candidates after full scan

* test(memory): pin bounded selector inputs

* fix(memory): preserve bounded recall candidates

* fix(memory): preserve lexical recall candidates

* fix(memory): prioritize lexical model candidates

* fix(core): preserve UTF-16 manifest boundaries

* fix(memory): address recall review feedback

* test(memory): measure recall rollout gate against the pre-change scorer

RFC QwenLM#7040 gates the multilingual precision change on evidence that English
Recall@5 and no-result precision do not regress. Add a labeled 45-case
corpus and an evaluation harness that scores both the shipped deterministic
selector and a frozen copy of the pre-change scorer over it, so the gate is
reproducible rather than asserted.

* fix(memory): deliver a deterministic fast recall result on the initial turn

The initial-turn budget is 100 ms, but recall awaits the model selector,
which is a network side query with a 30 s ceiling. The budget therefore
expires on the common path and delivery falls through to the ToolResult
point — which a tool-free turn never reaches, so the result is discarded as
no_safe_delivery_point. That is the case memory matters most for.

Publish the deterministic candidates that selectModelCandidateDocuments
already computes, before blocking on the selector, and inject them when the
budget expires. The refined result still lands at ToolResult, with documents
the fast phase already delivered filtered out.

phase telemetry now carries both stages: phase is the delivery stage, strategy
is the selection method, and they are orthogonal.

* docs(memory): record the fast-path decision and phase/strategy split

* test(memory): report the mixed-language slice in the rollout gate

* docs(memory): align recall docs on the deterministic fast path

memory-system.md documented recall selection but never documented delivery, so
the delivery telemetry from QwenLM#7393 was undocumented and the fast path had no
home in the canonical reference. Add a delivery section and the delivery event
table, and correct two docs that still described the single-path behaviour.

* fix(memory): use Array<T> for the fast-path test doc lists

@typescript-eslint/array-type forbids T[] for non-simple types.

* docs(memory): clarify recall delivery telemetry

* fix(memory): report already-delivered recall count

* docs(memory): align recall delivery claims

* fix(memory): rank ties by recency and record fast-delivered discards

Three review follow-ups on the recall reliability change.

Tie-break: `selectRelevantAutoMemoryDocuments` broke score ties with
`type.localeCompare`, which orders feedback < project < reference < user.
That was tolerable while the result was five documents wide; the fast path
takes only MAX_FAST_RECALL_DOCS = 2, so a tied user-typed document was
dropped every time — the exact memory a tool-free turn exists to surface.
Ties now fall to recency, then to input order, which keeps the
project-before-user precedence the concatenation already establishes.

Corpus: the case labeled `semantic-no-lexical` had no relevant documents,
so it was a no-result case wearing the wrong label and nothing measured the
cost of "no lexical match, no score". Relabel it and add three genuine
answerable-but-lexically-disjoint cases. Both scorers return nothing for
them, so the slice sits outside the quality floor and is asserted
separately: the fast path closes the timing gap, not the matching gap.
Tool-free delivery is 92.3%, not 100%, and the residual is that slice.

Telemetry: a tool-free turn logs its terminal event from the discard path,
which did not apply the fast-phase exclusion. A turn whose every selected
document had already been fast-delivered was recorded as
`no_safe_delivery_point`, inflating the "memory never reached the model"
bucket with turns that got it. Apply the same rule the ToolResult consume
point uses; a partial overlap still reports the cancellation reason.

* docs(memory): state the candidate-cap trade and the per-turn document count

Two review follow-ups, documentation only. No behaviour change.

"Removes the 200-document cap" oversold the candidate change. What it does
is swap a per-scope, query-blind recency truncation for a global,
query-aware one, and the effect is not a uniform widening: at or under 200
documents nothing was excluded by count under either design, but the new
25,000-byte manifest budget is a ceiling the old path lacked; between 200
and 400 with neither scope over 200 the old path sent every document and
the new one sends at most 200, so fewer reach the model; only a scope over
200 is the case the change is actually for. Record all three, plus the fact
that the manifest budget packs rather than prefixes.

MAX_RELEVANT_DOCS = 5 bounds one prompt, not one turn. A fast delivery of
two plus a refined delivery of five disjoint documents puts seven in front
of the model; dedupe removes repeats, not the sum. This follows from
dropping combined fast/refined budget accounting, which was a deliberate
choice, but the number was never written down next to the constant that
reads like a hard cap.

* fix(memory): end the initial recall wait on the fast result, widen tokenization

The 100 ms initial budget was a fixed cost, and the evidence for it measured
the wrong thing. Deterministic *scoring* is microseconds, but the fast result
is only published once recall has enumerated, read, and parsed the memory
tree — and this branch removed the 200-document cap for recall, so that scan
grows with the tree. recall-scan-latency.test.ts adds that measurement
against a real temporary tree: ~29 ms at 200 topics, ~70 ms at 500, ~130 ms
at 1000.

So for any tree small enough to scan in time — the ordinary case — the fast
result was in hand tens of milliseconds before the budget expired, and the
rest of the budget was spent waiting on a model selector this design already
assumes will miss it. The wait now ends on whichever comes first: recall
settling, the fast result being published, cancellation, or the ceiling. The
preference order is unchanged, because the code after the wait still prefers
a settled recall. Past roughly a thousand topics the scan alone exceeds the
ceiling and the turn pays the full budget for nothing; that is recorded as a
known limitation rather than fixed, since the fix is a persistent catalog.

Tokenization kept only [a-z0-9]{3,} runs, so Cyrillic, Greek, Arabic, and
accented Latin produced no tokens at all and the deterministic path was
unconditionally silent for them. Keep whole runs of non-CJK letters, marks,
and digits instead. CJK is excluded per character rather than by alternation
order: \p{L} also matches Han, so a Latin-initial run would otherwise swallow
the CJK after it and turn abc漢字 into one token. Scripts without word
separators outside the CJK set still collapse to one run, which is recorded
rather than claimed as segmentation.

Two smaller follow-ups. The active-tool alias set is now derived once per
recall instead of once per scanned document, which mattered little under the
old 200-document cap and more without it. And the eval prints the Recall@5 a
query-blind random scorer would score on this corpus (20%), with a test
holding that floor at or below 25%, because a small corpus flatters every
design and the headline was unreadable without it.

* docs(memory): correct the initial-turn preference claim, pin it with a test

Local end-to-end verification on QwenLM#8716 found the claim added in 01ef7d7 —
"the preference order is unchanged: whatever ends the wait, a settled recall
is still delivered in preference to the fast result" — to be false in the
case that matters. `onFastResult` is published before recall issues the
selector request at all, so the recall promise cannot be settled when the
wait ends on the fast result. Measured against a selector settling in 15 ms,
comfortably inside the ceiling, the initial turn still delivers the
deterministic pair and discards the model's picks.

The behaviour is right and stays: a model side query does not return inside a
100 ms ceiling in production, so arbitrating would spend the rest of the
budget on every turn to win a race that does not happen, and the selector's
judgement still lands at ToolResult with the fast documents excluded. What
was wrong was the description. State it directly instead — on the initial
turn, once the deterministic scorer matches, the fast result wins regardless
of selector latency — and pin it with a test that fails when the early exit
is removed, so it reads as a decision rather than an accident.

Two measurements corrected while here. The scan crossover is machine-
dependent, not a fixed topic count: the same three sizes measure 9/21/46 ms
on faster hardware against 29/70/130 ms on the machine the tables were
written from, so the ceiling is not reached there at all. And
MAX_MODEL_CANDIDATE_DOCS = 200 is rarely the binding constraint —
MAX_MODEL_MANIFEST_BYTES is, at roughly 90-150 documents once absolute paths
and timestamps are counted. Measured runs sent 94 and 96 manifest lines where
the document cap would have allowed 200, which also explains why the recency
reserve has to be interleaved rather than appended.

---------

Co-authored-by: yiliang114 <jinjing.zzj@gmail.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.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.

4 participants