From f4669f34cff3dbe4cb2b7c410974eaff5644d20e Mon Sep 17 00:00:00 2001 From: vizi0uz Date: Mon, 6 Jul 2026 15:25:34 -0300 Subject: [PATCH] feat(honcho): add list mode to honcho_conclude so delete can resolve a real conclusion id honcho_conclude's delete action was unreachable in practice: no tool ever surfaced a real conclusion id for the model to pass as delete_id. honcho_search only searches the separate Message resource space, and the SDK's ConclusionScope.list()/.query() (which do return real Conclusion.id values) were never wired into any tool. Adds an optional list mode to honcho_conclude (query to search, omit to browse recent conclusions), backed by a new HonchoSessionManager.list_conclusions(). No new tool, no changes to the create/delete signatures or their conclusions_of() routing. --- plugins/memory/honcho/__init__.py | 30 ++++++-- plugins/memory/honcho/session.py | 77 ++++++++++++++----- tests/honcho_plugin/test_session.py | 113 +++++++++++++++++++++++++++- 3 files changed, 190 insertions(+), 30 deletions(-) diff --git a/plugins/memory/honcho/__init__.py b/plugins/memory/honcho/__init__.py index 48567633842b..ae29bff16311 100644 --- a/plugins/memory/honcho/__init__.py +++ b/plugins/memory/honcho/__init__.py @@ -171,12 +171,15 @@ CONTEXT_SCHEMA = { CONCLUDE_SCHEMA = { "name": "honcho_conclude", "description": ( - "Write or delete a CONCLUSION — a persistent, derived fact about a peer that " + "Write, delete, or list CONCLUSIONS — persistent, derived facts about a peer that " "feeds their long-term profile (card + representation). Use this to record " "something durable you've learned about the peer (a stable preference, a " "correction, a standing constraint) so future sessions carry it forward. " - "You MUST pass exactly one of `conclusion` (to create) or `delete_id` (to " - "delete); passing neither, or both, is an error. Deletion exists only for " + "You MUST pass exactly one of `conclusion` (to create), `delete_id` (to " + "delete), or `list` (to list/search); any other combination is an error. " + "A deletion ID is an opaque server-generated string: first call with `list=true` " + "and optionally `query`, then pass the returned ID as `delete_id`. " + "Deletion exists only for " "PII removal — for merely wrong facts, write a corrected conclusion instead; " "Honcho self-heals contradictions over time. This is a WRITE tool: to read " "the profile use honcho_profile / honcho_context, and to search what was " @@ -187,11 +190,19 @@ CONCLUDE_SCHEMA = { "properties": { "conclusion": { "type": "string", - "description": "A factual statement to persist. Provide this when creating a conclusion. Do not send it together with delete_id.", + "description": "A factual statement to persist. Provide this when creating a conclusion. Do not send it together with delete_id or list.", }, "delete_id": { "type": "string", - "description": "Conclusion ID to delete for PII removal. Provide this when deleting a conclusion. Do not send it together with conclusion.", + "description": "Conclusion ID to delete for PII removal. Provide this when deleting a conclusion. Do not send it together with conclusion or list. Get this id from a prior `list` call — never guess it.", + }, + "list": { + "type": "boolean", + "description": "Set to true to list or search stored conclusions (with their ids) instead of creating or deleting one. Do not send together with conclusion or delete_id.", + }, + "query": { + "type": "string", + "description": "Optional semantic search query, used only when `list` is true. Omit to list the most recent conclusions instead of searching.", }, "peer": { "type": "string", @@ -1564,13 +1575,18 @@ class HonchoMemoryProvider(MemoryProvider): elif tool_name == "honcho_conclude": delete_id = (args.get("delete_id") or "").strip() conclusion = args.get("conclusion", "").strip() + list_mode = bool(args.get("list")) peer = args.get("peer", "user") has_delete_id = bool(delete_id) has_conclusion = bool(conclusion) - if has_delete_id == has_conclusion: - return tool_error("Exactly one of conclusion or delete_id must be provided.") + if sum([has_delete_id, has_conclusion, list_mode]) != 1: + return tool_error("Exactly one of conclusion, delete_id, or list must be provided.") + if list_mode: + query = (args.get("query") or "").strip() or None + conclusions = self._manager.list_conclusions(self._session_key, query=query, peer=peer) + return json.dumps({"conclusions": conclusions}) if has_delete_id: ok = self._manager.delete_conclusion(self._session_key, delete_id, peer=peer) if ok: diff --git a/plugins/memory/honcho/session.py b/plugins/memory/honcho/session.py index ec1c4f3a6bc7..9fe53eaef5c9 100644 --- a/plugins/memory/honcho/session.py +++ b/plugins/memory/honcho/session.py @@ -1199,6 +1199,23 @@ class HonchoSessionManager: return "\n\n".join(lines) + def _conclusions_scope(self, session: Any, target_peer_id: str) -> Any: + """Resolve the ConclusionScope for observing target_peer_id. + + Shared by create/delete/list_conclusions so the observer/observed + routing (self-conclusions vs. AI-observes-others vs. peer-owned) + stays consistent across all three. + """ + if target_peer_id == session.assistant_peer_id: + observer = self._get_or_create_peer(session.assistant_peer_id) + return observer.conclusions_of(session.assistant_peer_id) + elif self._ai_observe_others: + observer = self._get_or_create_peer(session.assistant_peer_id) + return observer.conclusions_of(target_peer_id) + else: + target_peer = self._get_or_create_peer(target_peer_id) + return target_peer.conclusions_of(target_peer_id) + def create_conclusion(self, session_key: str, content: str, peer: str = "user") -> bool: """Write a conclusion about a target peer back to Honcho. @@ -1228,16 +1245,7 @@ class HonchoSessionManager: logger.warning("Could not resolve conclusion peer '%s' for session '%s'", peer, session_key) return False - if target_peer_id == session.assistant_peer_id: - assistant_peer = self._get_or_create_peer(session.assistant_peer_id) - conclusions_scope = assistant_peer.conclusions_of(session.assistant_peer_id) - elif self._ai_observe_others: - assistant_peer = self._get_or_create_peer(session.assistant_peer_id) - conclusions_scope = assistant_peer.conclusions_of(target_peer_id) - else: - target_peer = self._get_or_create_peer(target_peer_id) - conclusions_scope = target_peer.conclusions_of(target_peer_id) - + conclusions_scope = self._conclusions_scope(session, target_peer_id) conclusions_scope.create([{ "content": content.strip(), "session_id": session.honcho_session_id, @@ -1264,15 +1272,7 @@ class HonchoSessionManager: return False try: target_peer_id = self._resolve_peer_id(session, peer) - if target_peer_id == session.assistant_peer_id: - observer = self._get_or_create_peer(session.assistant_peer_id) - scope = observer.conclusions_of(session.assistant_peer_id) - elif self._ai_observe_others: - observer = self._get_or_create_peer(session.assistant_peer_id) - scope = observer.conclusions_of(target_peer_id) - else: - target_peer = self._get_or_create_peer(target_peer_id) - scope = target_peer.conclusions_of(target_peer_id) + scope = self._conclusions_scope(session, target_peer_id) scope.delete(conclusion_id) logger.info("Deleted conclusion %s for %s", conclusion_id, session_key) return True @@ -1280,6 +1280,45 @@ class HonchoSessionManager: logger.error("Failed to delete conclusion %s: %s", conclusion_id, e) return False + def list_conclusions( + self, + session_key: str, + query: str | None = None, + peer: str = "user", + limit: int = 20, + ) -> list[dict]: + """List or semantically search stored conclusions, including their ids. + + This is the lookup path `honcho_conclude`'s delete action needs: + Conclusion.id is a server-generated nanoid that no other Honcho tool + surfaces (honcho_search only searches Messages, a separate resource). + + Args: + session_key: Session key for peer resolution. + query: Optional semantic search query. Omit to list recent conclusions. + peer: Peer alias or explicit peer ID. + limit: Max conclusions to return. + + Returns: + List of {"id": ..., "content": ...} dicts, or [] on failure/no session. + """ + session = self._cache.get(session_key) + if not session: + return [] + try: + target_peer_id = self._resolve_peer_id(session, peer) + if target_peer_id is None: + return [] + scope = self._conclusions_scope(session, target_peer_id) + if query: + conclusions = scope.query(query, top_k=limit) + else: + conclusions = scope.list(size=limit).items + return [{"id": c.id, "content": c.content} for c in conclusions] + except Exception as e: + logger.debug("Honcho list_conclusions failed: %s", e) + return [] + def set_peer_card(self, session_key: str, card: list[str], peer: str = "user") -> list[str] | None: """Update a peer's card. diff --git a/tests/honcho_plugin/test_session.py b/tests/honcho_plugin/test_session.py index cb73cc0da5e7..9114c8bd03c8 100644 --- a/tests/honcho_plugin/test_session.py +++ b/tests/honcho_plugin/test_session.py @@ -412,6 +412,52 @@ class TestPeerLookupHelpers: "session_id": session.honcho_session_id, }]) + def test_list_conclusions_uses_query_when_query_given(self): + mgr, session = self._make_cached_manager() + assistant_peer = MagicMock() + scope = MagicMock() + assistant_peer.conclusions_of.return_value = scope + mgr._get_or_create_peer = MagicMock(return_value=assistant_peer) + scope.query.return_value = [ + SimpleNamespace(id="nano1", content="User prefers dark mode"), + ] + + result = mgr.list_conclusions(session.key, query="dark mode") + + assert result == [{"id": "nano1", "content": "User prefers dark mode"}] + scope.query.assert_called_once_with("dark mode", top_k=20) + scope.list.assert_not_called() + + def test_list_conclusions_uses_list_when_no_query(self): + mgr, session = self._make_cached_manager() + assistant_peer = MagicMock() + scope = MagicMock() + assistant_peer.conclusions_of.return_value = scope + mgr._get_or_create_peer = MagicMock(return_value=assistant_peer) + scope.list.return_value = SimpleNamespace( + items=[SimpleNamespace(id="nano2", content="Robert likes vinyl")] + ) + + result = mgr.list_conclusions(session.key) + + assert result == [{"id": "nano2", "content": "Robert likes vinyl"}] + scope.list.assert_called_once_with(size=20) + scope.query.assert_not_called() + + def test_list_conclusions_returns_empty_list_on_exception(self): + mgr, session = self._make_cached_manager() + assistant_peer = MagicMock() + scope = MagicMock() + assistant_peer.conclusions_of.return_value = scope + mgr._get_or_create_peer = MagicMock(return_value=assistant_peer) + scope.list.side_effect = RuntimeError("boom") + + assert mgr.list_conclusions(session.key) == [] + + def test_list_conclusions_returns_empty_list_without_cached_session(self): + mgr = HonchoSessionManager() + assert mgr.list_conclusions("missing-session") == [] + class TestConcludeToolDispatch: def test_conclude_schema_has_no_anyof(self): @@ -421,6 +467,8 @@ class TestConcludeToolDispatch: assert params["type"] == "object" assert "conclusion" in params["properties"] assert "delete_id" in params["properties"] + assert "list" in params["properties"] + assert "query" in params["properties"] assert "anyOf" not in params assert "oneOf" not in params assert "allOf" not in params @@ -529,7 +577,7 @@ class TestConcludeToolDispatch: result = provider.handle_tool_call("honcho_conclude", {}) parsed = json.loads(result) - assert parsed == {"error": "Exactly one of conclusion or delete_id must be provided."} + assert parsed == {"error": "Exactly one of conclusion, delete_id, or list must be provided."} provider._manager.create_conclusion.assert_not_called() provider._manager.delete_conclusion.assert_not_called() @@ -545,7 +593,7 @@ class TestConcludeToolDispatch: {"conclusion": "User prefers dark mode", "delete_id": "conc-123"}, ) parsed = json.loads(result) - assert parsed == {"error": "Exactly one of conclusion or delete_id must be provided."} + assert parsed == {"error": "Exactly one of conclusion, delete_id, or list must be provided."} provider._manager.create_conclusion.assert_not_called() provider._manager.delete_conclusion.assert_not_called() @@ -558,7 +606,7 @@ class TestConcludeToolDispatch: provider._manager = MagicMock() result = provider.handle_tool_call("honcho_conclude", {"conclusion": " "}) parsed = json.loads(result) - assert parsed == {"error": "Exactly one of conclusion or delete_id must be provided."} + assert parsed == {"error": "Exactly one of conclusion, delete_id, or list must be provided."} provider._manager.create_conclusion.assert_not_called() def test_honcho_conclude_rejects_whitespace_only_delete_id(self): @@ -570,9 +618,66 @@ class TestConcludeToolDispatch: provider._manager = MagicMock() result = provider.handle_tool_call("honcho_conclude", {"delete_id": " "}) parsed = json.loads(result) - assert parsed == {"error": "Exactly one of conclusion or delete_id must be provided."} + assert parsed == {"error": "Exactly one of conclusion, delete_id, or list must be provided."} provider._manager.delete_conclusion.assert_not_called() + def test_honcho_conclude_list_mode_dispatches_to_manager(self): + """list=true with a query should return conclusions (with ids) via list_conclusions.""" + import json + provider = HonchoMemoryProvider() + provider._session_initialized = True + provider._session_key = "telegram:123" + provider._manager = MagicMock() + provider._manager.list_conclusions.return_value = [ + {"id": "nano1", "content": "User prefers dark mode"}, + ] + + result = provider.handle_tool_call("honcho_conclude", {"list": True, "query": "dark mode"}) + + parsed = json.loads(result) + assert parsed == {"conclusions": [{"id": "nano1", "content": "User prefers dark mode"}]} + provider._manager.list_conclusions.assert_called_once_with( + "telegram:123", + query="dark mode", + peer="user", + ) + + def test_honcho_conclude_list_mode_omits_query_when_not_given(self): + """list=true without a query should list recent conclusions (query=None).""" + import json + provider = HonchoMemoryProvider() + provider._session_initialized = True + provider._session_key = "telegram:123" + provider._manager = MagicMock() + provider._manager.list_conclusions.return_value = [] + + result = provider.handle_tool_call("honcho_conclude", {"list": True}) + + json.loads(result) + provider._manager.list_conclusions.assert_called_once_with( + "telegram:123", + query=None, + peer="user", + ) + + def test_honcho_conclude_rejects_list_with_conclusion(self): + """list=true combined with conclusion violates exactly-one-of and is rejected.""" + import json + provider = HonchoMemoryProvider() + provider._session_initialized = True + provider._session_key = "telegram:123" + provider._manager = MagicMock() + + result = provider.handle_tool_call( + "honcho_conclude", + {"conclusion": "User prefers dark mode", "list": True}, + ) + + parsed = json.loads(result) + assert parsed == {"error": "Exactly one of conclusion, delete_id, or list must be provided."} + provider._manager.list_conclusions.assert_not_called() + provider._manager.create_conclusion.assert_not_called() + def test_sync_turn_strips_leaked_memory_context_before_honcho_ingest(self): provider = HonchoMemoryProvider() provider._session_key = "telegram:123"