diff --git a/docs/spec/X9-mcp-tool-contract.md b/docs/spec/X9-mcp-tool-contract.md index 82f0ad1..a0f7ebe 100644 --- a/docs/spec/X9-mcp-tool-contract.md +++ b/docs/spec/X9-mcp-tool-contract.md @@ -38,8 +38,8 @@ מקביל ל-[X6 INV-UI3](X6-ui-api-contract.md). **הנדסי.** **מקורות:** Anthropic — *MCP / tool result conventions* (https://modelcontextprotocol.io/) · JSON-RPC 2.0 (result/error envelope) (https://www.jsonrpc.org/specification) · RFC 9457 (Problem Details) | סטטוס: verified -**אכיפה:** wrapper-תשובה משותף בכל הכלים. **כיום אין** — מעורב. -**הפרה ידועה:** `search_*` מחזיר `"לא נמצאו תוצאות."` או JSON; חלק `{error}`, חלק raise ([gap-audit GAP-48](gap-audit.md)). +**אכיפה:** wrapper-תשובה משותף בכל הכלים — `tools/envelope.py` (`ok`/`empty`/`err`), SSoT יחיד שמחליף את 5 ה-`_ok`/`_err` המשוכפלים. **GAP-48 בתהליך — פרוסה 1 (משפחת-חיפוש, 2026-06-06):** `search_decisions`/`search_case_documents`/`find_similar_cases`/`search_internal_decisions` מחזירים `{status,data,message}` (status ∈ ok/empty/error — מבחין הצלחה/ריק/שגיאה). צרכני-ה-API ב-`web/app.py` מפרקים דרך `envelope_unwrap` כדי לשמר את חוזה-ה-UI↔API (X6) ללא-שינוי. נותרו ~73 כלים — מיגרציה הדרגתית לפי-משפחה. +**הפרה ידועה:** שאר הכלים עדיין מעורבים (raw payload / `{error}` / `{status,message}` אד-הוק) — ייושרו בפרוסות הבאות של GAP-48. ### INV-TOOL2: שמות עקביים + חיפוש לפי-קורפוס **כלל:** שמות-הכלים עוקבים אחר convention אחיד, ושם משקף התנהגות. כלי-חיפוש מובחנים **לפי הקורפוס** diff --git a/docs/spec/gap-audit.md b/docs/spec/gap-audit.md index 4197ee7..c6978af 100644 --- a/docs/spec/gap-audit.md +++ b/docs/spec/gap-audit.md @@ -201,7 +201,8 @@ - **סטטוס חלקי (פרוסה 1, 2026-06-06):** ✅ **GAP-44** — נוסף `get_appraiser_facts` (ה-get המקביל ל-extract, INV-TOOL4); ✅ **GAP-53** — נוסף `_clamp_limit` (תקרה 200, INV-TOOL5) על ~13 כלי list/search + הוספת limit ל-`list_chair_feedback` (שהיה ללא תקרה). - **סטטוס חלקי (פרוסה 2, 2026-06-06):** ✅ **GAP-52** (INV-TOOL3 idempotency) — `case_create`/`precedent_attach`/`document_upload` מחזירים קיים במקום כפילות (בדיקת-מפתח ברמת-אפליקציה; document_upload לפי SHA-256 → מדלג OCR/embed כפול); ✅ **GAP-45** (INV-TOOL4 visibility) — נוסף `extraction_status` שחושף עומק תור-החילוץ (metadata/halacha) + גיל הבקשה הוותיקה. - **סטטוס חלקי (פרוסה 3, 2026-06-06):** ✅ **GAP-51** (set_outcome SSoT, הכרעת-יו"ר "3 תוצאות + הוצאת betterment_levy") — קנוני `rejection/partial_acceptance/full_acceptance` ב-`lessons.VALID_OUTCOMES`; `OUTCOME_LABELS_HE` (עברית-ב-UI SSoT); `canonical_outcome()` ל-legacy; `betterment_levy`→`PRACTICE_AREA_OVERRIDES` (override לפי practice_area); block_writer/set_outcome/drafting/web-ui יושרו; נתונים נורמלו (9 שורות + גיבוי). -- **סטטוס חלקי (פרוסה 4, 2026-06-06):** ✅ **GAP-47 (חלק provenance, INV-TOOL4/G9)** — `draft_section` חושף בפלט `document_id`+`page`+`score` לכל קטע ב-`case_documents`/`precedents` (ה-provenance כבר נשלף ב-`search_similar` ונזרק; כעת מוחזר), כך שהכותב יכול לעקוב אל המקור ולצטטו. תוספתי, לא-שובר. **נותר ב-GAP-47:** העברת הנחיות-יו"ר מ-`analysis-and-research.md` ל-DB (`get_chair_directions`) — שינוי-מסלול גדול יותר הנוגע ל-UI של דפנה ולזרימת-האנליסט; לפרוסה נפרדת. נותר ב-FU-14: GAP-48 (envelope), GAP-49/50 (מיזוג+rename — שובר), GAP-54 (איחוד קליטת-פסיקה). +- **סטטוס חלקי (פרוסה 4, 2026-06-06):** ✅ **GAP-47 (חלק provenance, INV-TOOL4/G9)** — `draft_section` חושף בפלט `document_id`+`page`+`score` לכל קטע ב-`case_documents`/`precedents` (ה-provenance כבר נשלף ב-`search_similar` ונזרק; כעת מוחזר), כך שהכותב יכול לעקוב אל המקור ולצטטו. תוספתי, לא-שובר. **נותר ב-GAP-47:** העברת הנחיות-יו"ר מ-`analysis-and-research.md` ל-DB (`get_chair_directions`) — שינוי-מסלול גדול יותר הנוגע ל-UI של דפנה ולזרימת-האנליסט; לפרוסה נפרדת. +- **סטטוס חלקי (פרוסה 5, 2026-06-06):** 🔄 **GAP-48 (envelope אחיד, INV-TOOL1) — תחילת מיגרציה הדרגתית.** נוצר `tools/envelope.py` כ-SSoT יחיד (`ok`/`empty`/`err` → `{status,data,message}`, status מבחין הצלחה/ריק/שגיאה) המחליף 3 מוסכמות סותרות (raw payload / `{error}` / `{status,message}` אד-הוק) ו-5 עותקי `_ok`/`_err` משוכפלים. **משפחת-החיפוש הראשונה הומרה** (`search_decisions`/`search_case_documents`/`find_similar_cases`/`search_internal_decisions`); `web/app.py` מפרק דרך `envelope_unwrap` לשמירת חוזה-ה-API (X6) ללא-שינוי; טסט `test_search_domain_scope` עודכן לחוזה החדש (5/5 ✅). **החלטה הנדסית:** הדרגתי לפי-משפחה ולא big-bang — מפת-צרכנים (Explore) הראתה ש-server.py הוא pass-through, web-ui מבודד (`/api/*`), ורק 17 כלים נצרכים ישירות מ-app.py — כך הסיכון לסוכנים החיים ממוזער. נותרו ~73 כלים בפרוסות הבאות. נותר ב-FU-14: GAP-48 (שאר המשפחות), GAP-49/50 (מיזוג+rename — שובר), GAP-54 (איחוד קליטת-פסיקה). ### FU-15 — deploy/env/secrets - **מכסה:** GAP-55..62 · **invariants:** INV-ENV1–ENV5 · **effort:** M · **תלויות:** — diff --git a/mcp-server/src/legal_mcp/tools/envelope.py b/mcp-server/src/legal_mcp/tools/envelope.py new file mode 100644 index 0000000..b5eb4ef --- /dev/null +++ b/mcp-server/src/legal_mcp/tools/envelope.py @@ -0,0 +1,58 @@ +"""מעטפת-תשובה קנונית לכלי-ה-MCP (GAP-48 / INV-TOOL1). + +מקור-אמת יחיד לצורת-הפלט של כל כלי. כל כלי שעבר מיגרציה מחזיר מחרוזת-JSON +אחידה ``{status, data, message}`` — לא list-לפעמים, string-לפעמים, ``{error}`` +לפעמים. שלושת המצבים מובחנים מפורשות (INV-TOOL1): + + status — "ok" הצלחה עם payload ב-``data``. + "empty" שאילתה תקינה שלא החזירה תוצאות (מובחן מ-error). + "error" הקריאה נכשלה; ``message`` מסביר. + data — ה-payload (list/dict/scalar) או null. + message — הערה קריאה-לאדם (עברית), בעיקר ל-empty/error. + +מחליף את ה-``_ok``/``_err`` שהשתכפלו ב-5 קבצי-כלים עם 3 מוסכמות שונות (G2). + +צרכן-API ב-``web/`` שמעביר פלט-כלי ל-HTTP חייב **לפרק** את המעטפת +(להחזיר ``data``) כדי לשמר את חוזה-ה-UI↔API (X6) — ראה ``envelope_unwrap``. +""" + +from __future__ import annotations + +import json +from typing import Any + + +def _dump(obj: dict) -> str: + return json.dumps(obj, ensure_ascii=False, indent=2, default=str) + + +def ok(data: Any = None, message: str = "") -> str: + """הצלחה עם payload.""" + return _dump({"status": "ok", "data": data, "message": message}) + + +def empty(message: str = "", data: Any = None) -> str: + """שאילתה תקינה ללא תוצאות — מובחן מהצלחה (data לא-ריק) ומשגיאה.""" + return _dump({"status": "empty", "data": [] if data is None else data, "message": message}) + + +def err(message: str, *, data: Any = None) -> str: + """כשל בקריאה.""" + return _dump({"status": "error", "data": data, "message": message}) + + +def envelope_unwrap(parsed: Any) -> Any: + """פירוק מעטפת לצורך צרכן-API שמשמר חוזה-HTTP ישן. + + בהינתן dict-מעטפת מפורסר (``json.loads`` של פלט-כלי שעבר מיגרציה): + status=="ok" → מחזיר ``data`` (ה-payload המקורי, למשל list). + status in {empty,error}→ מחזיר ``{"message": message}`` (תאימות-לאחור + לצורה שצרכני-ה-API החזירו על מחרוזת-פרוזה). + קלט שאינו מעטפת (אין מפתח ``status``) מוחזר כמות-שהוא — בטוח לתקופת-המעבר + שבה רק חלק מהכלים עברו מיגרציה. + """ + if isinstance(parsed, dict) and "status" in parsed and "data" in parsed: + if parsed["status"] == "ok": + return parsed["data"] + return {"message": parsed.get("message", "")} + return parsed diff --git a/mcp-server/src/legal_mcp/tools/search.py b/mcp-server/src/legal_mcp/tools/search.py index f1390ce..7b20607 100644 --- a/mcp-server/src/legal_mcp/tools/search.py +++ b/mcp-server/src/legal_mcp/tools/search.py @@ -2,12 +2,12 @@ from __future__ import annotations -import json import logging import time from uuid import UUID from legal_mcp.services import db, embeddings, hybrid_search, practice_area as pa, telemetry +from legal_mcp.tools.envelope import empty, err, ok logger = logging.getLogger(__name__) @@ -53,8 +53,8 @@ async def search_decisions( # search to its domain. This is a data anomaly — BLOCK rather than # silently running a cross-domain search for a specific case. if not practice_area: - return ( - f"שגיאה: לא ניתן לקבוע את התחום המשפטי (practice_area) של תיק " + return err( + f"לא ניתן לקבוע את התחום המשפטי (practice_area) של תיק " f"{case_number}. לתיק אין practice_area מוגדר ולא ניתן להסיק אותו " f"ממספר התיק. זוהי אנומליית נתונים — נא להגדיר את ה-practice_area " f"של התיק (למשל דרך case_update) לפני הרצת חיפוש מסונן לתיק זה." @@ -88,7 +88,7 @@ async def search_decisions( ) if not results: - return "לא נמצאו תוצאות." + return empty("לא נמצאו תוצאות.") formatted = [] for r in results: @@ -103,7 +103,7 @@ async def search_decisions( "image_thumbnail": r.get("image_thumbnail_path"), }) - return json.dumps(formatted, ensure_ascii=False, indent=2) + return ok(formatted) async def search_case_documents( @@ -120,7 +120,7 @@ async def search_case_documents( """ case = await db.get_case_by_number(case_number) if not case: - return f"תיק {case_number} לא נמצא." + return err(f"תיק {case_number} לא נמצא.") case_uuid = UUID(case["id"]) query_emb = await embeddings.embed_query(query) @@ -143,7 +143,7 @@ async def search_case_documents( ) if not results: - return f"לא נמצאו תוצאות בתיק {case_number}." + return empty(f"לא נמצאו תוצאות בתיק {case_number}.") formatted = [] for r in results: @@ -157,7 +157,7 @@ async def search_case_documents( "image_thumbnail": r.get("image_thumbnail_path"), }) - return json.dumps(formatted, ensure_ascii=False, indent=2) + return ok(formatted) async def find_similar_cases( @@ -216,7 +216,7 @@ async def find_similar_cases( ) if not results: - return "לא נמצאו תיקים דומים." + return empty("לא נמצאו תיקים דומים.") # Deduplicate by case_number, keep best score per case. # image-only rows still carry case_number from the join. @@ -240,7 +240,7 @@ async def find_similar_cases( "match_type": r.get("match_type", "text"), }) - return json.dumps(formatted, ensure_ascii=False, indent=2) + return ok(formatted) async def search_internal_decisions( @@ -296,7 +296,7 @@ async def search_internal_decisions( ) if not results: - return "לא נמצאו החלטות ועדת ערר רלוונטיות." + return empty("לא נמצאו החלטות ועדת ערר רלוונטיות.") # Cap primary results back to ``limit`` (we over-fetched only to seed # the citation expansion below — the user asked for ``limit`` items). @@ -334,7 +334,7 @@ async def search_internal_decisions( for row in cited_rows: formatted.append(_format_internal_row(row, match_type="cited_by")) - return json.dumps(formatted, ensure_ascii=False, indent=2) + return ok(formatted) def _format_internal_row(r: dict, *, match_type: str = "primary") -> dict: diff --git a/mcp-server/tests/test_search_domain_scope.py b/mcp-server/tests/test_search_domain_scope.py index 66968f7..59cb414 100644 --- a/mcp-server/tests/test_search_domain_scope.py +++ b/mcp-server/tests/test_search_domain_scope.py @@ -84,7 +84,8 @@ def test_explicit_practice_area_used(patched: dict) -> None: # explicit value must not trigger a case lookup assert patched["cases"] == {} # ran -> JSON result, not an error string - assert json.loads(out)[0]["content"] == "hit" + assert json.loads(out)["status"] == "ok" + assert json.loads(out)["data"][0]["content"] == "hit" def test_case_practice_area_used(patched: dict) -> None: @@ -98,7 +99,8 @@ def test_case_practice_area_used(patched: dict) -> None: ) assert len(patched["hybrid"]) == 1 assert patched["hybrid"][0]["practice_area"] == "betterment_levy" - assert json.loads(out)[0]["content"] == "hit" + assert json.loads(out)["status"] == "ok" + assert json.loads(out)["data"][0]["content"] == "hit" def test_case_empty_practice_area_derived_from_prefix(patched: dict) -> None: @@ -113,7 +115,8 @@ def test_case_empty_practice_area_derived_from_prefix(patched: dict) -> None: ) assert len(patched["hybrid"]) == 1 assert patched["hybrid"][0]["practice_area"] == "betterment_levy" - assert json.loads(out)[0]["content"] == "hit" + assert json.loads(out)["status"] == "ok" + assert json.loads(out)["data"][0]["content"] == "hit" def test_case_undeterminable_is_blocked(patched: dict) -> None: @@ -128,11 +131,10 @@ def test_case_undeterminable_is_blocked(patched: dict) -> None: ) # hybrid search must NOT have been called assert patched["hybrid"] == [] - # returns a Hebrew error string, not JSON - assert out.startswith("שגיאה") - assert "7777/25" in out - with pytest.raises(json.JSONDecodeError): - json.loads(out) + # GAP-48: returns the {status,data,message} envelope with status="error" + parsed = json.loads(out) + assert parsed["status"] == "error" + assert "7777/25" in parsed["message"] def test_no_case_no_practice_area_proceeds(patched: dict) -> None: @@ -141,4 +143,5 @@ def test_no_case_no_practice_area_proceeds(patched: dict) -> None: assert len(patched["hybrid"]) == 1 assert patched["hybrid"][0]["practice_area"] is None assert patched["cases"] == {} - assert json.loads(out)[0]["content"] == "hit" + assert json.loads(out)["status"] == "ok" + assert json.loads(out)["data"][0]["content"] == "hit" diff --git a/web/app.py b/web/app.py index e3b249b..305a566 100644 --- a/web/app.py +++ b/web/app.py @@ -32,6 +32,7 @@ import httpx from legal_mcp import config from legal_mcp.services import chunker, db, embeddings, extractor, git_sync, metrics as metrics_service, processor, proofreader, research_md from legal_mcp.tools import cases as cases_tools, search as search_tools, workflow as workflow_tools, drafting as drafting_tools, precedents as precedents_tools +from legal_mcp.tools.envelope import envelope_unwrap # Import integration clients (same directory) _web_dir = Path(__file__).resolve().parent @@ -2095,7 +2096,9 @@ async def api_search(query: str, limit: int = 10, section_type: str = ""): """Semantic search across decisions and documents.""" result = await search_tools.search_decisions(query, limit, section_type) try: - return json.loads(result) + # GAP-48: tool now returns the {status,data,message} envelope; unwrap it + # to preserve the legacy API shape (list on hits, {"message"} otherwise). + return envelope_unwrap(json.loads(result)) except json.JSONDecodeError: return {"message": result} @@ -2105,7 +2108,8 @@ async def api_case_search(case_number: str, query: str, limit: int = 10): """Semantic search within a specific case's documents.""" result = await search_tools.search_case_documents(case_number, query, limit) try: - return json.loads(result) + # GAP-48: unwrap the tool envelope, keep the legacy API shape. + return envelope_unwrap(json.loads(result)) except json.JSONDecodeError: return {"message": result}