From 1b7ca5ecf1f12bf58f6c0e28f7e738536afea7dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=93=88=E5=9F=BA=E7=B1=B3?=
 <140241684+BlueX888@users.noreply.github.com>
Date: Mon, 12 Oct 2026 22:45:08 +0800
Subject: [PATCH] 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>
---
 .../backends/deermem/deermem/core/storage.py  |  35 +++-
 backend/tests/test_deermem_self_contained.py  | 149 +++++++++++++++++-
 2 files changed, 177 insertions(+), 7 deletions(-)

diff --git a/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/storage.py b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/storage.py
index dce3399ae2..9f38aef87c 100644
--- a/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/storage.py
+++ b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/storage.py
@@ -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():
diff --git a/backend/tests/test_deermem_self_contained.py b/backend/tests/test_deermem_self_contained.py
index 6c5d14ae1c..66093549cc 100644
--- a/backend/tests/test_deermem_self_contained.py
+++ b/backend/tests/test_deermem_self_contained.py
@@ -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={