Skip to content

Commit 63dda4c

Browse files
nickrociclaude
andcommitted
perf(priming): incremental embedding index — re-encode only changed entries
The deadlock fix (prev commit) serialises rebuilds, but each stale rebuild still re-encoded the WHOLE corpus on the priming hot path. The Scholar writes constantly during a session, so the first priming request after each write paid a full ~552-doc re-encode → over the 2s budget → lexical fallback. That's the real "priming feels slow during curation". embeddings.load_or_build now does an INCREMENTAL rebuild when the cache is compatible-but-stale: reuse the cached embedding row for every entry whose path + mtime are unchanged, encode only new/modified entries, drop deleted ones. Falls back to a full build for first-build / force / model-or-dir change / corrupt cache. Reproduction (continuous staleness + 8 concurrent requests): 8/8 in ~1.4s total (~1s each) — vs ~42s each on the full-rebuild path. Unit test asserts only the changed entry is encoded (batches [3, 1], not [3, 3]). Also corrects the embeddings.py threading note: it claimed "parallel rebuilds are wasteful but correct" — they actually deadlock MPS; all inference is serialised via _inference.py. tools/search 172 tests, daemon 684, pyright 0, ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 58f4be1 commit 63dda4c

2 files changed

Lines changed: 159 additions & 13 deletions

File tree

tools/search/embeddings.py

Lines changed: 110 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,16 @@
2323
swallows unpickle errors and rebuilds.
2424
2525
Threading:
26-
- Reads (``search``) are safe to call concurrently.
27-
- ``save_index`` is concurrency-safe (see ``bm25.save_pickled``): racing
28-
saves to the same path each write a unique temp file then atomically
29-
replace, so they never interleave into a corrupt file. Last writer wins.
30-
``build_index`` has no shared state, so parallel rebuilds are wasteful
31-
(duplicated encode work) but correct.
26+
- ``search``, ``build_index`` and ``_incremental_build`` all call
27+
``model.encode()``, which is NOT thread-safe on MPS: concurrent encodes from
28+
different threads DEADLOCK (and corrupt each other's tensors). The daemon
29+
serialises ALL inference — search and (incremental) rebuild — onto a single
30+
thread via ``agent_mem_daemon/_inference.py``; callers outside that funnel
31+
must not run these concurrently. (The earlier note here — "parallel rebuilds
32+
are wasteful but correct" — was wrong: they hang.)
33+
- ``save_index`` is concurrency-safe (see ``bm25.save_pickled``): racing saves
34+
to the same path each write a unique temp file then atomically replace, so
35+
they never interleave into a corrupt file. Last writer wins.
3236
"""
3337

3438
from __future__ import annotations
@@ -325,6 +329,90 @@ def build_index(
325329
)
326330

327331

332+
def _incremental_build(
333+
cached: EmbeddingIndex,
334+
knowledge_dir: Path,
335+
model_name: str,
336+
) -> EmbeddingIndex:
337+
"""Rebuild re-encoding ONLY the entries that changed since ``cached``.
338+
339+
Reuses ``cached``'s embedding row for every entry whose path is unchanged and
340+
whose mtime hasn't advanced (matching :func:`is_stale`); encodes only new or
341+
modified entries; drops deleted ones. A full :func:`build_index` re-encodes
342+
the whole corpus — expensive on the priming hot path, which rebuilds after
343+
every Scholar write. The ``model.encode`` here is funnelled onto the daemon's
344+
single inference thread by the callers (``_inference.run``), so it stays
345+
MPS-safe. Falls back to a full build if the cached rows can't be reused.
346+
"""
347+
knowledge_dir = knowledge_dir.expanduser().resolve()
348+
n_cached = cached.embeddings.shape[0] if cached.embeddings.ndim == 2 else 0
349+
by_path = {rec.path: i for i, rec in enumerate(cached.docs)}
350+
351+
docs: list[_DocRecord] = []
352+
rows: list[_FloatArray] = []
353+
encode_texts: list[str] = []
354+
encode_slots: list[int] = []
355+
for md in _iter_markdown(knowledge_dir):
356+
try:
357+
mtime = md.stat().st_mtime
358+
except OSError:
359+
continue
360+
path = str(md)
361+
ci = by_path.get(path)
362+
if ci is not None and ci < n_cached and cached.docs[ci].mtime + 1e-6 >= mtime:
363+
# Unchanged → reuse the cached embedding row (refresh stored mtime so
364+
# the next staleness check is exact).
365+
docs.append(_DocRecord(path=path, mtime=mtime, raw_text=cached.docs[ci].raw_text))
366+
rows.append(cached.embeddings[ci])
367+
continue
368+
try:
369+
raw = md.read_text(encoding="utf-8")
370+
except (OSError, UnicodeDecodeError):
371+
continue
372+
text = _embedding_text(raw).strip()
373+
if not text:
374+
continue
375+
docs.append(_DocRecord(path=path, mtime=mtime, raw_text=raw))
376+
rows.append(np.zeros((0,), dtype=np.float32)) # placeholder; overwritten below
377+
encode_texts.append(text)
378+
encode_slots.append(len(docs) - 1)
379+
380+
if not docs:
381+
return EmbeddingIndex(
382+
knowledge_dir=knowledge_dir,
383+
model_name=model_name,
384+
docs=[],
385+
embeddings=np.zeros((0, 0), dtype=np.float32),
386+
built_at=time.time(),
387+
)
388+
389+
if encode_texts:
390+
model = _load_model(model_name)
391+
fresh = model.encode(
392+
[_format_doc(t, model_name) for t in encode_texts],
393+
convert_to_numpy=True,
394+
normalize_embeddings=True,
395+
show_progress_bar=False,
396+
batch_size=8,
397+
).astype(np.float32)
398+
for i, slot in enumerate(encode_slots):
399+
rows[slot] = fresh[i]
400+
401+
try:
402+
embeddings: _FloatArray = np.vstack(rows).astype(np.float32)
403+
except ValueError:
404+
# Ragged rows (corrupt / dimension-mismatched cache) — full rebuild.
405+
return build_index(knowledge_dir, model_name=model_name)
406+
407+
return EmbeddingIndex(
408+
knowledge_dir=knowledge_dir,
409+
model_name=model_name,
410+
docs=docs,
411+
embeddings=embeddings,
412+
built_at=time.time(),
413+
)
414+
415+
328416
def save_index(index: EmbeddingIndex, path: Path | None = None) -> Path:
329417
"""Pickle the index atomically. Returns the path written.
330418
@@ -343,11 +431,15 @@ def load_or_build(
343431
force_rebuild: bool = False,
344432
index_path: Path | None = None,
345433
) -> EmbeddingIndex:
346-
"""Load the persisted index if fresh, otherwise rebuild and save.
434+
"""Load the persisted index if fresh; otherwise rebuild (incrementally when
435+
possible) and save.
347436
348437
"Fresh" means the index was built against the same ``knowledge_dir`` and
349438
``model_name``, every file we tracked is still present with the same mtime,
350-
and no new ``.md`` files have appeared.
439+
and no new ``.md`` files have appeared. When the cache is compatible but
440+
stale we re-encode ONLY the changed/new entries (see :func:`_incremental_build`)
441+
rather than the whole corpus — the corpus rebuild is what made priming slow
442+
on the hot path after every Scholar write.
351443
"""
352444
knowledge_dir = knowledge_dir.expanduser().resolve()
353445
target = index_path or _default_index_path(knowledge_dir)
@@ -358,12 +450,17 @@ def load_or_build(
358450
cached is not None
359451
and cached.knowledge_dir == knowledge_dir
360452
and cached.model_name == model_name
361-
and not is_stale(
362-
cached.docs,
363-
{str(p): p.stat().st_mtime for p in _iter_markdown(knowledge_dir)},
364-
)
365453
):
366-
return cached
454+
current = {str(p): p.stat().st_mtime for p in _iter_markdown(knowledge_dir)}
455+
if not is_stale(cached.docs, current):
456+
return cached
457+
# Compatible but stale → re-encode only the entries that changed.
458+
index = _incremental_build(cached, knowledge_dir, model_name)
459+
try:
460+
save_index(index, target)
461+
except OSError:
462+
pass
463+
return index
367464

368465
index = build_index(knowledge_dir, model_name=model_name)
369466
try:

tools/search/test_embeddings.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
import time
1616
from pathlib import Path
1717

18+
import numpy as np
1819
import pytest
1920

21+
import embeddings
2022
from embeddings import (
2123
EmbeddingHit,
2224
EmbeddingIndex,
@@ -191,6 +193,53 @@ def test_load_or_build_caches_and_rebuilds(tmp_path: Path) -> None:
191193
assert third.embeddings.shape[0] == len(third.docs) == 5
192194

193195

196+
def test_load_or_build_incremental_reencodes_only_changed(
197+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
198+
) -> None:
199+
"""A stale-but-compatible index re-encodes ONLY the changed entries, not the
200+
whole corpus — the fix for the priming-hot-path full rebuild after every
201+
Scholar write. On the old code this re-encoded everything (batches [3, 3]).
202+
"""
203+
knowledge = tmp_path / "knowledge"
204+
knowledge.mkdir()
205+
for name in ("a", "b", "c"):
206+
(knowledge / f"{name}.md").write_text(f"# {name}\nbody about {name}\n", encoding="utf-8")
207+
208+
batches: list[int] = []
209+
210+
class _FakeModel:
211+
def encode(self, texts: list[str], **_kwargs: object) -> object:
212+
batches.append(len(texts))
213+
arr = np.zeros((len(texts), 4), dtype=np.float32)
214+
for i, text in enumerate(texts):
215+
arr[i, len(text) % 4] = 1.0
216+
return arr
217+
218+
fake = _FakeModel()
219+
monkeypatch.setattr(embeddings, "_load_model", lambda *_a, **_k: fake)
220+
221+
first = load_or_build(knowledge)
222+
assert batches == [3] # full build encoded all 3 entries
223+
assert len(first.docs) == 3
224+
225+
# Modify ONE entry and push its mtime into the future so the index is stale.
226+
(knowledge / "b.md").write_text("# b\ntotally different body now\n", encoding="utf-8")
227+
future = time.time() + 5
228+
os.utime(knowledge / "b.md", (future, future))
229+
230+
second = load_or_build(knowledge)
231+
assert batches == [3, 1] # incremental encoded ONLY the changed entry
232+
assert len(second.docs) == 3
233+
234+
# Unchanged entries kept their exact cached embedding rows (reused, not re-encoded).
235+
def _row(idx: EmbeddingIndex, name: str) -> object:
236+
pos = next(i for i, d in enumerate(idx.docs) if d.path.endswith(name))
237+
return idx.embeddings[pos]
238+
239+
assert np.array_equal(_row(first, "a.md"), _row(second, "a.md"))
240+
assert np.array_equal(_row(first, "c.md"), _row(second, "c.md"))
241+
242+
194243
# ── Edge cases that don't need the model loaded ──────────────────────────────
195244

196245

0 commit comments

Comments
 (0)