mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-08-10 14:58:46 +00:00
* refactor(memory): pluggable MemoryManager interface for backend onboarding
Optimize the MemoryManager interface layer so new backends (mem0/openviking)
onboard with less code and the contract stays stable as capabilities are
added. A minimal backend now implements only from_config + add + get_context
(verified by test_memory_manager_interface.py::_MinimalBackend onboarding via
the factory); the factory no longer knows a backend's private hooks.
- MemoryManager: ABC -> pydantic BaseModel; three-tier methods (tier-1
add/get_context abstract; tier-2 management defaults; tier-3 optional hooks
warm/reload/fact + on_pre_compress/on_turn_start). Dropped 3 self-serving
hooks. 6 hasattr probe sites -> direct call + try/except NotImplementedError.
- from_config classmethod: factory thins to resolve + inject storage_path +
collect host hooks + call from_config; DeerMem-specific hook consumption
moved from factory to DeerMem.from_config.
- Invariants: @model_validator (mode='tool' requires search via supports_search
ClassVar); DeerMemConfig storage_path-is-file check moved here from factory.
- Async: aadd/aget_context/asearch default to the sync path (speculative).
- Callbacks: MemoryCallbacks + LangfuseMemoryCallbacks; on_memory_llm_call
subsumes tracing_callback (same signature/timing/mutation); deleted the
tracing_callback field. DeerMem decoupled from langfuse (portability).
- noop keeps read-op empty overrides (avoids router 500s on the
disable-memory-via-noop path); only delete/export inherit the base raise.
Behavior preserved: 661 passed / 13 skipped. Docs: backends/README.md rewritten
(three-tier + from_config + callbacks); samples README updated; removed stale
private doc paths.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(memory): 501 on unsupported read/manage endpoints + accurate warm log
Review follow-up on the three-tier MemoryManager refactor.
- Read/manage endpoints (GET /memory, /memory/export, /memory/status,
DELETE /memory, POST /memory/import) and the /memory/reload fallback now
catch NotImplementedError -> 501, matching the fact-CRUD endpoints. The
hasattr->try/except migration had skipped these: they were @abstractmethod
before (every backend implemented them, so they never raised), so once they
became tier-2 default-raise a minimal backend (only add + get_context) hit a
raw 500 -- there is no global NotImplementedError handler. get_memory is
shared via _get_memory_or_501 (covers /memory, export, status, reload
fallback). noop is unchanged: its read-op empty overrides never raise.
- warm() base default returns None (tri-state: True=warmed, False=failed,
None=nothing to warm) so the Gateway lifespan logs "skipping" for a
non-DeerMem backend (e.g. noop) instead of the inaccurate "warmed
successfully" it never earned. DeerMem.warm keeps True/False.
- Tests: 6 router 501 tests (read/manage + reload fallback) + 2 lifespan
warm-log tests (None->skipping, False->warning); conformance/pluggable
assert warm() is None.
705 passed / 13 skipped; lint clean.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(memory): review follow-ups - search-flag consistency, client reload, backend_config purity
Address review feedback on the three-tier MemoryManager refactor:
- [Medium] supports_search/search drift: the invariant now requires the
supports_search ClassVar flag to MATCH whether search() is actually
overridden (type(self).search is not MemoryManager.search), so the flag
can't drift from the impl. Catches both directions at instantiation: a
backend that overrides search() but forgets supports_search=True (was a
misleading tool-mode rejection), and one that sets the flag without
overriding (was a runtime NotImplementedError on the first memory_search).
noop sets supports_search=True to match its search() override. Conformance
adds drift + consistent-backend tests.
- [Low] client.reload_memory fallback: wrap the get_memory fallback so a
minimal backend (only add + get_context) surfaces a clean NotImplementedError
("implements neither reload_memory nor get_memory") instead of an uncaught
propagation -- mirrors the router's 501. Test added.
- [Low] backend_config purity: DeerMem.from_config restores backend_config to
the pure data the host passed after model_post_init parses the injected hooks
into DeerMemConfig (self._config, PrivateAttr); the field stays serializable
(no callables/LLM) and matches the README ("host hooks NOT in backend_config").
Test asserts purity + hooks wired.
- [Low] CHANGELOG: breaking-change note that mode='tool' + non-search backend
now fails fast at startup (was silently empty) so operators recognize it on
upgrade.
- [Nit] .gitignore: drop the env-specific .tmp-pytest/ entry (--basetemp is
local-only, not make test/CI).
709 passed / 13 skipped; lint clean.
Co-Authored-By: Claude <noreply@anthropic.com>
* docs(changelog): correct memory tool-mode fail-fast note
The CHANGELOG entry said mode='tool' + a non-search backend "(e.g. noop)"
fails fast at startup, but noop overrides search() (returns []) and sets
supports_search=True (required by the consistency invariant), so noop IS
search-capable and noop+tool does NOT fail fast. The fail-fast only affects a
custom backend that onboards without overriding search(). Reworded to drop the
misleading noop example and state both shipping backends implement search().
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
201 lines
9.0 KiB
Python
201 lines
9.0 KiB
Python
"""Pluggable memory manager: the factory resolves the configured backend.
|
|
|
|
Covers the drop-in contract end-to-end:
|
|
- short name -> registered backend (deermem / noop);
|
|
- dotted path (``module.Attr`` and ``module:Attr``) -> the same class;
|
|
- unknown value -> raise (fail-fast: a wrong store is a silent data-integrity footgun).
|
|
|
|
Also pins the noop empty-memory behaviour and the ``hasattr`` capability
|
|
probing surface (reload_memory + fact CRUD) that the gateway/client rely on.
|
|
|
|
Each test resets the singleton + backend cache and sets the config, so they
|
|
are independent of order.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
from deerflow.agents.memory import (
|
|
MemoryManager,
|
|
get_memory_manager,
|
|
reset_memory_manager,
|
|
)
|
|
from deerflow.agents.memory.backends.deermem.deer_mem import DeerMem
|
|
from deerflow.agents.memory.backends.noop.noop_manager import NoopMemoryManager
|
|
from deerflow.config.memory_config import MemoryConfig, get_memory_config, set_memory_config
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _isolate_memory_manager():
|
|
"""Reset the singleton + restore config around every test."""
|
|
orig = get_memory_config()
|
|
reset_memory_manager()
|
|
yield
|
|
set_memory_config(orig)
|
|
reset_memory_manager()
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"manager_class, expected",
|
|
[
|
|
("deermem", DeerMem),
|
|
("noop", NoopMemoryManager),
|
|
("deerflow.agents.memory.backends.deermem.deer_mem.DeerMem", DeerMem),
|
|
("deerflow.agents.memory.backends.deermem.deer_mem:DeerMem", DeerMem),
|
|
("deerflow.agents.memory.backends.noop.noop_manager.NoopMemoryManager", NoopMemoryManager),
|
|
("deerflow.agents.memory.backends.noop.noop_manager:NoopMemoryManager", NoopMemoryManager),
|
|
],
|
|
)
|
|
def test_resolves_configured_backend(manager_class: str, expected: type[MemoryManager]) -> None:
|
|
set_memory_config(MemoryConfig(manager_class=manager_class))
|
|
manager = get_memory_manager()
|
|
assert isinstance(manager, expected)
|
|
# singleton: a second call returns the same instance
|
|
assert get_memory_manager() is manager
|
|
|
|
|
|
def test_unknown_backend_raises_instead_of_falling_back() -> None:
|
|
"""An unknown manager_class is a config error: raise, don't silently fall
|
|
back to DeerMem (memory is persistent state -- a wrong store is a silent
|
|
data-integrity footgun)."""
|
|
set_memory_config(MemoryConfig(manager_class="bogus-backend"))
|
|
with pytest.raises(ValueError, match="bogus-backend"):
|
|
get_memory_manager()
|
|
|
|
|
|
def test_noop_runs_with_empty_memory() -> None:
|
|
set_memory_config(MemoryConfig(manager_class="noop"))
|
|
manager = get_memory_manager()
|
|
assert manager.get_context(user_id="u") == ""
|
|
assert manager.search("anything") == []
|
|
assert manager.get_memory(user_id="u") == {"facts": []}
|
|
# writes are no-ops; memory stays empty
|
|
manager.add("t", [], agent_name=None, user_id="u")
|
|
manager.add_nowait("t", [], agent_name=None, user_id="u")
|
|
assert manager.get_memory(user_id="u") == {"facts": []}
|
|
|
|
|
|
def test_tier3_hooks_have_defaults_noop_inherits() -> None:
|
|
"""warm/reload/fact CRUD are tier-3 hooks ON the ABC with defaults (no more
|
|
``hasattr`` probing): noop inherits ``warm``=None (nothing to warm) and
|
|
fact-CRUD/reload raise ``NotImplementedError``. DeerMem overrides the ones
|
|
it supports (covered elsewhere)."""
|
|
set_memory_config(MemoryConfig(manager_class="noop"))
|
|
noop = get_memory_manager()
|
|
assert noop.warm() is None # inherited default (nothing to warm)
|
|
with pytest.raises(NotImplementedError):
|
|
noop.reload_memory(user_id="u")
|
|
with pytest.raises(NotImplementedError):
|
|
noop.create_fact("x", user_id="u")
|
|
with pytest.raises(NotImplementedError):
|
|
noop.delete_fact("x", user_id="u")
|
|
with pytest.raises(NotImplementedError):
|
|
noop.update_fact("x", user_id="u")
|
|
reset_memory_manager()
|
|
|
|
|
|
def test_deermem_search_works_delete_export_are_stubs() -> None:
|
|
set_memory_config(MemoryConfig(manager_class="deermem"))
|
|
deermem = get_memory_manager()
|
|
# search is implemented (substring match) -- returns a list, does not raise.
|
|
assert isinstance(deermem.search("q", user_id="u"), list)
|
|
# delete_memory / export_memory remain unimplemented stubs this phase.
|
|
with pytest.raises(NotImplementedError):
|
|
deermem.delete_memory(user_id="u")
|
|
with pytest.raises(NotImplementedError):
|
|
deermem.export_memory(user_id="u")
|
|
|
|
|
|
def test_factory_raises_when_storage_path_is_existing_file(tmp_path) -> None:
|
|
"""A storage_path that resolves to an existing FILE is a config error: DeerMem
|
|
treats storage_path as a root directory, so a file would make save's mkdir
|
|
raise NotADirectoryError (silent write failure). Fail loud at startup (#1)."""
|
|
file_path = tmp_path / "mem.json"
|
|
file_path.write_text("{}", encoding="utf-8")
|
|
set_memory_config(MemoryConfig(manager_class="deermem", backend_config={"storage_path": str(file_path)}))
|
|
with pytest.raises(ValueError, match="existing file"):
|
|
get_memory_manager()
|
|
|
|
|
|
def test_migration_drops_file_style_legacy_storage_path(caplog) -> None:
|
|
"""A legacy top-level storage_path that looks like a file (ends in .json) is
|
|
dropped, not carried verbatim -- DeerMem now treats storage_path as a root
|
|
directory, so carrying 'memory.json' would orphan per-user memory / hit
|
|
NotADirectoryError. Dropping lets the factory inject runtime_home (per-user
|
|
location unchanged). Non-file legacy fields still migrate; empty values are
|
|
skipped silently (#1, #6)."""
|
|
from deerflow.config.memory_config import load_memory_config_from_dict
|
|
|
|
with caplog.at_level("WARNING", logger="deerflow.config.memory_config"):
|
|
load_memory_config_from_dict({"storage_path": "memory.json", "max_facts": 50})
|
|
cfg = get_memory_config()
|
|
assert "storage_path" not in cfg.backend_config # file-style dropped
|
|
assert cfg.backend_config.get("max_facts") == 50 # non-file legacy still migrates
|
|
assert any("looks like a file path" in r.message for r in caplog.records)
|
|
|
|
|
|
def test_empty_storage_path_factory_injects_runtime_home(tmp_path, monkeypatch) -> None:
|
|
"""Empty/absent storage_path -> factory injects runtime_home() as the root, so
|
|
per-user memory lands at {runtime_home}/users/{uid}/memory.json (matches
|
|
pre-abstraction per-user location). Pins the zero-config default (reviewer #1)."""
|
|
import deerflow.config.runtime_paths as rp
|
|
|
|
monkeypatch.setattr(rp, "runtime_home", lambda: tmp_path)
|
|
set_memory_config(MemoryConfig(manager_class="deermem")) # no storage_path
|
|
manager = get_memory_manager()
|
|
assert Path(manager._config.storage_path) == tmp_path
|
|
manager.create_fact("hello", user_id="u1", agent_name="test-agent")
|
|
# per-user dir created under the injected runtime_home root
|
|
user_dirs = [p.name for p in (tmp_path / "users").iterdir() if p.is_dir()]
|
|
assert len(user_dirs) == 1
|
|
|
|
|
|
def test_shutdown_flush_has_default_and_noop_is_noop_success() -> None:
|
|
"""``shutdown_flush`` is a tier-2 method with a default (True -- backends
|
|
without a buffer have nothing to drain), NOT abstract; noop inherits/overrides
|
|
to True. Only ``add`` / ``get_context`` are tier-1 abstract."""
|
|
assert "add" in MemoryManager.__abstractmethods__
|
|
assert "get_context" in MemoryManager.__abstractmethods__
|
|
assert "shutdown_flush" not in MemoryManager.__abstractmethods__
|
|
reset_memory_manager()
|
|
set_memory_config(MemoryConfig(manager_class="noop"))
|
|
noop = get_memory_manager()
|
|
assert noop.shutdown_flush(1.0) is True
|
|
reset_memory_manager()
|
|
|
|
|
|
def test_deermem_shutdown_flush_delegates_to_queue_flush_sync() -> None:
|
|
"""DeerMem.shutdown_flush delegates to its queue's bounded flush_sync,
|
|
forwarding the host-owned timeout budget unchanged."""
|
|
reset_memory_manager()
|
|
set_memory_config(MemoryConfig(manager_class="deermem"))
|
|
deermem = get_memory_manager()
|
|
with patch.object(deermem._queue, "flush_sync", return_value=True) as spy:
|
|
assert deermem.shutdown_flush(7.0) is True
|
|
spy.assert_called_once_with(7.0)
|
|
reset_memory_manager()
|
|
|
|
|
|
def test_deermem_shutdown_flush_drains_a_pending_update() -> None:
|
|
"""End-to-end: a pending update sitting in DeerMem's queue is drained
|
|
within the timeout on shutdown_flush (the loss-on-exit bug the ABC method
|
|
fixes). The drain skips inter-item sleep so the budget goes to LLM calls."""
|
|
from deerflow.agents.memory.backends.deermem.deermem.core.queue import ConversationContext
|
|
|
|
reset_memory_manager()
|
|
set_memory_config(MemoryConfig(manager_class="deermem"))
|
|
deermem = get_memory_manager()
|
|
# Inject a mock updater so no real LLM call is made; both items "succeed".
|
|
mock_updater = MagicMock()
|
|
mock_updater.update_memory.return_value = True
|
|
deermem._queue._updater = mock_updater
|
|
deermem._queue._queue = [ConversationContext(thread_id=f"t{i}", messages=["m"], agent_name="lead_agent") for i in range(3)]
|
|
assert deermem.shutdown_flush(5.0) is True
|
|
assert deermem._queue.pending_count == 0
|
|
assert mock_updater.update_memory.call_count == 3
|
|
reset_memory_manager()
|