fix(hindsight): overhaul hindsight memory plugin and memory setup wizard
- Dedicated asyncio event loop for Hindsight async calls (fixes aiohttp session leaks) - Client caching (reuse instead of creating per-call) - Local mode daemon management with config change detection and auto-restart - Memory mode support (hybrid/context/tools) and prefetch method (recall/reflect) - Proper shutdown with event loop and client cleanup - Disable HindsightEmbedded.__del__ to avoid GC loop errors - Update API URLs (app -> ui.hindsight.vectorize.io, api_url -> base_url) - Setup wizard: conditional fields (when clause), dynamic defaults (default_from) - Switch dependency install from pip to uv (correct for uv-based venvs) - Add hindsight-all to plugin.yaml and import mapping - 12 new tests for dispatch routing and setup field filtering Original PR #5044 by cdbartholomew.
This commit is contained in:
Chris Bartholomew
committed by
Teknium
Teknium
parent
93aa01c71c
commit
28e1e210ee
@@ -547,3 +547,253 @@ class TestPluginMemoryDiscovery:
|
||||
"""load_memory_provider returns None for unknown names."""
|
||||
from plugins.memory import load_memory_provider
|
||||
assert load_memory_provider("nonexistent_provider") is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Sequential dispatch routing tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestSequentialDispatchRouting:
|
||||
"""Verify that memory provider tools are correctly routed through
|
||||
memory_manager.has_tool() and handle_tool_call().
|
||||
|
||||
This is a regression test for a bug where _execute_tool_calls_sequential
|
||||
in run_agent.py had its own inline dispatch chain that skipped
|
||||
memory_manager.has_tool(), causing all memory provider tools to fall
|
||||
through to the registry and return "Unknown tool". The fix added
|
||||
has_tool() + handle_tool_call() to the sequential path.
|
||||
|
||||
These tests verify the memory_manager contract that both dispatch
|
||||
paths rely on: has_tool() returns True for registered provider tools,
|
||||
and handle_tool_call() routes to the correct provider.
|
||||
"""
|
||||
|
||||
def test_has_tool_returns_true_for_provider_tools(self):
|
||||
"""has_tool returns True for tools registered by memory providers."""
|
||||
mgr = MemoryManager()
|
||||
provider = FakeMemoryProvider("ext", tools=[
|
||||
{"name": "ext_recall", "description": "Ext recall", "parameters": {}},
|
||||
{"name": "ext_retain", "description": "Ext retain", "parameters": {}},
|
||||
])
|
||||
mgr.add_provider(provider)
|
||||
|
||||
assert mgr.has_tool("ext_recall")
|
||||
assert mgr.has_tool("ext_retain")
|
||||
|
||||
def test_has_tool_returns_false_for_builtin_tools(self):
|
||||
"""has_tool returns False for agent-level tools (terminal, memory, etc.)."""
|
||||
mgr = MemoryManager()
|
||||
provider = FakeMemoryProvider("ext", tools=[
|
||||
{"name": "ext_recall", "description": "Ext", "parameters": {}},
|
||||
])
|
||||
mgr.add_provider(provider)
|
||||
|
||||
assert not mgr.has_tool("terminal")
|
||||
assert not mgr.has_tool("memory")
|
||||
assert not mgr.has_tool("todo")
|
||||
assert not mgr.has_tool("session_search")
|
||||
assert not mgr.has_tool("nonexistent")
|
||||
|
||||
def test_handle_tool_call_routes_to_provider(self):
|
||||
"""handle_tool_call dispatches to the correct provider's handler."""
|
||||
mgr = MemoryManager()
|
||||
provider = FakeMemoryProvider("hindsight", tools=[
|
||||
{"name": "hindsight_recall", "description": "Recall", "parameters": {}},
|
||||
{"name": "hindsight_retain", "description": "Retain", "parameters": {}},
|
||||
])
|
||||
mgr.add_provider(provider)
|
||||
|
||||
result = json.loads(mgr.handle_tool_call("hindsight_recall", {"query": "alice"}))
|
||||
assert result["handled"] == "hindsight_recall"
|
||||
assert result["args"] == {"query": "alice"}
|
||||
|
||||
def test_handle_tool_call_unknown_returns_error(self):
|
||||
"""handle_tool_call returns error for tools not in any provider."""
|
||||
mgr = MemoryManager()
|
||||
provider = FakeMemoryProvider("ext", tools=[
|
||||
{"name": "ext_recall", "description": "Ext", "parameters": {}},
|
||||
])
|
||||
mgr.add_provider(provider)
|
||||
|
||||
result = json.loads(mgr.handle_tool_call("terminal", {"command": "ls"}))
|
||||
assert "error" in result
|
||||
|
||||
def test_multiple_providers_route_to_correct_one(self):
|
||||
"""Tools from different providers route to the right handler."""
|
||||
mgr = MemoryManager()
|
||||
builtin = FakeMemoryProvider("builtin", tools=[
|
||||
{"name": "builtin_tool", "description": "Builtin", "parameters": {}},
|
||||
])
|
||||
external = FakeMemoryProvider("hindsight", tools=[
|
||||
{"name": "hindsight_recall", "description": "Recall", "parameters": {}},
|
||||
])
|
||||
mgr.add_provider(builtin)
|
||||
mgr.add_provider(external)
|
||||
|
||||
r1 = json.loads(mgr.handle_tool_call("builtin_tool", {}))
|
||||
assert r1["handled"] == "builtin_tool"
|
||||
|
||||
r2 = json.loads(mgr.handle_tool_call("hindsight_recall", {"query": "test"}))
|
||||
assert r2["handled"] == "hindsight_recall"
|
||||
|
||||
def test_tool_names_include_all_providers(self):
|
||||
"""get_all_tool_names returns tools from all registered providers."""
|
||||
mgr = MemoryManager()
|
||||
builtin = FakeMemoryProvider("builtin", tools=[
|
||||
{"name": "builtin_tool", "description": "B", "parameters": {}},
|
||||
])
|
||||
external = FakeMemoryProvider("ext", tools=[
|
||||
{"name": "ext_recall", "description": "E1", "parameters": {}},
|
||||
{"name": "ext_retain", "description": "E2", "parameters": {}},
|
||||
])
|
||||
mgr.add_provider(builtin)
|
||||
mgr.add_provider(external)
|
||||
|
||||
names = mgr.get_all_tool_names()
|
||||
assert names == {"builtin_tool", "ext_recall", "ext_retain"}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Setup wizard field filtering tests (when clause and default_from)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestSetupFieldFiltering:
|
||||
"""Test the 'when' clause and 'default_from' logic used by the
|
||||
memory setup wizard in hermes_cli/memory_setup.py.
|
||||
|
||||
These features are generic — any memory plugin can use them in
|
||||
get_config_schema(). Currently used by the hindsight plugin.
|
||||
"""
|
||||
|
||||
def _filter_fields(self, schema, provider_config):
|
||||
"""Simulate the setup wizard's field filtering logic.
|
||||
|
||||
Returns list of (key, effective_default) for fields that pass
|
||||
the 'when' filter.
|
||||
"""
|
||||
results = []
|
||||
for field in schema:
|
||||
key = field["key"]
|
||||
default = field.get("default")
|
||||
|
||||
# Dynamic default
|
||||
default_from = field.get("default_from")
|
||||
if default_from and isinstance(default_from, dict):
|
||||
ref_field = default_from.get("field", "")
|
||||
ref_map = default_from.get("map", {})
|
||||
ref_value = provider_config.get(ref_field, "")
|
||||
if ref_value and ref_value in ref_map:
|
||||
default = ref_map[ref_value]
|
||||
|
||||
# When clause
|
||||
when = field.get("when")
|
||||
if when and isinstance(when, dict):
|
||||
if not all(provider_config.get(k) == v for k, v in when.items()):
|
||||
continue
|
||||
|
||||
results.append((key, default))
|
||||
return results
|
||||
|
||||
def test_when_clause_filters_fields(self):
|
||||
"""Fields with 'when' are skipped if the condition doesn't match."""
|
||||
schema = [
|
||||
{"key": "mode", "default": "cloud"},
|
||||
{"key": "api_url", "default": "https://api.example.com", "when": {"mode": "cloud"}},
|
||||
{"key": "api_key", "default": None, "when": {"mode": "cloud"}},
|
||||
{"key": "llm_provider", "default": "openai", "when": {"mode": "local"}},
|
||||
{"key": "llm_model", "default": "gpt-4o-mini", "when": {"mode": "local"}},
|
||||
{"key": "budget", "default": "mid"},
|
||||
]
|
||||
|
||||
# Cloud mode: should see mode, api_url, api_key, budget
|
||||
cloud_fields = self._filter_fields(schema, {"mode": "cloud"})
|
||||
cloud_keys = [k for k, _ in cloud_fields]
|
||||
assert cloud_keys == ["mode", "api_url", "api_key", "budget"]
|
||||
|
||||
# Local mode: should see mode, llm_provider, llm_model, budget
|
||||
local_fields = self._filter_fields(schema, {"mode": "local"})
|
||||
local_keys = [k for k, _ in local_fields]
|
||||
assert local_keys == ["mode", "llm_provider", "llm_model", "budget"]
|
||||
|
||||
def test_when_clause_no_condition_always_shown(self):
|
||||
"""Fields without 'when' are always included."""
|
||||
schema = [
|
||||
{"key": "bank_id", "default": "hermes"},
|
||||
{"key": "budget", "default": "mid"},
|
||||
]
|
||||
fields = self._filter_fields(schema, {"mode": "cloud"})
|
||||
assert [k for k, _ in fields] == ["bank_id", "budget"]
|
||||
|
||||
def test_default_from_resolves_dynamic_default(self):
|
||||
"""default_from looks up the default from another field's value."""
|
||||
provider_models = {
|
||||
"openai": "gpt-4o-mini",
|
||||
"groq": "openai/gpt-oss-120b",
|
||||
"anthropic": "claude-haiku-4-5",
|
||||
}
|
||||
schema = [
|
||||
{"key": "llm_provider", "default": "openai"},
|
||||
{"key": "llm_model", "default": "gpt-4o-mini",
|
||||
"default_from": {"field": "llm_provider", "map": provider_models}},
|
||||
]
|
||||
|
||||
# Groq selected: model should default to groq's default
|
||||
fields = self._filter_fields(schema, {"llm_provider": "groq"})
|
||||
model_default = dict(fields)["llm_model"]
|
||||
assert model_default == "openai/gpt-oss-120b"
|
||||
|
||||
# Anthropic selected
|
||||
fields = self._filter_fields(schema, {"llm_provider": "anthropic"})
|
||||
model_default = dict(fields)["llm_model"]
|
||||
assert model_default == "claude-haiku-4-5"
|
||||
|
||||
def test_default_from_falls_back_to_static_default(self):
|
||||
"""default_from falls back to static default if provider not in map."""
|
||||
schema = [
|
||||
{"key": "llm_model", "default": "gpt-4o-mini",
|
||||
"default_from": {"field": "llm_provider", "map": {"groq": "openai/gpt-oss-120b"}}},
|
||||
]
|
||||
|
||||
# Unknown provider: should fall back to static default
|
||||
fields = self._filter_fields(schema, {"llm_provider": "unknown_provider"})
|
||||
model_default = dict(fields)["llm_model"]
|
||||
assert model_default == "gpt-4o-mini"
|
||||
|
||||
def test_default_from_with_no_ref_value(self):
|
||||
"""default_from keeps static default if referenced field is not set."""
|
||||
schema = [
|
||||
{"key": "llm_model", "default": "gpt-4o-mini",
|
||||
"default_from": {"field": "llm_provider", "map": {"groq": "openai/gpt-oss-120b"}}},
|
||||
]
|
||||
|
||||
# No provider set at all
|
||||
fields = self._filter_fields(schema, {})
|
||||
model_default = dict(fields)["llm_model"]
|
||||
assert model_default == "gpt-4o-mini"
|
||||
|
||||
def test_when_and_default_from_combined(self):
|
||||
"""when clause and default_from work together correctly."""
|
||||
provider_models = {"groq": "openai/gpt-oss-120b", "openai": "gpt-4o-mini"}
|
||||
schema = [
|
||||
{"key": "mode", "default": "local"},
|
||||
{"key": "llm_provider", "default": "openai", "when": {"mode": "local"}},
|
||||
{"key": "llm_model", "default": "gpt-4o-mini",
|
||||
"default_from": {"field": "llm_provider", "map": provider_models},
|
||||
"when": {"mode": "local"}},
|
||||
{"key": "api_url", "default": "https://api.example.com", "when": {"mode": "cloud"}},
|
||||
]
|
||||
|
||||
# Local + groq: should see llm_model with groq default, no api_url
|
||||
fields = self._filter_fields(schema, {"mode": "local", "llm_provider": "groq"})
|
||||
keys = [k for k, _ in fields]
|
||||
assert "llm_model" in keys
|
||||
assert "api_url" not in keys
|
||||
assert dict(fields)["llm_model"] == "openai/gpt-oss-120b"
|
||||
|
||||
# Cloud: should see api_url, no llm_model
|
||||
fields = self._filter_fields(schema, {"mode": "cloud"})
|
||||
keys = [k for k, _ in fields]
|
||||
assert "api_url" in keys
|
||||
assert "llm_model" not in keys
|
||||
|
||||
Reference in New Issue
Block a user