mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(memory): filter honcho search by category instead of stamping it on unfiltered results (#6981)
* fix(memory): filter honcho search by category instead of stamping it on unfiltered results HonchoMemoryManager.search sent an identical unfiltered query whether or not a category was requested, then stamped the requested category on every hit, so memory_search(category=...) returned unfiltered results attested to be that category (e.g. a work fact labeled "preference") — the opposite of the contract, where category filters before the top_k slice and only facts with that exact category are returned. Send the category as a real server-side metadata filter (Honcho applies "metadata" filters in its ranked query, before the limit) and label each result from the record's own metadata, falling back to "memory" as before. HonchoClient.search gains a keyword-only filters argument that is only put on the wire when set, so unscoped requests are byte-identical to before. * docs(memory): note honcho category filtering requires stored metadata
This commit is contained in:
parent
dfb32ef52b
commit
816ebd5563
@ -66,6 +66,9 @@ class HonchoClient:
|
||||
return str(data.get("representation") or "")
|
||||
return ""
|
||||
|
||||
def search(self, workspace: str, query: str, *, limit: int = 5) -> list[dict[str, Any]]:
|
||||
data = self._post(f"/v3/workspaces/{workspace}/search", {"query": query, "limit": limit})
|
||||
def search(self, workspace: str, query: str, *, limit: int = 5, filters: dict[str, Any] | None = None) -> list[dict[str, Any]]:
|
||||
payload: dict[str, Any] = {"query": query, "limit": limit}
|
||||
if filters:
|
||||
payload["filters"] = filters
|
||||
data = self._post(f"/v3/workspaces/{workspace}/search", payload)
|
||||
return list(data) if isinstance(data, list) else []
|
||||
|
||||
@ -89,7 +89,14 @@ def _stable_id(raw: str) -> str:
|
||||
|
||||
|
||||
class HonchoMemoryManager(MemoryManager):
|
||||
"""MemoryManager backed by a Honcho v3 instance (self-hosted or hosted)."""
|
||||
"""MemoryManager backed by a Honcho v3 instance (self-hosted or hosted).
|
||||
|
||||
Known limitation: ``search`` scopes ``category`` server-side to messages
|
||||
stored with ``metadata.category``, but ``add()`` writes plain metadata-less
|
||||
messages and fact CRUD is unsupported, so nothing this backend stores ever
|
||||
carries a category — category queries return empty results here until the
|
||||
write path gains metadata support (see ``search``).
|
||||
"""
|
||||
|
||||
_config: HonchoConfig = PrivateAttr(default=None)
|
||||
_client: Any = PrivateAttr(default=None)
|
||||
@ -223,14 +230,26 @@ class HonchoMemoryManager(MemoryManager):
|
||||
agent_name: str | None = None,
|
||||
category: str | None = None,
|
||||
) -> list[dict[str, Any]]:
|
||||
"""``category`` must filter, not label: it scopes the Honcho query itself
|
||||
(a ``metadata`` filter the server applies before its ``limit``, per the
|
||||
base-class contract), and each result is labeled from its own metadata so
|
||||
hits are never attested to a category they do not have.
|
||||
|
||||
Known limitation: that filter only matches messages stored with
|
||||
``metadata.category``, and nothing this backend writes has it — ``add()``
|
||||
sends plain ``{"peer_id", "content"}`` messages with no metadata, and
|
||||
fact CRUD is unsupported, so no other write path can set it either. A
|
||||
``category`` query therefore returns empty results in this backend
|
||||
until the write path gains metadata support."""
|
||||
workspace = self._workspace(user_id)
|
||||
if workspace is None:
|
||||
return []
|
||||
results = self._read_or_fallback([], lambda: self._client.search(workspace, query, limit=top_k))
|
||||
filters = {"metadata": {"category": category}} if category else None
|
||||
results = self._read_or_fallback([], lambda: self._client.search(workspace, query, limit=top_k, filters=filters))
|
||||
return [
|
||||
{
|
||||
"content": item.get("content", ""),
|
||||
"category": category or "memory",
|
||||
"category": (item.get("metadata") or {}).get("category") or "memory",
|
||||
"session_id": item.get("session_id"),
|
||||
"peer_id": item.get("peer_id"),
|
||||
"created_at": item.get("created_at"),
|
||||
|
||||
@ -36,7 +36,7 @@ class _BlockingFakeClient:
|
||||
self._block()
|
||||
return "rep"
|
||||
|
||||
def search(self, ws: str, query: str, *, limit: int = 5) -> list[Any]:
|
||||
def search(self, ws: str, query: str, *, limit: int = 5, filters: dict[str, Any] | None = None) -> list[Any]:
|
||||
self._block()
|
||||
return []
|
||||
|
||||
|
||||
@ -3,6 +3,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
from types import SimpleNamespace
|
||||
|
||||
import httpx
|
||||
@ -336,6 +337,26 @@ class TestHonchoClient:
|
||||
assert b'"alice"' in seen[2][2] and b'"deerflow"' in seen[2][2]
|
||||
assert b'"messages"' in seen[3][2]
|
||||
|
||||
def test_search_payload_carries_filters_only_when_present(self):
|
||||
"""``filters`` must reach the wire exactly as given (Honcho applies
|
||||
``metadata`` filters server-side, before its ``limit``), while the
|
||||
unfiltered payload stays byte-identical to the pre-filter shape so
|
||||
deployed Honcho versions see no change on unscoped searches."""
|
||||
|
||||
seen: list[dict] = []
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
if request.url.path.endswith("/search"):
|
||||
seen.append(json.loads(request.content))
|
||||
return httpx.Response(200, json=[])
|
||||
return httpx.Response(200, json={"id": "x"})
|
||||
|
||||
c = _client_with_handler(handler)
|
||||
c.search("ws1", "q", limit=5)
|
||||
assert seen == [{"query": "q", "limit": 5}]
|
||||
c.search("ws1", "q", limit=5, filters={"metadata": {"category": "preference"}})
|
||||
assert seen[-1] == {"query": "q", "limit": 5, "filters": {"metadata": {"category": "preference"}}}
|
||||
|
||||
def test_errors_wrap_as_honcho_request_error(self):
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
return httpx.Response(500, text="boom")
|
||||
@ -380,6 +401,7 @@ class _FakeClient:
|
||||
# not narrowly typed to the client's own exception class.
|
||||
self.raise_exc_cls: type[BaseException] = HonchoRequestError
|
||||
self.closed = False
|
||||
self.search_results = [{"content": "found", "peer_id": "deerflow", "session_id": "df-t", "created_at": "2026-01-01"}]
|
||||
|
||||
def _maybe_raise(self, name):
|
||||
if self.raise_on == name:
|
||||
@ -406,10 +428,10 @@ class _FakeClient:
|
||||
self.calls.append(("rep", (ws, pid)))
|
||||
return self.representation_text
|
||||
|
||||
def search(self, ws, query, *, limit=5):
|
||||
def search(self, ws, query, *, limit=5, filters=None):
|
||||
self._maybe_raise("search")
|
||||
self.calls.append(("search", (ws, query, limit)))
|
||||
return [{"content": "found", "peer_id": "deerflow", "session_id": "df-t", "created_at": "2026-01-01"}]
|
||||
self.calls.append(("search", (ws, query, limit, filters)))
|
||||
return self.search_results[:limit]
|
||||
|
||||
def close(self):
|
||||
self.closed = True
|
||||
@ -574,6 +596,39 @@ class TestHonchoManagerRead:
|
||||
results = mgr.search("deploy", top_k=3, user_id="u1")
|
||||
assert results[0]["content"] == "found"
|
||||
|
||||
def test_search_category_scopes_the_wire_request(self):
|
||||
"""Regression: ``category`` used to be dropped from the request entirely
|
||||
-- the manager.py contract wants it to filter BEFORE the ``top_k`` slice
|
||||
and memory_search (tools.py) promises "Only facts with this exact
|
||||
category are returned". It must reach Honcho as a ``metadata`` filter,
|
||||
which the server applies to its ranked query before its ``limit``."""
|
||||
mgr, fake = _manager(workspace_overrides={"u1": "shared"})
|
||||
mgr.search("tell me about the user", top_k=5, user_id="u1", category="preference")
|
||||
assert fake.calls[-1] == ("search", ("shared", "tell me about the user", 5, {"metadata": {"category": "preference"}}))
|
||||
|
||||
def test_search_labels_results_from_their_own_metadata(self):
|
||||
"""Regression: every unfiltered hit used to come back stamped with the
|
||||
*requested* category; labels must come from each record's own metadata
|
||||
(default ``memory``), never from the request."""
|
||||
mgr, fake = _manager(workspace_overrides={"u1": "shared"})
|
||||
fake.search_results = [
|
||||
{"content": "User works at Acme on backend systems", "peer_id": "alice", "session_id": "df-t1", "created_at": "2026-01-01"},
|
||||
{"content": "User likes hiking in the mountains", "metadata": {"category": "preference"}, "peer_id": "alice", "session_id": "df-t2", "created_at": "2026-01-02"},
|
||||
]
|
||||
results = mgr.search("tell me about the user", top_k=5, user_id="u1", category="preference")
|
||||
assert [(r["category"], r["content"]) for r in results] == [
|
||||
("memory", "User works at Acme on backend systems"),
|
||||
("preference", "User likes hiking in the mountains"),
|
||||
]
|
||||
|
||||
def test_search_without_category_keeps_the_wire_request_and_labels_unchanged(self):
|
||||
"""Guard for the unfiltered path the fix must not disturb: no ``filters``
|
||||
key is sent and the neutral ``memory`` label stays."""
|
||||
mgr, fake = _manager(workspace_overrides={"u1": "shared"})
|
||||
results = mgr.search("q", top_k=3, user_id="u1")
|
||||
assert fake.calls[-1] == ("search", ("shared", "q", 3, None))
|
||||
assert results == [{"content": "found", "category": "memory", "session_id": "df-t", "peer_id": "deerflow", "created_at": "2026-01-01"}]
|
||||
|
||||
def test_search_without_user_is_empty(self):
|
||||
mgr, _ = _manager()
|
||||
assert mgr.search("q", user_id=None) == []
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user