Skip to content

Commit f8495bb

Browse files
cdeustclaude
andcommitted
fix(recall): align handler output with declared outputSchema (#46)
Three drifts between mcp_server/handlers/recall.py's outputSchema and its handler implementation made every recall MCP call fail with ``Output validation error: 'memories' is a required property``: 1. Key names — handler returned ``results``/``total``/``query_intent`` while the schema required ``memories``/``count``/``intent``. 2. Intent enum — schema allowed 6 values but core.query_intent.QueryIntent defines 11. Any query falling back to QueryIntent.GENERAL ("general") would have failed enum validation even after the key rename. 3. Early return — the ``if not args or not args.get("query")`` branch returned ``{"results": [], "total": 0}``, missing the schema-required ``memories`` key entirely. Fix: • _handler_impl returns the schema-aligned ``memories``/``count``/ ``intent`` keys AND keeps ``results``/``total``/``query_intent`` as back-compat aliases for one minor release. • Early return now includes the schema-required keys plus the back-compat ones, so even no-query calls validate. • outputSchema enum broadened to match QueryIntent's full set (added: instruction, event_order, summarization, preference, general) so the classifier's GENERAL fallback no longer fails MCP validation. Two new tests prove the fix: • test_issue_46_schema_aligned_keys — early-return + empty-query paths carry the required keys. • test_issue_46_intent_in_schema_enum — every public string constant on QueryIntent must be in the broadened schema enum. Reported by @darval. Verified locally; the broken pre-existing test_recall_stored_memory unrelated to this fix (fails due to sqlite_compat missing ``heat`` column — separate sweep). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a1a3c1b commit f8495bb

2 files changed

Lines changed: 86 additions & 2 deletions

File tree

mcp_server/handlers/recall.py

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,24 @@
5858
},
5959
"intent": {
6060
"type": "string",
61+
# source: mcp_server/core/query_intent.py::QueryIntent — every
62+
# value the classifier can emit must be in this enum or MCP
63+
# output validation rejects the response. Previously the
64+
# schema was narrower than the classifier's range, so any
65+
# query falling back to QueryIntent.GENERAL ("general")
66+
# failed validation. Issue #46.
6167
"enum": [
6268
"temporal",
6369
"causal",
6470
"semantic",
6571
"entity",
6672
"knowledge_update",
6773
"multi_hop",
74+
"instruction",
75+
"event_order",
76+
"summarization",
77+
"preference",
78+
"general",
6879
],
6980
"description": "Classified query intent that drove the signal-weight profile.",
7081
},
@@ -262,7 +273,16 @@ def _track_recall_replay(results: list[dict], store: Any) -> None:
262273
async def _handler_impl(args: dict[str, Any] | None = None) -> dict[str, Any]:
263274
"""Retrieve memories: pg_recall base + production enrichments."""
264275
if not args or not args.get("query"):
265-
return {"results": [], "total": 0}
276+
# Issue #46: even the early-return must satisfy the outputSchema's
277+
# required keys (`memories`). The legacy `results`/`total` are
278+
# kept for back-compat with consumers that haven't migrated yet.
279+
return {
280+
"memories": [],
281+
"count": 0,
282+
"intent": "semantic",
283+
"results": [],
284+
"total": 0,
285+
}
266286

267287
query = args["query"]
268288
domain, directory = args.get("domain"), args.get("directory")
@@ -315,11 +335,21 @@ async def _handler_impl(args: dict[str, Any] | None = None) -> dict[str, Any]:
315335

316336
intent_info = classify_query_intent(query)
317337
intent = intent_info.get("intent", QueryIntent.GENERAL)
338+
# Issue #46: align the response with the declared outputSchema while
339+
# preserving the legacy keys (`results`/`total`/`query_intent`) for
340+
# consumers that already migrated to them. The schema enum was
341+
# broadened to include every QueryIntent value so the classifier's
342+
# `general` fallback no longer fails validation.
318343
return {
344+
"memories": results,
345+
"count": len(results),
346+
"intent": str(intent),
347+
# Back-compat aliases — drop after one minor release once consumers
348+
# migrate to the schema-aligned key names.
319349
"results": results,
320350
"total": len(results),
321-
"low_signal_dropped": low_signal_dropped,
322351
"query_intent": intent,
352+
"low_signal_dropped": low_signal_dropped,
323353
"dispatch_tier": "pg",
324354
"signals": {},
325355
"enhancements": build_enhancements(query, intent, "pg", settings),

tests_py/handlers/test_recall.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,60 @@ def test_empty_query_returns_empty(self):
5555
result = asyncio.run(recall_handler({"query": ""}))
5656
assert result["results"] == []
5757

58+
def test_issue_46_schema_aligned_keys(self):
59+
"""Issue #46: every recall response must satisfy the MCP outputSchema.
60+
61+
Schema requires: ``memories`` (array). Strictly enforced by Claude
62+
Code's MCP host, which would otherwise reject the response with
63+
``Output validation error: 'memories' is a required property``.
64+
65+
Back-compat aliases (``results``/``total``/``query_intent``) stay
66+
for one minor release so existing consumers don't break.
67+
"""
68+
# Early-return path (no query)
69+
result = asyncio.run(recall_handler(None))
70+
assert "memories" in result, "issue #46: schema requires 'memories' key"
71+
assert "count" in result, "issue #46: schema declares 'count'"
72+
assert "intent" in result, "issue #46: schema declares 'intent'"
73+
assert result["memories"] == []
74+
assert result["count"] == 0
75+
# Back-compat aliases still present
76+
assert "results" in result and "total" in result
77+
78+
# Empty-query path
79+
result = asyncio.run(recall_handler({"query": ""}))
80+
assert "memories" in result
81+
assert result["memories"] == []
82+
assert "intent" in result
83+
84+
def test_issue_46_intent_in_schema_enum(self):
85+
"""Issue #46 drift 2: any string the classifier emits must be in
86+
the schema's intent enum, or MCP validation rejects it. Verify
87+
the early-return uses an enum-valid value and that the broadened
88+
enum includes every QueryIntent constant (so the GENERAL fallback
89+
no longer fails)."""
90+
from mcp_server.core.query_intent import QueryIntent
91+
# The schema's broadened enum should include EVERY public string
92+
# constant on QueryIntent so the classifier can't produce an
93+
# out-of-enum value.
94+
schema_enum = {
95+
"temporal", "causal", "semantic", "entity", "knowledge_update",
96+
"multi_hop", "instruction", "event_order", "summarization",
97+
"preference", "general",
98+
}
99+
for attr in dir(QueryIntent):
100+
if attr.startswith("_") or attr != attr.upper():
101+
continue
102+
value = getattr(QueryIntent, attr)
103+
if isinstance(value, str):
104+
assert value in schema_enum, (
105+
f"QueryIntent.{attr}={value!r} missing from recall "
106+
f"outputSchema enum — would fail MCP validation"
107+
)
108+
# Early-return uses a schema-valid intent
109+
result = asyncio.run(recall_handler(None))
110+
assert result["intent"] in schema_enum
111+
58112
def test_recall_stored_memory(self):
59113
# Store a memory first
60114
asyncio.run(

0 commit comments

Comments
 (0)