feat(v2.11): callers pass their deadline; the server degrades inside it, and repeats hit a memo - #43
Merged
Merged
Conversation
…it, and repeats hit a memo
Ten days of recall_log on a 10,315-row library (2026-09-04 → 09-14), 1,860
recalls. Median latency had tripled since 09-01 (327 → 974 ms) and 46% of
hybrid calls were ending as hybrid_novec — the embedding timed out and the
call fell back to FTS. The embedding API itself measured 170–480 ms. The
loss was structural, between two timeouts that nobody had lined up:
hook HTTP budget 1500 ms (the author's hook; the bundled
hooks' default was 800)
server embedding timeout 2500 ms (novec p50 = 2543 ms; the 5000 in
.env.local never reached the process)
So the hook always gave up first. On the hook side, 1,385 hybrid calls:
683 (49%) finished after the hook had already aborted — the server kept
waiting for the embedding, then ran FTS + fusion for nobody. 311 of the 660
novec calls were followed within 8 s (median 1.7 s) by the same query again
from the same source: the hook's spawn fallback, a cold node process, paying
a second embedding. Concurrency made it worse, not the network: calls with
another recall within ±2 s degraded 74% of the time, solo calls 27%. A vec
KNN over 10k rows costs ~90 ms of synchronous time per call on the single
event loop, so bursts queue behind each other and embedding responses wait.
deadline_ms
POST /recall accepts the caller's remaining budget. recallMemoriesHybrid
bounds the embedding wait to deadline − 150 (floor 200, never above
EMBEDDING_TIMEOUT_MS) and degrades to FTS *inside* it. One call, an answer
within budget, no spawn, no zombie. Both bundled hooks send their HTTP
budget − 100 (transit); the two margins are documented together in
recallMemoriesHybrid so the compounding is visible. Their HTTP default
rises 800 → 1500: the 800 was FTS-era reasoning ("a healthy server answers
in single-digit ms"), and now that a longer wait cannot become a zombie
there is no reason to keep it tight. Callers that do not pass a deadline
(MCP tools, long-budget context builders) keep the env timeout.
query memo
23% of recall queries recur within 10 minutes — sequential repeats: the
same question asked again, a second session asking what the first just
asked. Embeddings are deterministic; the repeat is pure latency and
upstream load. 10-minute, 512-entry memo, opt-in on the recall path only.
Only a well-formed vector (non-empty, configured dimension) is memoised,
so an upstream hiccup cannot become sticky for that text. No in-flight
coalescing — the window that matters is minutes, not milliseconds.
observability — and the census that was never upstream
getEmbeddingStats() (calls / memoHits / timeouts / clamped / failures) is
included in getMemoryStats(). But getMemoryStats() had no reachable
surface for it: the author's private fork split /health (liveness) from
/stats (census) on 09-03 and never upstreamed it, so /health here still
ran three full scans per probe and there was no /stats at all. This
upstreams the split: GET /health is O(1) with a statsEndpoint pointer,
GET /stats returns the census with timing, and the memory_stats MCP tool
prints the embedding counters. "Fixed but invisible" is the failure the
last two weeks kept finding — the first draft of this PR had it too.
shrink guard
isStillCarried treats a path referred to by its bare file name as carried
("scripts/pull-x.py" → "pull-x.py"). A real supersede that replaced a
5-step recipe with a 1-step one still named every retired script by
basename and the guard flagged all four as lost — on an actively-recalled
chain (access_count 20). A guard that cries wolf on the rows people read
is a guard people learn to ignore. Basenames must be ≥ 8 chars, file-
shaped, and not universally generic: "frontend/package.json" superseded
by text naming only "backend/package.json" has dropped a real fact.
Review fixes (fanout on the draft — 1 P0, 4 of 5 P1s taken, 3 P2s):
- P0: /stats did not exist upstream; README and this PR's own test plan
pointed at it. Upstreamed the /health→/stats split (above).
- P1: hooks/tool-recall-pre.mjs had not been given deadline_ms — it fires
per tool call, so it mattered at least as much. Done.
- P1: two independent 150 ms margins stacked to 300 on an 800 ms default,
leaving 500 ms for a 170–480 ms call. Default → 1500, hook margin → 100,
both margins documented in one place.
- P1: memo comment implied concurrent fan-out; it serves sequential
repeats. Comment fixed; no coalescing added (not worth the subtlety).
- P1: basename rule matched package.json / index.mjs. Generic-name
blocklist + test.
- P2: empty/wrong-dimension replies are no longer memoised; a
clamped-and-successful call is tested; deadlineMs documented on both
recall entry points.
Not changed here, on purpose:
- The 90 ms synchronous vec KNN (brute force over all rows). Quantization
or an ANN index is a separate change; with deadlines in place it no
longer converts into thrown-away work.
- Whether dashscope is routed through the proxy (a Clash rule) — an
operator decision; measured 120–400 ms either way today.
- The hook's trigger breadth (18% of calls inject nothing after fusion).
Tests: embedding-timeout 7 → 21 (per-call deadline below/above the env
timeout, clamped-and-successful, recall-path deadline, memo hit/miss/opt-in,
counters); supersede-shrink 24 → 28 (basename carriage, Windows separators,
short names and generic names still warn). All 21 suites pass on the merged
tree; a boot smoke confirms /health is liveness-only and /stats carries the
counters.
Co-Authored-By: 千夏 <qianxia@clawgamers.com>
MXAntian
force-pushed
the
feat/recall-deadline-memo
branch
from
September 14, 2026 08:16
388c0af to
4cbbcb9
Compare
MXAntian
marked this pull request as ready for review
September 14, 2026 08:18
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
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.
Summary
Ten days of
recall_log(1,860 recalls): median latency had tripled since 09-01 (327 → 974 ms) and 46% of hybrid calls ended ashybrid_novec. The API itself is 170–480 ms. The loss is between two timeouts nobody lined up:.env.localsays 5000 — never reached the process)deadline_msonPOST /recall: the server bounds its embedding wait todeadline − 150(floor 200, ≤EMBEDDING_TIMEOUT_MS) and degrades to FTS inside the caller's budget. Both bundled hooks sendhttpTimeoutMs − 100; their default rises 800 → 1500 (FTS-era number; a longer wait can no longer become a zombie)./health→/statssplit upstreamed:/healthis O(1) liveness;GET /statsis the census and carriesembeddingcounters (calls / memoHits / timeouts / clamped / failures); thememory_statstool prints them.package.json,index.mjs, …).Deliberately not in this PR: the ~90 ms synchronous brute-force vec KNN (quantization/ANN is its own change); whether dashscope is proxied (operator's Clash rule); hook trigger breadth.
Review fixes (fanout on the draft — 1 P0, 4/5 P1, 3 P2)
/statsdid not exist upstream; README and this PR's test plan pointed at it/health→/statssplit;memory_statstool prints the counters; boot smoke verifies bothhooks/tool-recall-pre.mjsnot givendeadline_ms(fires per tool call)recallMemoriesHybridpackage.json/index.mjsfrontend/package.json→package.jsonstill warns)deadlineMsundocumented onrecallForClients/recallMemoriesHybridTest plan
embedding-timeout.integration.test.mjs7 → 21;supersede-shrink.integration.test.mjs24 → 28cli-supersedesandprompt-recall-triggerthat arrived on main)/healthkeys = liveness only +statsEndpoint;/statsok in 1 ms with all five counters; version 2.11.0/statscommit becomes redundant), restart server via watchdog (now injects fresh.env.local), then one day ofrecall_log: hook-side novec→same-query pairs ≈ 0,/stats embedding.clampedandmemoHits> 0🤖 Generated with Claude Code