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)}