Skip to content

Commit d78115a

Browse files
fix(memory): guard against self-supersede hiding recalled preferences (#2452)
A saved priority-sender preference could end up with `superseded_by` pointing at **its own id** — the update-consolidation path dedups identical content back into the same row and then marked that row superseded — so `recall(category=preference)` returned **0 results**. Asking "what priority-sender rules do I have saved?" in a new session answered "none" even though the rule was saved. Now the consolidation path only supersedes when a genuinely new row was created, and the memory store hard-rejects a self-supersede, so saved preferences stay recallable. ## Test plan - [x] Unit — `tests/unit/test_memory_store.py` + `tests/unit/test_memory_mixin.py` → **507 passed** (incl. the two regression tests) - [x] Real-world, Linux + macOS — reproduced the exact dedup-collapse trigger through the real installed package (log shows `knowledge deduped id=…` — same id), then `get_by_category("preference")` returns the pref with `superseded_by: None`; sqlite `SELECT id FROM knowledge WHERE superseded_by = id` → `[]` - [x] Agent UI, Linux + macOS — in a brand-new session, asked "what priority-sender rules do I have saved?" → the RECALL tool fired and the agent answered with the planted preference `zephyr-otter-9x@example.com` **Real-world Agent UI evidence** (planted, unguessable fact `zephyr-otter-9x@example.com`): ![macOS Agent UI — recall in a new session](https://raw.githubusercontent.com/amd/gaia/m59-autofix-evidence/testing/2452/2446_macos_ui_recall.png) ![Linux Agent UI — recall in a new session](https://raw.githubusercontent.com/amd/gaia/m59-autofix-evidence/testing/2452/2446_linux_ui_recall.png) Closes #2446 Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
1 parent 6bf96e5 commit d78115a

4 files changed

Lines changed: 71 additions & 5 deletions

File tree

src/gaia/agents/base/memory.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,9 +1364,13 @@ def _execute_extraction_operations(
13641364
source="llm_extract",
13651365
context=self._memory_context,
13661366
)
1367-
# Mark old as superseded and remove from FAISS
1368-
store.update(old_id, superseded_by=new_id)
1369-
self._faiss_remove(old_id)
1367+
# Only supersede when store() actually created a new row.
1368+
# Dedup can collapse near-identical content back into old_id,
1369+
# which would point superseded_by at the row itself and hide
1370+
# it from every active query (recall, get_by_category).
1371+
if new_id != old_id:
1372+
store.update(old_id, superseded_by=new_id)
1373+
self._faiss_remove(old_id)
13701374
# Embed the new item
13711375
try:
13721376
vec = self._embed_text(op["content"])

src/gaia/agents/base/memory_store.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,6 +1324,15 @@ def update(
13241324
When set, this item is considered historical/inactive and will be
13251325
excluded from active queries (search, get_by_*, system prompt).
13261326
"""
1327+
# A row may never supersede itself — that would set superseded_by to its
1328+
# own id and hide it from every active query (recall, get_by_category).
1329+
if superseded_by is not None and superseded_by == knowledge_id:
1330+
raise ValueError(
1331+
f"update(): superseded_by ({superseded_by}) must not equal the "
1332+
f"row's own id; a self-supersede makes the row unrecallable. "
1333+
f"Only supersede when a genuinely new row was created."
1334+
)
1335+
13271336
# Normalize empty strings to None — same semantics as store().
13281337
# An empty-string entity or domain would differ from NULL in SQL and
13291338
# break entity-scoped dedup, index filtering, and stats queries.

tests/unit/test_memory_mixin.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2371,6 +2371,40 @@ def test_extraction_drops_privileged_categories(self, extract_host):
23712371
assert "permission" not in cats
23722372
assert "system" not in cats
23732373

2374+
def test_update_dedup_does_not_self_supersede(self, extract_host):
2375+
"""Update-consolidation over near-identical content stays recallable.
2376+
2377+
Regression for #2446: store() dedups near-identical content back into
2378+
the same row and returns old_id; the update path must NOT then mark the
2379+
row as superseded_by=old_id, which would hide it from recall.
2380+
"""
2381+
extract_host._memory_context = "global"
2382+
store = extract_host._memory_store
2383+
2384+
old_id = store.store(
2385+
category="preference",
2386+
content="Prioritize email from alice@example.com",
2387+
source="llm_extract",
2388+
context="global",
2389+
)
2390+
2391+
# An "update" whose content dedups back into the same row.
2392+
ops = [
2393+
{
2394+
"op": "update",
2395+
"knowledge_id": old_id,
2396+
"category": "preference",
2397+
"content": "Prioritize email from alice@example.com",
2398+
}
2399+
]
2400+
existing_items = [{"id": old_id, "category": "preference", "confidence": 0.4}]
2401+
extract_host._execute_extraction_operations(ops, existing_items)
2402+
2403+
# The preference must still be recallable — not self-superseded.
2404+
results = store.get_by_category("preference", context="global")
2405+
assert any(r["id"] == old_id for r in results)
2406+
assert all(r["superseded_by"] is None for r in results)
2407+
23742408
def test_after_process_query_stores_conversation(self, extract_host):
23752409
"""_after_process_query() stores both user and assistant turns."""
23762410
extract_host._memory_session_id = "test-session-extraction"

tests/unit/test_memory_store.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,23 @@ def test_update_returns_false_for_nonexistent_id(self, store):
165165
fake_id = str(uuid.uuid4())
166166
assert store.update(fake_id, content="new content") is False
167167

168+
def test_update_rejects_self_supersede(self, store):
169+
"""update() refuses to point superseded_by at the row's own id.
170+
171+
A self-supersede would hide the row from every active query
172+
(recall, get_by_category) — see issue #2446.
173+
"""
174+
entry_id = store.store(
175+
category="preference",
176+
content="Prioritize email from alice@example.com",
177+
)
178+
with pytest.raises(ValueError, match="must not equal"):
179+
store.update(entry_id, superseded_by=entry_id)
180+
181+
# The row must remain recallable — the rejected update changed nothing.
182+
results = store.get_by_category("preference")
183+
assert any(r["id"] == entry_id for r in results)
184+
168185
def test_delete_entry(self, store):
169186
"""delete() removes an entry."""
170187
entry_id = store.store(category="fact", content="Temporary fact")
@@ -3963,13 +3980,15 @@ def test_get_all_knowledge_excludes_superseded_by_default(self, store):
39633980

39643981
def test_get_all_knowledge_includes_superseded_when_requested(self, store):
39653982
"""get_all_knowledge(include_superseded=True) includes superseded items."""
3983+
# Contents must be distinct enough to NOT dedup into one row, else
3984+
# store() returns the same id and the supersede would be a self-supersede.
39663985
old_id = store.store(
39673986
category="fact",
3968-
content="Superseded item included when requested test",
3987+
content="Historical note about the old deployment target",
39693988
)
39703989
new_id = store.store(
39713990
category="fact",
3972-
content="Active item included when requested test",
3991+
content="Fresh replacement pointing at a brand new value",
39733992
)
39743993
store.update(old_id, superseded_by=new_id)
39753994

0 commit comments

Comments
 (0)