Merge pull request 'fix(precedents): חילוץ-מטא-דאטה ממלא תחום (practice_area) ושם-יו"ר לכל החלטת-ועדה' (#288) from worktree-halacha-metadata-fixes into main

mcp-server/src/legal_mcp/services/precedent_metadata_extractor.py

@@ -27,6 +27,10 @@ from uuid import UUID
 
 from legal_mcp.config import parse_llm_json
 from legal_mcp.services import db, gemini_session
+from legal_mcp.services.practice_area import (
+    DOMAIN_PRACTICE_AREAS,
+    derive_domain_practice_area,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -58,6 +62,7 @@ METADATA_EXTRACTION_PROMPT = """אתה מסייע משפטי בכיר. קרא א
 {
   "case_name_short": "שם קצר ל-3-6 מילים (למשל 'אהרון ברק' או 'ב. קרן-נכסים'). אל תכלול מספר תיק. שם המבקש/העורר העיקרי. אם זו החלטה מאוחדת — שם הצד המוביל.",
   "appeal_subtype": "תת-סוג ספציפי בתוך תחום המשפט (למשל 'תכנית רחביה', 'מימוש במכר', 'תמ\\"א 38', 'שימוש חורג', 'סופיות ההחלטה'). מילה אחת או צירוף קצר.",
+  "practice_area": "תחום-העל המשפטי — אחד מ-3 בלבד: 'rishuy_uvniya' (רישוי ובנייה / היתרי בנייה / שימוש חורג / הקלות / תכנון), 'betterment_levy' (היטל השבחה — חיוב בעל מקרקעין בגין עליית-שווי מאישור תכנית), 'compensation_197' (פיצויים לפי סעיף 197 לחוק התכנון והבנייה — פגיעה במקרקעין ע\\\"י תכנית). קבע לפי מהות הסכסוך כפי שהוא עולה מהטקסט. אם לא ברור לאיזה מהשלושה — מחרוזת ריקה (אל תנחש).",
   "summary": "תקציר עניני 2-3 משפטים: מה הייתה השאלה, מה הוכרע. בלי שיפוט.",
   "headnote": "headnote בסגנון נבו: 1-2 משפטים שמסכמים את העיקרון שנקבע/יושם בפסק. למשל 'תכנית רחביה — היטל השבחה במימוש במכר — אין לחייב כשהזכויות צפות'.",
   "key_quote": "ציטוט מילולי בודד, 30-100 מילים, שמייצג את לב הפסק. חייב להופיע מילה במילה בטקסט. אם אין ציטוט מתאים — מחרוזת ריקה.",
@@ -68,7 +73,7 @@ METADATA_EXTRACTION_PROMPT = """אתה מסייע משפטי בכיר. קרא א
   "proceeding_type": "אחד מ-2 (רק להחלטות ועדת ערר): 'ערר' (הליך ערר עיקרי על החלטת ועדה מקומית) / 'בל\\\"מ' (בקשה להארכת מועד להגשת ערר). זהה דרך כותרת המסמך: 'ערר (ועדות ערר ...) NNNN/YY' → 'ערר'; 'בל\\\"מ NNNN/YY' או נושא 'בקשה להארכת מועד להגשת ערר' → 'בל\\\"מ'. בפסיקת בית משפט (לא ועדת ערר) — מחרוזת ריקה.",
   "court": "שם הערכאה כפי שהוא מופיע בכותרת (למשל 'בית המשפט העליון', 'בית המשפט המחוזי בירושלים בשבתו כבית משפט לעניינים מנהליים', 'ועדת הערר לתכנון ובניה פיצויים והיטלי השבחה — מחוז ירושלים'). מחרוזת ריקה אם לא ניתן לזהות.",
   "case_number_clean": "מספר הערר/תיק כפי שמופיע בכותרת — רק הספרות והאלכסון, למשל '1062/24' או '8031/21'. ללא המילה 'ערר', ללא שם הצדדים, ללא סוגריים. אם יש כמה עררים מאוחדים — הרשום הראשון. מחרוזת ריקה אם לא ניתן לזהות.",
-  "chair_name": "שם יו\\\"ר ההרכב — רלוונטי **רק להחלטות ועדת ערר**, לא לפסקי בית משפט. חפש בכותרת/חתימה: 'עו\\\"ד דפנה תמיר, יו\\\"ר ועדת הערר', 'בפני: עו\\\"ד פלוני אלמוני (יו\\\"ר)'. השאר שם פרטי+משפחה בלי תוארים ('עו\\\"ד', 'אדריכל'). אם זה פסק דין של בית משפט — מחרוזת ריקה.",
+  "chair_name": "שם יו\\\"ר ההרכב של **ההחלטה הזו** — רלוונטי **רק להחלטות ועדת ערר**, לא לפסקי בית משפט. כמעט תמיד מופיע — בשני מקומות: (א) בכותרת/רובריקה בראש המסמך, ליד 'בפני:' / 'בהרכב:' / רשימת חברי הוועדה; (ב) בבלוק-החתימה בסוף ההחלטה, אחרי 'ההחלטה ניתנה' — שם מופיעים זה-לצד-זה מזכיר/ת הוועדה והיו\\\"ר (למשל בשתי עמודות: בצד אחד 'פלוני, עו\\\"ד / מזכיר ועדת הערר' ובצד השני 'אלמוני, עו\\\"ד / יו\\\"ר ועדת הערר'). **קח את השם שמעליו/לצדו כתוב 'יו\\\"ר' — לא את המזכיר/ה.** השאר שם פרטי+משפחה בלבד, בלי תוארים ('עו\\\"ד', 'אדריכל', 'עו\\\"ד דפנה תמיר'→'דפנה תמיר'). **אזהרה קריטית:** אל תיקח שם יו\\\"ר של פסק/החלטה אחרים ש**מצוטטים** בגוף ההחלטה (למשל 'כפי שנקבע ברשותה של יו\\\"ר פלונית בערר אחר...') — אלה תקדימים מצוטטים, לא היו\\\"ר של ההחלטה הנוכחית. אם זה פסק דין של בית משפט — מחרוזת ריקה.",
   "district": "מחוז ועדת הערר — רלוונטי **רק להחלטות ועדת ערר**. ערכים מותרים: 'ירושלים', 'תל אביב', 'מרכז', 'חיפה', 'צפון', 'דרום', 'ארצית'. זהה מהכותרת ('ועדת הערר לתכנון ובניה — מחוז ירושלים' → 'ירושלים'; 'ועדות ערר - תכנון ובנייה תל אביב-יפו' → 'תל אביב'). אם זה פסק דין של בית משפט — מחרוזת ריקה.",
   "parties": "שמות הצדדים בשורה אחת בצורה 'עורר נ\\' משיב' — בדיוק כפי שמופיעים בכותרת/רובריקה. בלי הדגשה, בלי מספר-תיק, בלי תוארים מיותרים. למשל 'ישיבת חברת אהבת שלום נ\\' תאיה' או 'ראם חיים נ\\' הוועדה המקומית לתכנון ובניה ירושלים'. אם הצדדים אינם מופיעים בטקסט (למשל החלטה שמתחילה בגוף בלי רובריקה) — מחרוזת ריקה. **אל תמציא שמות.**",
   "citation_prefix": "קידומת-ההליך של פסיקת בית-משפט בלבד, כפי שמופיעה בראש הכותרת: ע\\"א / רע\\"א / בג\\"ץ / עע\\"מ / עת\\"מ / ע\\"פ / דנ\\"א / ת\\"א וכד'. **רק לפסקי בית-משפט (עליון/מנהלי)** — להחלטות ועדת-ערר השאר ריק (הקוד גוזר 'ערר'/'בל\\"מ' מעצמו). אם לא ברור — מחרוזת ריקה."
@@ -169,6 +174,16 @@ async def extract_metadata(case_law_id: UUID | str) -> dict:
         out["case_name_short"] = result["case_name_short"].strip()
     if isinstance(result.get("appeal_subtype"), str):
         out["appeal_subtype"] = result["appeal_subtype"].strip()
+    if isinstance(result.get("practice_area"), str):
+        # Closed domain enum (axis B). Anything else (incl. the legacy
+        # multi-tenant 'appeals_committee' value or free text) is dropped so a
+        # slip can't write an unrenderable value into the radio facet — the
+        # deterministic case_number-prefix derivation in apply_to_record is the
+        # authoritative source anyway; this is the content fallback for court
+        # rulings whose docket prefix doesn't encode the domain.
+        pa = result["practice_area"].strip()
+        if pa in DOMAIN_PRACTICE_AREAS:
+            out["practice_area"] = pa
     if isinstance(result.get("summary"), str):
         out["summary"] = result["summary"].strip()
     if isinstance(result.get("headnote"), str):
@@ -380,6 +395,25 @@ async def apply_to_record(
         else:
             fields_to_update["case_number"] = cn_clean
 
+    # practice_area — the domain facet (axis B) that drives the /precedents radio
+    # and search filters. The LLM never set it historically (it was passed in as
+    # read-only context), so committee/court uploads that left it blank stayed
+    # blank forever. Fill when empty, preferring the DETERMINISTIC case_number
+    # prefix (1xxx→rishuy, 8xxx→היטל, 9xxx→197 — authoritative for ועדת-ערר
+    # dockets, INV-AH rule-based) and falling back to the LLM's content
+    # classification for court rulings whose docket prefix doesn't encode a
+    # domain. Built off the EFFECTIVE case_number so a same-run normalization is
+    # seen. Abstains (no write) when neither yields a domain value.
+    if not (record.get("practice_area") or "").strip():
+        eff_cn = (
+            fields_to_update.get("case_number") or record.get("case_number") or ""
+        )
+        pa = derive_domain_practice_area(eff_cn) or (
+            suggested.get("practice_area") or ""
+        ).strip()
+        if pa in DOMAIN_PRACTICE_AREAS:
+            fields_to_update["practice_area"] = pa
+
     # parties — store the extracted "עורר נ' משיב" line (the re-derivable basis for
     # the deterministic citation). Only fill when empty; chair edits are preserved.
     if not (record.get("parties") or "").strip():
@@ -387,11 +421,30 @@ async def apply_to_record(
         if p:
             fields_to_update["parties"] = p
 
-    # chair_name / district — only for internal_committee rows. The DB CHECK
+    # chair_name / district — for ANY ועדת-ערר decision, regardless of how it
-    # forces these to be non-empty, so the upload endpoint stamps the row
+    # entered the corpus. Previously gated on source_kind=='internal_committee',
-    # with "(טרם חולץ)" as a placeholder. Treat that placeholder as empty
+    # which silently skipped committee decisions uploaded via the EXTERNAL
-    # so the LLM-extracted value can overwrite it.
+    # precedent path (source_kind='external_upload', source_type='appeals_committee'
-    if record.get("source_kind") == "internal_committee":
+    # — e.g. another district's decision pulled from נבו): the chair sat in the
+    # signature block but was never extracted. The CHECK only forces non-empty for
+    # internal_committee, so writing a chair onto an external_upload row is safe;
+    # for internal rows the upload endpoint stamps "(טרם חולץ)" which we treat as
+    # empty. The LLM prompt already abstains (empty) for court rulings, so this is
+    # additionally gated on the decision actually being a committee one — never a
+    # court ruling. Derive "is committee" from the effective source_type/level so a
+    # same-run fill is seen.
+    eff_st_chair = (
+        fields_to_update.get("source_type") or record.get("source_type") or ""
+    ).strip()
+    eff_lvl_chair = (
+        fields_to_update.get("precedent_level") or record.get("precedent_level") or ""
+    ).strip()
+    is_committee_decision = (
+        record.get("source_kind") == "internal_committee"
+        or eff_st_chair == "appeals_committee"
+        or eff_lvl_chair.startswith("ועדת_ערר")
+    )
+    if is_committee_decision:
         cur_chair = (record.get("chair_name") or "").strip()
         if cur_chair in ("", PLACEHOLDER_PENDING_EXTRACTION):
             s = (suggested.get("chair_name") or "").strip()

mcp-server/tests/test_metadata_extract_chair_practice_area.py

@@ -0,0 +1,165 @@
+"""Regression tests for two metadata-merge gaps surfaced from /precedents:
+
+1. chair_name was filled ONLY for source_kind='internal_committee', so ועדת-ערר
+   decisions uploaded via the EXTERNAL precedent path (source_kind='external_upload',
+   source_type='appeals_committee' — e.g. 1132-09-24, a Tel-Aviv decision pulled
+   from נבו) never got their chair extracted even though it sits in the signature.
+
+2. practice_area (the /precedents radio facet) was never set by extraction — it was
+   passed to the LLM as read-only context only. Committee/court uploads that left it
+   blank stayed blank, so the radio rendered nothing selected. It is now derived
+   deterministically from the case_number prefix (authoritative for ועדת-ערר dockets)
+   with the LLM's content classification as the fallback for court dockets whose
+   prefix doesn't encode a domain.
+
+Runs fully OFFLINE — monkeypatches the ``db`` calls ``apply_to_record`` makes.
+"""
+
+from __future__ import annotations
+
+import asyncio
+from uuid import uuid4
+
+import pytest
+
+from legal_mcp.services import db, precedent_metadata_extractor as pme
+
+
+def _run(coro):
+    loop = asyncio.new_event_loop()
+    try:
+        return loop.run_until_complete(coro)
+    finally:
+        loop.close()
+
+
+def _wire_db(monkeypatch, record: dict) -> dict:
+    """Stub the db calls apply_to_record makes; return a dict that captures the
+    kwargs passed to update_case_law."""
+    captured: dict = {}
+
+    async def _get(_cid):
+        return dict(record)
+
+    async def _update(_cid, **fields):
+        captured.update(fields)
+        return {**record, **fields}
+
+    async def _collides(_cn, _cid):
+        return False
+
+    monkeypatch.setattr(db, "get_case_law", _get)
+    monkeypatch.setattr(db, "update_case_law", _update)
+    monkeypatch.setattr(db, "case_number_collides", _collides)
+    # citation_formatted is pre-set in every fixture below, so the deterministic
+    # formatter is never reached — stub defensively anyway.
+    monkeypatch.setattr(db, "format_precedent_citation", lambda *a, **k: "")
+    return captured
+
+
+def test_external_committee_decision_gets_chair_name(monkeypatch):
+    """source_kind=external_upload + source_type=appeals_committee → chair filled."""
+    record = {
+        "source_kind": "external_upload",
+        "source_type": "appeals_committee",
+        "case_number": "1132-09-24",
+        "chair_name": "",
+        "district": "תל אביב",
+        "practice_area": "rishuy_uvniya",
+        "citation_formatted": "ערר ... 1132-09-24",
+    }
+    captured = _wire_db(monkeypatch, record)
+    suggested = {"chair_name": "מיכל דגני הלברשטם", "district": "תל אביב"}
+    out = _run(pme.apply_to_record(uuid4(), suggested))
+    assert out["updated"] is True
+    assert captured.get("chair_name") == "מיכל דגני הלברשטם"
+
+
+def test_court_ruling_never_gets_chair_name(monkeypatch):
+    """A court ruling is not a committee decision — chair must stay empty even if
+    the model slips and returns one."""
+    record = {
+        "source_kind": "external_upload",
+        "source_type": "court_ruling",
+        "precedent_level": "עליון",
+        "case_number": 'ע"א 4768/22',
+        "chair_name": "",
+        "district": "",
+        "practice_area": "betterment_levy",
+        "citation_formatted": 'ע"א 4768/22',
+    }
+    captured = _wire_db(monkeypatch, record)
+    suggested = {"chair_name": "פלוני אלמוני"}
+    _run(pme.apply_to_record(uuid4(), suggested))
+    assert "chair_name" not in captured
+
+
+def test_practice_area_derived_from_case_number_prefix(monkeypatch):
+    """8xxx docket → betterment_levy, deterministically, even if the LLM
+    suggested nothing (or something else)."""
+    record = {
+        "source_kind": "external_upload",
+        "source_type": "appeals_committee",
+        "case_number": "8126-03-25",
+        "chair_name": "פלונית",
+        "district": "ירושלים",
+        "practice_area": "",
+        "citation_formatted": "ערר ... 8126-03-25",
+    }
+    captured = _wire_db(monkeypatch, record)
+    out = _run(pme.apply_to_record(uuid4(), {}))
+    assert out["updated"] is True
+    assert captured.get("practice_area") == "betterment_levy"
+
+
+def test_practice_area_falls_back_to_llm_for_court_docket(monkeypatch):
+    """A Supreme-Court docket prefix (4xxx) encodes no domain → use the LLM's
+    content classification."""
+    record = {
+        "source_kind": "external_upload",
+        "source_type": "court_ruling",
+        "precedent_level": "עליון",
+        "case_number": 'ע"א 4768/22',
+        "chair_name": "",
+        "district": "",
+        "practice_area": "",
+        "citation_formatted": 'ע"א 4768/22',
+    }
+    captured = _wire_db(monkeypatch, record)
+    out = _run(pme.apply_to_record(uuid4(), {"practice_area": "betterment_levy"}))
+    assert captured.get("practice_area") == "betterment_levy"
+
+
+def test_practice_area_not_overwritten_when_present(monkeypatch):
+    """An existing practice_area (chair-set or earlier derivation) is preserved —
+    the prefix derivation only fills the blank."""
+    record = {
+        "source_kind": "external_upload",
+        "source_type": "appeals_committee",
+        "case_number": "8126-03-25",  # prefix would say betterment_levy
+        "chair_name": "פלונית",
+        "district": "ירושלים",
+        "practice_area": "compensation_197",  # but a human said 197 — keep it
+        "citation_formatted": "ערר ... 8126-03-25",
+    }
+    captured = _wire_db(monkeypatch, record)
+    _run(pme.apply_to_record(uuid4(), {"practice_area": "rishuy_uvniya"}))
+    assert "practice_area" not in captured
+
+
+def test_invalid_llm_practice_area_is_dropped(monkeypatch):
+    """The LLM returning a non-domain value (legacy 'appeals_committee' / free text)
+    must not be written — and with no usable prefix, practice_area stays blank."""
+    record = {
+        "source_kind": "external_upload",
+        "source_type": "court_ruling",
+        "precedent_level": "עליון",
+        "case_number": 'ע"א 4768/22',
+        "chair_name": "",
+        "district": "",
+        "practice_area": "",
+        "citation_formatted": 'ע"א 4768/22',
+    }
+    captured = _wire_db(monkeypatch, record)
+    _run(pme.apply_to_record(uuid4(), {"practice_area": "appeals_committee"}))
+    assert "practice_area" not in captured

web-ui/src/components/precedents/precedent-edit-sheet.tsx

@@ -73,7 +73,18 @@ export function PrecedentEditSheet({ caseLawId, onOpenChange }: Props) {
   // record arrives (including after save+refetch). Using setState during
   // render avoids the one-frame flash that useEffect would produce.
   const [syncedRecordId, setSyncedRecordId] = useState<string | null>(null);
-  if (record && record.id !== syncedRecordId) {
+  // Re-arm the sync on close so the NEXT open re-pulls the latest server record.
+  // The component is always mounted, so without this the form syncs once per
+  // precedent-id for the component's lifetime — and shows stale fields (e.g. a
+  // practice_area / chair_name that background metadata extraction filled AFTER
+  // the last open) until a full page refresh. Resetting on close makes reopening
+  // the sheet reflect the freshest record (which usePrecedent re-fetches while a
+  // row is mid-extraction). Both guards flip to false, so this render-phase
+  // setState terminates.
+  if (!open && syncedRecordId !== null) {
+    setSyncedRecordId(null);
+  }
+  if (open && record && record.id !== syncedRecordId) {
     setSyncedRecordId(record.id as string);
     setForm({
       case_number: record.case_number || "",

web-ui/src/lib/api/precedent-library.ts

@@ -295,6 +295,15 @@ export function usePrecedent(id: string | null) {
       ),
     enabled: Boolean(id),
     staleTime: 30_000,
+    /* Poll while THIS precedent is mid-extraction (text/halacha/metadata) so the
+     * detail catches up after the local MCP drainer fills fields like
+     * practice_area / chair_name — otherwise the open edit sheet shows stale
+     * empties until a hard refresh. Stops once the row settles (mirrors the
+     * list poller in usePrecedents). */
+    refetchInterval: (query) => {
+      const data = query.state.data;
+      return data && isPrecedentActive(data) ? 5000 : false;
+    },
   });
 }