From 9a322726ae89e2f242f8f20fb8f854b7867ba7a0 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 8 Jul 2026 01:50:11 +0530 Subject: [PATCH] fix(mem0): prune dead get_all, wire rerank config default, warn on MEM0_HOST env override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the salvage: - get_all() pruned from the ABC and all three backends: mem0_list (its only caller) was removed by the recall-tuning commit, leaving new, tested, unreachable code — including SelfHostedBackend's _MAX_TOP_K over-fetch workaround. Tests for it dropped; fake-class stubs remain harmlessly. (The #52921 true-total fix lives on in the PR history if a lister ever returns.) - The persisted rerank config key was write-only (setup prompted for it, nothing read it). initialize() now parses it into _rerank_default and mem0_search uses it when the model doesn't pass rerank explicitly; per-call args still win. Guard test added. - Platform-mode setup now warns when MEM0_HOST is set in the environment: the json host-clear can't help there (_load_config seeds host from the env var, docs tell users to put it in .env) — the user would silently keep routing to the self-hosted server. - SelfHostedBackend: connect-level retries (httpx.HTTPTransport(retries=2)) so a single transient blip doesn't count toward the provider breaker; transport now injectable and the test helper uses the real __init__ instead of mirroring it via __new__. - plugin.yaml description no longer leads with reranking (off by default, platform-only); docs em-dash typo fixed. --- plugins/memory/mem0/__init__.py | 11 +++- plugins/memory/mem0/_backend.py | 36 +++-------- plugins/memory/mem0/_setup.py | 11 ++++ plugins/memory/mem0/plugin.yaml | 2 +- tests/plugins/memory/test_mem0_backend.py | 63 ++----------------- tests/plugins/memory/test_mem0_v3.py | 9 +++ .../user-guide/features/memory-providers.md | 2 +- 7 files changed, 44 insertions(+), 90 deletions(-) diff --git a/plugins/memory/mem0/__init__.py b/plugins/memory/mem0/__init__.py index 751efd0effbc..35f413ed37a7 100644 --- a/plugins/memory/mem0/__init__.py +++ b/plugins/memory/mem0/__init__.py @@ -204,6 +204,7 @@ class Mem0MemoryProvider(MemoryProvider): self._host = "" self._user_id = _DEFAULT_USER_ID self._agent_id = "hermes" + self._rerank_default = False self._channel = "cli" # gateway channel name (cli/telegram/discord/...) self._sync_thread = None self._prefetch_thread = None @@ -353,6 +354,14 @@ class Mem0MemoryProvider(MemoryProvider): configured = None self._user_id = configured or kwargs.get("user_id") or _DEFAULT_USER_ID self._agent_id = self._config.get("agent_id", "hermes") + # Persisted rerank preference (setup wizard / mem0.json). Used as the + # DEFAULT for mem0_search when the model doesn't pass ``rerank`` + # explicitly; per-call args still win. Platform-only feature — other + # backends accept-and-ignore the flag. + _rr = self._config.get("rerank", False) + self._rerank_default = ( + _rr.lower() in ("true", "1", "yes") if isinstance(_rr, str) else bool(_rr) + ) self._channel = kwargs.get("platform") or "cli" self._backend = self._create_backend() if self._backend and not self._atexit_registered: @@ -527,7 +536,7 @@ class Mem0MemoryProvider(MemoryProvider): return tool_error("Missing required parameter: query") try: top_k = max(1, min(int(args.get("top_k", 10)), 50)) - rerank_raw = args.get("rerank", False) + rerank_raw = args.get("rerank", getattr(self, "_rerank_default", False)) if isinstance(rerank_raw, str): rerank = rerank_raw.lower() not in ("false", "0", "no") else: diff --git a/plugins/memory/mem0/_backend.py b/plugins/memory/mem0/_backend.py index d68a70a9e5a7..7292fd76658d 100644 --- a/plugins/memory/mem0/_backend.py +++ b/plugins/memory/mem0/_backend.py @@ -13,10 +13,6 @@ class Mem0Backend(ABC): def search(self, query: str, *, filters: dict, top_k: int = 10, rerank: bool = False) -> list[dict]: ... - @abstractmethod - def get_all(self, *, filters: dict, page: int = 1, page_size: int = 100) -> dict: - ... - @abstractmethod def add( self, @@ -61,12 +57,6 @@ class PlatformBackend(Mem0Backend): response = self._client.search(query, filters=filters, top_k=top_k, rerank=rerank) return _unwrap_results(response) - def get_all(self, *, filters: dict, page: int = 1, page_size: int = 100) -> dict: - response = self._client.get_all(filters=filters, page=page, page_size=page_size) - results = response.get("results", []) if isinstance(response, dict) else response - count = response.get("count", len(results)) if isinstance(response, dict) else len(results) - return {"results": results, "count": count} - def add( self, messages: list, @@ -101,16 +91,20 @@ class SelfHostedBackend(Mem0Backend): ``/search`` routes. """ - _MAX_TOP_K = 10000 - - def __init__(self, api_key: str, host: str): + def __init__(self, api_key: str, host: str, transport=None): import httpx headers = {"Content-Type": "application/json"} if api_key: headers["X-API-Key"] = api_key # omitted only for AUTH_DISABLED servers + # Connect-level retries smooth over transient blips so a single + # dropped SYN doesn't count toward the provider failure breaker. + # ``transport`` is injectable for tests (httpx.MockTransport). + if transport is None: + transport = httpx.HTTPTransport(retries=2) self._client = httpx.Client( base_url=host.rstrip("/"), headers=headers, timeout=30.0, + transport=transport, ) def _json(self, method: str, path: str, **kwargs) -> Any: @@ -125,14 +119,6 @@ class SelfHostedBackend(Mem0Backend): body["filters"] = filters # user_id belongs in filters (top-level is deprecated) return _unwrap_results(self._json("POST", "/search", json=body)) - def get_all(self, *, filters: dict, page: int = 1, page_size: int = 100) -> dict: - params: dict[str, Any] = {k: v for k, v in (filters or {}).items() if v} - params["top_k"] = self._MAX_TOP_K - all_results = _unwrap_results(self._json("GET", "/memories", params=params)) - total = len(all_results) - start = (page - 1) * page_size - return {"results": all_results[start : start + page_size], "count": total} - def add( self, messages: list, @@ -270,14 +256,6 @@ class OSSBackend(Mem0Backend): response = self._memory.search(query, filters=filters, top_k=top_k) return _unwrap_results(response) - def get_all(self, *, filters: dict, page: int = 1, page_size: int = 100) -> dict: - response = self._memory.get_all(filters=filters) - all_results = _unwrap_results(response) - total = len(all_results) - start = (page - 1) * page_size - results = all_results[start : start + page_size] - return {"results": results, "count": total} - def add( self, messages: list, diff --git a/plugins/memory/mem0/_setup.py b/plugins/memory/mem0/_setup.py index 021705aea9da..c81e5d1f0293 100644 --- a/plugins/memory/mem0/_setup.py +++ b/plugins/memory/mem0/_setup.py @@ -300,6 +300,17 @@ def _setup_platform(hermes_home: str, config: dict, flags: dict[str, str]) -> No # (existing.update), so a popped key would survive; an empty value overwrites # it and reads as falsy at routing time. provider_config["host"] = "" + # The json-file clear above can't help when the host comes from the + # environment: _load_config() seeds ``host`` from MEM0_HOST, and the + # docs tell self-hosted users to put MEM0_HOST in ~/.hermes/.env. Warn + # so the user knows platform mode won't take effect until it's removed. + if os.environ.get("MEM0_HOST", "").strip(): + print( + "\n ⚠ MEM0_HOST is set in your environment " + f"({os.environ['MEM0_HOST']}). It overrides platform mode — " + "remove it from ~/.hermes/.env (or unset it) or Hermes will keep " + "routing to the self-hosted server." + ) from hermes_cli.config import save_config config["memory"]["provider"] = "mem0" diff --git a/plugins/memory/mem0/plugin.yaml b/plugins/memory/mem0/plugin.yaml index 4c67a02d97d1..c6ae10acca8d 100644 --- a/plugins/memory/mem0/plugin.yaml +++ b/plugins/memory/mem0/plugin.yaml @@ -1,5 +1,5 @@ name: mem0 version: 1.3.0 -description: "Mem0 — server-side LLM fact extraction with semantic search, reranking, and automatic deduplication." +description: "Mem0 — server-side LLM fact extraction with semantic search, automatic deduplication, and opt-in reranking (platform mode)." pip_dependencies: - mem0ai>=2.0.10,<3 diff --git a/tests/plugins/memory/test_mem0_backend.py b/tests/plugins/memory/test_mem0_backend.py index da3adeb95e8e..94f6d1443b37 100644 --- a/tests/plugins/memory/test_mem0_backend.py +++ b/tests/plugins/memory/test_mem0_backend.py @@ -68,13 +68,6 @@ class TestPlatformBackend: assert isinstance(result, list) assert result[0]["id"] == "m1" - def test_get_all_forwards_pagination(self): - backend, client = self._make() - result = backend.get_all(filters={"user_id": "u1"}, page=2, page_size=50) - assert client.calls[0][1]["page"] == 2 - assert client.calls[0][1]["page_size"] == 50 - assert "count" in result - def test_add_forwards_kwargs(self): backend, client = self._make() msgs = [{"role": "user", "content": "hi"}] @@ -168,21 +161,6 @@ class TestOSSBackend: backend.search("q", filters={}, rerank=True) assert "rerank" not in memory.calls[0][2] - def test_get_all_ignores_pagination(self): - """OSSBackend accepts page/page_size but does NOT forward to Memory.get_all().""" - backend, memory = self._make() - result = backend.get_all(filters={"user_id": "u1"}, page=2, page_size=50) - call_kwargs = memory.calls[0][1] - assert "page" not in call_kwargs - assert "page_size" not in call_kwargs - assert result["count"] == 1 - - def test_get_all_returns_envelope(self): - backend, _ = self._make() - result = backend.get_all(filters={"user_id": "u1"}) - assert "results" in result - assert "count" in result - def test_add_forwards_kwargs(self): backend, memory = self._make() msgs = [{"role": "user", "content": "hi"}] @@ -243,20 +221,14 @@ class _StubServer: def _backend(server, api_key="adminkey", host="http://sh:8888"): - """Build a SelfHostedBackend whose client routes through the stub transport. + """Build a SelfHostedBackend routed through the stub transport. - Mirrors __init__'s header setup but injects MockTransport so no real socket - is opened. + Uses the real __init__ (via the injectable ``transport`` kwarg) so the + constructor's header/base_url setup is exercised by every test here. """ - backend = SelfHostedBackend.__new__(SelfHostedBackend) - headers = {"Content-Type": "application/json"} - if api_key: - headers["X-API-Key"] = api_key - backend._client = httpx.Client( - base_url=host.rstrip("/"), headers=headers, - transport=httpx.MockTransport(server.handler), + return SelfHostedBackend( + api_key, host, transport=httpx.MockTransport(server.handler) ) - return backend class TestSelfHostedBackend: @@ -294,31 +266,6 @@ class TestSelfHostedBackend: assert req.headers["x-api-key"] == "adminkey" assert "authorization" not in req.headers - # --- get_all / pagination ------------------------------------------- - - def test_get_all_passes_user_id_as_query_param(self): - s = _StubServer() - _backend(s).get_all(filters={"user_id": "u1"}, page=1, page_size=3) - req = s.requests[-1] - assert req.method == "GET" and req.url.path == "/memories" - assert req.url.params["user_id"] == "u1" - - def test_get_all_slices_requested_page(self): - s = _StubServer(rows=10) - out = _backend(s).get_all(filters={"user_id": "u1"}, page=2, page_size=3) - # We over-fetch (top_k=_MAX_TOP_K) and slice the page locally... - assert [r["id"] for r in out["results"]] == ["m3", "m4", "m5"] - # ...so count is the TRUE total, not the page size (issue #52921). - assert out["count"] == 10 - - def test_get_all_requests_max_top_k_for_accurate_count(self): - # Always request _MAX_TOP_K (not page*page_size) so `count` reflects the - # real total — self-hosted GET /memories returns no count field (#52921). - s = _StubServer(rows=10) - _backend(s).get_all(filters={"user_id": "u1"}, page=1, page_size=3) - req = s.requests[-1] - assert int(req.url.params["top_k"]) == SelfHostedBackend._MAX_TOP_K - # --- add / update / delete ------------------------------------------ def test_add_posts_messages_and_identity(self): diff --git a/tests/plugins/memory/test_mem0_v3.py b/tests/plugins/memory/test_mem0_v3.py index fffb2f7eb3fc..060eed375593 100644 --- a/tests/plugins/memory/test_mem0_v3.py +++ b/tests/plugins/memory/test_mem0_v3.py @@ -77,6 +77,15 @@ class TestMem0V3Tools: provider.handle_tool_call("mem0_search", {"query": "test", "rerank": False}) assert backend.captured[0][2]["rerank"] is False + def test_search_rerank_config_default_used_when_arg_absent(self, monkeypatch): + """The persisted mem0.json ``rerank`` preference is the tool default; + a per-call arg still wins (see the explicit-False override above).""" + backend = FakeBackend() + provider = self._make_provider(monkeypatch, backend) + provider._rerank_default = True # as initialize() sets from config + provider.handle_tool_call("mem0_search", {"query": "test"}) + assert backend.captured[0][2]["rerank"] is True + def test_add_uses_content_param(self, monkeypatch): backend = FakeBackend() provider = self._make_provider(monkeypatch, backend) diff --git a/website/docs/user-guide/features/memory-providers.md b/website/docs/user-guide/features/memory-providers.md index 929bb640fcf4..ea8aec293bdf 100644 --- a/website/docs/user-guide/features/memory-providers.md +++ b/website/docs/user-guide/features/memory-providers.md @@ -321,7 +321,7 @@ Server-side LLM fact extraction with semantic search, reranking, and automatic d | | | |---|---| -| **Best for** | Hands-off memory management Mem0 handles extraction automatically | +| **Best for** | Hands-off memory management — Mem0 handles extraction automatically | | **Requires** | `pip install mem0ai` + API key (platform), a running Mem0 server (self-hosted dashboard), or an LLM + vector store (OSS) | | **Data storage** | Mem0 Cloud (platform), your own Mem0 server (self-hosted dashboard), or in-process (OSS) | | **Cost** | Mem0 pricing (platform) / free (self-hosted or OSS) |