diff --git a/src/agent_loop.py b/src/agent_loop.py index a4525e93c..5effc54b5 100644 --- a/src/agent_loop.py +++ b/src/agent_loop.py @@ -262,6 +262,11 @@ _DOMAIN_RULES = { - Use `manage_settings` for preferences and tool enable/disable. - Use named tools over `app_api` when a named wrapper exists. - `app_api` is only for safe UI/API actions without a named tool; do not use it for shell, package installs, engine rebuilds, or sensitive auth/admin paths.""", + "contacts": """\ +## Contacts rules +- Use `resolve_contact` to look up a contact's email or phone number by name. Searches the CardDAV address book and sent email history. +- Use `manage_contact` to list, add, update, or delete contacts in the address book. +- Do NOT use `manage_memory` for contact lookups — contact details live in the address book, not memory.""", } _DOMAIN_TOOL_MAP = { @@ -274,6 +279,7 @@ _DOMAIN_TOOL_MAP = { "sessions": {"create_session", "list_sessions", "manage_session", "send_to_session", "search_chats"}, "files": {"bash", "python", "read_file", "write_file", "edit_file", "grep", "glob", "ls", "get_workspace"}, "settings": {"manage_settings", "manage_endpoints", "manage_mcp", "manage_webhooks", "manage_tokens", "app_api"}, + "contacts": {"resolve_contact", "manage_contact"}, } def _domain_rules_for_tools(tool_names: set) -> list[str]: @@ -797,6 +803,8 @@ def _classify_agent_request(messages: List[Dict], last_user: str) -> Dict[str, o domains.add("files") if has(r"\b(endpoint|api token|mcp|webhook|preference|configure|config|setting)\b"): domains.add("settings") + if has(r"\b(contact|contacts|phone|phone number|address book|vcard)\b"): + domains.add("contacts") low_signal = not continuation and not domains return { diff --git a/tests/test_tool_rag_contacts_domain.py b/tests/test_tool_rag_contacts_domain.py new file mode 100644 index 000000000..a1f8660ae --- /dev/null +++ b/tests/test_tool_rag_contacts_domain.py @@ -0,0 +1,72 @@ +"""Regression: the agent tool-RAG domain classifier had no contacts domain, +so contact-lookup requests matched no domain, were flagged low_signal, and had +tool retrieval SKIPPED entirely — the model only received ALWAYS_AVAILABLE tools +(manage_memory, ask_user, update_plan) and never `resolve_contact`/`manage_contact`, +so it could not look up contacts from the CardDAV address book (it looped on +manage_memory instead). + +Root cause: `_classify_agent_request` in src/agent_loop.py sets +`low_signal = not continuation and not domains`; with no `contacts` domain, +prompts like "What is Massimo's contact?" matched nothing → low_signal → +retrieval skipped. + +The classifier is deterministic string matching (no embeddings / no DB), so it +can be exercised directly. +""" + +from src.agent_loop import ( + _classify_agent_request, + _DOMAIN_TOOL_MAP, + _DOMAIN_RULES, + _domain_rules_for_tools, +) + + +def _classify(text): + return _classify_agent_request([{"role": "user", "content": text}], text) + + +def test_contact_lookup_requests_get_contacts_domain(): + """Contact-lookup phrasings must match the `contacts` domain and NOT be + treated as low-signal (which would skip tool retrieval).""" + prompts = [ + "What is Massimo's contact?", + "What's John's phone number?", + "Show me my contacts", + "Look up Kevin's contact info", + "Find Alice's phone number", + ] + for p in prompts: + intent = _classify(p) + assert "contacts" in intent["domains"], f"expected contacts domain for: {p!r}" + assert intent["low_signal"] is False, f"must not be low_signal: {p!r}" + + +def test_contact_management_requests_get_contacts_domain(): + """Add/update/delete contact phrasings also resolve to the contacts domain.""" + for p in ("add a new contact", "update Bob's phone number", "delete that contact", + "save this person to contacts"): + intent = _classify(p) + assert "contacts" in intent["domains"], f"expected contacts domain for: {p!r}" + + +def test_contacts_domain_seeds_resolve_and_manage_contact(): + """The domain must seed the actual contacts tools so they are offered even + when semantic retrieval misses.""" + assert _DOMAIN_TOOL_MAP["contacts"] == {"resolve_contact", "manage_contact"} + + +def test_contacts_domain_has_a_rule_pack(): + """Every domain in _DOMAIN_TOOL_MAP needs a matching _DOMAIN_RULES entry, + otherwise _domain_rules_for_tools raises KeyError when the tools are selected.""" + assert "contacts" in _DOMAIN_RULES + rules = _domain_rules_for_tools({"resolve_contact"}) + assert any("Contacts rules" in r for r in rules) + + +def test_non_contact_requests_do_not_match_contacts_domain(): + """Guard against over-triggering: ordinary prompts must not be flagged contacts.""" + assert "contacts" not in _classify("what is the capital of France")["domains"] + assert "contacts" not in _classify("reply to the latest email in my inbox")["domains"] + assert "contacts" not in _classify("generate an image of a sunset")["domains"] + assert "contacts" not in _classify("what's 2 plus 2")["domains"]