diff --git a/CHANGELOG.md b/CHANGELOG.md index ca83013..51b318a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Steward routing no longer triggers on words inside its own explanation. Capability + extraction reads the declared `DELEGATE:` line instead of substring-matching + capability domains across the whole response, where ordinary English routed + requests — "description" contains the housekeeper domain "script", "acknowledge" + contains "knowledge" and "know". A spurious capability meant a real agent call, + including web searches, on queries that needed none. + ## [2.4.2] - 2026-07-19 ### Fixed diff --git a/src/agents/steward/service.py b/src/agents/steward/service.py index cea08b3..cf4aa50 100644 --- a/src/agents/steward/service.py +++ b/src/agents/steward/service.py @@ -19,35 +19,88 @@ from .schemas import ConversationContext, StewardRecommendation logger = get_logger(__name__) +_DELEGATE_LINE_RE = re.compile(r"^[ \t]*DELEGATE:[ \t]*(.+)$", re.IGNORECASE | re.MULTILINE) + + +def _mentions(needle: str, haystack: str) -> bool: + """Whole-word containment. Substring matching is what made this go wrong.""" + return re.search(rf"(? list[str]: """ - Extract capability names from Steward's text response. + Extract capability names from the Steward's declared delegation. - Uses keyword matching to find mentioned capabilities. + The prompt instructs the Steward to answer in a fixed shape:: + + DELEGATE: to + REASON: ... + COMPLEXITY: ... + CONTEXT: ... + + Only the DELEGATE line states intent; the rest is free prose. An earlier + version substring-matched capability *domains* across the whole response, + which routed on ordinary English: "description" contains "script" and + "discover" contains "cover" (both housekeeper domains), "acknowledge" + contains "knowledge" and "know" (librarian, biographer), and "economy" + contains "my" (biographer). Any REASON line could therefore summon agents + the Steward never asked for, and a spurious librarian is a real + multi-second web call. + + It also made prose length a routing input, so anything that shortened the + Steward's output — such as disabling model thinking — would look like it had + improved routing. + + Resolution is layered, most explicit first: + 1. a DELEGATE line beginning with a capability name — the documented shape + 2. a capability named anywhere on a DELEGATE line + 3. a capability *domain* on a DELEGATE line, for a loosely worded answer + 4. no DELEGATE line: capability names only, never domains Args: text: Steward's plain text analysis Returns: - List of capability names (e.g., ['tatlock_core']) + List of capability names (e.g. ['tatlock_core']), de-duplicated. """ - text_lower = text.lower() registry = get_household_registry() capabilities = registry.get_all_capabilities() + delegate_lines = [line.strip().lower() for line in _DELEGATE_LINE_RE.findall(text or "")] - found_caps = [] + found_caps: list[str] = [] - for cap in capabilities: - # Check if capability name is mentioned - if cap.name.lower() in text_lower: - found_caps.append(cap.name) + def _add(name: str) -> None: + if name not in found_caps: + found_caps.append(name) + + if not delegate_lines: + # Either the Steward judged no capability necessary — the prompt's + # conversational path, whose correct answer is [] — or it ignored the + # format. Names only: domain words are ordinary English and would fire + # on any prose, which is the bug described above. + haystack = (text or "").lower() + for cap in capabilities: + if _mentions(cap.name.lower(), haystack): + _add(cap.name) + return found_caps + + for line in delegate_lines: + leading = next((c for c in capabilities if line.startswith(c.name.lower())), None) + if leading is not None: + _add(leading.name) continue - # Check if any domains are mentioned - for domain in cap.domains: - if domain.lower() in text_lower: - found_caps.append(cap.name) - break + named = [c for c in capabilities if _mentions(c.name.lower(), line)] + if named: + for cap in named: + _add(cap.name) + continue + + # Last resort. Scoped to this line, so the REASON and CONTEXT prose that + # caused the original misrouting can no longer reach it. + for cap in capabilities: + if any(_mentions(domain.lower(), line) for domain in cap.domains): + _add(cap.name) return found_caps diff --git a/tests/agents/steward/test_steward_service.py b/tests/agents/steward/test_steward_service.py index 5cec948..e0516ce 100644 --- a/tests/agents/steward/test_steward_service.py +++ b/tests/agents/steward/test_steward_service.py @@ -8,7 +8,12 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest from src.agents.steward.schemas import ConversationContext, StewardRecommendation -from src.agents.steward.service import analyze_request, format_steward_note, _build_enriched_query +from src.agents.steward.service import ( + _build_enriched_query, + _extract_capabilities, + analyze_request, + format_steward_note, +) from src.core.startup import register_household_members @@ -283,3 +288,89 @@ class TestBuildEnrichedQuery: result = _build_enriched_query(query, memory_context) assert result == query + + +class TestExtractCapabilities: + """Capability extraction reads the declared DELEGATE line, not free prose. + + The prompt tells the Steward to state its choice on a DELEGATE line and to + explain itself on REASON/COMPLEXITY/CONTEXT lines. An earlier version + substring-matched capability domains across the entire response, so ordinary + English in the explanation routed requests: "description" contains the + housekeeper domain "script", "acknowledge" contains "know". These tests pin + that the explanation can no longer influence routing. + """ + + # (prose, why it used to misroute) + SUBSTRING_TRAPS = [ + ("The user wants a description of the algorithm.", "script -> housekeeper"), + ("I should discover what the answer is.", "cover -> housekeeper"), + ("That sounds fantastic, let me compute it.", "fan -> housekeeper"), + ("I acknowledge the request to add two numbers.", "knowledge/know -> librarian, biographer"), + ("The user asks about the economy myth.", "my -> biographer"), + ("Convert 98.6 Fahrenheit to Celsius.", "temperature is a housekeeper domain"), + ] + + @pytest.mark.parametrize("prose,reason", SUBSTRING_TRAPS) + def test_reason_prose_cannot_add_capabilities(self, prose, reason): + """Explanatory prose must not summon agents the Steward did not request.""" + text = f"DELEGATE: tatlock_core to calculate\nREASON: {prose}\nCOMPLEXITY: simple" + + assert _extract_capabilities(text) == ["tatlock_core"], f"regression: {reason}" + + def test_delegate_line_task_text_does_not_leak(self): + """A domain word inside the task description must not add a capability. + + "home" is a housekeeper domain, but this is plainly a memory recall. + """ + text = "DELEGATE: biographer to recall the user's home address\nREASON: personal data" + + assert _extract_capabilities(text) == ["biographer"] + + def test_multiple_delegate_lines(self): + """Each DELEGATE line contributes its capability, in order, deduplicated.""" + text = ( + "DELEGATE: biographer to recall the user's location\n" + "DELEGATE: librarian to search_web for the forecast\n" + "DELEGATE: biographer to recall preferences\n" + ) + + assert _extract_capabilities(text) == ["biographer", "librarian"] + + def test_capability_named_later_on_the_line(self): + """A loosely worded DELEGATE line still resolves by name.""" + text = "DELEGATE: ask the librarian to search the web" + + assert _extract_capabilities(text) == ["librarian"] + + def test_domain_fallback_within_delegate_line(self): + """With no capability named, domains on the DELEGATE line still resolve.""" + text = "DELEGATE: turn on the lights in the kitchen" + + assert _extract_capabilities(text) == ["housekeeper"] + + def test_conversational_response_selects_nothing(self): + """No DELEGATE line means no capability, which is the prompt's chat path.""" + text = "This is a simple greeting. No capabilities are needed. COMPLEXITY: simple" + + assert _extract_capabilities(text) == [] + + def test_malformed_response_still_routes_by_name(self): + """If the format is ignored, a named capability is still honoured.""" + text = "I think the librarian should handle this research request." + + assert _extract_capabilities(text) == ["librarian"] + + def test_malformed_response_does_not_route_on_domains(self): + """...but bare prose must not route on domain words alone.""" + text = "The user wants a description of home automation, and I acknowledge it." + + assert _extract_capabilities(text) == [] + + def test_case_insensitive_delegate_marker(self): + text = "delegate: Librarian to search_web" + + assert _extract_capabilities(text) == ["librarian"] + + def test_empty_input(self): + assert _extract_capabilities("") == []