fix(memory): keep non-core fact categories instead of rewriting to other (#6733)

* fix(memory): keep non-core fact categories instead of rewriting to other

_normalize_category rewrote any category outside CORE_CATEGORIES to
"other" and parked the original label in a write-only
categoryExtension field, so memory_add(category="personal") -- an
example label in the tool's own docstring -- persisted as "other" and
memory_search(category="personal") ("Only facts with this exact category
are returned") never matched it. get_memory documents kept reporting
"other" as well.

Keep the caller's label as the stored category so it round-trips
through search, documents, and the retrieval index.

* fix(memory): finish migrations interrupted across the category change

A pre-native-category Gateway committed a per-agent legacy JSON migration
as category="other" + categoryExtension=<label> and could stop before
deleting the legacy JSON (crash / failed unlink). The retained JSON holds
the label natively, so _migrate_locked compares "personal" against its
own committed "other"/"personal" alias, raises MemoryStorageCorruption,
and fails every subsequent read, reload and migration retry for that
agent (HTTP 500 at the Gateway) -- a recoverable state the previous
build recovered from.

Fold the legacy alias into _migration_equivalent's category comparison
("other"/<label> equals <label>/no-extension) so retries recognize and
complete recovery, while a genuinely different label still stops the
migration with the legacy file preserved.

Also drop a stale categoryExtension when a fact is normalized away from
"other": _normalize_fact deep-copies unknown fields through, so a legacy
fact updated to its native category kept surfacing the leftover
extension, contradicting the Gateway contract that the field is only
set "when category is 'other'".

Both regressions are pinned by replaying the historical writer in tests.

---------

Co-authored-by: BlueX888 <BlueX888@users.noreply.github.com>
This commit is contained in:
哈基米 2026-10-12 22:45:08 +08:00 committed by GitHub
parent 22ddb77f83
commit 1b7ca5ecf1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 177 additions and 7 deletions

View File

@ -248,10 +248,31 @@ def _normalize_category(fact: dict[str, Any]) -> None:
raw_category = fact.get("category", "context")
if not isinstance(raw_category, str):
raise ValueError("fact.category must be a string")
category = raw_category or "context"
if category not in CORE_CATEGORIES:
fact.setdefault("categoryExtension", category)
fact["category"] = "other"
fact["category"] = raw_category or "context"
# ``categoryExtension`` is only meaningful while ``category`` is "other"
# (pre-native-category builds parked non-core labels there). Once a label
# is stored natively -- including a legacy "other"+extension fact updated
# to a real category -- the leftover field would contradict the Gateway's
# documented meaning, so drop it.
if fact["category"] != "other":
fact.pop("categoryExtension", None)
def _category_comparison_key(fact: dict[str, Any]) -> tuple[Any, ...]:
"""Category pair with the legacy ``other`` alias folded into the label.
Pre-native-category builds persisted a label outside CORE_CATEGORIES as
``category="other"`` plus ``categoryExtension=<label>``; this build stores
the label natively. A migration interrupted after its Markdown commit but
before the legacy JSON delete must still recognize its own result on retry,
so ``"other"``/<label> compares equal to <label>/no-extension here, while
genuinely different labels or extensions stay different.
"""
category = fact.get("category")
extension = fact.get("categoryExtension")
if category == "other" and isinstance(extension, str) and extension:
return extension, None
return category, extension
def _require_string_list(fact: dict[str, Any], field: str) -> None:
@ -1104,8 +1125,10 @@ class FileMemoryStorage(MemoryStorage):
@staticmethod
def _migration_equivalent(left: dict[str, Any], right: dict[str, Any]) -> bool:
ignored = {"revision", "createdAt", "updatedAt", "scope", "title", "schemaVersion"}
return {key: value for key, value in left.items() if key not in ignored} == {key: value for key, value in right.items() if key not in ignored}
ignored = {"revision", "createdAt", "updatedAt", "scope", "title", "schemaVersion", "category", "categoryExtension"}
left_material = {key: value for key, value in left.items() if key not in ignored}
right_material = {key: value for key, value in right.items() if key not in ignored}
return left_material == right_material and _category_comparison_key(left) == _category_comparison_key(right)
def _load_memory_file(self, path: Path) -> dict[str, Any] | None:
if not path.exists():

View File

@ -13,6 +13,7 @@ injected onto the updater so no network is needed.
from __future__ import annotations
import json
import sys
from pathlib import Path
@ -20,11 +21,12 @@ import pytest
from langchain_core.messages import AIMessage, HumanMessage
from deerflow.agents.memory.backends.deermem.deer_mem import DeerMem
from deerflow.agents.memory.backends.deermem.deermem.core import storage as deermem_storage
from deerflow.agents.memory.backends.deermem.deermem.core.message_processing import (
filter_messages_for_memory,
)
from deerflow.agents.memory.backends.deermem.deermem.core.storage import FileMemoryStorage
from deerflow.agents.memory.manager import MemoryCallbacks
from deerflow.agents.memory.manager import MemoryCallbacks, MemoryCorruptionError
@pytest.fixture
@ -54,6 +56,25 @@ def _deermem_with_fake_llm(backend_config=None, payload=None, callbacks=None) ->
return dm
def _legacy_alias_normalize_category(fact: dict) -> None:
"""The pre-native-category ``storage._normalize_category``.
Historical builds parked any label outside CORE_CATEGORIES in
``categoryExtension`` and stored ``category="other"``; migration-retry and
update-path regressions below replay data an interrupted such build left
behind, so the tests need the exact historical writer.
"""
raw_category = fact.get("category", "context")
if not isinstance(raw_category, str):
raise ValueError("fact.category must be a string")
category = raw_category or "context"
if category not in deermem_storage.CORE_CATEGORIES:
fact.setdefault("categoryExtension", category)
fact["category"] = "other"
else:
fact["category"] = category
def test_add_swallows_queue_full_so_backpressure_does_not_break_caller(deermem_data_dir, caplog) -> None:
"""Regression: QueueFull raised under backpressure is caught in
DeerMem.add (the backend owns the queue, so it owns the degradation) so
@ -537,6 +558,132 @@ def test_search_survives_non_float_confidence(deermem_data_dir):
assert [r["id"] for r in results] == ["b", "a", "c"]
def test_create_fact_category_round_trips_into_search_and_documents(deermem_data_dir):
"""A non-core category label must survive the write and stay searchable.
Regression: ``storage._normalize_category`` rewrote every label outside
CORE_CATEGORIES to "other" (parking the original in a write-only
``categoryExtension`` field), so a fact added via
``memory_add(category="personal")`` -- an example label in the memory_add
docstring -- was persisted as "other", never matched
``memory_search(category="personal")`` ("Only facts with this exact category
are returned"), and kept reporting "other" in get_memory documents.
"""
dm = DeerMem(backend_config={"storage_path": str(deermem_data_dir)})
memory_data, fact_id = dm.create_fact(
"The user lives in Paris",
category="personal",
confidence=0.9,
user_id="u1",
)
assert fact_id is not None
assert next(f for f in memory_data["facts"] if f["id"] == fact_id)["category"] == "personal"
document = dm.get_memory(user_id="u1")
assert next(f for f in document["facts"] if f["id"] == fact_id)["category"] == "personal"
results = dm.search("Paris", top_k=10, user_id="u1", category="personal")
assert [f["id"] for f in results] == [fact_id]
def test_interrupted_legacy_migration_retry_recovers_aliased_category(deermem_data_dir, monkeypatch):
"""Regression: a migration interrupted after its Markdown commit recovers.
An older build (pre-native-category ``_normalize_category``) committed a
per-agent legacy JSON migration and stopped before deleting the JSON. Its
Markdown fact holds ``category="other"`` + ``categoryExtension="personal"``
while the retained JSON holds ``category="personal"``. The next build's
migration candidate keeps "personal" natively, so the retry must recognize
the alias as the same fact and finish recovery (delete the JSON) instead of
raising a migration conflict that fails every read and retry for the agent
(HTTP 500 at the Gateway). A genuinely different label must still conflict.
"""
dm = DeerMem(backend_config={"storage_path": str(deermem_data_dir)})
memory_path = dm._storage._get_memory_file_path("researcher", user_id="u1")
legacy_path = FileMemoryStorage._legacy_agent_memory_path(dm._storage, memory_path, "researcher")
legacy_path.parent.mkdir(parents=True)
legacy_path.write_text(
json.dumps(
{
"version": "1.0",
"lastUpdated": "2026-09-01T00:00:00Z",
"user": {},
"history": {},
"facts": [
{
"id": "fact_interrupted_personal",
"content": "The user's sister is called Lea",
"category": "personal",
"confidence": 0.9,
"source": "manual",
"createdAt": "2026-09-01T00:00:00Z",
"revision": 1,
}
],
}
),
encoding="utf-8",
)
retained = legacy_path.read_bytes()
native_normalize = deermem_storage._normalize_category
monkeypatch.setattr(deermem_storage, "_normalize_category", _legacy_alias_normalize_category)
dm.get_memory(agent_name="researcher", user_id="u1") # the old Gateway commits the migration
legacy_path.write_bytes(retained) # ... and stops before the unlink
monkeypatch.setattr(deermem_storage, "_normalize_category", native_normalize)
# The retry must complete recovery instead of raising a conflict.
document = dm.get_memory(agent_name="researcher", user_id="u1")
assert not legacy_path.exists()
fact = next(f for f in document["facts"] if f["id"] == "fact_interrupted_personal")
# The already-committed Markdown fact is adopted as-is, not rewritten.
assert (fact["category"], fact.get("categoryExtension")) == ("other", "personal")
# Recovery must not relax genuine conflicts: a retained JSON label that
# disagrees with the committed alias still stops the migration and keeps
# the legacy file for a human decision.
payload = json.loads(retained)
payload["facts"][0]["category"] = "work"
legacy_path.write_text(json.dumps(payload), encoding="utf-8")
with pytest.raises(MemoryCorruptionError):
dm.get_memory(agent_name="researcher", user_id="u1")
assert legacy_path.exists()
def test_update_legacy_aliased_fact_to_native_category_clears_extension(deermem_data_dir, monkeypatch):
"""Regression: rewriting a legacy "other"+extension fact drops the stale extension.
``_normalize_fact`` deep-copies every key, so a legacy fact persisted as
``category="other"`` + ``categoryExtension="personal"`` that is updated to
the now-native ``category="personal"`` used to keep the stale extension --
contradicting the Gateway contract that the field is only set "when
category is 'other'". The update must clear it, and the rewritten fact
must be findable by its native label.
"""
dm = DeerMem(backend_config={"storage_path": str(deermem_data_dir)})
native_normalize = deermem_storage._normalize_category
monkeypatch.setattr(deermem_storage, "_normalize_category", _legacy_alias_normalize_category)
_, fact_id = dm.create_fact(
"The user's sister is called Lea",
category="personal",
agent_name="researcher",
user_id="u1",
)
monkeypatch.setattr(deermem_storage, "_normalize_category", native_normalize)
stored = next(f for f in dm.get_memory(agent_name="researcher", user_id="u1")["facts"] if f["id"] == fact_id)
assert (stored["category"], stored.get("categoryExtension")) == ("other", "personal")
updated = dm.update_fact(fact_id, category="personal", agent_name="researcher", user_id="u1")
fact = next(f for f in updated["facts"] if f["id"] == fact_id)
assert fact["category"] == "personal"
assert "categoryExtension" not in fact
assert fact["revision"] == 2
results = dm.search("sister", top_k=10, user_id="u1", agent_name="researcher", category="personal")
assert [f["id"] for f in results] == [fact_id]
def test_query_search_records_access_but_default_injection_does_not(deermem_data_dir):
dm = DeerMem(
backend_config={