mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-18 14:52:04 +00:00
fix(mem0): prune dead get_all, wire rerank config default, warn on MEM0_HOST env override
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.
This commit is contained in:
parent
53edf6f983
commit
9a322726ae
7 changed files with 44 additions and 90 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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) |
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue