Chaim
357a5238c4
Design for unifying the two parallel ingest paths (ingest_precedent / ingest_internal_decision) into one canonical pipeline parameterized by an IntakeSpec config object — Template Method skeleton + Strategy injection. Closes the GAP-02 root cause (missing metadata queue on internal path) by making a skipped step structurally impossible. Architecture choice verified against 3+ authoritative sources (refactoring.guru Template-Method/Replace-Conditional, Fowler FlagArgument, Strategy pattern). DB check (2026-05-30): no migration needed — 0/56 internal rows lack metadata, 0 invalid enums; multimodal backfill (42 rows) tracked as TaskMaster #61.2 / FU-3. Covers GAP-01/02/04/05 · provides INV-ING1/ING3/G2/G4 · TaskMaster #59. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
142 lines
11 KiB
Markdown
142 lines
11 KiB
Markdown
# FU-1 — איחוד מסלול-הקליטה (Unified Ingest Path) — עיצוב
|
||
|
||
**סטטוס:** מאושר-לעיצוב · **תאריך:** 2026-05-30 · **ענף:** TBD (ייפתח בביצוע)
|
||
**מכסה:** GAP-01, GAP-02, GAP-04, GAP-05 · **מספק:** INV-ING1, INV-ING3, INV-G2, INV-G4
|
||
**מקורות:** [docs/spec/01-ingest.md](../../spec/01-ingest.md), [docs/spec/gap-audit.md](../../spec/gap-audit.md) · **משימה:** TaskMaster #59 (legal-ai)
|
||
**סוג-עבודה:** pure-code · **מיגרציה:** אין (אומת מול DB 2026-05-30 — ראה §6)
|
||
|
||
---
|
||
|
||
## 1. הבעיה
|
||
|
||
שתי פונקציות-קליטה מקבילות לישויות-אחיות, שמשכפלות את צעדי 2–10 של הפייפליין ומתפצלות
|
||
בפרטים:
|
||
|
||
- `services/precedent_library.py::ingest_precedent` (פסיקה חיצונית, `source_kind='external_upload'`)
|
||
- `services/internal_decisions.py::ingest_internal_decision` (החלטות-ועדה, `source_kind='internal_committee'`)
|
||
|
||
מסלולים מקבילים גוררים drift — צעד שקיים באחד וחסר באחר. הביטוי הקונקרטי: **GAP-02** —
|
||
המסלול הפנימי מתזמן רק `request_halacha_extraction` ולא `request_metadata_extraction`,
|
||
ולכן החלטות-ועדה נקלטו בלי metadata. שש אסימטריות נוספות (GAP-01/04/05) מתועדות ב-01-ingest §4.
|
||
|
||
## 2. ההכרעה האדריכלית (מאומתת)
|
||
|
||
**Template Method skeleton + Strategy via config object.** פונקציה קנונית אחת מריצה את
|
||
שלד-הפייפליין (סדר-צעדים אכיף); כל מה שמשתנה לפי סוג נישא ב-config object מוזרק (`IntakeSpec`).
|
||
שתי הפונקציות הציבוריות נשמרות כ-API בעל-שם ומאצילות לליבה.
|
||
|
||
| החלטה | נימוק | מקורות (≥3) |
|
||
|-------|--------|-------------|
|
||
| מסלול קנוני יחיד (לא 2 מקבילים, לא ליבה-משותפת בין 2 כניסות) | Template Method: "אלגוריתמים כמעט-זהים עם הבדלים קטנים" → שלד אחד אוכף סדר-צעדים, צעד-חסר נעשה בלתי-אפשרי | refactoring.guru (Template Method); SourceMaking (Strategy); ADF parameterized-pipelines |
|
||
| שמירת `ingest_precedent`/`ingest_internal_decision` כ-API ציבורי | Fowler FlagArgument: "separate methods communicate more clearly"; ליבה משותפת מוסתרת כשהלוגיקה שזורה | martinfowler.com/bliki/FlagArgument; ardalis; luzkan smells |
|
||
| ווריאציה ב-config object (`IntakeSpec`), לא boolean-flags | flag-argument הוא code smell; config object נותן בהירות + הרחבה | Fowler; ardalis; dev.to flag-anti-pattern |
|
||
| `validate` כ-callable, `enum_fields` כ-data | callable להטרוגני (Strategy idiomatic ב-Python first-class), data להומוגני ("אל תכפה הכל ל-strategy") | Strategy/Wikipedia; Functional-Strategy dev.to; Strategy-in-Python Medium |
|
||
| `create_record` כ-callable מוזרק, לא `if source_kind` | Replace Conditional with Polymorphism + factory injection ("tell, don't ask") | refactoring.guru (Replace-Conditional); code-maze (Factory+DI); c-sharpcorner |
|
||
|
||
## 3. מבנה מודולים
|
||
|
||
**מודול חדש:** `mcp-server/src/legal_mcp/services/ingest.py`
|
||
|
||
```
|
||
services/ingest.py ← חדש (בית המסלול הקנוני)
|
||
├── IntakeSpec (frozen dataclass — מתאר-הסוג)
|
||
├── async ingest_document(spec, *, file_path|text, inputs, progress) ← ליבה: צעדים 1–10
|
||
├── _stage_file(src, root, subdir) (אחיד — מאוחד משני הקבצים)
|
||
├── _coerce_date / _safe_filename (אחיד — היום משוכפל)
|
||
└── _embed_pages(case_law_id, pdf, n) (עובר מ-precedent_library.py — צעד 7 אחיד)
|
||
```
|
||
|
||
**API ציבורי — חתימה ללא שינוי לקוראים:**
|
||
- `precedent_library.py::ingest_precedent(...)` → בונה `_EXTERNAL_SPEC`, קורא `ingest.ingest_document(...)`.
|
||
- `internal_decisions.py::ingest_internal_decision(...)` → בונה `_INTERNAL_SPEC`, קורא `ingest.ingest_document(...)`.
|
||
|
||
**לא זז (גבול FU-2):** `db.create_external_case_law` / `db.create_internal_committee_decision`
|
||
נשארות נפרדות; מנותבות דרך `IntakeSpec.create_record`. כל שאר הפונקציות בשני קבצי-השירות
|
||
(search_*, migrate_*, reextract_*, process_pending_extractions, enrich_*) **לא נוגעים בהן**.
|
||
|
||
**הקוראים שלא משתנים:** MCP tools (`tools/precedent_library.py`, `tools/internal_decisions.py`)
|
||
וה-HTTP API ב-`web/` ממשיכים לקרוא לאותן שתי פונקציות ציבוריות.
|
||
|
||
## 4. ה-IntakeSpec
|
||
|
||
```python
|
||
@dataclass(frozen=True)
|
||
class IntakeSpec:
|
||
source_kind: str # 'external_upload' | 'internal_committee'
|
||
id_field: str # 'citation' | 'case_number' (לוג/שגיאות)
|
||
staging_root: Path # PRECEDENT_LIBRARY_DIR | INTERNAL_DECISIONS_DIR
|
||
staging_subdir: Callable[[dict], str] # inputs → subdir (source_type | district | 'other')
|
||
validate: Callable[[dict], None] # מרים ValueError (citation-guard / chair_name-חובה)
|
||
enum_fields: dict[str, frozenset[str]] # נאכף לשני הסוגים (GAP-04)
|
||
derive: Callable[[dict], dict] # שדות-נגזרים (district, proceeding_type); identity לחיצוני
|
||
display_name_fallback: str # שם-השדה כשחסר case_name ('citation'|'case_number')
|
||
create_record: Callable[..., Awaitable[dict]] # create_external_case_law | create_internal_committee_decision
|
||
```
|
||
|
||
הליבה `ingest_document` **לא יודעת** איזה סוג רץ — רק מפעילה את ה-hooks בנקודות מוגדרות.
|
||
|
||
## 5. הפייפליין הקנוני (צעדים 1–10, לפי 01-ingest §2)
|
||
|
||
סדר-הביצוע בפועל (ה-DB-create מוקדם — נדרש `case_law_id` לפני אחסון chunks; תואם את הקוד הקיים):
|
||
|
||
| # | צעד | אחיד? | מקור-וריאציה |
|
||
|---|------|-------|---------------|
|
||
| 1 | ולידציית-קלט + enums | מנגנון אחיד | `spec.validate` + `spec.enum_fields` |
|
||
| 2 | גזירת-שדות | מנגנון אחיד | `spec.derive` (identity לחיצוני) |
|
||
| 3 | Stage file | מנגנון אחיד | `spec.staging_root` + `spec.staging_subdir` |
|
||
| 4 | Extract text (טקסט-ריק = כשל מדווח) | ✅ מלא | — (internal גם מקבל `text` ישיר, בלי קובץ) |
|
||
| 5 | Strip Nevo preamble | ✅ מלא | — |
|
||
| 6 | **DB create → `case_law_id`** (ספציפי-לסוג) | מנותב | `spec.create_record` (+ `display_name_fallback`) |
|
||
| 7 | Chunk (hierarchical/flat לפי `PARENT_DOC_RETRIEVAL_ENABLED`) | ✅ מלא | — (flag, לא סוג) |
|
||
| 8 | Embed children + Store chunks | ✅ מלא | — |
|
||
| 9 | **Multimodal page-image embed** (flag+PDF+page_count>0) | ✅ מלא | — (**GAP-05 fix**: היה רק בחיצוני) |
|
||
| 10 | **Queue metadata extraction** | ✅ מלא | — (**GAP-02 fix**: היה רק בחיצוני) |
|
||
| 11 | Queue halacha extraction | ✅ מלא | — |
|
||
| 12 | Set statuses (extraction=completed, halacha=pending) | ✅ מלא | — |
|
||
|
||
> הערה: 01-ingest §2 ממספר 1–10 בלי למנות מפורשות את ה-DB-create; כאן הוא צעד 6 כי הוא קודם
|
||
> ל-chunking בקוד בפועל. שגיאת-עיבוד אחרי create → `extraction_status=failed` (כמו היום).
|
||
|
||
**אילוץ `claude_session`:** הליבה רק **מתזמנת** (`request_*_extraction` — כתיבת-DB טהורה).
|
||
אין import של `halacha_extractor`/`precedent_metadata_extractor` במסלול-הקליטה — נשמר כפי שהיום.
|
||
|
||
## 6. שינויי-התנהגות וסיכון
|
||
|
||
| שינוי | השפעה | סיכון |
|
||
|--------|--------|--------|
|
||
| **GAP-02**: internal עכשיו מתזמן metadata | החלטות-ועדה חדשות יקבלו headnote/summary/tags | אין — תיקון; רשומות קיימות כבר מלאות (0/56 חסר) |
|
||
| **GAP-04**: ולידציית-enums על internal | קלט עם practice_area לא-חוקי יידחה בקליטה | נמוך — כל 56 הקיימות חוקיות; בודקים שקוראי-internal מעבירים ערכים חוקיים |
|
||
| **GAP-05 multimodal**: internal PDF עכשיו מטמיע עמודים | החלטות-ועדה חדשות PDF יקבלו page-images | אין (non-fatal); קיימות → backfill ב-**TaskMaster 61.2 (FU-3)** |
|
||
| **GAP-05 fallback/staging/derive/guard**: מאוחדים | התנהגות זהה, מסלול אחד | אין — citation-guard נשמר ב-`_EXTERNAL_SPEC.validate` |
|
||
|
||
**אין מיגרציה (אומת מול DB 2026-05-30):** internal_committee = 56 רשומות; metadata חסר = **0**;
|
||
enums לא-חוקיים = **0**; multimodal: 14/56 יש (42 חסר → FU-3 #61.2). הריפקטור משנה רק התנהגות
|
||
*קדימה*; אינו נוגע בנתונים שמורים.
|
||
|
||
## 7. אסטרטגיית בדיקה
|
||
|
||
pytest offline עם monkeypatch לכל גבולות-ה-I/O (db, embeddings, chunker, extractor) — כתבנית
|
||
[tests/test_search_domain_scope.py](../../../mcp-server/tests/test_search_domain_scope.py)
|
||
ו-[tests/test_precedent_corpus_isolation.py](../../../mcp-server/tests/test_precedent_corpus_isolation.py).
|
||
קובץ חדש: `mcp-server/tests/test_unified_ingest.py`. רץ עם `.venv` המקומי.
|
||
|
||
מקרי-בדיקה (TDD — נכשלים לפני, עוברים אחרי):
|
||
1. **regression GAP-02** — `ingest_internal_decision` מתזמן גם metadata **וגם** halacha (לוכד את הבאג המקורי).
|
||
2. שני הסוגים זורמים דרך `ingest.ingest_document` (לא דרך גוף-קוד נפרד).
|
||
3. ולידציית-enum דוחה `practice_area` לא-חוקי בשני הסוגים (GAP-04).
|
||
4. citation-guard עדיין חוסם ציטוט `ערר`/`בל"מ` במסלול החיצוני.
|
||
5. staging-subdir נפתר נכון (source_type לחיצוני, district לפנימי, 'other' ל-fallback).
|
||
6. מסלול-`text` (פנימי, בלי קובץ) ומסלול-`file_path` שניהם עובדים.
|
||
7. multimodal מותנה flag+PDF+page_count — **לא** בסוג-ה-intake; PDF פנימי → מטמיע, text → לא.
|
||
8. fallback לשם-תצוגה: חסר case_name → נופל למזהה הקנוני הנכון לכל סוג.
|
||
9. אידמפוטנטיות-חתימה: ערכי-החזרה של שתי הפונקציות הציבוריות נשמרים (תאימות-קוראים).
|
||
|
||
## 8. סדר-ביצוע
|
||
|
||
1. כתיבת `test_unified_ingest.py` (אדום).
|
||
2. `services/ingest.py` — `IntakeSpec` + `ingest_document` + הזזת `_embed_pages` + helpers אחידים.
|
||
3. `_EXTERNAL_SPEC` + צמצום `ingest_precedent` ל-wrapper.
|
||
4. `_INTERNAL_SPEC` + צמצום `ingest_internal_decision` ל-wrapper.
|
||
5. הרצת הבדיקות (ירוק) + lint.
|
||
6. בדיקת-עשן: import של שני קבצי-השירות + ה-MCP tools (ללא שבירת חתימות).
|