SIGN IN SIGN UP

perf(remember): move the embedding call out of the global save lock (#1423)

* perf(remember): move the embedding call out of the mem:remember lock

mem::remember wrapped its whole body in withKeyedLock("mem:remember", ...),
including the vectorIndexAddGuarded call, which makes a network round-trip
to the embedding provider. Every save serialized behind that network call,
so two unrelated saves that happened to land at the same time queued
behind each other's embedding latency even though they touch different
memory ids and do not conflict.

Split mem::remember so the locked callback covers only the read-dedup-write
critical section (candidate generation, Jaccard supersede check, the
memory's KV writes, and the BM25 index update) and returns the saved
memory. The embedding call, the mem::cascade-update fire-and-forget
trigger, and the final log line now run after the lock releases, using
the memory the locked section already wrote.

Dedup/supersede correctness is unchanged: two concurrent saves for
identical content still serialize on the read-dedup-write section (that
part is still inside the lock), so the second call's candidate scan still
sees the first call's already-written memory and correctly supersedes it.
Only the embedding network call, which never reads or writes the
supersede state, moved outside.

Added test/remember-lock-scope.test.ts: one test with a slow fake
embedder proves two concurrent saves' embed calls overlap in wall time
instead of serializing, and one test proves two concurrent saves of
identical content still dedup correctly (first becomes v1, second
supersedes it as v2) even with the slow embedder in place.

* fix(remember): keep superseded memories out of the vector index

The embedding call for a saved memory runs outside the mem:remember
lock so concurrent saves can overlap. If a second save supersedes the
first memory while its embedding is still pending, the pending vector
add could finish after the supersede's vectorIndexRemove call and put
the superseded memory back into the vector index.

Move the final vector-index write into the same mem:remember lock the
supersede path already uses, guarded by a fresh isLatest check read
right before the write. The embedding computation itself stays outside
the lock, so concurrent saves still overlap on the slow part; only the
cheap check-and-write is serialized against supersession.

Added a test that races two identical concurrent saves with a slow
embedder and asserts the superseded memory's id never lands in the
vector index.

* perf(remember): schedule an index save after the vector commit and split failure logging

The deferred vector-index add scheduled no index save after it landed,
so if the embedding call outlasted the 5s debounce window from the
earlier BM25 add, the just-added vector could sit unsaved until some
unrelated write happened to reschedule it. Call scheduleIndexSave()
right after the commit's vector-index add.

vectorIndexAddGuarded ran the deferred commit callback inside the same
try/catch as the embedding call, so a failure in the commit (for
example the KV read that rechecks isLatest) was logged as "embed
failed" with the embedding provider's name, which is misleading when
diagnosing a KV problem. The embed call and the commit callback now
have their own try/catch with distinct log messages.

Added a test for each: one confirms scheduleIndexSave is called again
after the vector commit resolves, the other confirms a throwing commit
callback is logged as a commit failure, not an embed failure.

Also addressed the review's flaky wall-clock assertion in
remember-lock-scope.test.ts: dropped the elapsed-time bound and kept
the start/end cross-check assertions, which already prove the two
embeds overlapped without depending on machine timing.
R
Rohit Ghumare committed
c2df01a62c6438c22a494faac249ab974814a3d5
Parent: 00411d6
Committed by GitHub <noreply@github.com> on 9/28/2026, 6:40:04 AM