mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
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.
This commit is contained in:
parent
01d1a663e1
commit
f4669f34cf
3 changed files with 190 additions and 30 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue