Skip to content

fix: evicted observations leave ghost entries in search indexes#1005

Open
fix2015 wants to merge 1 commit into
rohitg00:mainfrom
fix2015:fix/eviction-ghost-entries
Open

fix: evicted observations leave ghost entries in search indexes#1005
fix2015 wants to merge 1 commit into
rohitg00:mainfrom
fix2015:fix/eviction-ghost-entries

Conversation

@fix2015

@fix2015 fix2015 commented Jul 3, 2026

Copy link
Copy Markdown

Found a bug in the eviction logic — when observations are evicted from KV, they're never removed from the BM25 or vector indexes. This means searches return IDs for evicted entries, but the KV lookup in the second pass gets null, silently dropping results and under-filling search pages.

The forget and auto-forget code paths already handle this correctly (they call both `getSearchIndex().remove()` and `vectorIndexRemove()` alongside the KV delete). Applied the same pattern to eviction.

Summary by CodeRabbit

  • Bug Fixes
    • Improved memory cleanup so deleted items are also removed from search and vector results.
    • Prevented stale or expired memories from continuing to appear in retrieval-based features.
    • Ensured index updates are saved after eviction, except during dry runs.

when evict deleted observations from KV, it never removed them from
the BM25 or vector indexes. this meant search would return IDs for
evicted entries, but the second-pass KV lookup would get null, silently
dropping results and under-filling search pages. added index removal
calls to match how forget and auto-forget already handle it.
@vercel

vercel Bot commented Jul 3, 2026

Copy link
Copy Markdown

@fix2015 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

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 183a8773-c725-45a5-913c-0c4a7405ad6a

📥 Commits

Reviewing files that changed from the base of the PR and between 93ae9bc and b7d3c8d.

📒 Files selected for processing (1)
  • src/functions/evict.ts

📝 Walkthrough

Walkthrough

The eviction function in evict.ts now synchronizes the search and vector indexes when deleting entities. Deletions of low-importance observations, project-capped observations, expired memories, and old non-latest memories additionally remove index entries, and pending index changes are flushed at the end when not in dry-run mode.

Changes

Eviction index synchronization

Layer / File(s) Summary
Import index utilities
src/functions/evict.ts
Imports getSearchIndex, vectorIndexRemove, and flushIndexSave from ./search.js.
Remove index entries on deletion
src/functions/evict.ts
Adds search index and vector index removal calls following KV deletions across four eviction paths (low-importance observations, project-capped observations, expired memories, old non-latest memories).
Flush index changes
src/functions/evict.ts
Calls flushIndexSave() at the end of eviction when not in dryRun mode to persist index changes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant EvictFunction
  participant KVStore
  participant SearchIndex
  participant VectorIndex

  EvictFunction->>KVStore: delete evicted entity
  KVStore-->>EvictFunction: deletion confirmed
  EvictFunction->>SearchIndex: remove(id)
  EvictFunction->>VectorIndex: vectorIndexRemove(id)
  Note over EvictFunction: repeats for each eviction path
  alt not dryRun
    EvictFunction->>SearchIndex: flushIndexSave()
  end
Loading

Possibly related PRs

  • rohitg00/agentmemory#636: Applies the same index cleanup pattern of getSearchIndex().remove(...), vectorIndexRemove(...), and flushIndexSave to other delete/evict paths.

Suggested reviewers: rohitg00

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main fix: evicted observations were left in search indexes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)


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

Comment @coderabbitai help to get the list of available commands.

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