feat(channels): recall relevant memory per message - #7157
Merged
Conversation
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: Real and well-specified. Issue #6360 describes a concrete gap — channel memory is injected as a complete document only at session establishment, so long-lived sessions miss newly relevant facts and precision degrades as memory grows. This is an observed product limitation, not theoretical hardening. Direction: Aligned. Per-message bounded recall is a natural next step for channel memory. The issue was marked Size: 344 production logic lines (237+78 in Approach: Scope feels right. The recall selector is a pure function in its own file — easy to test and reason about in isolation. The generation-based race protection for memory reads is a clean pattern that avoids stale-context injection without needing a storage migration or model calls. The Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实且明确。Issue #6360 描述了一个具体的产品缺口——channel memory 仅在 session 建立时以完整文档注入，长生命周期 session 会错过新相关事实，memory 增长后精度下降。这是已观测的产品限制，不是理论性加固。 方向：对齐。逐消息有界召回是 channel memory 的自然演进。Issue 已标记 规模：344 行生产代码（ 方案：范围合理。召回选择器是独立文件中的纯函数——易于测试和理解。基于 generation 的 memory read 竞争保护是一个干净的模式，无需存储迁移或模型调用即可避免过期上下文注入。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Review: per-message channel-memory recall (head
|
Contributor
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. |
Collaborator
Author
|
Addressed the oversized-entry finding in commit 39cc390: when the highest-ranked relevant fact alone exceeds the 1,200-code-point recall budget, normal turns now inject a bounded copy with an explicit [truncated] marker while leaving the stored entry unchanged. Added a regression test that pins the hard budget and non-mutation behavior. Verification: 504 focused tests passed, plus build, typecheck, Prettier, and focused ESLint. The fallback recency suggestion is intentionally deferred because it is a non-blocking ranking-policy change rather than a correctness fix for this PR. |
qwen-code-ci-bot
approved these changes
Jul 18, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jul 18, 2026
Collaborator
Re-verification (round 2): oversized-fact recall fix — head
|
| # | Severity | Round-1 finding | Status |
|---|---|---|---|
| 1 | Medium | Store accepts entries up to 2,000 cp but recall skipped anything over the 1,200-cp budget — a 1,201–2,000 cp fact was unrecallable on normal turns even as the sole relevant match | ✅ Fixed — a top-ranked relevant fact that alone exceeds the budget is now recalled truncated to exactly 1,200 cp with a [truncated] suffix. Confirmed by the new unit test, a real-flow E2E, and an old-vs-new A/B (below) |
| 2 | Low | Zero-overlap fallback keeps store order (oldest first), so the 3 oldest short facts monopolize the fallback slots |
What I ran
1. Unit suite + typecheck — standalone run in packages/channels/base at 39cc39074: 839/839 tests green across 16 files (channel-memory-recall.test.ts now 16 tests incl. the new truncation test; ChannelBase.test.ts 488 covering the recall integration paths), tsc --noEmit clean.
2. Real-flow E2E (21/21 assertions) — a tsx harness driving the real ChannelBase with the real packages/core channel-memory store (actual CHANNEL.json on disk under an isolated QWEN_HOME); the only fake piece is a recording ChannelAgentBridge that captures the exact prompt sent to the agent:
- S1 (the fix): a 1,500-cp fact stored for real on disk is recalled on a matching message — the prompt on the bridge contains the recall block with the fact truncated to exactly 1,200 cp ending in
[truncated], real entry id kept; the stored entry stays 1,500 cp on disk (truncation operates on a copy). - S2 (A/B): identical inputs through the round-1 head's selector return
[](fact silently dropped); the new selector returns the truncated entry. - S3 (boundary): exactly 1,200 cp → recalled whole, no suffix; 1,201 cp → truncated.
- S4 (regression): small relevant facts still recalled verbatim.
- S5 (astral safety): a 1,407-cp emoji fact truncates on a code-point boundary — no lone surrogate (
isWellFormed()), last content char is a whole 🚀. - S6: finding 2 re-confirmed (fallback = 3 oldest short facts).
- S7: when a smaller relevant fact outranks the oversized one, the smaller one wins and the oversized non-first fact is skipped, as before.
Fix audit notes
- Truncation returns a copy (
{ ...entry, text }); the store is never mutated — E2E-verified against the on-disk document after the turn. sanitizePromptTextstill runs after truncation informatRelevantChannelMemoryContext, so the injection-hardening surface from round 1 is unchanged.- Slicing is
Array.from-based (code points), astral-safe (S5). - The guard
score > 0 && selected.length === 0 && entryCodePoints > MAXfires only for the top-ranked relevant fact. A fallback entry can't qualify anyway: the fallback filter requires normalized length ≤ 120 cp and NFKC can at most halve length, so a >1,200-cp raw text can never enter the fallback list — thescore > 0term is belt-and-braces. - Behavior note (not a bug): when the oversized fact ranks first it consumes the whole budget, so lower-ranked relevant facts are dropped in its favor — the sensible priority order given the hard cap;
breakvscontinueis outcome-equivalent there since the budget is exhausted.
Verdict
LGTM — merge-ready at 39cc3907476e4eab8edcee068edbf535e5d13d31. Finding 1 is fixed exactly along the lines recommended in round 1; finding 2 (Low) remains open but is not blocking.
中文版本(点击展开)
复验(第二轮):超长记忆条目召回修复 — head 39cc390
这是对我第一轮 review(#issuecomment-5010890015)的跟进。自第一轮以来的增量只有一个提交 39cc39074 fix(channels): recall oversized memory facts,针对第一轮的 Finding 1。我在隔离 worktree 中于 PR head 重新构建并验证。
第一轮发现 → 在 39cc390 的状态
| # | 严重度 | 第一轮发现 | 状态 |
|---|---|---|---|
| 1 | 中 | store 允许最长 2,000 码点的条目,但召回预算是 1,200 码点,超预算条目直接被跳过 —— 1,201–2,000 码点的事实即使是唯一相关匹配,普通轮次也无法召回 | ✅ 已修复 —— 排名第一的相关事实若单独超出预算,现在会被截断到恰好 1,200 码点(带 [truncated] 后缀)后召回。经新增单测、真实流 E2E 和新旧 A/B 对比确认(见下) |
| 2 | 低 | 零重叠 fallback 保持 store 顺序(最旧优先),最旧的 3 条短事实垄断 fallback 名额 |
验证内容
1. 单测套件 + 类型检查 —— 在 39cc39074 的 worktree 中对 packages/channels/base 独立运行:839/839 全绿,共 16 个文件(channel-memory-recall.test.ts 现有 16 个测试,含新增截断测试;ChannelBase.test.ts 488 个覆盖召回集成路径),tsc --noEmit 干净。
2. 真实流 E2E(21/21 断言) —— tsx harness 驱动真实 ChannelBase + 真实 packages/core channel-memory store(隔离 QWEN_HOME 下的真实磁盘 CHANNEL.json);唯一伪造件是记录型 ChannelAgentBridge,用于捕获发送给 agent 的确切 prompt:
- **S1(本修复):**真实落盘的 1,500 码点事实在匹配消息上被召回 —— 桥上捕获的 prompt 含召回块,事实被截断到恰好 1,200 码点、以
[truncated]结尾,真实条目 id 保留;磁盘上条目仍是 1,500 码点(截断作用于副本)。 - **S2(A/B):**相同输入经第一轮 head 的选择器返回
[](事实被静默丢弃);新选择器返回截断条目。 - **S3(边界):**恰好 1,200 码点 → 完整召回、无后缀;1,201 码点 → 截断。
- **S4(回归):**小事实仍原样召回。
- **S5(增补平面安全):**1,407 码点 emoji 事实按码点边界截断 —— 无孤立代理项(
isWellFormed()),最后一个内容字符是完整的 🚀。 - **S6:**Finding 2 复确认(fallback = 最旧 3 条短事实)。
- **S7:**当更小的相关事实得分更高时,小事实胜出,非首位的超长事实照旧被跳过。
修复审计要点
- 截断返回副本(
{ ...entry, text }),store 永不被改写 —— E2E 已对轮次结束后的磁盘文档验证。 formatRelevantChannelMemoryContext中sanitizePromptText仍在截断之后执行,第一轮验证的注入加固面不变。- 切片基于
Array.from(码点),对增补平面安全(S5)。 - 守卫
score > 0 && selected.length === 0 && entryCodePoints > MAX仅对排名第一的相关事实生效。fallback 条目本就无法命中:fallback 过滤要求规范化长度 ≤ 120 码点,而 NFKC 最多使长度减半,>1,200 码点的原文不可能进入 fallback 列表 ——score > 0属于双保险。 - 行为说明(非缺陷):超长事实排名第一时会占满整个预算,排名更低的相关事实为其让位 —— 在硬上限下这是合理的优先级;此处
break与continue结果等价,因为预算已耗尽。
结论
**LGTM —— 在 39cc3907476e4eab8edcee068edbf535e5d13d31 达到可合并状态。**Finding 1 完全按第一轮建议的方向修复;Finding 2(低)仍开放但不阻塞。
Collaborator
Author
|
🤖 Could not address the latest feedback automatically (round 1/5). A human should take over this PR. What I found before stopping: Review Address SummaryPR: #7157 — per-message channel-memory recall Finding 1 — Medium: facts of 1201–2000 code points are permanently unrecallableDecision: Already fixed (commit The current selection loop already handles the case: when no entry has been selected yet ( The reviewer's probe ( Finding 2 — Low: fallback slots monopolized by oldest short factsDecision: Implemented. Reversed the fallback array so that newer short facts surface before older ones. Changes in
Run log: https://github.com/QwenLM/qwen-code/actions/runs/29642851252 |
wenshao
requested changes
Jul 18, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Not reviewed: reverse-audit — stopped at the five-round hard cap without two consecutive dry rounds.
— Codex GPT-5 via Qwen Code /review
| selected.length === 0 && | ||
| entryCodePoints > CHANNEL_MEMORY_RECALL_MAX_CODE_POINTS | ||
| ) { | ||
| selected.push(truncateEntryToRecallBudget(entry)); |
Collaborator
There was a problem hiding this comment.
[Critical] The selector scores the complete fact but this branch injects only its prefix and stops considering later facts. If the only matching term is after code point 1,200, the prompt gets unrelated text plus the truncation marker and loses the fact that made the entry relevant; this also conflicts with issue #6360's requirement to return complete entries and skip entries that do not fit. Remove this special truncation branch so oversized entries continue to the next candidate, and add a regression where the match occurs beyond the budget.
— Codex GPT-5 via Qwen Code /review
Comment on lines
+4428
to
+4429
| ...(await this.channelMemory.listChannelMemoryEntries( | ||
| memoryTarget, |
Collaborator
There was a problem hiding this comment.
[Critical] This await crosses the group-history snapshot boundary. While it is pending, an unmentioned group message is recorded immediately; the earlier accepted turn then drains and clears that later message, attaches it before the wrong current message, and deprives the next trigger of it. Drain/snapshot group history before starting the recall read, and cover a deferred list read with an unmentioned message arriving during it.
— Codex GPT-5 via Qwen Code /review
| appendChannelMemory: vi.fn().mockResolvedValue({ changed: true }), | ||
| clearChannelMemory: vi.fn().mockResolvedValue({ changed: true }), | ||
| }; | ||
| it('recomputes recall from the latest entry snapshot on every normal turn', async () => { |
Collaborator
There was a problem hiding this comment.
[Suggestion] This replacement removes the full-document bidi-sanitization and astral-code-point truncation assertions, although loops and webhooks still use that formatter. A regression that leaks a bidi override or mishandles 6,001 emoji would now pass because the remaining cases cover structured recall or ASCII truncation. Relocate those two assertions to focused runLoopPrompt full-memory tests.
— Codex GPT-5 via Qwen Code /review
| state = { generation: 0, readers: 0 }; | ||
| this.channelMemoryReads.set(key, state); | ||
| } | ||
| state.readers += 1; |
Collaborator
There was a problem hiding this comment.
[Suggestion] The shared per-target reader refcount has no test with two simultaneous reads for the same target. If the first reader releases before a mutation while the second remains active, a cleanup regression could remove the state and let the second stale read escape invalidation while every single-reader race test stays green. Add a two-reader test that resolves one read, mutates, then verifies the remaining stale read is rejected.
— Codex GPT-5 via Qwen Code /review
Comment on lines
+1205
to
1207
| const sessionContext = await this.prepareUnattendedSessionContext( | ||
| sessionId, | ||
| target, |
Collaborator
There was a problem hiding this comment.
[Suggestion] The webhook copy of stale-read revalidation is not exercised by a mutation race. A webhook can read old memory while a human updates the same target; a regression in this duplicated branch could inject the stale snapshot and suppress the corrected one on the next webhook while loop tests remain green. Add a deferred webhook read with a same-target mutation before resolution.
— Codex GPT-5 via Qwen Code /review
| }); | ||
|
|
||
| it('invalidates injected memory only after confirming a natural update', async () => { | ||
| it('recalls the latest snapshot after confirming a natural update', async () => { |
Collaborator
There was a problem hiding this comment.
[Suggestion] Despite the test name, the mocked list snapshot never changes and the prompt text is never asserted: updateChannelMemoryEntry only reports changed while listChannelMemoryEntries keeps returning Use staging. A stale-recall regression therefore passes. Back the mock with mutable entries, replace the entry on update, and assert the final prompt contains Use production and not Use staging.
— Codex GPT-5 via Qwen Code /review
| { | ||
| channelName: 'test-chan', | ||
| chatId: 'group-1', | ||
| threadId: 'thread-1', |
Collaborator
There was a problem hiding this comment.
[Suggestion] The threaded replacement verifies only that the exact thread target was listed, not that the selected thread memory reaches the thread prompt. A regression that lists the right target but drops relevant entries during threaded prompt construction would pass, while the ordering test covers only an unthreaded group. Assert the third bridge prompt contains the relevant-memory block and the threaded current message.
— Codex GPT-5 via Qwen Code /review
Comment on lines
+5245
to
+5248
| const loopJob = ( | ||
| id: string, | ||
| senderId: string, | ||
| chatId: string, |
Collaborator
There was a problem hiding this comment.
[Suggestion] Cross-target isolation is tested only with different chat IDs, even though both generation keys and invalidation depend on threadId. A regression that invalidates thread-2 when only thread-1 changes would still pass and cause unnecessary rereads/reinjection in the neighboring topic. Let loopJob accept threadId and add same-chat, different-thread mutation and in-flight-read cases.
— Codex GPT-5 via Qwen Code /review
| if (!first) continue; | ||
|
|
||
| if (/\p{Script=Latin}/u.test(first)) { | ||
| if (run.length >= 2) result.add(`latin:${run.join('')}`); |
Collaborator
There was a problem hiding this comment.
[Critical] Splitting Latin and decimal runs before applying the one-character filters makes mixed identifiers such as v4, S3, and o3 unmatchable. With long API v1 through API v4 facts, the query API v4 gives all entries only the generic api score, so stable ordering plus the three-entry cap injects v1-v3 and omits the exact v4 fact. Preserve a token for contiguous mixed Latin/decimal identifiers and add exact-version ranking coverage.
— Codex GPT-5 via Qwen Code /review
| function normalize(text: string): string { | ||
| return text | ||
| .normalize('NFKC') | ||
| .toLowerCase() |
Collaborator
There was a problem hiding this comment.
[Critical] Lowercasing is not Unicode full case folding. A supported Latin fact containing Straße normalizes to straße while the ordinary uppercase query STRASSE normalizes to strasse, yielding no overlap; with three earlier generic route facts, the exact fact is ranked fourth and omitted. Use Unicode full/NFKC case folding before tokenization and add Straße/STRASSE ranking coverage.
— Codex GPT-5 via Qwen Code /review
This was referenced Jul 18, 2026
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR adds deterministic, per-message channel-memory recall for normal DM, group, and thread messages. Each eligible turn selects a bounded relevant subset from the latest structured facts using normalized Latin/number terms and CJK bigrams, with a short-fact fallback, stable ordering, and fixed entry and character limits.
Recalled facts are rendered as explicitly untrusted context before configured instructions and the channel isolation boundary. Loop and webhook tasks keep their existing complete-memory behavior, while memory mutations refresh unattended context without replaying one-time static instructions.
Target-scoped active-read generations prevent updates, removals, or clears from racing pending normal or unattended reads and injecting stale facts. Existing access gates, single-session isolation, storage, mutation confirmation, daemon persistence, and secret protection remain unchanged.
Why it's needed
Channel memory was previously injected as a complete document only when session context was first established. Long-lived sessions could miss newly relevant facts, and complete-document injection becomes less precise as memory grows. Per-message bounded recall keeps explicit memory current and focused without another model call, embeddings, or a storage migration.
Reviewer Test Plan
How to verify
Save several channel-memory facts, including related long facts and unrelated short facts, then send different normal messages in the same DM, group, or thread. Confirm each turn receives at most three complete relevant facts, newly saved or updated facts appear on the next turn, unrelated long facts are omitted, and one-time configured instructions are not replayed.
Confirm rejected senders, messages that fail group mention/access gates, single-scope sessions, and recognized agent slash commands perform no recall. Confirm memory-list failures still deliver the user prompt with a sanitized log entry.
Confirm loop and webhook tasks still receive complete channel memory once per unattended context, and a successful same-target mutation refreshes that context without replaying static instructions. The focused concurrency tests cover clear races, mutation during pending reads, post-read microtask races, and unrelated-target isolation.
Evidence (Before & After)
N/A - prompt assembly and channel lifecycle behavior are covered by unit/integration-style tests rather than UI output.
Tested on
Environment (optional)
Node.js workspace on macOS. Focused channel tests: 503 passed. Full repository build, typecheck, and lint passed before the final rebase; focused tests, build, and typecheck passed again on the latest upstream main.
Risk & Scope
Linked Issues
Fixes #6360
中文说明
本 PR 做了什么
本 PR 为普通单聊、群聊和话题消息增加确定性的逐消息 channel memory 召回。每个通过现有权限检查的消息都会从最新结构化事实中选择一个有界的相关子集，选择算法使用规范化后的拉丁字母/数字词项和 CJK bigram，并提供短事实 fallback、稳定排序以及固定的条目数和字符数上限。
召回事实会作为明确标记的不可信上下文，放在配置 instructions 和 channel 隔离边界之前。Loop 和 webhook 任务继续保持现有的完整 memory 行为；memory 变更后 unattended context 会刷新，但不会重复注入一次性静态 instructions。
按 target 隔离的活跃读取 generation 可以防止更新、删除或清空操作与尚未完成的普通/后台读取竞争，从而避免旧事实进入 prompt。现有访问控制、single session 隔离、存储、变更确认、daemon 持久化和 secret 保护行为保持不变。
为什么需要
此前 channel memory 只会在 session context 首次建立时以完整文档注入。长生命周期 session 可能看不到新近相关的事实，而且 memory 增长后完整文档注入会越来越不精确。逐消息有界召回不需要额外模型调用、embedding 或存储迁移，就能让显式记忆保持最新且聚焦。
Reviewer Test Plan
如何验证
保存多条 channel memory，包括相关的长事实和无关的短事实，然后在同一个单聊、群聊或话题内发送不同普通消息。确认每轮最多注入三条完整相关事实，新保存或更新的事实会在下一轮出现，无关长事实不会注入，并且一次性配置 instructions 不会重复出现。
确认被拒绝的 sender、未通过群聊 mention/访问控制的消息、single-scope session 和已识别的 agent slash command 都不会执行召回。确认 memory 列表读取失败时用户消息仍会送到 agent，日志内容经过安全清洗。
确认 loop 和 webhook 任务仍然在每个 unattended context 中接收一次完整 channel memory，同 target 成功变更后会刷新 memory 且不会重复静态 instructions。聚焦并发测试覆盖 clear race、读取期间 mutation、读取完成后的 microtask race 以及无关 target 隔离。
Evidence (Before & After)
N/A - 这是 prompt 组装和 channel 生命周期行为，通过单元/集成风格测试验证，不涉及 UI 输出。
Tested on
Environment (optional)
macOS Node.js workspace。聚焦 channel 测试 503 项通过。最终 rebase 前全仓 build、typecheck 和 lint 通过；rebase 到最新 upstream main 后再次通过聚焦测试、build 和 typecheck。
Risk & Scope
Linked Issues
Fixes #6360