fix(memory): improve recall reliability and candidate coverage - #8716
Conversation
|
Re-run after substantial growth since the last gate pass — the PR gained the deterministic fast path, the query-aware candidate set, and the eval corpora. Gate re-checked from scratch on
Moving on to code review. 🔍 中文说明自上次门禁通过以来 PR 有显著增长——新增了确定性 fast path、query 感知的候选集以及评估语料。门禁已在
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewFull re-read of the production diff at Independent baseline. For "selected memory misses a tool-free first turn" my proposal is a bounded await at the initial consume point with early exit on settle/abort, exactly-once guards, and the budget miss left pending for the ToolResult point — plus, since the selector round trip dominates any short budget, delivering the deterministic candidates the selector was already going to score. For "ASCII-only scorer that scores everything" NFKC + script-aware runs + CJK bigrams, a lexical gate, field weights, and a bounded query window. For the 200-document pre-relevance truncation, a query-aware candidate set with some bound — uncapping the scan puts tree size inside the 100 ms budget, so a bound is mandatory. The PR matches this baseline on every point; I found no simpler path it missed. What I checked and what holds:
No correctness blockers, no convention violations. The remaining caveats are documented design limits, not defects: lexical-only matching is silent on semantic-only queries, per-turn document count can reach 7 (fast 2 + refined 5, individually bounded), and a very slow scan can exceed the ceiling — each named in Risk & Scope with its trade. sequenceDiagram
participant P1 as User
participant P2 as GeminiClient
participant P3 as Recall
participant P4 as Model selector
P1->>P2: query arrives
P2->>P3: recall starts on UserQuery
P3->>P3: scan tree, deterministic score
P3-->>P2: fast result, top 2 docs
P2-->>P1: injected into first prompt, within the 100 ms ceiling
P3->>P4: side query with ranked candidates
P4-->>P3: model selection settles later
P3-->>P2: refined result at the ToolResult turn
P2-->>P1: remaining docs, fast-delivered ones excluded
Files changed (17)
TestingUnattended CI run — the evidence below is the PR's own CI on the reviewed commit, fetched via the API; I did not build or run PR code. All
The ubuntu unit suite (including the recall corpus, the delivery matrix, and the fake-timer budget pins), Desktop Shell on both OSes, and the web-shell smoke all succeeded; the skipped lanes are merge-queue-only by ci.yml design. The central behavioural claim is substantiated beyond the unit suite by a maintainer's real-stack A/B run posted in this thread on head Sandboxed verification would settle the one remaining gap: Not verified: live-turn behaviour on Windows/Linux beyond the CI unit suite (see above), and cancellation during the bounded wait beyond unit tests — both standing caveats, neither blocking. 中文说明代码审查在 独立基线。 对"选中的 memory 错过无工具首轮",我的方案是:在初始消费点加有界等待,settle/abort 可提前退出,exactly-once 守卫,预算未命中留给 ToolResult 消费点——并且由于 selector 往返主导任何短预算,直接把 selector 本来就要评分的确定性候选投出去。对"ASCII-only 且什么都打分的评分器":NFKC + 按文字分组的 token + CJK bigram、词面门槛、字段权重、有界查询窗口。对 200 篇先截断后评估的问题:query 感知的候选集且必须保留某种上限——解除扫描上限后树的规模进入了 100 ms 预算。PR 在每一点上与该基线一致;我没有找到它遗漏的更简路径。 逐项核对结论:
无正确性阻塞项,无规范违规。其余保留项是已记录的设计局限而非缺陷:纯词面匹配对语义型查询保持沉默、单回合计文档数可达 7(fast 2 + refined 5,各自有界)、极慢扫描可能超上限——Risk & Scope 中逐一点名并给出取舍。 测试无人值守 CI 运行——以下证据是 PR 自身 CI 在被审 commit 上的结果,经 API 获取;我未构建或运行 PR 代码。 ubuntu 单测(含 recall 语料、投递矩阵、假计时器预算钉子)、两个平台的 Desktop Shell、web-shell smoke 全部成功;被跳过的 lane 按 ci.yml 设计只在 merge queue 运行。 核心行为声明已有超出单测的实证:线程中 maintainer 在 head 沙箱验证可补齐唯一剩余缺口: 未验证:CI 单测之外的 Windows/Linux 实时回合行为(见上)、单测之外的等待中取消——均为既有保留项,均不构成阻塞。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. Stepping back: this PR started as a bounded-wait fix and grew into the question the measurements kept forcing — "what should the first turn deliver when the selector cannot win a 100 ms race?" The answer it landed on is the honest one: publish the deterministic candidates the selector was already going to score, cap them at two, and let the model's judgement land later at the existing ToolResult point. That is not scope creep; each growth step has a measurement in this thread behind it, including the one that overturned the author's own earlier claim about wait preference — corrected in the final commit, comment and test together, after a maintainer's mutation found it. Against my independent proposal, the PR matches on both halves and exceeds mine where it matters: I would have kept the 200-document cap and lived with old-but-matching documents staying invisible; the candidate-set rework fixes that with a bound still in place, and the asymmetry it introduces for Forget is tracked in #9378 rather than ignored. The implementation reads like it will age well — dense state machine, but every branch has a reason comment and a test that fails when the reason is removed. The evidence chain is complete for what CI can see: green suite on the reviewed commit, a 51-case corpus scored against the frozen pre-change scorer, a scan-latency measurement that decides whether the fast path delivers at all, and a maintainer's real-stack A/B on the commit one back — whose production behaviour is byte-identical to this one. The one gap (live-turn timing on Windows/Linux) is named with its remedy above, and it does not block: the machinery is portable APIs and the platform-split CI is green. Approving, pinned to the reviewed commit. — Qwen Code · qwen3.8-max Reviewed at |
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. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 781 passed · 0 failed · 781 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:781 通过 · 0 失败 · 781 总计 Verification reportPR 8716 Deep Verification —
|
| cell | observable oracle | head | base |
|---|---|---|---|
| AB-A: recall settles after 40 ms | memory prompt in first turn.run request; dispatch latency |
delivered, dispatch @42 ms | missed, dispatch @8 ms |
| AB-B: recall settles synchronously (positive control, both arms) | memory prompt present | delivered @1 ms | delivered @0 ms |
| AB-C: recall never settles | memory absent; recall aborted at dispatch; dispatch latency |
absent, dispatch @100 ms, abortedAtDispatch=false |
absent, dispatch @0 ms |
| AB-D: parent abort @20 ms during wait | memory absent; recall aborted; dispatch < budget | absent, dispatch @21 ms | absent, dispatch @0 ms (abort bridge pre-existing) |
Witnesses: 02-delivery-ab-head-bounded-wait.png, 03-delivery-ab-base-zero-wait-miss.png.
The AB-A row is the load-bearing flip — same harness, same environment, only
client.ts differs, and delivery goes absent → present. AB-C shows the budget is real
(dispatch at exactly the 100 ms timer, never early) and that a deadline miss does not
abort the recall, preserving the ToolResult path. One nuance: sampling
recallSignal.aborted at stream end reads true on both arms whenever the turn
produced no tool call — that is the pre-existing no_safe_delivery_point cleanup, not
a deadline abort; the assertion therefore samples at the dispatch seam.
Cron and ToolResult consume points pass waitMs = 0 (gate is
messageType === UserQuery ? 100 : 0 at the single consume site) — the claimed
zero-wait paths hold by construction; the prefetch itself is only created when managed
memory is available and enabled, so the wait cannot fire when the feature is off.
PR's own tests as A/B control
The HEAD test files were run verbatim against base source: 23 failed | 339 passed
(362) on base vs 362/362 on head (04-pr-tests-on-base-23-red.png). The red set
is exactly the new behavior: 22 recall tests (every CJK/NFKC/gating/window case) plus
should inject auto-memory when recall settles inside the initial wait budget, whose
failure quotes the missing ## Relevant memory prompt in the first request — the
intended assertion, not an import/compile break. The other 339 (all pre-existing
behavior) are green on both arms — no collateral movement on the covered surface.
Scorer fallback A/B (secondary claim 1)
scorer-ab.mjs imports selectRelevantAutoMemoryDocuments from both dist builds and
runs 37 cells: the PR's 31 labeled cases plus six verifier-added siblings of the same
root cause — half-width Katakana (NFKC), a Kangxi radical (NFKC), an astral
Extension-B Han pair (code-point iteration), CJK punctuation splitting, a Latin
fi ligature inside a doc body, and an uppercase-ASCII regression guard.
Head 37/37; base wrong on 21 cells (witness 01-scorer-ab-37-cells-flip.png):
pure-CJK queries return [] on base (zero tokens), and no-match queries select up to 5
arbitrary non-empty-body docs on base (expected [] → got 5 docs), the exact
"selected because the body is non-empty" defect the design doc names. All sibling
shapes flip clean; no cell that base handled correctly regressed on head.
Timing ladder (200 docs = scanner cap, ~0.5 KB bodies, per rung):
| query | head | base |
|---|---|---|
| 2 k ASCII distinct tokens | 5.3 ms | 6.0 ms |
| 5 k | 3.3 ms | 11.0 ms |
| 20 k | 3.6 ms | 42.5 ms |
| 20 k CJK | 2.7 ms | 0.1 ms (zero tokens, early exit) |
Head is flat (the 64-token bound), base grows linearly with query token count — the
"bounded work" claim measured, not read. Query here is the local user's own prompt and
memory docs are user/project files, so this is a robustness bound rather than an
attacker-facing surface; the bound holds either way.
Vacuity and mutation matrix
Reverting is unnecessary as a separate step — the base-source run above is the
whole-hunk revert, and it fails the intended assertions (quoted above). Per-guard
mutations on head (mutation-matrix.sh, junit failure counts of the 42-test suite;
witness 05-mutation-matrix-6-of-6-killed.png):
| mutation | guard | failures/42 | verdict |
|---|---|---|---|
| unmutated control | — | 0 | green |
M1 remove lexicalScore === 0 gate |
no-match gating | 1 | killed |
M2 remove body .slice(0, 1200) window |
surfaced-window scoring | 1 | killed |
M3 remove .normalize('NFKC') |
NFKC | 1 | killed |
| M4 disable bigram emission | CJK bigrams | 15 | killed |
M5 unbounded tokens (edgeSize = ∞) |
token bound | 2 | killed |
| M6 title weight 4→1 (positive control) | field weighting | 4 | killed |
No survivors; the positive control proves the suite can go red. The client-side wait
block is likewise load-bearing (its revert = base arm of the delivery A/B, where the
inject test and AB-A fail). The cancel-during-wait test passes on base too — it pins
cancel semantics that pre-date this PR and cannot discriminate the wait itself; the
delivery A/B cells AB-C/AB-D cover that axis instead. Noted for completeness, not a
defect.
Targeted gates (head)
vitest run src/core/client.test.ts src/memory/recall.test.ts: 362/362 (320 + 42).npm run typecheck -w @qwen-code/qwen-code-core: clean. Liveness proven in both
directions: the same gate reported 14 real errors when my scratch harness had unused
imports, and exited 0 once they were removed.eslinton the four changed files: clean; liveness proven with a planted
no-unused-varsviolation that was reported, then removed.
Reviewer Test Plan walkthrough
| step | status | evidence |
|---|---|---|
| 1. recall settles ≤100 ms → in first request | verified | AB-A (40 ms → delivered), PR inject test |
| 2. pending >100 ms → proceeds, not aborted, ToolResult consumes once | verified | AB-C (dispatch @100 ms, abortedAtDispatch=false), PR ToolResult-delivery test green |
| 3. cancel during wait → wait ends, aborted, no stale delivery | verified | AB-D (dispatch @21 ms, recall aborted, memory absent) |
| 4. labeled multilingual fallback corpus | verified | scorer harness 37/37 incl. bounded work + edge preservation |
| 5. model selector + active-tool filtering unchanged | verified | those tests green on both arms (unchanged code paths) |
Findings
None. No blocking or advisory defects found. Specifically checked and not present:
wait firing when managed memory is disabled; waits on Cron/ToolResult paths; deadlock in
the wait's abort/settle race (worst case resolves via the promise's .then(finish));
listener leak (finish removes itself; the promise handler is bounded by recall
settling); scoring/window inconsistency (both the surfaced body window and the scoring
window are the same raw 1200-char prefix, NFKC applied after slicing on both sides);
regressions on previously-correct ASCII behavior (all English corpus cells green on
both arms).
Not covered
- Per-commit attribution: checkout is depth 2; only the head commit
ec0c746is
reachable,aa0bc54is behind the shallow boundary (verified, not assumed:
rev-list HEAD^1..HEAD^2returns 1 commit vs 2 in the metadata snapshot). The
aggregateHEAD^1..HEADdiff is what was verified. The two commit subjects map
cleanly onto the two changes each A/B isolates (client.ts delivery vs recall.ts
scoring), so the isolation loss is small. - End-to-end run against a real model selector endpoint (the model-primary path is
unchanged by this PR; the heuristic fallback was exercised directly). - The PR's local-bundle E2E claim (
MEMORY_PRESENTwith a mock endpoint) was not
replayed — superseded by the wire-seam delivery A/B above, which observes the same
injection point. - Windows/macOS behavior (pure JS, no platform branches in the diff).
- The 200-document candidate cap, explicitly out of scope per the PR description and
design doc. - Per-commit vacuity of the second commit's hunks in isolation (same reason as
attribution); the mutation matrix covers each guard individually instead.
Methodology
Environment: CI verify container (node v22.23.2, Linux), merge-ref checkout of
refs/pull/8716/merge at depth 2; npm ci + npm run build pre-run at HEAD. The
base control was a git worktree at HEAD^1 with packages/core recompiled via the
root tsc; compiled entry points were realpath-asserted into their own tree and
code-identity-grepped before use. Delivery harness: a verifier-authored vitest file
(byte-identical in both trees, scaffold copied from client.test.ts lines 1–706)
driving the real GeminiClient.sendMessageStream with a mocked Config/transport
environment — the unit under test (consume/wait wiring) runs unstubbed; oracles sample
the turn.run dispatch seam (request parts, latency, recall-signal state). Scorer
harness: direct import of the compiled selectRelevantAutoMemoryDocuments from both
trees. Mutation matrix: perl-applied single-point mutants, suite re-run, file restored
(diff-verified) after each. Raw logs in logs/ (scorer-ab.log, delivery-head.log,
delivery-base.log, prtests-on-base.log, mutation-matrix.log); harness scripts in
this directory; terminal witnesses in evidence/ captured via scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
yiliang114
left a comment
There was a problem hiding this comment.
The implementation paths look correct. I found one non-blocking test gap inline; it does not block merge.
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.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.7)
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 #7040 sets no numeric overhead target; fix attribution
|
Closeout for the automated review findings (addressed in 7a079d1): Fixed (7) — each pin verified by mutation check (the corresponding regression turns the new test red):
Deferred (1) — tokenizer dedup against packages/channels/base: cross-package extraction is out of this PR's scope; replied in-thread, tracked as follow-up. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 813 passed · 0 failed · 813 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:813 通过 · 0 失败 · 813 总计 Verification reportPR 8716 Deep Verification (round 2) —
|
| # | previous-round measurement @ ec0c746 | status at 7a079d1 |
|---|---|---|
| 1 | Delivery A/B: 40 ms settle delivered @42 ms; never-settle held 100 ms, not aborted; abort ends wait @21 ms | stands — re-measured: 30 ms settle delivered @32 ms; never-settle held 100 ms, not aborted; abort ends wait @20 ms |
| 2 | Scorer corpus: head 37/37; base wrong on 21; bounded timing ladder | stands — re-measured on a corpus grown to 38 cells (hiragana doc added by the PR itself): head 38/38; base wrong on 22; ladder flat on head (3.2–8.4 ms), superlinear on base (78→839 ms) |
| 3 | PR tests on base: 23 red = exact predicted new-behavior set | stands — re-measured: 25 red = exact predicted set (+2 client wait tests added by the delta commits) |
| 4 | Mutation matrix: 6/6 killed, 0 survivors | stands — re-measured and extended: 14/14 killed, 0 survivors (delta pins now included) |
| 5 | Gates: typecheck/eslint clean, liveness proven | stands — re-measured clean with fresh liveness probes |
| 6 | Findings: none | stands — none found this round either (see Findings) |
Central claim and A/B proof
Central claim: the UserQuery initial request now waits up to 100 ms for an
in-flight managed-memory recall; a result settling inside the budget is injected into
the first model request; a deadline miss leaves the recall running (not aborted) for
the existing ToolResult delivery point. Secondary claims: (1) the selector-failure
fallback scorer handles CJK/mixed/NFKC queries and stops selecting lexically
non-matching documents; (2) cancellation during the bounded wait ends it early with no
stale delivery.
Control construction: git worktree add tmp/base-tree HEAD^1 (merge-ref checkout, so
HEAD^1 = base tip d91c66119b), packages/core rebuilt there with the repo's own
tsc (node_modules/typescript/lib/tsc.js --build), plus the gitignored
generated/git-commit.ts artifact regenerated with base-HEAD values. The PR touches no
package.json/lockfile (verified with git diff --exit-code), and the base tree
reuses the head's nested packages/core/node_modules (identical versions), so sharing
the root node_modules is a clean control. packages/core has no @qwen-code/*
dependencies; readlink -f on both dist entry points resolved inside their own trees,
so no workspace symlink could leak head code into the control. Emitted code identity
was asserted directly: base dist carries the old split(/[^a-z0-9]+/) tokenizer and no
INITIAL_MEMORY_RECALL_WAIT_MS/RECALL_TOKEN_RUN; head dist the inverse. The only
dist file-list delta was src/generated/git-commit.js (untracked codegen artifact,
display constants only).
Delivery A/B (verifier-authored cells, byte-identical file in both trees, run via the repo's vitest)
| cell | observable oracle | head | base |
|---|---|---|---|
| AB-A: recall settles after 30 ms | memory prompt in first turn.run request; dispatch latency |
delivered, dispatch @32 ms | missed, dispatch @8 ms |
| AB-B: recall settles synchronously (positive control, both arms) | memory prompt present | delivered @0 ms | delivered @0 ms |
| AB-C: recall never settles | memory absent; recall aborted at dispatch; dispatch latency |
absent, dispatch @100 ms, abortedAtDispatch=false |
absent, dispatch @0 ms |
| AB-D: parent abort @20 ms during wait | memory absent; recall aborted; dispatch < budget | absent, dispatch @20 ms, abortedAtDispatch=true |
absent, dispatch @0 ms |
Witnesses: 02-delivery-ab-head-bounded-wait.png, 03-delivery-ab-base-zero-wait-miss.png.
AB-A is the load-bearing flip — same harness, same environment, only client.ts
differs, delivery goes absent → present, and dispatch lands ~2 ms after settle (the
wait ends early, it does not run out the budget). AB-C shows the budget is real
(dispatch at the 100 ms timer, not earlier) and that a deadline miss does not abort
the recall, preserving the ToolResult path. AB-D bounds the abort path.
Cron and ToolResult consume points are zero-wait by construction and now pinned by
tests: the initial consume site sits inside the UserQuery || Cron branch and passes
messageType === UserQuery ? 100 : 0 (mutant C4 → Cron test red), the ToolResult site
relies on the waitMs = 0 default (mutant C6 → ToolResult test red) and is on a
branch the initial site never reaches.
PR's own tests as A/B control
The HEAD test files were run verbatim against base source: 25 failed | 341 passed
(366) on base vs 366/366 on head (04-pr-tests-on-base-red-set.png). The red
set is exactly the new behavior, adjudicated by scripted membership checks: 2 client
tests (the fake-timers budget test — fails with expected "spy" to not be called at all, but actually been called 1 times, i.e. base dispatched inside the 99 ms hold;
and the settle-early test — fails quoting the missing ## Relevant memory prompt) +
18 corpus cases + 5 recall unit tests. The other 341 (all pre-existing behavior) are
green on both arms. The 18 red corpus cases match cell-for-cell the scorer's
independent base OBSERVE set — two instruments, one set. The remaining 4 new client
tests pass on base (cancel semantics and zero-wait paths pre-date the PR; their value
is mutation-pinning at head, verified below), which the red-set adjudication encodes
rather than assuming.
Scorer fallback A/B (secondary claim 1)
scorer-ab.mjs imports selectRelevantAutoMemoryDocuments from both dist builds and
runs 38 cells: the PR's 32 labeled cases plus six verifier-added siblings of the same
root cause (half-width Katakana via NFKC, a Kangxi radical via NFKC, an astral
Extension-B Han pair probing code-point iteration, CJK punctuation splitting, a Latin
fi ligature inside a doc body, and an uppercase-ASCII regression guard).
Head 38/38; base wrong on 22 cells (witness 01-scorer-ab-corpus-flip.png):
pure-CJK/hiragana/NFKC queries return [] on base (zero tokens), and the two
token-bearing no-match queries select 5 arbitrary non-empty-body docs on base
(No lexical match → body-only.md n=5, Unrelated English terms → body-only.md n=5)
— the exact "selected because the body is non-empty" defect. The four token-less
no-match queries return [] on both arms (base via a zero-token early exit, head via
the lexical gate — same outcome, different cause; both encoded). A scripted regression
guard confirms no base-correct cell regressed on head.
Harness note, for honesty: the first scorer run showed 2 FAILs, both adjudicated as
harness defects, not PR defects — one sibling fixture was ambiguous (a decoy doc
legitimately outscored the intended winner under the PR's own field weighting), and
the no-match class was over-encoded (expected all six to be non-empty on base when
only token-bearing queries exhibit the defect). Fixtures/encoding corrected, re-run
green; the pre-fix log is not the one cited.
Timing ladder (200 docs = scanner cap, ~0.5 KB bodies, min of 3):
| query | head | base |
|---|---|---|
| 2 k ASCII distinct tokens | 3.2 ms | 77.9 ms |
| 5 k | 3.9 ms | 196.6 ms |
| 20 k | 8.4 ms | 838.9 ms |
| 20 k CJK | 4.0 ms | 0.05 ms (zero tokens, early exit) |
Head is flat (the 64-token bound); base grows superlinearly in unique token count.
Input provenance is unchanged from the previous round: the query is the local user's
own prompt and memory docs are user/project files, so this is a robustness bound
rather than an attacker-facing surface; the bound holds either way.
Vacuity and mutation matrix
Whole-hunk revert is unnecessary as a separate step — the base-source run above is
the whole-hunk revert, and it fails the intended behavioral assertions (quoted above).
Per-guard single-point mutants on head, each with its own vitest run and
restore-verified (git diff --quiet after every mutant; witness
05-mutation-matrix-scripted-run.png; C6/C7 were run with the identical discipline):
| mutation | guard | failures | killed by |
|---|---|---|---|
| unmutated control (client, filtered) | — | 0/323 | green |
| unmutated control (recall) | — | 0/43 | green |
C1 INITIAL_MEMORY_RECALL_WAIT_MS 100→50 |
budget constant | 1 | budget fake-timers test |
| C7 budget 100→150 | budget constant (upward) | 1 | same test — the pin catches both directions |
C2 drop handle.promise.then(finish, finish) |
settle-early wait exit | 1 | settle-early test |
C3 drop pendingMemoryPrefetch !== handle re-check |
post-wait stale-handle guard | 1 | replaced-during-wait test |
| C4 ternary → always 100 | Cron zero-wait gate | 1 | Cron test |
| C5 drop abort listener in wait | cancel ends wait | 1 | cancel-during-wait test |
C6 waitMs default 0→100 |
ToolResult zero-wait default | 1 | ToolResult zero-wait test |
R1 drop lexicalScore === 0 gate |
no-match gating | 1 | type-boost test's "keyword alone ⇒ []" assertion |
R2 drop body .slice(0, 1200) |
surfaced-window scoring | 1 | window test |
R3 drop .normalize('NFKC') |
NFKC | 1 | NFKC full-width API case |
| R4 disable bigram emission | CJK bigrams | 16 | all pure-CJK/hiragana cases + bounds |
R5 unbounded edgeSize |
token bound | 2 | bounds + tail-refresh |
R6 drop typeBoost |
type boost flips winner | 1 | type-boost flip test |
R7 drop \p{Script=Hiragana} class |
hiragana coverage | 1 | hiragana-only case |
14/14 killed, 0 survivors, positive controls green. Every delta-commit pin claim
("settle-early", "Cron and ToolResult stay zero-wait", "post-wait replacement guard",
"type boost flips the winner", "hiragana-only coverage", "budget changes can no longer
pass unnoticed") is independently mutation-verified — including the four new client
tests that pass on base: each is pinned against a head-only mutant, which is exactly
the shape a regression pin should have. Attribution check: each mutant killed exactly
its named test (R4/R5 the cases that exercise the removed mechanism); no mutant killed
an unrelated test, and no shared-name pre-existing test went red under any mutant.
Targeted gates (head)
vitest run src/core/client.test.ts src/memory/recall.test.ts: 366/366
(323 + 43).tsc --noEmit(packages/core): clean. Liveness proven twice: it reported the real
TS6133/TS6192 diagnostics of my scratch harness (exit 2) before the harness was
removed, and a plantednumber = 'string'probe file produced its diagnostic and
was removed; the post-removal run is exit 0.eslinton the four changed files: clean; liveness proven with a planted unused-var
that was reported, then removed.- Base-control hygiene gates (counted above): lockfile untouched, dist code identity
on both arms, realpath isolation, file-list parity modulo the generated artifact.
Reviewer Test Plan walkthrough
| step | status | evidence |
|---|---|---|
| 1. recall settles ≤100 ms → in first request | verified | AB-A (30 ms → delivered @32 ms), PR inject tests green |
| 2. pending >100 ms → proceeds, not aborted, ToolResult consumes once | verified | AB-C (dispatch @100 ms, abortedAtDispatch=false), PR ToolResult-delivery test green incl. the new recallSignal.aborted === false assertion |
| 3. cancel during wait → wait ends, aborted, no stale delivery | verified | AB-D (dispatch @20 ms, recall aborted, memory absent) + cancel-during-wait test (kills C5) |
| 4. labeled multilingual fallback corpus incl. bounded work, edges, empty for unrelated/single-char | verified | scorer 38/38 + ladder + bounds/refresh/window unit tests |
| 5. model selector + active-tool filtering unchanged | verified | those shared-name tests green on both arms; the diff touches only the wait block and the heuristic scorer |
Findings
None. No blocking or advisory defects found. Specifically checked and not present:
wait firing when managed memory is disabled (prefetch only created when available +
enabled); waits on Cron/ToolResult paths (ternary + branch structure + mutants C4/C6);
stale handle consumed after a mid-wait replacement (guard + C3); deadlock in the
wait's abort/settle race (worst case resolves through the promise's .then(finish));
listener leak (finish removes itself; the promise handler is bounded by recall
settling); scoring/window inconsistency (surfaced window and scoring window are the
same raw 1200-char prefix, NFKC applied after slicing on both sides); regressions on
previously-correct ASCII behavior (scripted no-regression guard over the corpus);
upward budget drift (C7).
Not covered
- Per-commit attribution: checkout is depth 2;
git rev-list HEAD^1..HEAD^2
returns 1 commit locally vs 4 in the metadata snapshot (verified with
rev-parse --is-shallow-repository= true, not assumed). The aggregate
HEAD^1..HEADdiff is what was verified. The delta commits' subjects map cleanly
onto the pins the mutation matrix now verifies individually, so the isolation loss
is small. - End-to-end run against a real model selector endpoint (the model-primary path is
unchanged by this PR; the heuristic fallback was exercised directly). - The PR's local-bundle E2E claim (
MEMORY_PRESENTwith a mock endpoint) was not
replayed — superseded by the wire-seam delivery A/B, which observes the same
injection point. - Windows/macOS behavior (pure JS, no platform branches in the diff).
- The 200-document candidate cap, explicitly out of scope per the PR description and
design doc. - Harness incident, fully recovered: the first mutation-matrix run was invalid —
itsgit checkoutrestore used a cwd-relative path, so restores silently failed and
mutations accumulated (visible in the monotone failure counts). Detected via the
script's own restore checks, the log was quarantined
(mutation-matrix-v1-INVALID-accumulated.log), the tree restored and verified
clean, and the matrix re-run with a hard-abort restore guard. No v1 number is cited
anywhere in this report. This is a verifier-harness defect, not a PR issue. - Environmental note: a direct
tsc --buildofpackages/corereports one TS7016
declaration error in the untouchedshellExecutionService.ts(@lydell/node-pty
exports resolution in the shared rootnode_modules); emission was unaffected and
the repo's owntsc --noEmitgate is clean at head, so this is recorded as an
environment artifact, not a PR regression.
Methodology
Environment: CI verify container (node v22.23.2, Linux), merge-ref checkout of
refs/pull/8716/merge at depth 2; npm ci + npm run build pre-run at HEAD. The
base control was a git worktree at HEAD^1 with packages/core recompiled via the
repo's own tsc and the gitignored generated/git-commit.ts regenerated with base
values; compiled entry points were realpath-asserted into their own tree and
code-identity-grepped before use. Delivery harness: a verifier-authored vitest file
(byte-identical in both trees; scaffold = client.test.ts lines 1–706 plus
verifier cells with runtime arm detection) driving the real
GeminiClient.sendMessageStream with the scaffold's mocked Config/transport
environment — the unit under test (consume/wait wiring) runs unstubbed; oracles sample
the turn.run dispatch seam (request parts, latency, recall-signal state) and write
per-cell JSONL records. Scorer harness: direct import of the compiled
selectRelevantAutoMemoryDocuments from both trees. Mutation matrix: perl-applied
single-point mutants, vitest junit parsed by parse-junit.mjs, file restored and
git diff --quiet-verified after each (hard abort otherwise). All logs were then
cross-checked by adjudicate.mjs (56 scripted checks: red-set membership, encoded
totals, required-red presence, matrix kill/total invariants, delivery cell ranges) —
56/56 pass. Raw logs in logs/ (delivery jsonl + vitest logs, scorer-ab.log,
prtests-on-base.log, mutation-matrix.log + quarantined v1, adjudication.log,
gate logs, test-name lists); harness scripts in this directory; terminal witnesses in
evidence/ captured via scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Local verification on a real stackI built both sides and ran the real CLI against a recording OpenAI-compatible provider, rather than relying on the unit suite alone. Setup — base 1. Delivery behaviour
A and E are the bug this PR is about: on base, a recall that resolves a few tens of ms after the turn starts is simply dropped when the turn has no tool call, and it stayed dropped in 0/10 attempts. D is the fallback's other defect — every non-empty document scored, so an unrelated query pulled three memories into the prompt. 2. The deterministic fallback, side by sideCompiled Base returns nothing for any CJK query (the ASCII tokenizer produces zero tokens) and returns the entire corpus for queries that match nothing. Head returns the expected document in every case, and an empty list for the negative cases including single-character CJK. 3. Latency costMeasured as the gap between the recall side-query hitting the provider and the turn's first request hitting it (median of 3):
So the early-exit path is real: a fast recall costs roughly its own latency, not the full budget, and the worst case stays inside the advertised 100 ms. Projects with no memories pay nothing. 4. Tests and mutation checks
Notes for the merge decision
Behaviour matches what the description claims, the regression surface I probed is clean, and both defects reproduce on base and are fixed on head. No blocking findings from my side. 中文版本本地真实环境验证我没有只跑单测,而是把两侧都完整构建出来,用真实 CLI 打一个会记录请求的 OpenAI 兼容 mock provider。 环境 — base 1. 投递行为
A 和 E 就是本 PR 要修的问题:base 上只要 recall 比回合起步晚几十毫秒,而该回合又没有工具调用,结果就被直接丢掉,10 次尝试全丢。D 则是 fallback 的另一个缺陷 —— 只要文档正文非空就有分数,于是完全无关的 query 也会把三条 memory 塞进 prompt。 2. 确定性 fallback 对比用两棵树编译产物里的 3. 延迟代价以「recall side-query 到达 provider」到「本回合首个请求到达 provider」的间隔衡量(3 次中位数):
也就是说提前退出这条路是真的生效的:快 recall 只花它自己的耗时,不会等满预算,最坏情况仍在承诺的 100 ms 以内;没有 memory 的项目零代价。 4. 测试与变异验证head 上 合并前需要知道的几点
行为与描述一致,我探到的回归面是干净的,两个缺陷在 base 上都能复现、在 head 上都已修复。我这边没有阻塞性发现。 |
|
Qwen Code review request accepted. Review is queued in workflow run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
10 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R5-4 onFastResult contract test — already reported (comment 3767026881)
- R5-5 phase:'fast' telemetry unasserted — already reported (comment 3767026891)
- delivery-eval duplicate metric tautological — already reported (comment 3773476158)
- R5-15 fast-branch surfacedPaths coverage — already reported (comment 3767026973)
- R5-19 discardedDocIds dead accounting — already reported (comment 3767026998)
- R7-7 flowchart 否-edge discard — already reported (comment 3779201375)
- R5-32 exactly-once second-consume untested — already reported (comment 3767027079)
- delivery-eval bypasses active-tool filtering — already reported (comment 3773476201)
- R5-12 INITIAL_BUDGET_MS hand-mirrored constant — already reported (comment 3767026945)
- R4-16 real-time 100 ms wait in rewritten slow-recall test — already reported (comment 3758656077)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/memory/recall-eval.test.ts — no such file or directory; src/memory/recall-delivery-eval.test.ts — no such file or directory; src/memory/recall.test.ts — no such file or directory; src/memory/relevanceSelector.test.ts — no such file or directory; src/memory/memoryLifecycle.integration.test.ts — no such file or directory; and 1 more.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
packages/core/src/memory/recall-delivery-eval.test.ts:227 — [review] anyDeliveryRate docstring claims relevant-doc delivery; the metric counts any deliverypackages/core/src/telemetry/types.ts:1607 — [review] phase docstring says when a result reached the model; discard events hardcode phase:'refined' and strategy:'none' is unenumeratedpackages/core/src/memory/recall-delivery-eval.test.ts:353 — [review] expect(p50).toBeLessThanOrEqual(p95) is a tautology; probe-confirmed defect variants keep it green
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 10 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan(非阻断):src/memory/recall-eval.test.ts — no such file or directory; src/memory/recall-delivery-eval.test.ts — no such file or directory; src/memory/recall.test.ts — no such file or directory; src/memory/relevanceSelector.test.ts — no such file or directory; src/memory/memoryLifecycle.integration.test.ts — no such file or directory; and 1 more。
收敛姿态下延后(第 9 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — ran all 5 rounds (cap) without two consecutive dry rounds; rounds 1-4 kept reporting candidates that verification confirmed or dropped as re-reports of existing threads.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; unit suites ran on Linux only.
Test Plan (not a blocker): src/memory/recall-eval.test.ts — no such file or directory; src/memory/recall-delivery-eval.test.ts — no such file or directory; src/memory/recall.test.ts — no such file or directory; src/memory/relevanceSelector.test.ts — no such file or directory; src/memory/memoryLifecycle.integration.test.ts — no such file or directory; and 1 more.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
packages/core/src/core/client.ts:929 — [probe] Cron turns can synchronously inject the deterministic fast memory resultpackages/core/src/memory/recall-delivery-eval.test.ts:45 — [probe] RECALL_AT hand-mirrors unexported MAX_RELEVANT_DOCS with no comment or importpackages/core/src/memory/__fixtures__/auto-memory-recall-eval.json:377 — [probe] NFKC halfwidth-katakana composition axis pinned by no testpackages/core/src/memory/__fixtures__/auto-memory-recall-eval.json:63 — [probe] Placeholder-body strip (_No entries yet._) pinned by no eval casepackages/core/src/memory/recall-delivery-eval.test.ts:276 — [probe] Delivery table prints two structurally identical first-turn rows as distinct metricspackages/core/src/memory/recall-eval.test.ts:350 — [probe] maxSelectedDocs floor cannot pin the production selection cappackages/core/src/memory/__fixtures__/auto-memory-recall-eval.json:363 — [probe] Pure-ASCII query categorized 'mixed' dilutes the mixed-script slicepackages/core/src/memory/recall-delivery-eval.test.ts:227 — [probe] anyDeliveryRate docstring claims 'relevant document' but the metric counts any delivered doc
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — ran all 5 rounds (cap) without two consecutive dry rounds; rounds 1-4 kept reporting candidates that verification confirmed or dropped as re-reports of existing threads。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; unit suites ran on Linux only。
Test Plan(非阻断):src/memory/recall-eval.test.ts — no such file or directory; src/memory/recall-delivery-eval.test.ts — no such file or directory; src/memory/recall.test.ts — no such file or directory; src/memory/relevanceSelector.test.ts — no such file or directory; src/memory/memoryLifecycle.integration.test.ts — no such file or directory; and 1 more。
收敛姿态下延后(第 9 轮,非阻断)——已记录,本轮不要求修改:共 8 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
Follow-up issues filed for the deferred items from the review rounds, so nothing is silently dropped:
The remaining round-9 deferred items are test-harness polish (docstring wording, tautological assertion, hand-mirrored constants) recorded in the review ledger above and intentionally not blocking per the convergence posture. |
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.
… 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.
…kenization
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.
Summary: what this solves, how, and what it touchesPosting a roll-up now that the review follow-ups have landed ( The problemTwo independent failures, both of which made managed memory unreliable rather than merely imperfect. Selected memory did not reach the model. Recall starts asynchronously on UserQuery, and the initial request consumed it with a zero-wait poll — so memory made it into the first prompt only when the timing happened to work out. When it didn't, delivery fell through to the ToolResult point, which a turn that makes no tool call never reaches; the result was discarded as Selection itself was broken outside ASCII, and too permissive inside it. The deterministic scorer tokenized only The solutionDelivery. One recall lifecycle and model-primary selection, with a single deterministic stage in front of it — deliberately not RFC #7040's original two-result shared-scan architecture, whose cross-phase bookkeeping is the source of the duplicate-injection bug class the RFC itself warned about. The initial wait is a ceiling, not a fixed cost — it ends on whichever comes first: recall settling, the deterministic result being published, cancellation, or 100 ms. That detail turned out to matter more than expected; see the measurement below. Selection. NFKC normalization; whole-run tokens for non-CJK letters ( What the measurements changed about the designThe original evidence cited deterministic scoring latency (~0.03 ms) as proof the budget was cheap. That measured the wrong thing. The fast result is only published once recall has enumerated, read, and parsed the memory tree — and this PR removed the 200-document cap for recall, so that scan grows with the tree.
At 1000 topics the split is ~140 ms scan against ~8 ms ranking, so the scan is the whole story. Two conclusions followed. For an ordinary tree the fast result was in hand tens of milliseconds before the ceiling and the rest of the budget was spent waiting on a selector this design already assumes will miss it — which is why the wait now ends on the fast result. Past roughly a thousand topics the scan alone exceeds the ceiling, so the turn pays the full budget and delivers nothing; that is recorded as a known limitation rather than fixed, since the real fix is a persistent catalog and that stays out of scope. Impact assessmentWho is affected. Only sessions with managed auto-memory enabled and at least one memory topic. With memory off or the tree empty there is no pending prefetch and no wait, so the path is inert. What is deliberately not affected. The Latency. Previously zero added latency. As originally written this PR added up to a fixed 100 ms to every UserQuery turn with memory enabled. As it stands now it adds the scan time — single-digit milliseconds for a typical tree of tens of topics, ~29 ms at 200 — bounded at 100 ms. User-visible behaviour changes.
Observability. Known limitations, stated rather than hidden. The fast result carries no model judgement. Queries sharing no token with their document produce nothing on the deterministic path, so a tool-free turn asking one still gets nothing — the Rollback. No migration, no persisted state, no new dependency, no new public setting. A revert is clean. Verification
|
Local verification on a real stack — head
|
| arm | main req − selector req | fast-delivery latency_ms |
end-to-end vs base |
|---|---|---|---|
| base, query matches | −4 ms | n/a | — |
| head, query matches | +1 ms | 31 ms | +39 ms |
| head, no lexical match | +79 ms | n/a | +82 ms |
| head with the early exit removed | +78 ms | 124 ms | +76 ms |
| empty memory tree | — | n/a | no measurable cost |
Both mutations are one-line edits to head's own bundle, so nothing else moves:
handle.fastResultRef.onArrive = finish→= void 0(the behaviour before01ef7d7d): the wait runs to the ceiling, fast delivery latency goes 31 ms → 124 ms. The early exit is doing exactly what the commit message claims.b.doc.mtimeMs - a.doc.mtimeMs→a.doc.type.localeCompare(b.doc.type): with three documents tied on score the fast result flips fromtie-user.md, tie-reference.mdtotie-project.md, tie-reference.md— the user document is dropped, which is the failure the tie-break comment describes.
3. Observations for the reviewer
None of these are defects; they are consequences worth having on the record before merge.
- On a tool-free turn the selector's judgement effectively never reaches the first prompt once the deterministic scorer matches anything.
onFastResultfires immediately before the selector request is issued, sohandle.settledAtis still null when the wait ends and the "prefer a settled recall over the fast result" branch below it is unreachable. Measured with the selector settling in 15 ms — comfortably inside the budget — head still injects the heuristic pair and discards the model's two picks asno_safe_delivery_point; the same bundle with the early exit removed injectsrefined / modelinstead. The PR body lists "the fast result carries no model judgment" as the main risk; this measurement shows the trade is not limited to slow selectors. It is still strictly better than base, which delivered nothing in that same run. - Per-turn document total reaches 7, confirmed end-to-end (2 fast + 5 disjoint refined in one turn, against 5 on base) — exactly as the PR body documents.
- Candidate count drops in the under-cap case, also as documented: 150 project + 150 user documents, neither scope over 200 → base sends 300 manifest lines to the selector, head sends 94 (the 25 KB manifest budget binds well before the 200-document cap). Fewer, but query-ranked, and in the over-cap case that is the difference between the matching document being first and being absent.
- The scan gate is ~3× faster here than the PR body's Linux numbers: 200 topics 9.1 ms, 500 topics 21.4 ms, 1000 topics 46.4 ms — all inside the 100 ms ceiling. The stated "past roughly a thousand topics the scan exceeds the ceiling" did not reproduce on this machine, so that limitation is machine-dependent and the PR body states the conservative case.
4. Gates on macOS (the row the PR body marks as not rerun)
Head worktree, macOS 26.6 / arm64 / Node 24.18.1:
packages/coresrc/memory+src/core/client.test.ts— 37 files, 929 passed, 0 failed.team-memory-sync.test.ts > unstages the team path when the commit fails, which the PR body reports as an unrelated pre-existing failure on Linux, passes here.- The two eval files reproduce the PR body's tables exactly (overall Recall@5 45.2% → 92.9%, top-1 46.2% → 97.4%, cjk 0% → 100%, delivery 0% → 92.9% at every latency scenario above the budget, duplicate delivery 0%).
tsc --noEmit -p packages/core— clean, no output (thesharperror the PR body mentions is not present on this checkout).eslinton the five changed source files — clean.
5. Not covered
Cancellation inside the initial window (Ctrl-C / Esc) was not driven; Windows was not run; selector latency is injected on loopback rather than a real network round trip; the ordinary-corpus scenarios use 7 documents, with scale exercised only in the 251- and 300-document runs; single machine, so the absolute timings are indicative, not portable.
Assessment. Every behavioural claim in the PR body that I could drive end-to-end reproduces on a real stack, and the two mutation checks show the newest commit's two lines are load-bearing. The documented trade-offs are real and I measured both directions of each. Observation 1 is the only item I would want a deliberate answer on before merge — it is a design choice the PR already names, just broader in scope than "slow selectors". From my side this is merge-ready.
中文说明
本地真实环境验证 —— head 01ef7d7dc4
我重新构建了两侧,用真实的 CLI bundle 打到一个会记账的 OpenAI 兼容 provider 上,磁盘上放真实的 .qwen/memory 树,而不是只看单测。这次是在我上一轮验证(当时 head 是 6085ef0)之后新增的两个 commit 上重新验证:8a5a524(按 mtime 的同分排序、所有丢弃路径都记 already_delivered)和 01ef7d7d(初始等待在 fast 结果处结束、扩大分词覆盖)。
环境。 base 用 merge-base 9a5b07c0b1,head 用 01ef7d7dc4,各自独立 worktree 里 npm install + npm run build + npm run bundle,每次运行都用该树自己的 dist/cli.js。先跑 bundle 完整性闸门:head 里有 onFastResult / fastDeliveredPaths / already_delivered / Script=Hiragana token 类,base 一个都没有。每次运行都用一次性项目目录 + QWEN_CODE_MEMORY_LOCAL=1,隔离的 HOME 和 QWEN_CODE_MEMORY_BASE_DIR,语料写成真实 topic 文件并控制 mtime。provider 按报文里的 Available memories: 标记识别 recall 侧查询,可以独立地延迟或让它失败;主模型的回复直接回显「这次请求里带了哪些 ## Relevant memory 文档」,所以对话记录本身就说明了投递结果。每次运行三个判据面:provider 请求台账(模型真正收到了什么,带到达时间戳)、本地文件 exporter 导出的 qwen-code.memory.recall{,.delivery} 遥测、以及 TUI。
1. 交付
同一个 query、同一棵 memory 树、同一个 provider、无工具的首轮 —— base 的首个 prompt 是空的,head 投递成功(见上方英文部分的截图;第二张是 CJK 场景,改动前的 tokenizer 对它根本产生不了 token)。
逐条来自台账和遥测:
- 无工具回合、selector 超预算。 base:初始请求里没有 memory,recall 之后带着 3 篇模型选中的文档 settle,最终以
no_safe_delivery_point丢弃。head:phase=fast, delivery_point=initial, strategy=heuristic, docs_selected=2, latency_ms=32,两篇文档确实在第一个请求体里。 - 去重。 有工具的回合里,head 的第二个请求带了两个 memory 块 —— fast 的两篇,加上只含
release-checklist.md的 refined 块,即唯一一篇没被 fast 投过的文档(refined / tool_result / model / docs_selected=1)。没有任何文档出现两次。base 则在tool_result一次性投三篇。 already_delivered记账。 selector 恰好重选了 fast 那两篇时,head 记already_delivered,base 记no_safe_delivery_point。部分重叠时(选中 3 篇、fast 投过 2 篇)head 正确保留了原取消原因。- Scorer 精度。 selector 失败、query 与任何文档都没有词面重叠:base 投了 5 篇完全不相关的文档;head 什么都没投(
no_relevant_results)。 - Scorer 分词。 selector 失败、中文 query:base 是
strategy=none, docs_selected=0—— query 产生了零个 token;head 选中了cjk-deploy.md。 - 候选覆盖。 单个 scope 251 篇、唯一词面命中的那篇最旧:base 的 manifest 是 200 行且目标不在里面;head 的 manifest 是 96 行且目标是第 1 行,随后被投递。
2. 开销,以及对 head bundle 的两处单行变异
每个 arm 跑 5 次取中位数。在 provider 侧量「selector 请求」到「主请求」的间隔,可以把 CLI 约 1.2 s 的启动噪声抵消掉——否则 100 ms 的预算会被完全淹没。
| arm | 主请求 − selector 请求 | fast 交付 latency_ms |
相对 base 的端到端 |
|---|---|---|---|
| base,query 命中 | −4 ms | 不适用 | — |
| head,query 命中 | +1 ms | 31 ms | +39 ms |
| head,无词面命中 | +79 ms | 不适用 | +82 ms |
| head 去掉提前退出 | +78 ms | 124 ms | +76 ms |
| 空 memory 树 | — | 不适用 | 无可测开销 |
两处变异都只改 head 自己 bundle 里的一行,其它一切不变:
handle.fastResultRef.onArrive = finish→= void 0(即01ef7d7d之前的行为):等待跑满上限,fast 交付延迟从 31 ms 变成 124 ms。这一行确实在做 commit 说明里声称的事。b.doc.mtimeMs - a.doc.mtimeMs→a.doc.type.localeCompare(b.doc.type):三篇同分文档时,fast 结果从tie-user.md, tie-reference.md翻成tie-project.md, tie-reference.md—— user 文档被挤掉,正是 tie-break 注释里描述的那种失败。
3. 给 reviewer 的观察
以下都不是缺陷,但合入前值得留档。
- 无工具回合里,只要确定性 scorer 有命中,selector 的判断实际上永远进不了第一个 prompt。
onFastResult是在 selector 请求发出之前紧挨着触发的,所以等待结束时handle.settledAt必然还是 null,下面那段「优先用已 settle 的 recall 而不是 fast 结果」的分支不可达。实测让 selector 在 15 ms 内返回(远在预算之内),head 仍然注入 heuristic 的那两篇,把模型选的两篇按no_safe_delivery_point丢掉;同一个 bundle 去掉提前退出那一行后,注入的是refined / model。PR 正文把「fast 结果没有模型判断」列为主要风险;这次测量说明这个取舍并不只发生在 selector 慢的时候。不过它仍然严格优于 base —— 同一次运行里 base 什么都没投。 - 单回合文档总量会到 7 篇,端到端确认(一轮里 2 篇 fast + 5 篇不重叠的 refined,base 是 5 篇)—— 与 PR 正文写的完全一致。
- 未超旧上限的场景里候选数确实变少,也与正文一致:150 篇 project + 150 篇 user、两个 scope 都不超 200 时,base 给 selector 送 300 行 manifest,head 送 94 行(25 KB 的 manifest 预算比 200 篇的数量上限先生效)。变少了,但是按 query 排过序的;而在超上限的场景里,这个差别就是「命中文档排第一」和「命中文档根本不在」的区别。
- 本机的扫描闸门比 PR 正文的 Linux 数字快约 3 倍:200 篇 9.1 ms、500 篇 21.4 ms、1000 篇 46.4 ms —— 全部在 100 ms 上限之内。正文里「超过约 1000 篇时扫描本身就超上限」在本机没有复现,所以这个限制与机器相关,正文取的是保守的那一侧。
4. macOS 门禁(正文里标为「本轮未重跑」的那一行)
head worktree,macOS 26.6 / arm64 / Node 24.18.1:
packages/core的src/memory+src/core/client.test.ts—— 37 个文件,929 passed,0 failed。正文里报为「无关的既有失败」的team-memory-sync.test.ts > unstages the team path when the commit fails,在这里是通过的。- 两个 eval 文件的表格与正文完全一致(overall Recall@5 45.2% → 92.9%,top-1 46.2% → 97.4%,cjk 0% → 100%,超预算的每个 latency 场景交付 0% → 92.9%,重复交付 0%)。
tsc --noEmit -p packages/core—— 干净,无输出(正文提到的sharp类型报错在这个 checkout 上不存在)。- 对 5 个改动源文件跑
eslint—— 干净。
5. 未覆盖
初始窗口内的取消(Ctrl-C / Esc)没有驱动;Windows 没跑;selector 的延迟是在 loopback 上注入的,不是真实网络往返;普通语料场景只有 7 篇文档,规模只在 251 篇和 300 篇那两组里验证;单机结果,绝对耗时只作参考。
结论。 PR 正文里所有我能端到端驱动的行为性主张,在真实链路上都复现了;两处变异检查也证明最新那个 commit 的两行是起作用的。文中记录的取舍都是真的,我把每一项的两个方向都量了。第 3 节第 1 条是我合入前希望有个明确态度的唯一一项 —— 它是 PR 已经点名的设计取舍,只是覆盖面比「selector 慢的时候」更广。就我这边而言,可以合入。
…a test Local end-to-end verification on #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.
|
Thanks — the two single-line bundle mutations are the part I want to call out. Asserting that a line is load-bearing is cheap; showing that flipping it reproduces the exact failure the comment describes is not, and both of them landed on commits from the last day. Answering Observation 1 directly, since you asked for a deliberate one, and then two corrections your numbers force. Observation 1 — you are right, and the code said otherwiseYou are correct, and the claim I wrote in
One refinement to the framing: the branch is not unreachable, it is reachable only where no fast result exists — no The decision: the behaviour stays, the description was the defect. The trade you measured is real but its cost is bounded by something your harness deliberately removes. Selector latency is injected on loopback; in production this is a model side query with a 30 s abort ceiling, and it does not return inside 100 ms. So the case where arbitrating would win is one that essentially does not occur, while the cost of preserving the option is the remainder of the budget on every turn — your own table prices that at +39 ms versus +76 ms end-to-end. Paying ~37 ms per turn to win a race that does not start is the wrong side of that trade, and the selector's judgement is not lost either way: it lands at ToolResult with the fast documents excluded. What was missing is that this reads as an accident. Fixed in
Observation 3 — the document cap is not the binding constraint, and my docs said it wasYour 300 → 94 measurement is the more interesting number, and it does not follow from anything I documented. I had written the reduction as "at most 200 in total", implying Two consequences now written into Observation 4 — crossover is machine-dependent, corrected9.1 / 21.4 / 46.4 ms against my 29 / 70 / 130 ms is a 3× spread, so "past roughly a thousand topics the scan exceeds the ceiling" was never a topic count. It is now stated as a machine-dependent crossover — roughly a thousand topics on the slower of the two machines, not reached at all on the faster one — in the design doc, the limitations list, and the PR body, with the table labelled as the conservative side. The two environment failuresBoth of the caveats in my "Tested on" row turn out to be my machine, and your macOS run is what settles it: On section 5Cancellation inside the initial window is covered by unit tests rather than driven end-to-end — Head is now |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 1034 passed · 0 failed · 1034 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:1034 通过 · 0 失败 · 1034 总计 Verification reportPR 8716 verification — fix(memory): improve recall delivery and multilingual fallbackVerdict: 中文摘要
Central claim + A/BCentral claim: on a tool-free first turn, selected memory actually reaches the model — a bounded ≤100 ms wait ends on the deterministic fast result, which is injected when the selector is still in flight; the refined result lands later with fast-delivered docs excluded. Harness
Recall-seam A/B ( Secondary claim — multilingual selection ( Secondary claim — bounded candidates ( Manifest wire oracle ( PR's own eval tables reproduce exactly on this container (witness Findings (non-blocking)F1 — F2 — the eval's CJK slice is insensitive to bigram granularity. Mutation M5 (Han/Hiragana/Katakana removed) survives F3 — a Cron turn can opportunistically receive an already-published fast result. The fast-injection branch does not check Not covered
MethodologyEnvironment: Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.14. |
) * fix(memory): scan uncapped when selecting forget candidates Recall moved to the uncapped scanner in QwenLM#8716; forget did not. A document ranked past the 200-document cap could be recalled and injected into the prompt but never forgotten. Forget now scans uncapped, so its candidate universe matches recall's. The model-selection prompt renders every candidate, so it gets its own bound of 400: literal query matches first, then the most recently modified remainder. The heuristic fallback keeps scanning the full uncapped list. Indexer, status, and extraction stay capped on purpose, and the two design docs that recorded forget as capped now say otherwise. Refs: QwenLM#9378 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(memory): give each scope its own share of the forget prompt Review round 1. The 400-candidate bound ranked both scopes into one recency budget, so a store whose project entries are all newer than its user entries seated no user memory at all. The capped scanners this replaced ran per scope, so each scope always had seats. That made an old user entry unselectable by the model while recall could still inject it, which is the same asymmetry the PR set out to close. Each scope now keeps a 200-candidate quota and whatever a smaller scope leaves is handed to the other. Within a scope, literal query matches rank first and both groups are ordered newest first, so truncation is deterministic instead of scan-order, and the bound logs when it drops candidates. Also from review: the query normalisation and match predicate are now shared with selectByHeuristic so the two cannot drift; the user scan gets the best-effort guard recall.ts and extractionAgentPlanner.ts already carry; and the docstring and design docs no longer claim an unconditional guarantee the bound does not provide. Three tests, each verified against the mutation it is meant to catch: global ranking drops the user ids, an ascending sort drops the newest filler, and handing the fallback the bounded list returns 400 of 450 matches. Refs: QwenLM#9378 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(memory): bound the unconfirmed forget path and drop the silent scan guard Review round 2, all suggestions. MemoryManager.forget passed limit: MAX_SAFE_INTEGER and deletes without confirmation. With an uncapped scan and a heuristic fallback that substring matches the whole store, a one-character query matched nearly every entry in both scopes, where the capped scanners had held that same failure to one scan's worth of candidates. The limit is now the prompt bound, restoring the old ceiling. Round 1 added a best-effort catch on the user scan. That was wrong on two counts: scan.ts caps after reading and ordering the whole tree, so uncapping adds no read exposure to justify it, and swallowing the failure made forget report "no entries matched" for a scope it never read, then act on that answer by deleting. Reverted, with a comment saying why forget differs from recall here: a missed injection is recoverable, a missed deletion is not. normalizeForgetQuery now delegates to normalizeSummary so query matching and the post-selection re-match cannot drift apart, and one design-doc sentence no longer implies only semantic matches fall off the bound. Two tests, each verified against its mutation: the quota split is now exercised with both scopes over quota, where dropping it to 150 seats 250 project entries instead of 200; and the delete ceiling fails at 401 removals if the unbounded limit comes back. Refs: QwenLM#9378 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(memory): split forget's deletion seats per scope, and decouple the ceiling Review round 3. The deletion ceiling added last round truncated the heuristic fallback in candidate order, and listIndexedForgetCandidates pushes every user entry ahead of every project entry. With 450 matching user entries and 50 matching project ones and the side query down, forget deleted 400 user entries, zero project ones, and reported success. That is the reachability asymmetry this PR exists to remove, moved into the delete path. The per-scope allocation the model prompt already used is now shared with the heuristic, so each scope keeps its share of the limit and a smaller scope's unused seats go to the other. The ceiling is also its own constant now rather than an alias of the prompt bound. Resizing the model prompt is a cost decision and resizing this is a blast-radius decision; sharing one constant let the first silently widen the second. Two tests, each checked against its mutation: the 450-user/50-project shape returns zero project matches under a plain slice, and oldest-first ranking inside a scope drops that scope's newest entry from the prompt. Refs: QwenLM#9378 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(memory): pin the forget split at a small limit and the heuristic's own order Cross-review found both new tests mutation-survivable. Every case used a 400 limit, so hard-coding a 200 per-scope quota instead of deriving it from the budget still passed, and the recency case let the side query succeed, so it pinned the model prompt's ranking rather than selectByHeuristic's own comparator. One case at limit 5 with the side query failing covers both: it asserts the 3/2 split, which only holds if the quota comes from the budget, and that each scope contributes its newest entry, which fails if the comparator is reversed. Both mutants verified failing. Refs: QwenLM#9378 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(memory): share the forget recency comparator and log a bound deletion Review round 4, both suggestions. The mtime comparator was the last thing the model path and the heuristic path each typed for themselves, after this branch had already hoisted the query normaliser, the match predicate and the per-scope allocator so the two could not drift. Each site has its own test, so a one-sided ordering change would have updated its own test, passed CI, and left the sibling stale. Now one definition. The deletion cap also bound silently. The prompt bound warns when it truncates; the path that actually deletes did not, so a forget that removed 400 of 500 matches reported success and left no record of why recall kept injecting the rest. It now says so. No test for the new warning: it is a debug log line, and asserting on it would pin the wording rather than the behaviour. Refs: QwenLM#9378 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>























What this PR does
Makes native memory recall reliable in two independent ways: selected memory actually reaches the model, and selection works outside ASCII.
Delivery: recall gets a 100 ms ceiling before the initial user request — not a fixed cost. The wait ends on whichever comes first: recall settling, the deterministic result being published, cancellation, or the ceiling. A result that settles inside it is delivered immediately. If it does not, the deterministic candidates that
selectModelCandidateDocumentsalready computed for the model manifest are injected instead of nothing, capped at two documents. Recall stays alive, so the model-selected result still lands at the existing same-query ToolResult delivery point, with fast-delivered documents excluded and the prompt rebuilt from the remainder.Selection: the deterministic scorer, which now serves both the fast path and the selector-failure fallback, gets Unicode NFKC normalization, whole-run tokens for non-CJK letters (
\p{L}-based rather than[a-z0-9], so Cyrillic, Greek, Arabic, and accented Latin produce tokens instead of none), CJK code-point bigrams, isolated-CJK rejection, lexical-match gating, field-aware weighting, and bounded query tokens. Score ties break by recency and then by input order, never by document type — an alphabetical type comparison ranksuserbehind every other type, and with the fast result capped at two documents that would systematically drop user-level memory from exactly the turn the fast path exists to serve. Recall also swaps the shared scanner's per-scope 200-document cap for a global, query-aware candidate set with a recency reserve and a bounded manifest. This is a change of truncation key, not a lifted ceiling: at or under 200 documents nothing was excluded by count before either, and 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 to the selector and the new one sends at most 200, so fewer reach the model; a scope over 200 is the case the change is actually for, where an old but lexically matching document was permanently invisible.Model-first selection, cancellation, exactly-once delivery telemetry, active-tool filtering, and the existing prompt limits are preserved. No new dependency and no new public setting.
Why it's needed
The initial request originally did a zero-wait poll, so memory reached the first prompt only by luck. A 100 ms budget was the first fix, but measurement showed it is not sufficient alone: recall awaits the model selector, a network side query with a 30 s abort ceiling, so the budget is dominated by round-trip time rather than scheduler jitter. It expires on the common path, delivery falls through to ToolResult, and a turn that makes no tool call never reaches one, so the result is discarded as
no_safe_delivery_point. Tool-free turns are exactly the short, context-answered questions where user memory matters most.Separately, the deterministic fallback was ASCII-oriented and gave every non-empty document a positive score, so it could select non-matching documents and could not tokenize ordinary CJK queries.
Reviewer Test Plan
How to verify
The two eval files print their before/after tables to stdout with
--silent=false.Behavioral cases to confirm by review: a result settling inside the budget is delivered initially; a budget miss delivers the deterministic result and leaves recall alive for ToolResult; the later delivery never repeats a fast-delivered document; cancellation inside the initial window delivers nothing; a fast result never crosses a query boundary; no-result queries stay silent; a score tie does not push a user-typed document out of the two-document fast result; a result whose every document was already fast-delivered is discarded as
already_deliveredrather than as a lost one, while a partial overlap still reports the cancellation reason.Evidence (Before & After)
Full numbers, method, and limitations are in #8716 (comment).
Recall quality (
recall-eval.test.ts, 51-case / 25-document labeled corpus scored against a frozen copy of the pre-change scorer):Corpus scale, so the headline is readable: 25 documents, 51 cases. A query-blind scorer returning 5 random documents scores 20.0% Recall@5 on this pool, and a test keeps that floor at or below 25% with the measured result well clear of it.
The overall figures are held below 100% by the
semantic-no-lexicalslice: answerable queries that share no token with their document. Both the shipped scorer and the frozen pre-change one return nothing for all of them, so requiring a lexical match did not create that gap — but it does keep the deterministic path silent there, and the slice is measured on its own rather than folded into the quality floor.Delivery (
recall-delivery-eval.test.ts): above the budget, tool-free first-turn delivery improves 0% -> 92.9%; tool-using turns remain delivered one request later; duplicate delivery remains 0%.Scan latency (
recall-scan-latency.test.ts, new) is the measurement that was missing, and it is the one that decides whether the fast path delivers at all. Deterministic scoring costs p50 0.036 ms / p95 0.053 ms — but the fast result is only published once recall has enumerated, read, and parsed the memory tree, and this PR removed the 200-document cap for recall. Measured against a real temporary tree:Two consequences. For any tree small enough to scan in time — the ordinary case, tens of topics — the fast result is in hand long before the ceiling, so the wait now ends there rather than spending the rest on a selector this design already assumes will miss the budget. On slow enough I/O the scan alone exceeds the ceiling, and the turn then pays the full budget and still delivers nothing — worse than the zero-wait behaviour this PR replaced. The crossover is machine-dependent, not a fixed topic count: an independent run on faster hardware measured 9 / 21 / 46 ms for the same three sizes and never reached the ceiling. The table above is the conservative side. Ending the wait early bounds this case rather than fixing it, and the real fix (a persistent catalog) stays out of scope.
The residual 7.1% of tool-free delivery is exactly the
semantic-no-lexicalslice, asserted separately: the fast path closes the timing gap, not the matching gap. Only the model selector can serve a query with no lexical match, and on a tool-free turn it never lands.Selector latency is modeled rather than measured because a network round trip cannot be timed in a unit test, so delivery results are reported per latency scenario. Note that the delivery eval simulates the two designs rather than driving
tryConsumeMemoryPrefetch, so its duplicate-delivery figure is a property of the simulation; the shipped dedupe is covered byclient.test.ts("does not re-deliver a document the fast phase already injected"), which was verified to fail when the exclusion filter is removed.Tested on
01ef7d7d(macOS 26.6 / arm64 / Node 24.18.1): 37 files, 929 passed, 0 failed.tsc --noEmit -p packages/coreclean — thesharperror below does not reproduce there.eslintclean. Both eval tables reproduce exactly. See this verification commentpackages/coresrc/memory/+src/core/client.test.ts— 928 passed. One unrelated pre-existing failure,team-memory-sync.test.ts > unstages the team path when the commit fails, which reproduces on the untouched branch head and passes on macOS, confirming it is environmental (sandbox git-hook).eslintclean;tsc --noEmit -p packages/corereports only the pre-existingsharpSharpConstructortype error insrc/utils/image-view.tsEnvironment (optional)
Not applicable.
Risk & Scope
onFastResultis published before recall issues the selector request, so the recall promise is never settled when the wait ends on it; the settled-recall branch is reached at the initial delivery point only when no fast result exists. Verified end-to-end against a selector settling in 15 ms: the deterministic pair is delivered and the model's picks are discarded, where the pre-change build delivered nothing at all on that same turn. This is deliberate — a model side query does not return inside a 100 ms ceiling in production, so arbitrating would spend the rest of the budget every turn to win a race that does not happen — and the selector's judgement still reaches the model at ToolResult with the fast documents excluded. Two documents bounds the cost of being wrong.MAX_RELEVANT_DOCS = 5bounds one prompt, not one turn. A fast delivery of two documents followed by a refined delivery of five disjoint ones puts seven in front of the model — dedupe removes repeats, not the sum. This follows directly from dropping combined fast/refined budget accounting; both prompts stay individually bounded and each body is still truncated toMAX_DOC_BODY_CHARS, so the worst case is bounded and small, but it is not five. If a hard aggregate ceiling is wanted, the cheap version is passinglimit - fastDeliveredPaths.sizeas the refined limit.phaseis the delivery stage andstrategyis the selection method. Afastdelivery is alwaysheuristic; arefineddelivery ismodelnormally andheuristicwhen the selector failed.MemoryRecallDiscardReasongainsalready_delivered, which is now recorded on every discard path — not only at the ToolResult consume point — whenever the fast phase had already delivered every selected document, so theno_safe_delivery_pointbucket stops counting turns that did get their memory. A partial overlap still reports the cancellation reason.Known limitations:
ownerinsideownership). The corpus records one such case rather than hiding it.semantic-no-lexicalslice measures this; it is a limit the fast path does not remove.Linked Issues
Refs #7040.
中文说明
本 PR 做了什么
本 PR 从两个互相独立的方面提升原生 memory recall 的可靠性:一是选中的记忆确实进入模型,二是非 ASCII 查询也能正确参与选择。
交付方面:初始用户请求前,recall 会获得 100 ms 上限(不是固定开销)。等待以先到者为准结束:recall 完成、确定性结果发布、取消,或到达上限。如果结果在预算内完成,就立即交付。如果没有完成,则注入
selectModelCandidateDocuments已经为模型 manifest 计算出的确定性候选,而不是注入空结果,最多两个文档。recall 仍会继续运行,因此模型选择出的结果仍能在现有同 query 的 ToolResult 交付点进入请求;已经 fast 交付过的文档会被排除,prompt 会用剩余结果重建。选择方面:确定性 scorer 现在同时服务 fast path 和 selector 失败 fallback,并增加 Unicode NFKC 归一化、非 CJK 字母整串 token(基于
\p{L}而非[a-z0-9],因此西里尔、希腊、阿拉伯和带重音拉丁文都能产生 token)、CJK code-point bigram、孤立 CJK 拒绝、词面匹配门禁、字段加权和有界 query token。同分时按 mtime 降序、再按输入顺序排列,不再按文档 type——type 字典序会把user排在其他所有类型之后,而 fast 结果只取两篇,这会系统性地把用户级 memory 排除在 fast path 最该服务的那类回合之外。recall 还把 shared scanner 按 scope 各自 200 篇的上限,换成了全局、感知 query 的候选集(recency reserve + 有界 manifest)。这是换了截断依据,不是抬高了天花板:总量 ≤200 时两种设计都不按数量丢弃,但新增的 25,000 字节 manifest 上限是旧路径没有的;总量在 200–400 且单个 scope 不超 200 时,旧路径会把全部文档送给 selector,新路径最多送 200 篇,候选反而变少;只有单个 scope 超过 200 时才是这次真正要解决的场景——老而词法命中的文档原本永久不可见。模型优先选择、取消、exactly-once 交付 telemetry、active-tool 过滤和现有 prompt 限制都保持不变。没有新增依赖,也没有新增公开设置。
为什么需要
初始请求原来只做 zero-wait poll,因此记忆能否进入第一个 prompt 基本靠运气。100 ms 预算是第一步修复,但测量显示它单独不够:recall 会等待模型 selector,而模型 selector 是一次网络 side query,有 30 秒 abort 上限,所以预算主要受网络往返影响,而不是调度抖动。常见路径会超出预算,交付退到 ToolResult;如果这一轮没有工具调用,就永远到不了 ToolResult,于是结果会以
no_safe_delivery_point被丢弃。无工具回合通常正是那些短的、靠上下文回答的问题,也是用户记忆最有价值的场景。另外,确定性 fallback 原来偏 ASCII,并且会给每个非空文档一个正分,因此可能选中完全不匹配的文档,也无法 tokenize 常见 CJK 查询。
Reviewer 测试计划
如何验证
两个 eval 文件会在 stdout 输出 before/after 表格,需要使用
--silent=false查看。代码复核时应确认这些行为:预算内完成的结果会初始交付;预算 miss 会交付确定性结果并让 recall 继续等待 ToolResult;后续交付不会重复 fast 交付过的文档;初始窗口内取消不会交付任何内容;fast 结果不会跨 query 边界;无结果查询保持静默;同分不会把 user 类型文档挤出两篇的 fast 结果;选中文档若已被 fast 全部投递,丢弃时记为
already_delivered而不是记成"丢失",部分重叠仍记原取消原因。证据(前后对比)
完整数字、方法和限制见 https://github.com/QwenLM/qwen-code/pull/8716#issuecomment-5264740656。
Recall 质量方面(
recall-eval.test.ts,51 case / 25 文档标注语料,与变更前 scorer 的 frozen copy 对比):语料规模(便于解读上面的数字):25 篇文档、51 条 case。一个不看 query、随机返回 5 篇的 scorer 在这个池子上的 Recall@5 是 20.0%,测试会把这个下限卡在 25% 以内,实测结果远高于它。
overall 不到 100% 是被
semantic-no-lexical分片拉下来的:这些 query 可答,但与目标文档没有任何词面重叠。新旧两个 scorer 对它们都返回空,所以"要求词面匹配"并没有制造这个缺口,但它确实让确定性路径在这里保持沉默。该分片单独测量,不计入质量下限。交付方面(
recall-delivery-eval.test.ts):超过预算时,无工具 first-turn 交付从 0% 提升到 92.9%;有工具回合仍在下一次请求交付;重复交付保持 0%。扫描延迟(
recall-scan-latency.test.ts,新增)才是决定 fast path 能否交付的那个量测,之前缺失。确定性 打分成本是 p50 0.036 ms / p95 0.053 ms——但 fast 结果要等 recall 扫完并解析整棵 Memory 树才会发布,而本 PR 恰好去掉了 recall 的 200 文档上限。在真实临时 Memory 树上实测:两个结论。对能在预算内扫完的树(普通用户的常见情况,几十篇),fast 结果远早于上限就绪,所以等待现在到此为止,不再把剩余预算花在一个本设计已经假定赶不上的 selector 上。I/O 足够慢时扫描本身就超上限,该轮会付满预算且什么都投不到——比本 PR 替换掉的零等待更差。这个临界点与机器相关而不是固定篇数:另一台更快的机器上同样三档实测 9 / 21 / 46 ms,根本没到上限。上表取的是保守的那一侧。提前结束等待只能限制这种情况,消除不了它,真正的解法(持久化 catalog)仍在范围外。
剩下的 7.1% 正好就是
semantic-no-lexical分片,并有独立断言:fast path 解决的是时机问题,不是匹配问题。没有词面匹配的 query 只有 Model Selector 能覆盖,而无工具回合等不到 Selector。Selector latency 是建模而不是直接测量,因为单元测试无法真实计时一次网络往返,所以交付结果按不同 latency 场景报告。另需注意:delivery eval 是对两种设计的模拟,并没有真正驱动
tryConsumeMemoryPrefetch,所以其中的"重复交付"数字是模拟自身的性质;真正的去重由client.test.ts的does not re-deliver a document the fast phase already injected覆盖,该用例已验证移除 exclusion filter 时会失败。测试平台
01ef7d7d上独立验证(macOS 26.6 / arm64 / Node 24.18.1):37 个文件、929 passed、0 failed。tsc --noEmit -p packages/core干净——下面提到的sharp报错在那边不复现。eslint干净。两个 eval 表格完全复现。见该验证评论packages/core的src/memory/+src/core/client.test.ts—— 928 passed。仅 1 条无关的既有失败team-memory-sync.test.ts > unstages the team path when the commit fails,在未改动的分支 HEAD 上同样复现、且在 macOS 上通过,确认是环境问题(sandbox git hook)。eslint干净;tsc --noEmit -p packages/core只剩既有的src/utils/image-view.ts中sharp的SharpConstructor类型报错环境(可选)
不适用。
风险与范围
onFastResult在 recall 发出 selector 请求之前就发布了,所以等待在它这里结束时 recall promise 必然未 settle;初始交付点上「优先已 settle 的 recall」那个分支,只有在完全没有 fast 结果时才走得到。端到端验证过:让 selector 在 15 ms 内返回,投的仍然是确定性那两篇,模型选的被丢弃——而同一次运行里变更前的构建什么都没投。这是有意的取舍:生产环境里模型 side query 不可能在 100 ms 内返回,为此仲裁等于每轮都花掉剩余预算去赢一场不会发生的比赛;selector 的判断仍会在 ToolResult 交付点进入模型,并排除 fast 已投的文档。最多两篇限制了错排成本。MAX_RELEVANT_DOCS = 5限制的是单次注入,不是单轮总量。Fast 投递 2 篇、refined 又投递 5 篇全新文档时,本轮进入模型的是 7 篇——去重只消除重复,不压缩总和。这是放弃跨阶段预算核算的直接结果;两次 prompt 各自有界、每篇 body 仍截断到MAX_DOC_BODY_CHARS,因此最坏情况有界且不大,但它不等于 5。若需要硬性总量上限,最省事的做法是把 refined 的 limit 传成limit - fastDeliveredPaths.size。phase表示交付阶段,strategy表示选择方法。fast交付总是heuristic;refined交付通常是model,selector 失败时是heuristic。MemoryRecallDiscardReason新增already_delivered;只要 fast 阶段已经投递了全部选中文档,现在所有丢弃路径都记这个原因,而不只是 ToolResult 交付点,这样no_safe_delivery_point这一桶不再统计实际已拿到 memory 的回合。部分重叠仍记原取消原因。已知限制:
owner匹配ownership。语料中记录了这个 case,而不是隐藏它。semantic-no-lexical分片测量了这一点;这是 fast path 不解决的边界。关联 Issue
Refs #7040。