refactor(tests): per-model fakes, FakeMCP and session mocks come from tests/helpers (#2825, milestone 296 area 1, batch 2)

Second pass over the tests/ ledger after bbee0d0. fake_record(**attrs) is the
one MagicMock-with-real-attributes builder (to_dict mirrors them; the
note-2109 hazard documented once); fake_note/fake_task/fake_snippet/
fake_project/fake_milestone/fake_system/fake_rulebook/fake_topic/fake_rule
carry each model's ordinary defaults on top of it, replacing 14 per-file
factories (two rulebook trios in tool-vs-service wordings, _fake_task, _fake_ms,
_fake_project, _plan_note, _fake_snippet, two _snippet adapters now one-liners
over fake_snippet). FakeMCP replaces the five closure-over-a-list registrar
fakes (+ _Recorder); loc() and design_token_stub() replace the paired _loc /
_token / _T stand-ins; every hand-built async_session mock (9 helper defs and
14 inline copies) now starts from make_mock_session(). Call sites rewritten by
AST with each file's former defaults made explicit, so behaviour is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

tests/test_mcp_tool_milestones.py

@@ -6,20 +6,12 @@ import pytest
 from scribe.mcp.tools.milestones import (
     list_milestones, get_milestone, create_milestone, update_milestone,
 )
+from tests.helpers import fake_milestone
 
 
 pytestmark = pytest.mark.usefixtures("_bind_user")
 
 
-def _fake_ms(**overrides) -> MagicMock:
-    m = MagicMock()
-    base = {"id": 1, "project_id": 1, "title": "MS", "description": None,
-            "status": "active", "order_index": 0}
-    base.update(overrides)
-    m.to_dict.return_value = base
-    return m
-
-
 @pytest.mark.asyncio
 async def test_list_milestones_returns_dict_with_progress():
     rows = [{"id": 1, "title": "MS1", "status": "active", "task_count": 2}]
@@ -33,7 +25,7 @@ async def test_list_milestones_returns_dict_with_progress():
 
 @pytest.mark.asyncio
 async def test_create_milestone_passes_through():
-    m = _fake_ms(id=5)
+    m = fake_milestone(id=5)
     mock = AsyncMock(return_value=m)
     with patch("scribe.mcp.tools.milestones.milestones_svc.create_milestone", mock):
         out = await create_milestone(project_id=1, title="new", description="d")
@@ -45,7 +37,7 @@ async def test_create_milestone_passes_through():
 
 @pytest.mark.asyncio
 async def test_create_milestone_empty_description_becomes_none():
-    m = _fake_ms()
+    m = fake_milestone()
     mock = AsyncMock(return_value=m)
     with patch("scribe.mcp.tools.milestones.milestones_svc.create_milestone", mock):
         await create_milestone(project_id=1, title="t", description="")
@@ -55,7 +47,7 @@ async def test_create_milestone_empty_description_becomes_none():
 @pytest.mark.asyncio
 async def test_create_milestone_passes_body_through():
     """The milestone-as-plan body is forwarded to the service."""
-    m = _fake_ms(id=5)
+    m = fake_milestone(id=5)
     mock = AsyncMock(return_value=m)
     with patch("scribe.mcp.tools.milestones.milestones_svc.create_milestone", mock):
         await create_milestone(project_id=1, title="t", body="## Goal\n\nship")
@@ -64,7 +56,7 @@ async def test_create_milestone_passes_body_through():
 
 @pytest.mark.asyncio
 async def test_create_milestone_empty_body_becomes_none():
-    m = _fake_ms()
+    m = fake_milestone()
     mock = AsyncMock(return_value=m)
     with patch("scribe.mcp.tools.milestones.milestones_svc.create_milestone", mock):
         await create_milestone(project_id=1, title="t", body="")
@@ -73,7 +65,7 @@ async def test_create_milestone_empty_body_becomes_none():
 
 @pytest.mark.asyncio
 async def test_update_milestone_sends_body():
-    m = _fake_ms()
+    m = fake_milestone()
     mock = AsyncMock(return_value=m)
     with patch("scribe.mcp.tools.milestones.milestones_svc.update_milestone", mock):
         await update_milestone(project_id=1, milestone_id=5, body="new plan")
@@ -82,7 +74,7 @@ async def test_update_milestone_sends_body():
 
 @pytest.mark.asyncio
 async def test_get_milestone_returns_body_steps_and_rules():
-    m = _fake_ms(id=5, project_id=3, body="## Goal")
+    m = fake_milestone(id=5, project_id=3, body="## Goal")
     step = MagicMock()
     step.to_dict.return_value = {"id": 9, "title": "step 1", "status": "todo"}
     applicable = {"rules": [{"id": 1, "title": "r"}], "truncated": False,
@@ -112,7 +104,7 @@ async def test_get_milestone_raises_when_not_found():
 
 @pytest.mark.asyncio
 async def test_update_milestone_only_sends_non_default_fields():
-    m = _fake_ms()
+    m = fake_milestone()
     mock = AsyncMock(return_value=m)
     with patch("scribe.mcp.tools.milestones.milestones_svc.update_milestone", mock):
         await update_milestone(project_id=1, milestone_id=5, status="done")
@@ -124,7 +116,7 @@ async def test_update_milestone_only_sends_non_default_fields():
 @pytest.mark.asyncio
 async def test_update_milestone_order_index_negative_is_omitted():
     """order_index=-1 sentinel means leave unchanged."""
-    m = _fake_ms()
+    m = fake_milestone()
     mock = AsyncMock(return_value=m)
     with patch("scribe.mcp.tools.milestones.milestones_svc.update_milestone", mock):
         await update_milestone(project_id=1, milestone_id=5, order_index=-1)
@@ -134,7 +126,7 @@ async def test_update_milestone_order_index_negative_is_omitted():
 @pytest.mark.asyncio
 async def test_update_milestone_order_index_zero_is_explicit():
     """order_index=0 is a real value (top of list), not a sentinel."""
-    m = _fake_ms()
+    m = fake_milestone()
     mock = AsyncMock(return_value=m)
     with patch("scribe.mcp.tools.milestones.milestones_svc.update_milestone", mock):
         await update_milestone(project_id=1, milestone_id=5, order_index=0)