fix(memory): index memories into BM25 + backfill on startup (closes #257)

[rohitg00]

Closes #257.

What

memory_save returned 200 with a valid memory ID, but every subsequent memory_smart_search and memory_recall returned empty results — and direct REST /agentmemory/smart-search showed the same, ruling out the MCP layer. Per the issue:

[agentmemory] info Memory saved {"memId":"mem_moy3u6ua_8c6962b668e7","type":"fact"}
[agentmemory] info Smart search compact {"query":"BM25 test","results":0}

Recall was completely broken in BM25-only mode (no embedding provider configured). High priority — this makes the entire memory feature unusable for the self-hosted-without-embeddings slice of users.

Root cause

src/functions/remember.ts::mem::remember wrote the new Memory to KV.memories but never called getSearchIndex().add(...). Same shape of bug as #228 fixed for vectors-on-observe — the row hits durable storage, the search index never sees it. Compounded by rebuildIndex() (src/functions/search.ts) walking only sessions/observations and ignoring KV.memories, so even a restart-rebuild couldn't recover the corpus.

Fix

Three minimal changes:

File	Change
src/functions/remember.ts	After kv.set(KV.memories, ...), synthesize a CompressedObservation from the memory and call getSearchIndex().add(). Wrapped in try/catch so an indexing hiccup never blocks the durable save.
src/functions/search.ts	rebuildIndex() now walks KV.memories before sessions/observations. Skips memory.isLatest === false so superseded rows don't pollute results.
src/index.ts	Startup backfill for users upgrading from <0.9.5: when persisted BM25 is non-empty (no rebuild triggered), walk KV.memories and add anything not already indexed. SearchIndex.has(id) is the idempotency gate. indexPersistence.scheduleSave() persists the augmented index.
src/state/search-index.ts	New has(id: string): boolean method.

Test plan

test/remember-bm25-index.test.ts covers:

• SearchIndex.has() before / after add()
• Saved memory is findable by keyword search (the case the bug broke)
• Exact repro from the issue: mem_moy3u6ua_8c6962b668e7 with query "BM25 test" → returns the hit
• Concept-only matches (no title/content keyword overlap)
• Pre-existing test/search-index.test.ts (9 tests) still passes
• npx tsc --noEmit — no errors from changed files

5 new tests + 9 existing → 14/14.

Out of scope (filing as follow-ups)

• Removing superseded memories from BM25 when mem::cascade-update fires. Currently isLatest === false filter at result time prevents stale hits from surfacing, but the inverted index keeps growing. Not a regression vs current behavior.
• Persistence round-trip test for memories specifically — startup backfill covers the user-visible contract, but a dedicated IndexPersistence.save() → reload → memory.title-search test would tighten coverage.

Note on the user's "Issue exists since v0.9.0" finding

That tracks. Pre-v0.9.0 the path likely went through mem::observe → which DOES call getSearchIndex().add(). The mem::remember path that bypasses observation flow is newer, and the indexing call was never wired. The startup backfill in this PR retroactively fixes existing memories on next start.

Credit to @Nizar-BenHamida for the precise repro with logs and the elimination of MCP / engine layers from the diagnosis.

Summary by CodeRabbit

Release Notes

• New Features

  • Saved memories are now automatically indexed for searchability upon creation
  • Search results expanded to include all saved memories and decisions
  • System automatically backfills previously saved memories into the search index at startup

• Bug Fixes

  • Improved search reliability by ensuring all memories are consistently discoverable

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agentmemory	Ready	Preview, Comment	May 9, 2026 1:19pm

[coderabbitai]

📝 Walkthrough

Walkthrough

Memory objects are now indexed into the BM25 search index. Indexing occurs immediately after save, during batch rebuild, and via startup backfill for pre-existing memories. A new SearchIndex.has() method enables duplicate-detection checks. Helper functions normalize Memory objects into the CompressedObservation shape expected by the indexer.

Changes

BM25 Memory Indexing Integration

Layer / File(s)	Summary
Search Index API Enhancement src/state/search-index.ts	SearchIndex exposes a new has(id: string): boolean method to check observation presence in the entries map.
Memory-to-Index Conversion Contract src/functions/remember.ts, src/functions/search.ts, test/remember-bm25-index.test.ts	Multiple files define memoryAsIndexable helpers that convert Memory into CompressedObservation shape, normalizing type to "decision", deriving sessionId, and mapping fields to facts/narrative.
Immediate Indexing on Memory Save src/functions/remember.ts	After persisting a new Memory, the remember function adds it to the BM25 index via getSearchIndex().add(); wrapped in try/catch to prevent save failures from indexing errors.
Batch Memory Indexing During Rebuild src/functions/search.ts	The rebuildIndex function now loads and indexes Memory entries from KV.memories in addition to per-session observations, filtering to latest entries with required title/content and accumulating a single count.
Startup Index Backfill src/index.ts	During startup, when BM25 already has data, the system iterates KV.memories, checks presence via bm25Index.has(), and adds missing latest memories; schedules persistence only if backfilled items are found.
BM25 Indexing Tests test/remember-bm25-index.test.ts	Adds test helpers (memoryAsIndexable, makeMemory) and validates SearchIndex.has() behavior, BM25 keyword search recall, and concept matching including a regression case for issue #257.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• rohitg00/agentmemory#78: Modifies mem::remember in src/functions/remember.ts to add ttlDays/forgetAfter handling, overlapping the same function where this PR adds immediate BM25 indexing.

Poem

🐰 A memory finds its nest,
Indexed swift, put to the test,
BM25's searching light,
Makes recall shine so bright!
Hop-hop, memories take flight! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(memory): index memories into BM25 + backfill on startup (closes #257)' accurately describes the main changes: fixing memory indexing into BM25 and implementing backfill on startup.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/257-bm25-memories-not-indexed

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/functions/remember.ts (1)

16-29: ⚡ Quick win

Consider extracting memoryAsIndexable to eliminate duplication.

The same conversion logic appears in four locations:

• src/functions/remember.ts (lines 16-29)
• src/functions/search.ts (lines 15-28)
• src/index.ts (inlined at lines 422-433)
• test/remember-bm25-index.test.ts (test fixture at lines 8-21)

While the PR summary notes a circular import concern, extracting this to a neutral location like src/state/memory-indexing.ts would eliminate the maintenance burden. Any future field mapping change currently requires updating four locations.

♻️ Suggested approach

Create src/state/memory-indexing.ts:

import type { CompressedObservation, Memory } from "../types.js";

export function memoryAsIndexable(memory: Memory): CompressedObservation {
  return {
    id: memory.id,
    sessionId: memory.sessionIds[0] ?? "memory",
    timestamp: memory.createdAt,
    type: "decision",
    title: memory.title,
    facts: [memory.content],
    narrative: memory.content,
    concepts: memory.concepts,
    files: memory.files,
    importance: memory.strength,
  };
}

Then import from src/functions/remember.ts, src/functions/search.ts, and src/index.ts. Since src/state/ modules typically don't import from src/functions/, this should avoid circular dependencies.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/functions/remember.ts` around lines 16 - 29, Extract the duplicate
mapping into a single exported utility called memoryAsIndexable in a new neutral
module (e.g., memory-indexing) and replace the four inlined copies with imports
of that function; specifically, move the current memoryAsIndexable
implementation (the function that maps Memory -> CompressedObservation using id,
sessionIds[0] ?? "memory", createdAt, type "decision", title, facts [content],
narrative content, concepts, files, importance strength) into the new module and
update the callers (remember.ts, search.ts, index.ts and the test fixture) to
import and use that exported memoryAsIndexable to avoid duplication and prevent
circular imports by keeping the new module dependency-free of higher-level
function modules.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/remember-bm25-index.test.ts`:
- Around line 23-39: The helper makeMemory creates two timestamps by calling new
Date().toISOString() twice; change it to capture the ISO timestamp once in a
local variable at the start of makeMemory and reuse that single value for both
createdAt and updatedAt so both fields are identical and Date is only invoked
once (update references to createdAt and updatedAt in the returned Memory object
accordingly).

---

Nitpick comments:
In `@src/functions/remember.ts`:
- Around line 16-29: Extract the duplicate mapping into a single exported
utility called memoryAsIndexable in a new neutral module (e.g., memory-indexing)
and replace the four inlined copies with imports of that function; specifically,
move the current memoryAsIndexable implementation (the function that maps Memory
-> CompressedObservation using id, sessionIds[0] ?? "memory", createdAt, type
"decision", title, facts [content], narrative content, concepts, files,
importance strength) into the new module and update the callers (remember.ts,
search.ts, index.ts and the test fixture) to import and use that exported
memoryAsIndexable to avoid duplication and prevent circular imports by keeping
the new module dependency-free of higher-level function modules.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 03f2d765-759f-41e6-9146-97b52e3eed54

📥 Commits

Reviewing files that changed from the base of the PR and between ab4a166 and 0c21c56.

📒 Files selected for processing (5)

• src/functions/remember.ts
• src/functions/search.ts
• src/index.ts
• src/state/search-index.ts
• test/remember-bm25-index.test.ts

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Capture timestamp once and reuse.

The makeMemory helper calls new Date().toISOString() twice (lines 26-27). As per coding guidelines, capture the timestamp once and reuse it.

📅 Proposed fix

 function makeMemory(overrides: Partial<Memory> = {}): Memory {
+  const now = new Date().toISOString();
   return {
     id: "mem_test_001",
-    createdAt: new Date().toISOString(),
-    updatedAt: new Date().toISOString(),
+    createdAt: now,
+    updatedAt: now,
     type: "fact",

As per coding guidelines: "Capture timestamps once with new Date().toISOString() and reuse instead of calling Date multiple times".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	function makeMemory(overrides: Partial<Memory> = {}): Memory {
	return {
	id: "mem_test_001",
	createdAt: new Date().toISOString(),
	updatedAt: new Date().toISOString(),
	type: "fact",
	title: "BM25 test memory",
	content: "BM25 search returns this memory by keyword match",
	concepts: ["bm25", "search", "test"],
	files: [],
	sessionIds: [],
	strength: 7,
	version: 1,
	isLatest: true,
	...overrides,
	};
	}
	function makeMemory(overrides: Partial<Memory> = {}): Memory {
	const now = new Date().toISOString();
	return {
	id: "mem_test_001",
	createdAt: now,
	updatedAt: now,
	type: "fact",
	title: "BM25 test memory",
	content: "BM25 search returns this memory by keyword match",
	concepts: ["bm25", "search", "test"],
	files: [],
	sessionIds: [],
	strength: 7,
	version: 1,
	isLatest: true,
	...overrides,
	};
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/remember-bm25-index.test.ts` around lines 23 - 39, The helper makeMemory
creates two timestamps by calling new Date().toISOString() twice; change it to
capture the ISO timestamp once in a local variable at the start of makeMemory
and reuse that single value for both createdAt and updatedAt so both fields are
identical and Date is only invoked once (update references to createdAt and
updatedAt in the returned Memory object accordingly).