fix(memory): delete_all() now enumerates the full ID set, not just the first page - #6630
fix(memory): delete_all() now enumerates the full ID set, not just the first page#6630Solaris-star wants to merge 1 commit into
Conversation
…e first page Memory.delete_all() and AsyncMemory.delete_all() fetched the IDs to delete via vector_store.list(filters=filters) without passing top_k. Most vector stores default list(top_k) to 100, so delete_all() silently deleted only the first 100 memories for a scope and left the rest behind while still reporting success — a data-retention / privacy hazard (e.g. GDPR right-to- be-forgotten requests that appear to succeed). Pass top_k=10000, matching the full-enumeration cap already used for entity_store.list() elsewhere in this module. Adds sync + async regression tests asserting top_k=10000 is forwarded to the vector store (both fail on the unfixed code, pass with the fix). Fixes mem0ai#6627
|
|
AmirF194
left a comment
There was a problem hiding this comment.
Docker-verified this against the PR head (07b6f0f) with a fake vector store that mirrors Qdrant's/Chroma's real list(filters, top_k) signature: a single scroll/get call capped at limit=top_k, no offset or cursor.
top_k=10000 raises the threshold but does not close the gap #6627 describes. With 10,005 memories in one scope, delete_all() still leaves 5 behind and still logs success:
before delete_all: 10005 records for alice
after delete_all: 5 records for alice
Since vector_store.list() has no pagination cursor on any backend I checked, a fixed cap reproduces the same silent partial-delete for any scope larger than the cap, just at a higher count, and it is still silent (no error, no partial-completion signal).
A page-and-relist loop closes it for any size, since each round's list() call naturally returns the next slice once the prior page is deleted, no offset needed. #6629 on this repo does exactly that and I verified it leaves 0 of the same 10,005 records behind.
Non-blocking, just flagging that the cap is a mitigation rather than a fix for the class of bug the PR title claims to close.
Summary
Fixes #6627
Memory.delete_all()andAsyncMemory.delete_all()fetched the IDs to delete viavector_store.list(filters=filters)without passingtop_k. Most vector stores defaultlist(top_k)to 100, sodelete_all()silently deleted only the first 100 memories for a scope and left the rest behind — while still returning{"message": "Memories deleted successfully!"}.This is a data-retention / privacy hazard: a GDPR right-to-be-forgotten request can appear to succeed while personal data remains in the store.
Fix
Pass
top_k=10000tovector_store.list(), matching the full-enumeration cap already used forentity_store.list()in five other places in this module:Tests
Added sync + async regression tests asserting
top_k=10000is forwarded to the vector store.Verified both new tests fail on the unfixed code and pass with the fix.