From 0ec499d9b473eabd6e4e4bccb057e353a9487f4b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 21 Sep 2026 09:46:33 -0400 Subject: [PATCH] test: the milestone result shape moved, and its fake had a mock for a body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_milestone_search_is_its_own_shape asserts the result dict exactly, so adding `matched` / `matched_is` / `body_length` broke it — which is the test doing its job. Its fake was a bare MagicMock, so `m.body` autovivified: `matched` came out as a mock object and `body_length` as 0, and nothing in the old assertion touched either. Lesson #2833 is about exactly this, and the failure output is what surfaced it. The fake now carries a real body string and a report with the chunk that matched, so the assertion is about the product rather than about MagicMock's attribute behaviour. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- tests/test_mcp_tool_search.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/test_mcp_tool_search.py b/tests/test_mcp_tool_search.py index 138df7b..88d4ee1 100644 --- a/tests/test_mcp_tool_search.py +++ b/tests/test_mcp_tool_search.py @@ -211,16 +211,36 @@ async def test_milestone_search_is_its_own_shape_and_scopes_to_the_project(): from unittest.mock import MagicMock _user_id_ctx.set(7) + # `body` is a REAL string, not left to MagicMock's attribute autovivication: + # the result now reads it, and a mock body would make `matched` a mock + # object and `body_length` zero while the assertion still looked green + # (lesson #2833). ms = MagicMock(id=339, title="M3 — Metadata", description="works, editions", - status="active", project_id=30) - found = AsyncMock(return_value=[(0.81, ms)]) + status="active", project_id=30, + body="Step 4 — the metadata editions table.") + summary = AsyncMock(return_value=[{"id": 339, "total": 0, "completed": 0}]) - with patch("scribe.mcp.tools.search.semantic_search_milestones", found), \ + seen: dict = {} + + async def _milestone_search(*_a, **kw): + seen.update(kw) + kw["report"]["best_chunk"] = { + 339: {"index": 1, "text": "Step 4 — the metadata editions table."} + } + return [(0.81, ms)] + + with patch("scribe.mcp.tools.search.semantic_search_milestones", + _milestone_search), \ patch("scribe.services.milestones.get_project_milestone_summary", summary): out = await search(q="book metadata", content_type="milestone", project_id=30) - assert found.await_args.kwargs["project_id"] == 30 + assert seen["project_id"] == 30 assert out["results"] == [{ "id": 339, "title": "M3 — Metadata", "description": "works, editions", + # The plan body stays out; the passage that matched comes along, and + # says which of the two it is (#4243). + "matched": "Step 4 — the metadata editions table.", + "matched_is": "matched_passage", + "body_length": len("Step 4 — the metadata editions table."), "status": "active", "project_id": 30, "total": 0, "completed": 0, "similarity": 0.81, }]