Repository navigation
fix(memory): keep non-core fact categories instead of rewriting to other - #6733
WillemJiang merged 2 commits into
Conversation
_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.
github-actions
Bot
added
area:agents
willem-bd
left a comment
willem-bd
left a comment
There was a problem hiding this comment.
Reviewed against head 20ec78f. The fix is correct and well-targeted: I traced every surface that touches fact categories at head — the FTS5 retrieval filter is an exact-match SQL comparison (retrieval.py category = ?), consolidation and dedup compare/strip categories without enum assumptions, eviction only special-cases the core "correction" label, the gateway Fact model takes a free-form string, and the settings UI renders/edits category as free text — so non-core labels now round-trip everywhere memory_add/memory_search/get_memory promise. The regression test covers the create path. One suggestion below on a stale legacy field surviving the update path.
| if category not in CORE_CATEGORIES: | ||
| fact.setdefault("categoryExtension", category) | ||
| fact["category"] = "other" | ||
| fact["category"] = raw_category or "context" |
There was a problem hiding this comment.
Good fix. One follow-up: _normalize_fact deep-copies every key and the update paths carry unknown fields through (updater.update_memory_fact builds the candidate from the stored fact via get_fact/dict(fact)), so a legacy fact persisted as category: "other" + categoryExtension: "personal" that is later updated to category="personal" keeps the stale categoryExtension alongside the now-native category. The gateway still surfaces that field (backend/app/gateway/routers/memory.py:76 documents it as "Extension category when category is 'other'"), so it now contradicts its documented meaning. Consider clearing it here once the label is stored natively, e.g. if fact["category"] != "other": fact.pop("categoryExtension", None), and extending the new regression test with an update-path case (legacy fact rewritten to a non-core category) to pin the cleanup.
There was a problem hiding this comment.
Good catch — fixed in ec67df3 with your suggested shape: _normalize_category now drops categoryExtension whenever the category is not "other", so a legacy other/personal fact updated to personal through update_memory_fact no longer persists the stale extension, and the field keeps its documented meaning ("Extension category when category is 'other'").
The update-path case is pinned in test_update_legacy_aliased_fact_to_native_category_clears_extension (kept next to the round-trip test): it stores the fact under the historical alias, rewrites it to the native label, and asserts the extension is gone, the revision bumps, and memory_search(category="personal") now finds it.
willem-bd
left a comment
willem-bd
left a comment
There was a problem hiding this comment.
Read-only static review of pinned current revision; verified introduced findings below. No PR code or tests executed.
| if category not in CORE_CATEGORIES: | ||
| fact.setdefault("categoryExtension", category) | ||
| fact["category"] = "other" | ||
| fact["category"] = raw_category or "context" |
There was a problem hiding this comment.
[P2] Recognize legacy category aliases during migration retries
If an older Gateway commits a per-agent legacy JSON migration and stops, or unlink fails, before deleting the JSON, its Markdown fact contains category='other', categoryExtension='personal' while the retained JSON contains category='personal'. That recoverable state previously resumed successfully. This change also normalizes migration candidates, so _migrate_locked now compares personal/no extension with existing other/personal; _migration_equivalent compares both fields and raises MemoryStorageCorruption. Subsequent reads, reloads and migration retries for that agent fail, surfaced as HTTP 500 by the Gateway, instead of completing recovery. Preserve equivalence with the legacy category/extension representation during migration retries without relaxing genuine content conflicts, and add an interrupted pre-change migration regression test.
There was a problem hiding this comment.
Reproduced exactly as described — thanks for the trace. The interrupted pre-change state (Markdown committed as other + categoryExtension="personal", legacy JSON retained with personal) recovered fine before this PR and raised MemoryStorageCorruption on every read/reload/retry at head.
Fixed in ec67df3: _migration_equivalent now compares categories through _category_comparison_key, which folds the legacy alias — category="other" + non-empty categoryExtension=<label> compares equal to the native <label>/no-extension — so the retry recognizes its own committed result and finishes recovery (deletes the JSON, adopts the committed Markdown as-is).
Genuine content conflicts are not relaxed: a retained JSON label that disagrees with the committed alias (e.g. work vs other/personal) still raises, with the legacy file preserved. test_interrupted_legacy_migration_retry_recovers_aliased_category builds the historical on-disk state by replaying the pre-change _normalize_category and pins both sides.
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.
github-actions
Bot
added
size/M
Why
memory_adddocuments free-form category labels — "Category label for organization (default "context"). e.g. "preference", "correction", "behavior", "personal"" (backend/packages/harness/deerflow/agents/memory/tools.py:135-136) — andmemory_searchpromises "Only facts with this exact category are returned" (tools.py:64-65). Butstorage._normalize_category(backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/storage.py:252-254) rewrote every label outsideCORE_CATEGORIESto"other", parking the original in acategoryExtensionfield that no code in the repo reads (its only other reference is the gateway response model,backend/app/gateway/routers/memory.py:76). As a resultmemory_add(category="personal")— an example label from the tool's own docstring — was persisted ascategory="other",memory_search(category="personal")could never match it, andget_memory/gateway documents kept reporting"other".The same rewrite hit two more surfaces: the memory settings UI's category input is free text (
frontend/src/components/workspace/settings/memory-settings-page.tsx:804), and the fact-extraction prompt's own category enum includesknowledge(backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/prompts/fact_extraction.yaml:12), which is not inCORE_CATEGORIES— so LLM-extractedknowledgefacts were also folded into"other".What changed
_normalize_categorynow keeps the caller's label as the storedcategory(an empty string still falls back to"context"); it no longer rewrites non-core labels to"other".get_memorydocuments, the FTS5 retrieval index, and thememory_searchcategory filter.category/categoryExtensionvalues; the gateway'sFact.categoryExtensionfield still surfaces those legacy values. No data migration is performed.Surface area
categoryinstead of"other"plus a write-onlycategoryExtension.Bug fix verification
backend/tests/test_deermem_self_contained.py::test_create_fact_category_round_trips_into_search_and_documentsmainand green on this branch? yesValidation
Backend (
cd backend):make lint→ruff check .:All checks passed!;ruff format --check .:2037 files already formattedPYTHONPATH=. uv run pytest tests/test_deermem_self_contained.py -q→43 passed(includes the new regression test)PYTHONPATH=. uv run pytest tests/test_memory_storage.py tests/test_memory_normalize.py tests/test_memory_tools.py tests/test_memory_updater.py tests/test_memory_search.py tests/test_memory_relevance_retrieval.py tests/test_memory_router.py -q→278 passedPYTHONPATH=. uv run pytest tests/test_memory_consolidation.py tests/test_memory_eviction.py tests/test_memory_fact_dedup.py tests/test_memory_markdown_migration_cli.py tests/test_memory_manager_interface.py tests/test_memory_manager_pluggable.py tests/test_memory_retrieval_adapter.py tests/test_memory_retrieval_index_local.py tests/test_memory_queue.py tests/test_memory_middleware.py tests/test_memory_scope_gate.py tests/test_gateway_memory_shutdown_cancellation.py tests/test_load_memory_sample.py -q→243 passed, 1 skippedmake test(four parallel shards, full offline suite) →7413 passed, 123 skipped/7434 passed, 106 skipped/7408 passed, 130 skipped/7411 passed, 131 skipped— 29666 passed, 490 skipped, 0 failed. (One earliermake testinvocation hit a transient single-shard failure under load whose detail was not captured; two subsequent full runs of the same shards, including a cleanmake test, passed completely.)AI assistance
Tool(s) used: ZCode coding agent (GLM model), driving the BlueX888 account.
How you used it: The agent located the bug by reading the memory tool docstrings against the storage normalization code, wrote the regression test and the one-function fix, and ran the red/green, suite, and lint commands listed in Validation.