fix(evict): keep search index in sync on eviction

[shgew]

What

mem::evict now removes evicted observations and memories from the BM25/vector index in addition to deleting them from KV, mirroring the remember / auto-forget / retention delete paths.

Why

Pre-fix: evicted entries lingered in search results until a full rebuild, because the index removal hook was wired into every other delete path except eviction.

Tests

test/evict-index-sync.test.ts (new file, 2 cases that exercise the post-evict search consistency invariant). Targeted suite: 2/2 pass.

Compatibility

Bug fix. No env changes. Callers that already expected post-evict search consistency observe correct behavior; callers that relied on the stale index will see fresher results.

Summary by CodeRabbit

• Bug Fixes
  • Search and vector indexes are now kept in sync with data deletions during the eviction process.
  • Stale-session recovery now waits for completion before proceeding.
• Tests
  • Added coverage to verify eviction triggers the expected search/vector index removals and index persistence.

[vercel]

@shgew is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 51643069-d2c1-44ff-ad75-499c79a050bb

📥 Commits

Reviewing files that changed from the base of the PR and between c9ec53b and 8aeec3c.

📒 Files selected for processing (1)

• test/evict-index-sync.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• test/evict-index-sync.test.ts

---

📝 Walkthrough

Walkthrough

The eviction function now removes evicted observations and memories from both the search index (getSearchIndex().remove(...)) and vector index (vectorIndexRemove(...)) after each KV deletion path, then calls flushIndexSave() when evictions occurred outside dry-run mode. The stale-session recovery trigger is updated to waitForCompletion: true. A new test file covers these index-sync behaviors for observation and memory eviction scenarios.

Changes

Eviction Search/Vector Index Synchronization

Layer / File(s)	Summary
Index removal across all eviction paths src/functions/evict.ts	Imports getSearchIndex, vectorIndexRemove, and flushIndexSave; adds removal calls after low-importance observation, project-cap observation, expired memory, and non-latest memory deletions; calls flushIndexSave() at the end of any non-dryRun run with evictions; sets waitForCompletion: true on the stale-session recovery trigger.
Eviction index sync tests test/evict-index-sync.test.ts	Adds a Vitest suite with in-memory KV, SDK harness, and mocked search-index helpers, covering two scenarios: low-importance observation eviction and expired memory eviction, asserting KV deletion and calls to remove, vectorIndexRemove, and flushIndexSave.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• rohitg00/agentmemory#636: Implements the same pattern of calling getSearchIndex().remove(...), vectorIndexRemove(...), and flushIndexSave() after KV deletions on eviction/retention delete paths, which this PR extends to additional eviction paths in evict.ts.

Suggested reviewers

• rohitg00

Poem

🐇 Hop, hop, the index stays clean,
When memories vanish from the KV scene.
Search and vector, both get the call,
flushIndexSave catches them all.
No ghost ids left behind to haunt —
The rabbit tidies every data font! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 clearly summarizes the main change: ensuring the search index stays synchronized during evictions, which is the core bug fix described in the PR objectives.
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 unit tests (beta)

• Create PR with unit tests

---

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]

🧹 Nitpick comments (1)

test/evict-index-sync.test.ts (1)

74-74: ⚡ Quick win

Add beforeEach to clear mocks between tests.

The mocked functions (vectorIndexRemove, flushIndexSave, and the remove function from getSearchIndex) persist across test cases. Without clearing, assertions like toHaveBeenCalled() in the second test may pass due to calls from the first test, potentially masking failures.

🧹 Proposed fix

+import { beforeEach, describe, expect, it, vi } from "vitest";
-import { describe, expect, it, vi } from "vitest";

Then add the hook inside the describe block:

 describe("mem::evict keeps the search index in sync", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
   it("removes evicted low-importance observations from the index and flushes", async () => {

🤖 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/evict-index-sync.test.ts` at line 74, Inside the "mem::evict keeps the
search index in sync" describe block, add a beforeEach hook that clears all
mocks between test cases. This hook should call the appropriate mock clearing
functions (such as jest.clearAllMocks() or by resetting individual mocks like
vectorIndexRemove, flushIndexSave, and the remove function from getSearchIndex)
to ensure that mock call counts and assertions in each test are not affected by
calls from previous tests.

🤖 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.

Nitpick comments:
In `@test/evict-index-sync.test.ts`:
- Line 74: Inside the "mem::evict keeps the search index in sync" describe
block, add a beforeEach hook that clears all mocks between test cases. This hook
should call the appropriate mock clearing functions (such as
jest.clearAllMocks() or by resetting individual mocks like vectorIndexRemove,
flushIndexSave, and the remove function from getSearchIndex) to ensure that mock
call counts and assertions in each test are not affected by calls from previous
tests.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 30c9573c-2f6e-483d-b7cf-0e203e17a76d

📥 Commits

Reviewing files that changed from the base of the PR and between f6f9e3c and c9ec53b.

📒 Files selected for processing (2)

• src/functions/evict.ts
• test/evict-index-sync.test.ts