From 2f50a8cc797af8d9b456855f213d3eecde596325 Mon Sep 17 00:00:00 2001 From: Chaim Date: Wed, 5 Aug 2026 10:32:27 +0000 Subject: [PATCH] =?UTF-8?q?fix(retrieval):=20104=20=D7=94=D7=97=D7=9C?= =?UTF-8?q?=D7=98=D7=95=D7=AA=20=D7=95=D7=A2=D7=93=D7=95=D7=AA-=D7=A2?= =?UTF-8?q?=D7=A8=D7=A8=20=D7=94=D7=99=D7=95=20=D7=91=D7=9C=D7=AA=D7=99-?= =?UTF-8?q?=D7=A0=D7=A8=D7=90=D7=95=D7=AA=20=D7=91=D7=97=D7=99=D7=A4=D7=95?= =?UTF-8?q?=D7=A9=20=E2=80=94=20=D7=A1=D7=99=D7=A0=D7=95=D7=9F=20=D7=95?= =?UTF-8?q?=D7=9C=D7=90=20=D7=93=D7=99=D7=A8=D7=95=D7=92=20(#232)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `search_precedent_library` — הכלי שסוכני הכתיבה קוראים לו בפועל — לא חשף `source_kind` כלל, וכל שכבה מתחתיו נפלה לברירת-מחדל `external_upload`. התוצאה: `WHERE cl.source_kind = 'external_upload'` חתך 104 החלטות ועדות-ערר (29% מהקורפוס) עוד לפני הדירוג. ערר (חיפה) 83/16 — האסמכתה הישירה ביותר בקורפוס לטענת-סף ס-2 ב-1069-04-26 — לא הוחזרה גם בשאילתה כמעט-מילולית. ההיפותזה שנרשמה ב-#232 (הטיית verified/cite_count מדירה החלטות חדשות) **נבדקה ונפסלה**: כיסוי ה-verified כמעט זהה בשני הקורפוסים (55% מול 49%), ותקרת-ההטיה 0.22 לא הסבירה פער מול התאמה כמעט-מילולית. הסיבה הייתה סינום קשיח, לא דירוג. מה שונה - `_source_kind_clause()` — הגדרה אחת לאוצר-המילים של הסלקטור (G2), במקום שכפולו בכל אתר חיפוש/רשימה. `""`/`"all"` = כל הקורפוס; סלקטור לא-מוכר מרים ValueError במקום להגיע ל-SQL (הערך מוזרק ב-f-string, אז ה-whitelist הוא גם מה ששומר על זה בטוח). - ברירת-המחדל בכל שרשרת-החיפוש והרשימה: `""` = הקורפוס כולו. - `search_precedent_library` + `precedent_library_list` חושפים `source_kind` לסינון מפורש; `/api/precedent-library/search` מקבל אותו גם הוא כדי ש-UI ו-MCP לא יתפצלו. - נרמול מפריד במספר-תיק: `83/16` ו-`83-16` מחזירים את אותה שורה. אימות מול ה-DB החי — 4/4 החלטות ועדה חוזרות במקום **1** בשאילתה בלשון הלכה מאושרת (83-16, 1029-18, 1085-23, 1094-09-19 פדילה). 83/16 עלתה ל-0.711 מול 0.663 של 3213/97 שחסמה אותה קודם. רשימת-הקורפוס: 259 → 386. invariants: G1 (נרמול במקור, לא תיקון-תסמין בקריאה) · G2 (הגדרה אחת לסלקטור; UI ו-MCP על אותו מסלול) · §6 (סלקטור שגוי מתפוצץ, לא נבלע) טסטים: 5 חדשים (tests/test_source_kind_selector.py), אחד מהם נועל את ברירות-המחדל של 8 נקודות-הכניסה — זה בדיוק הבאג. 514 עוברים. --- mcp-server/src/legal_mcp/services/db.py | 88 +++++++++++++++---- .../src/legal_mcp/services/hybrid_search.py | 7 +- .../legal_mcp/services/precedent_library.py | 9 +- .../src/legal_mcp/tools/precedent_library.py | 14 ++- mcp-server/tests/test_source_kind_selector.py | 67 ++++++++++++++ web/app.py | 4 +- 6 files changed, 162 insertions(+), 27 deletions(-) create mode 100644 mcp-server/tests/test_source_kind_selector.py diff --git a/mcp-server/src/legal_mcp/services/db.py b/mcp-server/src/legal_mcp/services/db.py index 34d8e1a..be08245 100644 --- a/mcp-server/src/legal_mcp/services/db.py +++ b/mcp-server/src/legal_mcp/services/db.py @@ -5324,21 +5324,19 @@ async def list_external_case_law( search: str = "", limit: int = 100, offset: int = 0, - source_kind: str = "external_upload", + source_kind: str = "", ) -> list[dict]: """List chair-uploaded precedents, with simple filters. + source_kind="" (default) = the whole corpus — court rulings *and* + appeals-committee decisions. The old ``external_upload`` default hid + every committee decision from a plain listing (#232). source_kind="all_committees" expands to: source_kind='internal_committee' OR (source_kind='external_upload' AND source_type='appeals_committee'). """ pool = await get_pool() - if source_kind == "all_committees": - conditions = [ - "(source_kind = 'internal_committee' OR " - "(source_kind = 'external_upload' AND source_type = 'appeals_committee'))" - ] - else: - conditions = [f"source_kind = '{source_kind}'"] + sk_clause = _source_kind_clause(source_kind) + conditions = [sk_clause] if sk_clause else [] params: list = [] idx = 1 if practice_area: @@ -5358,13 +5356,19 @@ async def list_external_case_law( params.append(source_type) idx += 1 if search: + # Case-number separator normalisation (#232, trap 2): committee numbers + # are stored hyphenated ("83-16") while people write them with a slash + # ("83/16"). Fold both sides to '/' so either form finds the row — + # matching at the point of comparison rather than asking every caller + # to guess the stored form. conditions.append( f"(case_number ILIKE ${idx} OR case_name ILIKE ${idx} " - f"OR summary ILIKE ${idx} OR headnote ILIKE ${idx})" + f"OR summary ILIKE ${idx} OR headnote ILIKE ${idx} " + f"OR replace(case_number, '-', '/') ILIKE replace(${idx}, '-', '/'))" ) params.append(f"%{search}%") idx += 1 - where_sql = " AND ".join(conditions) + where_sql = " AND ".join(conditions) if conditions else "TRUE" params.extend([limit, offset]) sql = f""" SELECT id, case_number, case_name, court, date, practice_area, @@ -7775,6 +7779,45 @@ async def list_corroboration_for_halacha(halacha_id: UUID) -> list[dict]: ] +#: Accepted ``source_kind`` selectors. ``""``/``"all"`` mean *no filter* — +#: the whole corpus, court rulings and appeals-committee decisions alike. +_SOURCE_KIND_SELECTORS = frozenset( + {"", "all", "all_committees", "external_upload", "internal_committee", "cited_only"} +) + + +def _source_kind_clause(source_kind: str, column_prefix: str = "") -> str: + """Return the SQL predicate for a ``source_kind`` selector, or "" for none. + + Single definition for every caller (G2) — the selector vocabulary was + previously restated at each search/list site, which is how the + ``external_upload`` default silently hid 104 appeals-committee + decisions from ``search_precedent_library`` (#232). + + ``column_prefix`` is the table alias plus dot (e.g. ``"cl."``) or "" when + the query selects from ``case_law`` directly. + + Raises ValueError on an unknown selector rather than interpolating it — + the value reaches SQL by f-string, so the whitelist is also what keeps + that safe. + """ + sk = (source_kind or "").strip() + if sk not in _SOURCE_KIND_SELECTORS: + raise ValueError( + f"source_kind לא מוכר: {source_kind!r}. " + f"ערכים חוקיים: {sorted(_SOURCE_KIND_SELECTORS - {''})} או '' (הכל)" + ) + if sk in ("", "all"): + return "" + p = column_prefix + if sk == "all_committees": + return ( + f"({p}source_kind = 'internal_committee' OR " + f"({p}source_kind = 'external_upload' AND {p}source_type = 'appeals_committee'))" + ) + return f"{p}source_kind = '{sk}'" + + async def search_precedent_library_semantic( query_embedding: list[float], practice_area: str = "", @@ -7785,14 +7828,15 @@ async def search_precedent_library_semantic( subject_tag: str = "", limit: int = 10, include_halachot: bool = True, - source_kind: str = "external_upload", + source_kind: str = "", district: str = "", chair_name: str = "", ) -> list[dict]: - """Semantic search over precedents filtered by source_kind. + """Semantic search over precedents, optionally filtered by source_kind. - source_kind='external_upload' → court rulings (default) - source_kind='internal_committee' → appeals-committee decisions + source_kind='' → the whole corpus (default, #232) + source_kind='external_upload' → court rulings only + source_kind='internal_committee' → appeals-committee decisions only Returns merged halachot + chunks. Halachot are pre-distilled rules, so they get a small score boost. Only ``approved`` / ``published`` halachot @@ -7800,12 +7844,15 @@ async def search_precedent_library_semantic( of halacha review status. """ pool = await get_pool() + sk_clause = _source_kind_clause(source_kind, "cl.") halacha_filters = [ "h.review_status <> 'rejected'", # #153: include background; rank verified higher - f"cl.source_kind = '{source_kind}'", "cl.searchable = true", ] - chunk_filters = [f"cl.source_kind = '{source_kind}'", "cl.searchable = true"] + chunk_filters = ["cl.searchable = true"] + if sk_clause: + halacha_filters.append(sk_clause) + chunk_filters.append(sk_clause) h_params: list = [query_embedding, limit] c_params: list = [query_embedding, limit] h_idx = 3 @@ -8017,7 +8064,7 @@ async def search_precedent_library_lexical( appeal_subtype: str = "", is_binding: bool | None = None, subject_tag: str = "", - source_kind: str = "external_upload", + source_kind: str = "", district: str = "", chair_name: str = "", limit: int = 30, @@ -8043,12 +8090,15 @@ async def search_precedent_library_lexical( return [] pool = await get_pool() + sk_clause = _source_kind_clause(source_kind, "cl.") halacha_filters = [ "h.review_status <> 'rejected'", # #153: include background; rank verified higher - f"cl.source_kind = '{source_kind}'", "cl.searchable = true", ] - chunk_filters = [f"cl.source_kind = '{source_kind}'", "cl.searchable = true"] + chunk_filters = ["cl.searchable = true"] + if sk_clause: + halacha_filters.append(sk_clause) + chunk_filters.append(sk_clause) # $1 = query, $2 = limit. Filters append starting at $3. h_params: list = [query, limit] c_params: list = [query, limit] diff --git a/mcp-server/src/legal_mcp/services/hybrid_search.py b/mcp-server/src/legal_mcp/services/hybrid_search.py index 7fb306d..001bf8b 100644 --- a/mcp-server/src/legal_mcp/services/hybrid_search.py +++ b/mcp-server/src/legal_mcp/services/hybrid_search.py @@ -98,15 +98,16 @@ async def search_precedent_library_hybrid( is_binding: bool | None = None, subject_tag: str = "", include_halachot: bool = True, - source_kind: str = "external_upload", + source_kind: str = "", district: str = "", chair_name: str = "", max_per_case_law: int = 2, ) -> list[dict]: """Hybrid wrapper for precedent-library search. - source_kind='external_upload' → court rulings (default) - source_kind='internal_committee' → appeals-committee decisions + source_kind='' → the whole corpus (default, #232) + source_kind='external_upload' → court rulings only + source_kind='internal_committee' → appeals-committee decisions only max_per_case_law: MMR-style diversity cap — at most N hits per case_law_id in the final ranked list (default 2). Prevents a single precedent from monopolizing the result list when many of diff --git a/mcp-server/src/legal_mcp/services/precedent_library.py b/mcp-server/src/legal_mcp/services/precedent_library.py index 0cab8d8..e73f19a 100644 --- a/mcp-server/src/legal_mcp/services/precedent_library.py +++ b/mcp-server/src/legal_mcp/services/precedent_library.py @@ -477,7 +477,7 @@ async def list_precedents( precedent_level: str = "", source_type: str = "", search: str = "", - source_kind: str = "external_upload", + source_kind: str = "", limit: int = 100, offset: int = 0, ) -> list[dict]: @@ -503,12 +503,18 @@ async def search_library( subject_tag: str = "", limit: int = 10, include_halachot: bool = True, + source_kind: str = "", ) -> list[dict]: """Semantic search merging halachot (rule-level) and chunks (passage-level). Only ``approved`` / ``published`` halachot are returned, per chair-review policy. Chunks are returned regardless of halacha review status. + ``source_kind=""`` (default) covers the whole corpus — court rulings and + appeals-committee decisions together. It used to be hard-wired to + ``external_upload`` here, which made 104 committee decisions + unreachable through this entry point (#232). + When ``VOYAGE_RERANK_ENABLED`` is set, results are passed through voyage rerank-2 (cross-encoder). The +0.05 halacha boost from ``search_precedent_library_semantic`` is preserved before rerank @@ -529,4 +535,5 @@ async def search_library( is_binding=is_binding, subject_tag=subject_tag, include_halachot=include_halachot, + source_kind=source_kind, ) diff --git a/mcp-server/src/legal_mcp/tools/precedent_library.py b/mcp-server/src/legal_mcp/tools/precedent_library.py index 29e2c22..5ecd745 100644 --- a/mcp-server/src/legal_mcp/tools/precedent_library.py +++ b/mcp-server/src/legal_mcp/tools/precedent_library.py @@ -96,10 +96,13 @@ async def precedent_library_list( precedent_level: str = "", source_type: str = "", search: str = "", - source_kind: str = "external_upload", + source_kind: str = "", limit: int = 100, ) -> str: - """רשימה של פסיקה בקורפוס הסמכותי, עם פילטרים.""" + """רשימה של פסיקה בקורפוס הסמכותי, עם פילטרים. + + source_kind ריק (ברירת מחדל) = כל הקורפוס, כולל החלטות ועדות ערר. + """ rows = await precedent_library.list_precedents( practice_area=practice_area, court=court, @@ -266,8 +269,9 @@ async def search_precedent_library( subject_tag: str = "", limit: int = 10, include_halachot: bool = True, + source_kind: str = "", ) -> str: - """חיפוש סמנטי בקורפוס הפסיקה הסמכותית. + """חיפוש סמנטי בקורפוס הפסיקה הסמכותית — פסקי דין **והחלטות ועדות ערר**. מחזיר תוצאות מעורבות: הלכות (rule-level, מאושרות בלבד) + קטעי טקסט (passage-level). הלכות מקבלות boost קל בדירוג כי הן מזוקקות מראש. @@ -282,6 +286,9 @@ async def search_precedent_library( subject_tag: סינון לפי תגית נושא (לדוגמה "מועד_קביעת_שומה"). limit: מספר תוצאות מקסימלי. include_halachot: האם לכלול הלכות (ברירת מחדל: כן). + source_kind: ריק (ברירת מחדל) = כל הקורפוס — פסקי דין והחלטות ועדות + ערר יחד. "external_upload" = פסקי בתי משפט בלבד; + "internal_committee" = החלטות ועדות ערר בלבד. Returns: רשימה מדורגת. כל פריט הוא {"type": "halacha"|"passage", "score", ...}. """ @@ -299,6 +306,7 @@ async def search_precedent_library( subject_tag=subject_tag, limit=limit, include_halachot=include_halachot, + source_kind=source_kind, ) # X11 Phase 2 (#154): attach the incoming-citation authority breakdown so the # research agent can WEIGH and ARGUE authority ("הלכה שאומצה ב-N החלטות ועדת-ערר") diff --git a/mcp-server/tests/test_source_kind_selector.py b/mcp-server/tests/test_source_kind_selector.py new file mode 100644 index 0000000..3f6f3a5 --- /dev/null +++ b/mcp-server/tests/test_source_kind_selector.py @@ -0,0 +1,67 @@ +"""#232 — the source_kind selector must not silently hide a corpus. + +`search_precedent_library` defaulted to source_kind='external_upload' all the +way down the stack, so 104 appeals-committee decisions (29% of the corpus) +were unreachable through the entry point the writing agents actually call. +These tests pin the selector semantics; the end-to-end retrieval check lives +in scripts/test_retrieval_by_name.py (needs a live DB). +""" + +import pytest + +from legal_mcp.services.db import _source_kind_clause + + +def test_empty_selector_means_no_filter(): + """'' and 'all' must produce no predicate — the whole corpus.""" + assert _source_kind_clause("") == "" + assert _source_kind_clause("all") == "" + assert _source_kind_clause(" ") == "" + + +def test_named_kinds_produce_equality_predicate(): + assert _source_kind_clause("internal_committee", "cl.") == ( + "cl.source_kind = 'internal_committee'" + ) + assert _source_kind_clause("external_upload") == "source_kind = 'external_upload'" + + +def test_all_committees_expands_to_both_shapes(): + """Committee decisions live under two historical shapes — cover both.""" + clause = _source_kind_clause("all_committees", "cl.") + assert "cl.source_kind = 'internal_committee'" in clause + assert "cl.source_type = 'appeals_committee'" in clause + assert clause.startswith("(") and clause.endswith(")") + + +def test_unknown_selector_raises_rather_than_reaching_sql(): + """The value is f-string-interpolated, so the whitelist is the guard.""" + with pytest.raises(ValueError, match="source_kind"): + _source_kind_clause("'; DROP TABLE case_law; --") + with pytest.raises(ValueError): + _source_kind_clause("internal") + + +def test_default_of_the_search_entry_points_is_whole_corpus(): + """A regression guard on the defaults themselves — this is the bug.""" + import inspect + + from legal_mcp.services import hybrid_search, precedent_library + from legal_mcp.services import db as db_mod + from legal_mcp.tools import precedent_library as plib_tool + + for fn in ( + db_mod.search_precedent_library_semantic, + db_mod.search_precedent_library_lexical, + db_mod.list_external_case_law, + hybrid_search.search_precedent_library_hybrid, + precedent_library.search_library, + precedent_library.list_precedents, + plib_tool.search_precedent_library, + plib_tool.precedent_library_list, + ): + default = inspect.signature(fn).parameters["source_kind"].default + assert default == "", ( + f"{fn.__module__}.{fn.__qualname__} defaults source_kind to " + f"{default!r} — that hides a corpus from every caller (#232)" + ) diff --git a/web/app.py b/web/app.py index 1f31920..2c24d49 100644 --- a/web/app.py +++ b/web/app.py @@ -6992,7 +6992,7 @@ async def precedent_library_list( precedent_level: str = "", source_type: str = "", search: str = "", - source_kind: str = "external_upload", + source_kind: str = "", limit: int = 100, offset: int = 0, ): @@ -7020,6 +7020,7 @@ async def precedent_library_search( subject_tag: str = "", limit: int = 10, include_halachot: bool = True, + source_kind: str = "", ): if not q or len(q.strip()) < 2: return {"items": [], "count": 0} @@ -7032,6 +7033,7 @@ async def precedent_library_search( subject_tag=subject_tag, limit=limit, include_halachot=include_halachot, + source_kind=source_kind, ) return {"items": results, "count": len(results)}