Skip to content

feat(v2.11): callers pass their deadline; the server degrades inside it, and repeats hit a memo - #43

Merged
MXAntian merged 1 commit into
mainfrom
feat/recall-deadline-memo
Sep 14, 2026
Merged

MXAntian merged 1 commit into
mainfrom
feat/recall-deadline-memo

Conversation

@MXAntian

@MXAntian MXAntian commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

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 as hybrid_novec. The API itself is 170–480 ms. The loss is between two timeouts nobody lined up:

hook HTTP budget 1500 ms (bundled default was 800)
server embedding timeout in effect 2500 ms (.env.local says 5000 — never reached the process)
hook-side hybrid work finished after the hook had aborted 683 / 1385 = 49%
novec calls followed within 8 s by the same query (spawn fallback, second embedding) 311 / 660 = 47%
degrade rate: solo vs ±2 s concurrent 27% vs 74%
queries recurring within 10 min 23%
  • deadline_ms on POST /recall: the server bounds its embedding wait to deadline − 150 (floor 200, ≤ EMBEDDING_TIMEOUT_MS) and degrades to FTS inside the caller's budget. Both bundled hooks send httpTimeoutMs − 100; their default rises 800 → 1500 (FTS-era number; a longer wait can no longer become a zombie).
  • Query memo: 10-minute / 512-entry, opt-in on the recall path; only well-formed vectors are memoised.
  • /health/stats split upstreamed: /health is O(1) liveness; GET /stats is the census and carries embedding counters (calls / memoHits / timeouts / clamped / failures); the memory_stats tool prints them.
  • Shrink guard: a path referred to by its bare file name counts as carried, except universally generic names (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)

finding fix
P0 /stats did not exist upstream; README and this PR's test plan pointed at it upstreamed the /health/stats split; memory_stats tool prints the counters; boot smoke verifies both
P1 hooks/tool-recall-pre.mjs not given deadline_ms (fires per tool call) added, same contract
P1 two independent 150 ms margins stacked → 500 ms embedding budget on the 800 default default → 1500; hook margin → 100; both margins documented together in recallMemoriesHybrid
P1 memo comment implied concurrent fan-out comment corrected (sequential repeats); no coalescing added
P1 basename rule matched package.json / index.mjs generic-name blocklist + test (frontend/package.jsonpackage.json still warns)
P1 (declined) in-flight coalescing not worth the Symbol/option subtlety at this traffic; noted in the comment
P2 empty/wrong-dimension reply could be memoised for 10 min only well-formed vectors are memoised
P2 no test for clamped-and-successful added
P2 deadlineMs undocumented on recallForClients / recallMemoriesHybrid JSDoc added

Test plan

  • embedding-timeout.integration.test.mjs 7 → 21; supersede-shrink.integration.test.mjs 24 → 28
  • all 21 suites pass on the merged tree (incl. cli-supersedes and prompt-recall-trigger that arrived on main)
  • boot smoke: /health keys = liveness only + statsEndpoint; /stats ok in 1 ms with all five counters; version 2.11.0
  • CI on the amended head
  • after merge: rebase runtime (private /stats commit becomes redundant), restart server via watchdog (now injects fresh .env.local), then one day of recall_log: hook-side novec→same-query pairs ≈ 0, /stats embedding.clamped and memoHits > 0

🤖 Generated with Claude Code

…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
MXAntian force-pushed the feat/recall-deadline-memo branch from 388c0af to 4cbbcb9 Compare September 14, 2026 08:16
@MXAntian
MXAntian marked this pull request as ready for review September 14, 2026 08:18
@MXAntian
MXAntian merged commit 2bc02cd into main Sep 14, 2026
2 checks passed
@MXAntian
MXAntian deleted the feat/recall-deadline-memo branch September 14, 2026 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant