mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-19 11:06:18 +00:00
* feat(memory): add opt-in tolerant MarkdownMemoryStorage (fixes #3124) User-memory summary load is now tolerant of corrupt/partially written files: a Markdown summary (fenced `memory-json` block, best-effort structured fallback) or JSON is accepted, and an unrecoverable file recovers to an empty memory instead of raising MemoryStorageCorruption and taking down the agent. On-disk JSON format and the JSON UI are unchanged, so enabling `memory.storage_class: markdown` is fully opt-in and cannot break existing deployments. Co-authored-by: WorkBuddy <noreply@workbuddy.ai> * fix(markdown-memory): address review - schema-safe loader, greedy fence, quarantine, CI tests - drop lossy structured Markdown fallback (invalid-shape crash on load/save) - greedy fence match: ``` inside remembered strings round-trips losslessly - quarantine unreadable summary before returning None (no silent erase) - remove dead _render_memory_markdown (deferred to write-path change) - move tests to backend/tests/ so CI runs them; update storage_class docs * fix(markdown-memory): address review - schema-safe loader, greedy fence, quarantine, CI tests - drop lossy structured Markdown fallback (invalid-shape crash on load/save) - greedy fence match: ``` inside remembered strings round-trips losslessly - quarantine unreadable summary before returning None (no silent erase) - remove dead _render_memory_markdown (deferred to write-path change) - move tests to backend/tests/ so CI runs them; update storage_class docs * fix(markdown-memory): address review - schema-safe loader, greedy fence, quarantine, CI tests - drop lossy structured Markdown fallback (invalid-shape crash on load/save) - greedy fence match: ``` inside remembered strings round-trips losslessly - quarantine unreadable summary before returning None (no silent erase) - remove dead _render_memory_markdown (deferred to write-path change) - move tests to backend/tests/ so CI runs them; update storage_class docs * fix(markdown-memory): address review - schema-safe loader, greedy fence, quarantine, CI tests - drop lossy structured Markdown fallback (invalid-shape crash on load/save) - greedy fence match: ``` inside remembered strings round-trips losslessly - quarantine unreadable summary before returning None (no silent erase) - remove dead _render_memory_markdown (deferred to write-path change) - move tests to backend/tests/ so CI runs them; update storage_class docs * fix(markdown-memory): address review - schema-safe loader, greedy fence, quarantine, CI tests - drop lossy structured Markdown fallback (invalid-shape crash on load/save) - greedy fence match: ``` inside remembered strings round-trips losslessly - quarantine unreadable summary before returning None (no silent erase) - remove dead _render_memory_markdown (deferred to write-path change) - move tests to backend/tests/ so CI runs them; update storage_class docs * fix(memory): correct tolerant Markdown parsing and regression tests --------- Co-authored-by: WorkBuddy <noreply@workbuddy.ai> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Kris
GitHub
parent
74ab3cf818
commit
208ba7cc65
@ -1640,6 +1640,13 @@ and the [request contract](backend/docs/API.md#referencing-a-previous-conversati
|
||||
|
||||
### Long-Term Memory
|
||||
|
||||
For DeerMem, `memory.backend_config.storage_class: markdown` opts into tolerant
|
||||
summary reads while keeping JSON writes and the existing UI. A hand-edited
|
||||
`memory.json` can contain its JSON object inside a fenced `memory-json` block;
|
||||
embedded backticks and later fenced notes are supported. Unparseable summary
|
||||
text is moved to `memory.json.corrupt-<timestamp>` for recovery before rebuilding.
|
||||
`storage_path` must point to the data root directory, not an existing JSON file.
|
||||
|
||||
Gateway shutdown drains memory updates before closing the backend, even when
|
||||
shutdown is cancelled. Config reload failures are logged without aborting runtime
|
||||
teardown. For Kubernetes, budget `terminationGracePeriodSeconds` for all shutdown
|
||||
|
||||
@ -84,6 +84,14 @@ existing enabled behavior.
|
||||
|
||||
#### DeerMem storage contract
|
||||
|
||||
`memory.backend_config.storage_class: markdown` opts into tolerant summary
|
||||
reads; writes still use JSON. Only a JSON object in a closed `memory-json`
|
||||
fence is accepted from Markdown. Decode the value before checking its closing
|
||||
fence so embedded backticks and later fenced notes stay lossless. Parser and
|
||||
storage regressions belong in `tests/test_memory_storage.py`; configure a data
|
||||
root directory, not the existing manifest file. Legacy v1 reads still migrate
|
||||
and advance the revision. Unparseable summary text is quarantined before rebuild.
|
||||
|
||||
`FileMemoryStorage` owns canonical storage and the retrieval adapter.
|
||||
Do not reach into its private adapter state from higher layers.
|
||||
|
||||
|
||||
@ -53,7 +53,12 @@ class DeerMemConfig(BaseModel):
|
||||
)
|
||||
storage_class: str = Field(
|
||||
default="",
|
||||
description="Dotted class path for an alternative storage provider; empty (default) = FileMemoryStorage (no importlib, portable).",
|
||||
description=(
|
||||
"Dotted class path for an alternative storage provider, or a built-in alias: "
|
||||
"``file`` = FileMemoryStorage (default) or ``markdown`` = MarkdownMemoryStorage "
|
||||
"(tolerant load path, same JSON on disk); empty (default) = FileMemoryStorage "
|
||||
"(no importlib, portable)."
|
||||
),
|
||||
)
|
||||
strict_user_scope: bool = Field(
|
||||
default=False,
|
||||
|
||||
@ -0,0 +1,56 @@
|
||||
"""Markdown-aware parsing for DeerMem user-memory summaries.
|
||||
|
||||
This module is intentionally dependency-free so it can be unit-tested and
|
||||
imported without the rest of the DeerMem stack.
|
||||
|
||||
Design (read path only)
|
||||
-----------------------
|
||||
A Markdown summary carries its *lossless* state inside a fenced
|
||||
```` ```memory-json ```` block. When loading, the fenced JSON block is the
|
||||
only trusted Markdown representation: if it is present and parses to a JSON
|
||||
object it is returned verbatim; anything else (no fence, malformed fence,
|
||||
non-object JSON) yields ``None`` so the caller can decide policy (the
|
||||
default is to quarantine the unreadable file rather than silently rebuild
|
||||
over persistent state).
|
||||
|
||||
The JSON decoder locates the end of the value before the closing fence is
|
||||
checked. Backticks inside remembered strings cannot truncate the value, and
|
||||
later fenced notes cannot be accidentally consumed as part of the JSON.
|
||||
|
||||
A lossy structured parse of the human-readable sections is deliberately NOT
|
||||
provided: it cannot reproduce the manifest schema (``user``/``history`` must
|
||||
be objects, ``version``/``revision`` scalars) and previously surfaced as
|
||||
``ValueError``/``AttributeError`` crashes on the very hand-edited files the
|
||||
loader claimed to tolerate. Rendering Markdown is deferred to a future
|
||||
write-path change.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import re
|
||||
from typing import Any
|
||||
|
||||
_OPEN_FENCE_RE = re.compile(r"```memory-json[ \t]*\r?\n")
|
||||
_CLOSE_FENCE_RE = re.compile(r"[ \t\r\n]*\r?\n[ \t]*```[ \t]*(?:\r?\n|$)")
|
||||
|
||||
|
||||
def _parse_markdown_memory(raw: str) -> dict[str, Any] | None:
|
||||
"""Parse a Markdown summary into a dict, or None when nothing usable.
|
||||
|
||||
Only the fenced ```` ```memory-json ```` block is trusted. There is no
|
||||
structured fallback: without a valid fenced block the file cannot be
|
||||
mapped onto the manifest schema losslessly, so returning ``None`` (the
|
||||
caller quarantines and starts fresh) is the honest outcome.
|
||||
"""
|
||||
opening = _OPEN_FENCE_RE.search(raw)
|
||||
if opening is None:
|
||||
return None
|
||||
payload = raw[opening.end() :].lstrip()
|
||||
try:
|
||||
value, end = json.JSONDecoder().raw_decode(payload)
|
||||
except json.JSONDecodeError:
|
||||
return None
|
||||
if _CLOSE_FENCE_RE.match(payload, end) is None:
|
||||
return None
|
||||
return value if isinstance(value, dict) else None
|
||||
@ -0,0 +1,102 @@
|
||||
"""Opt-in Markdown-aware summary storage for DeerMem.
|
||||
|
||||
The default :class:`FileMemoryStorage` persists the user-memory *summary* as a
|
||||
single JSON document. Reasoning/thinking models occasionally emit malformed
|
||||
JSON, and a partially written summary historically raised
|
||||
``MemoryStorageCorruption`` and took down the whole agent.
|
||||
|
||||
``MarkdownMemoryStorage`` keeps the same on-disk JSON as the default for full
|
||||
backward compatibility, but its loader is *tolerant*:
|
||||
|
||||
* a corrupt or partially written summary no longer crashes the agent;
|
||||
* a Markdown summary is accepted only through its fenced ```` ```memory-json ````
|
||||
block, which is parsed losslessly;
|
||||
* when the on-disk file is unreadable (neither valid JSON nor a Markdown
|
||||
summary with a usable fenced block), it is *quarantined* as
|
||||
``memory.json.corrupt-<timestamp>`` before the loader returns ``None``.
|
||||
Quarantining keeps the content recoverable: returning ``None`` alone would
|
||||
make the next :meth:`save` rebuild the manifest from scratch (revision
|
||||
reset, no journal backup) and silently erase the unreadable state.
|
||||
|
||||
Writes still persist JSON (the write path is untouched). Hand-edited
|
||||
Markdown files are therefore a *read-time* convenience: the next write
|
||||
rewrites ``memory.json`` as JSON, so the Markdown rendering is temporary
|
||||
until a Markdown write path lands.
|
||||
|
||||
This is intentionally a small, additive change scoped to the load path only:
|
||||
the JSON UI and all other backends are untouched. Enabling it cannot break
|
||||
existing deployments.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from .markdown_format import _parse_markdown_memory
|
||||
from .storage import FileMemoryStorage, logger
|
||||
|
||||
|
||||
class MarkdownMemoryStorage(FileMemoryStorage):
|
||||
"""File-backed storage whose summary loader tolerates corrupt/Markdown.
|
||||
|
||||
Fully opt-in. Enable via ``memory.storage_class: markdown`` (or the full
|
||||
import path ``deerflow.agents.memory.backends.deermem.deermem.core.
|
||||
markdown_storage.MarkdownMemoryStorage``). The default JSON summary
|
||||
format is unchanged, so the existing JSON UI and all other backends keep
|
||||
working.
|
||||
"""
|
||||
|
||||
def _load_memory_file(self, path: Path) -> dict[str, Any] | None:
|
||||
if not path.exists():
|
||||
return None
|
||||
try:
|
||||
raw = path.read_text(encoding="utf-8")
|
||||
except (OSError, UnicodeError) as exc:
|
||||
logger.warning("Cannot read memory summary %s: %s", path, exc)
|
||||
return None
|
||||
|
||||
parsed: dict[str, Any] | None = None
|
||||
try:
|
||||
value = json.loads(raw)
|
||||
except json.JSONDecodeError:
|
||||
value = None
|
||||
if isinstance(value, dict):
|
||||
parsed = value
|
||||
else:
|
||||
# Not a JSON object: the opt-in Markdown summary is accepted only
|
||||
# through its lossless fenced ```memory-json block. No structured
|
||||
# fallback -- a best-effort section parse cannot reproduce the
|
||||
# manifest schema (object-shaped user/history) and previously
|
||||
# crashed load()/save() on the files it claimed to tolerate.
|
||||
parsed = _parse_markdown_memory(raw)
|
||||
|
||||
if parsed is not None:
|
||||
return parsed
|
||||
|
||||
logger.warning(
|
||||
"Memory summary %s is unreadable (neither valid JSON nor a Markdown summary with a usable ```memory-json block); quarantining the file so its content stays recoverable instead of being silently overwritten by the next save.",
|
||||
path,
|
||||
)
|
||||
self._quarantine_unreadable(path)
|
||||
return None
|
||||
|
||||
@staticmethod
|
||||
def _quarantine_unreadable(path: Path) -> None:
|
||||
"""Move an unreadable summary aside; the next save rebuilds from scratch.
|
||||
|
||||
Without this, returning ``None`` would let ``_commit_changes_locked``
|
||||
rebuild the manifest from ``create_empty_memory()`` and skip its
|
||||
recovery backup (which only runs when a current memory exists),
|
||||
silently destroying the previous state.
|
||||
"""
|
||||
stamp = datetime.now().strftime("%Y%m%d-%H%M%S-%f")
|
||||
target = path.with_name(f"{path.name}.corrupt-{stamp}")
|
||||
try:
|
||||
path.replace(target)
|
||||
logger.warning("Quarantined unreadable memory summary as %s", target)
|
||||
except OSError as exc:
|
||||
# Quarantine is best-effort: keep the tolerant-read guarantee.
|
||||
logger.error("Could not quarantine unreadable memory summary %s: %s", path, exc)
|
||||
@ -1843,6 +1843,10 @@ def create_storage(config: DeerMemConfig, retrieval: RetrievalPort | None = None
|
||||
storage_class_path = config.storage_class
|
||||
if not storage_class_path or storage_class_path == "file":
|
||||
return FileMemoryStorage(config, retrieval=retrieval)
|
||||
if storage_class_path == "markdown":
|
||||
from .markdown_storage import MarkdownMemoryStorage
|
||||
|
||||
return MarkdownMemoryStorage(config, retrieval=retrieval)
|
||||
try:
|
||||
module_path, class_name = storage_class_path.rsplit(".", 1)
|
||||
storage_class = getattr(importlib.import_module(module_path), class_name)
|
||||
|
||||
@ -1,11 +1,14 @@
|
||||
"""Tests for memory storage providers (DI: FileMemoryStorage(config) / create_storage)."""
|
||||
|
||||
import json
|
||||
import threading
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from deerflow.agents.memory.backends.deermem.deermem.config import DeerMemConfig
|
||||
from deerflow.agents.memory.backends.deermem.deermem.core import markdown_format as mf
|
||||
from deerflow.agents.memory.backends.deermem.deermem.core.markdown_storage import MarkdownMemoryStorage
|
||||
from deerflow.agents.memory.backends.deermem.deermem.core.paths import validate_agent_name
|
||||
from deerflow.agents.memory.backends.deermem.deermem.core.storage import (
|
||||
FileMemoryStorage,
|
||||
@ -178,3 +181,102 @@ class TestCreateStorage:
|
||||
def test_dotted_storage_class_resolves(self):
|
||||
storage = create_storage(DeerMemConfig(storage_class="deerflow.agents.memory.backends.deermem.deermem.core.storage.FileMemoryStorage"))
|
||||
assert isinstance(storage, FileMemoryStorage)
|
||||
|
||||
|
||||
class TestMarkdownMemoryStorage:
|
||||
"""Opt-in ``storage_class="markdown"``: tolerant load path (issue #3124)."""
|
||||
|
||||
def _markdown_storage_at(self, memory_file) -> MarkdownMemoryStorage:
|
||||
return MarkdownMemoryStorage(DeerMemConfig(storage_path=str(memory_file.parent.resolve())))
|
||||
|
||||
def test_markdown_alias_resolves(self):
|
||||
storage = create_storage(DeerMemConfig(storage_class="markdown"))
|
||||
assert isinstance(storage, MarkdownMemoryStorage)
|
||||
assert isinstance(storage, FileMemoryStorage)
|
||||
|
||||
@pytest.mark.parametrize(("version", "expected_revision"), [("1.0", 8), ("2.0", 7)])
|
||||
def test_corrupt_json_with_fenced_block_recovers(self, tmp_path, version, expected_revision):
|
||||
"""A partially written JSON summary that still carries a fenced
|
||||
```memory-json block is recovered losslessly instead of crashing."""
|
||||
memory_file = tmp_path / "memory.json"
|
||||
manifest = create_empty_memory()
|
||||
manifest["version"] = version
|
||||
if version == "2.0":
|
||||
manifest.pop("facts") # v2 manifests keep facts in separate files.
|
||||
manifest["revision"] = 7
|
||||
manifest["user"]["workContext"] = {"summary": "recovered"}
|
||||
fenced = '{"version": 1, "revision": 7, "user": {"lang": "zh"}\n```memory-json\n' + json.dumps(manifest, ensure_ascii=False) + "\n```"
|
||||
memory_file.write_text(fenced, encoding="utf-8")
|
||||
storage = self._markdown_storage_at(memory_file)
|
||||
loaded = storage.load()
|
||||
# Legacy manifests migrate to v2 on load and advance the revision.
|
||||
assert loaded["version"] == "2.0"
|
||||
assert loaded["revision"] == expected_revision
|
||||
assert loaded["user"]["workContext"]["summary"] == "recovered"
|
||||
|
||||
def test_hand_edited_markdown_without_fence_is_quarantined(self, tmp_path):
|
||||
"""Markdown without a fenced block cannot be mapped onto the manifest
|
||||
schema losslessly: load() must not crash AND must not return an
|
||||
invalid shape -- the file is quarantined so nothing is silently lost."""
|
||||
memory_file = tmp_path / "memory.json"
|
||||
body = "# DeerFlow Memory\n\n- version: 2\n- revision: 5\n\n## User\n- summary: likes tea\n"
|
||||
memory_file.write_text(body, encoding="utf-8")
|
||||
storage = self._markdown_storage_at(memory_file)
|
||||
loaded = storage.load() # must not raise
|
||||
assert isinstance(loaded, dict)
|
||||
quarantined = list(tmp_path.glob("memory.json.corrupt-*"))
|
||||
assert len(quarantined) == 1, "unreadable file must be preserved via quarantine"
|
||||
assert quarantined[0].read_text(encoding="utf-8") == body
|
||||
|
||||
def test_truncated_json_is_quarantined_before_rebuild(self, tmp_path):
|
||||
"""A truncated manifest must not be silently erased by the next save:
|
||||
the unreadable file is quarantined (revision reset is then safe)."""
|
||||
memory_file = tmp_path / "memory.json"
|
||||
truncated = '{"version": "2.0", "revision": 41, "user": {"work'
|
||||
memory_file.write_text(truncated, encoding="utf-8")
|
||||
storage = self._markdown_storage_at(memory_file)
|
||||
loaded = storage.load()
|
||||
assert isinstance(loaded, dict)
|
||||
assert storage.save(create_empty_memory()) is True
|
||||
quarantined = list(tmp_path.glob("memory.json.corrupt-*"))
|
||||
assert len(quarantined) == 1
|
||||
assert quarantined[0].read_text(encoding="utf-8") == truncated
|
||||
|
||||
def test_fenced_block_containing_backticks_parses_losslessly(self):
|
||||
"""Remembered code snippets must not terminate the JSON block."""
|
||||
manifest = create_empty_memory()
|
||||
manifest["user"]["workContext"] = {"summary": "prefers ```python\nprint('hi')\n``` snippets"}
|
||||
rendered = "# DeerFlow Memory\n\n```memory-json\n" + json.dumps(manifest, ensure_ascii=False) + "\n```\n"
|
||||
parsed = mf._parse_markdown_memory(rendered)
|
||||
assert parsed == manifest
|
||||
|
||||
@pytest.mark.parametrize("newline", ["\n", "\r\n"])
|
||||
def test_fenced_summary_with_trailing_code_block_loads_without_quarantine(self, tmp_path, newline):
|
||||
manifest = create_empty_memory()
|
||||
manifest["version"] = "2.0"
|
||||
manifest.pop("facts")
|
||||
manifest["revision"] = 7
|
||||
manifest["user"]["workContext"]["summary"] = "prefers ```python snippets```"
|
||||
body = newline.join(["# Memory", "```memory-json", json.dumps(manifest, indent=2), "", "```", "Notes:", "```python", "print('example')", "```", ""])
|
||||
memory_file = tmp_path / "memory.json"
|
||||
memory_file.write_text(body, encoding="utf-8")
|
||||
assert mf._parse_markdown_memory(body) == manifest
|
||||
loaded = self._markdown_storage_at(memory_file).load()
|
||||
assert loaded["revision"] == 7
|
||||
assert loaded["user"]["workContext"] == manifest["user"]["workContext"]
|
||||
assert not list(tmp_path.glob("memory.json.corrupt-*"))
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
[
|
||||
"# Memory without a fence",
|
||||
'```memory-json\n{"user": ',
|
||||
'```memory-json\n{"user": {}}',
|
||||
'```memory-json\n{"user": {}} trailing garbage\n```',
|
||||
'```memory-json\n{"user": {}}\n```python\nnotes\n```',
|
||||
"```memory-json\n[]\n```",
|
||||
"```memory-json\nnull\n```",
|
||||
],
|
||||
)
|
||||
def test_invalid_or_non_object_fenced_summary_rejected(self, body):
|
||||
assert mf._parse_markdown_memory(body) is None
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user