diff --git a/docs/superpowers/plans/2026-05-30-fu3-reindex-on-change.md b/docs/superpowers/plans/2026-05-30-fu3-reindex-on-change.md new file mode 100644 index 0000000..49c45a8 --- /dev/null +++ b/docs/superpowers/plans/2026-05-30-fu3-reindex-on-change.md @@ -0,0 +1,421 @@ +# FU-3: Re-Index on Content Change — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Detect content changes via a SHA-256 `content_hash`, expose a standalone `reindex_case_law` that re-embeds from stored `full_text` (no re-OCR, no file needed), and surface embedding-drift in the health-check — enforcing INV-G6 where embeddings can't be DB-GENERATED. + +**Architecture:** Two additive `case_law` columns (V23): `content_hash` (hash of current full_text, written at the create boundary) and `indexed_hash` (hash the current chunks/embeddings were built from, set by `mark_indexed` after a successful store). Stale ⇔ `content_hash IS DISTINCT FROM indexed_hash`. `reindex_case_law` reuses the canonical `_chunk_embed_store` over stored text. Backfill only computes hashes (no re-embed — existing rows keep their vectors). + +**Tech Stack:** Python 3.12, asyncpg, PostgreSQL@localhost:5433, voyage embeddings API, pytest offline, `.venv` at `mcp-server/.venv`. + +**Spec:** [docs/superpowers/specs/2026-05-30-fu3-reindex-on-change-design.md](../specs/2026-05-30-fu3-reindex-on-change-design.md) + +**Run tests:** `cd ~/legal-ai/mcp-server && .venv/bin/python -m pytest tests/test_reindex_on_change.py -v` + +--- + +## File Structure + +- **Modify** `mcp-server/src/legal_mcp/services/db.py` — `_content_hash`; V23 migration; `content_hash` in `create_external_case_law`/`create_internal_committee_decision`/`create_case`; `mark_indexed`; `list_stale_case_law`; `recompute_content_hashes`. +- **Modify** `mcp-server/src/legal_mcp/services/ingest.py` — `reindex_case_law`; call `mark_indexed` after `_chunk_embed_store` in `ingest_document`. +- **Modify** `mcp-server/src/legal_mcp/services/metrics.py` — `stale_embedding_case_law` count. +- **Modify** `mcp-server/src/legal_mcp/tools/precedent_library.py` + `server.py` — MCP tool `precedent_reindex`. +- **Create** `mcp-server/tests/test_reindex_on_change.py`. + +--- + +## Task 1: Failing tests + +**Files:** Create `mcp-server/tests/test_reindex_on_change.py` + +- [ ] **Step 1: Write the failing tests** + +```python +"""FU-3: re-index on content change (offline, monkeypatched I/O).""" +from __future__ import annotations + +import asyncio +from uuid import uuid4 + +import pytest + +from legal_mcp.services import db, ingest + + +def _run(coro): + return asyncio.run(coro) + + +# ── content_hash is deterministic ────────────────────────────────────── +def test_content_hash_deterministic(): + h1 = db._content_hash("פסק דין כלשהו") + h2 = db._content_hash("פסק דין כלשהו") + assert h1 == h2 and len(h1) == 64 # sha256 hex + + +def test_content_hash_empty_is_blank(): + assert db._content_hash("") == "" + assert db._content_hash(None) == "" + + +def test_content_hash_changes_with_text(): + assert db._content_hash("alpha") != db._content_hash("beta") + + +# ── mark_indexed copies content_hash → indexed_hash ───────────────────── +def test_mark_indexed_executes_update(monkeypatch): + seen = {} + + class _Conn: + async def execute(self, q, *a): + seen["q"] = q; seen["args"] = a + async def __aenter__(self): return self + async def __aexit__(self, *a): return False + + class _Pool: + def acquire(self): return _Conn() + + async def _pool(): return _Pool() + monkeypatch.setattr(db, "get_pool", _pool) + + cid = uuid4() + _run(db.mark_indexed(cid)) + assert "indexed_hash" in seen["q"] and "content_hash" in seen["q"] + assert seen["args"][0] == cid + + +# ── reindex_case_law re-embeds from stored text, no extractor/LLM ─────── +def test_reindex_case_law_uses_stored_text(monkeypatch): + cid = uuid4() + calls = {"chunk_embed_store": [], "mark_indexed": []} + + async def _get_case_law(x): + return {"id": cid, "full_text": "טקסט שמור של ההחלטה"} + monkeypatch.setattr(ingest.db, "get_case_law", _get_case_law) + + async def _ces(case_law_id, text, page_offsets, page_count, progress): + calls["chunk_embed_store"].append((case_law_id, text)) + return 5 + monkeypatch.setattr(ingest, "_chunk_embed_store", _ces) + + async def _mark(x): + calls["mark_indexed"].append(x) + monkeypatch.setattr(ingest.db, "mark_indexed", _mark) + + out = _run(ingest.reindex_case_law(cid)) + assert out["chunks"] == 5 and out["reindexed"] is True + assert calls["chunk_embed_store"][0][1] == "טקסט שמור של ההחלטה" + assert calls["mark_indexed"] == [cid] + + +def test_reindex_case_law_missing_row_raises(monkeypatch): + async def _none(x): return None + monkeypatch.setattr(ingest.db, "get_case_law", _none) + with pytest.raises(ValueError, match="not found"): + _run(ingest.reindex_case_law(uuid4())) +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `cd ~/legal-ai/mcp-server && .venv/bin/python -m pytest tests/test_reindex_on_change.py -v` +Expected: FAIL — `AttributeError: ... no attribute '_content_hash'` / `mark_indexed` / `reindex_case_law`. + +- [ ] **Step 3: Commit** + +```bash +cd ~/legal-ai +git add mcp-server/tests/test_reindex_on_change.py +git commit -m "test(reindex): failing tests for content-hash re-index (FU-3)" +``` + +--- + +## Task 2: V23 + hash helpers + content_hash at write + +**Files:** Modify `mcp-server/src/legal_mcp/services/db.py` + +- [ ] **Step 1: Ensure `hashlib` import + add `_content_hash`** + +READ the top imports of db.py. If `import hashlib` is absent, add it. Add this helper near `_canonical_case_number` (~line 1227): + +```python +def _content_hash(text: str) -> str: + """SHA-256 hex of the text — deterministic content fingerprint (FU-3/GAP-09). + + Empty/None → "" (a row with no text has no content fingerprint). + """ + if not text: + return "" + return hashlib.sha256(text.encode("utf-8")).hexdigest() +``` + +- [ ] **Step 2: Add `SCHEMA_V23_SQL` after `SCHEMA_V22_SQL` + wire it** + +READ near `SCHEMA_V22_SQL` and `_run_schema_migrations`. Add after the V22 block: + +```python +# ── V23: case_law content/indexed hashes — re-index on content change (GAP-09) ── +# content_hash = SHA-256 of current full_text (written at the create boundary). +# indexed_hash = the content_hash the CURRENT chunks/embeddings were built from +# (set by mark_indexed after a successful store). Stale ⇔ content_hash IS +# DISTINCT FROM indexed_hash. embedding can't be a GENERATED column (needs an +# API call), so freshness is enforced by detection + reindex_case_law + health-check. +SCHEMA_V23_SQL = """ +ALTER TABLE case_law ADD COLUMN IF NOT EXISTS content_hash text NOT NULL DEFAULT ''; +ALTER TABLE case_law ADD COLUMN IF NOT EXISTS indexed_hash text; +""" +``` +After `await conn.execute(SCHEMA_V22_SQL)` add `await conn.execute(SCHEMA_V23_SQL)`; bump the log line to `v1-v23`. + +- [ ] **Step 3: Write `content_hash` in the two case_law create functions** + +In `create_external_case_law` and `create_internal_committee_decision` (db.py ~2610-2760), the `INSERT ... ON CONFLICT ... DO UPDATE` was built in FU-2a. For EACH: +1. Add `content_hash` to the INSERT column list (append after the last data column, before the closing `)`). +2. Add a matching `$N` placeholder in VALUES (next number after the current max). +3. Add `content_hash = EXCLUDED.content_hash` to the `DO UPDATE SET` clause. +4. Append `_content_hash(full_text)` as the LAST positional arg in the `conn.fetchrow(..., )` call (matching the new `$N`). + +CRITICAL: the new placeholder number must equal `(current highest $N) + 1`, and the new arg must be appended LAST in the args tuple in the SAME order. Read the current SQL + args carefully and count. After editing, verify param count = placeholder count (Step 5 import check will catch a gross mismatch; the DB smoke in Task 6 confirms at runtime). + +- [ ] **Step 4: Write `content_hash` in `create_case`** + +In `create_case` (db.py ~1130-1165), the INSERT into `cases` — add `content_hash`? NO: `cases` is a different table (active appeal cases), and FU-3's scope is `case_law` (the corpus). Do NOT alter `create_case` or the `cases` table here. (The spec §3 mentioned create_case for normalization in FU-2a; for FU-3 hashing, scope is `case_law` only. Skip create_case.) + +- [ ] **Step 5: Add `mark_indexed`, `list_stale_case_law`, `recompute_content_hashes` (after `get_case_law`, ~line 2547)** + +```python +async def mark_indexed(case_law_id: UUID) -> None: + """Mark a case_law row's embeddings as built from its current content (FU-3). + + Sets indexed_hash := content_hash. Call AFTER a successful chunk+embed+store. + """ + pool = await get_pool() + async with pool.acquire() as conn: + await conn.execute( + "UPDATE case_law SET indexed_hash = content_hash WHERE id = $1", + case_law_id, + ) + + +async def list_stale_case_law(limit: int = 500) -> list[dict]: + """case_law rows whose embeddings are stale vs current content (GAP-09/INV-G6).""" + pool = await get_pool() + async with pool.acquire() as conn: + rows = await conn.fetch( + """SELECT id, case_number, source_kind + FROM case_law + WHERE coalesce(full_text, '') <> '' + AND content_hash IS DISTINCT FROM indexed_hash + ORDER BY created_at LIMIT $1""", + limit, + ) + return [dict(r) for r in rows] + + +async def recompute_content_hashes() -> dict: + """Backfill (FU-3): set content_hash for all rows; set indexed_hash=content_hash + only where chunks already exist (those are already embedded). Rows with text but + no chunks get indexed_hash=NULL → surface as stale. Hash-only; no re-embed.""" + pool = await get_pool() + updated = 0 + async with pool.acquire() as conn: + rows = await conn.fetch("SELECT id, full_text FROM case_law") + for r in rows: + ch = _content_hash(r["full_text"] or "") + has_chunks = await conn.fetchval( + "SELECT EXISTS(SELECT 1 FROM precedent_chunks WHERE case_law_id = $1)", + r["id"]) + await conn.execute( + "UPDATE case_law SET content_hash = $2, " + "indexed_hash = CASE WHEN $3 THEN $2 ELSE indexed_hash END WHERE id = $1", + r["id"], ch, bool(has_chunks)) + updated += 1 + return {"updated": updated} +``` + +- [ ] **Step 6: Run the helper tests** + +Run: `cd ~/legal-ai/mcp-server && .venv/bin/python -m pytest tests/test_reindex_on_change.py -k "content_hash or mark_indexed" -v` +Expected: `test_content_hash_*` (3) + `test_mark_indexed_executes_update` PASS. + +- [ ] **Step 7: Commit** + +```bash +cd ~/legal-ai +git add mcp-server/src/legal_mcp/services/db.py +git commit -m "feat(reindex): V23 content/indexed hashes + helpers + write content_hash (GAP-09, FU-3)" +``` + +--- + +## Task 3: `reindex_case_law` + mark_indexed on ingest + +**Files:** Modify `mcp-server/src/legal_mcp/services/ingest.py` + +- [ ] **Step 1: Call `mark_indexed` after successful chunk+embed+store in `ingest_document`** + +READ `ingest_document` — find the line `stored_chunks = await _chunk_embed_store(case_law_id, raw_text, page_offsets, page_count, progress)` (~line 184). Immediately AFTER it, add: + +```python + await db.mark_indexed(case_law_id) +``` +(After a fresh ingest, chunks were just built from the current text → indexed_hash = content_hash.) + +- [ ] **Step 2: Add `reindex_case_law` (append to ingest.py)** + +```python +async def reindex_case_law( + case_law_id: "UUID | str", + progress: ProgressCb | None = None, +) -> dict: + """Re-chunk + re-embed an existing case_law row from its STORED full_text (GAP-09). + + No re-extract / no re-OCR (uses the stored text — see feedback_no_reocr_retrofit) + and no LLM/CLI (only chunker + voyage embeddings), so it is safe to run anywhere. + Idempotent: store_precedent_chunks(_hierarchical) is DELETE-then-INSERT. + """ + progress = progress or _noop_progress + cid = case_law_id if isinstance(case_law_id, UUID) else UUID(str(case_law_id)) + row = await db.get_case_law(cid) + if not row: + raise ValueError(f"case_law not found: {cid}") + text = (row.get("full_text") or "").strip() + if not text: + raise ValueError("case_law has no stored full_text to re-index") + stored = await _chunk_embed_store(cid, text, None, 0, progress) + await db.mark_indexed(cid) + await progress("completed", 100, f"הוטמע מחדש: {stored} chunks") + return {"status": "completed", "case_law_id": str(cid), "chunks": stored, "reindexed": True} +``` +(`UUID`, `db`, `_chunk_embed_store`, `_noop_progress`, `ProgressCb` are already in ingest.py.) + +- [ ] **Step 3: Run reindex tests** + +Run: `cd ~/legal-ai/mcp-server && .venv/bin/python -m pytest tests/test_reindex_on_change.py -v` +Expected: ALL pass (incl `test_reindex_case_law_uses_stored_text`, `test_reindex_case_law_missing_row_raises`). + +- [ ] **Step 4: Commit** + +```bash +cd ~/legal-ai +git add mcp-server/src/legal_mcp/services/ingest.py +git commit -m "feat(reindex): reindex_case_law from stored text + mark_indexed on ingest (GAP-09, FU-3)" +``` + +--- + +## Task 4: Health-check drift count + +**Files:** Modify `mcp-server/src/legal_mcp/services/metrics.py` + +- [ ] **Step 1: Add `stale_embedding_case_law` count** + +READ metrics.py — the aggregation that holds `non_searchable_case_law` / `cases_with_stale_blocks` (added in FU-2a/FU-7). Add a sibling, mirroring the exact pattern: + +```python + stale_embedding_case_law = await conn.fetchval( + "SELECT COUNT(*) FROM case_law " + "WHERE coalesce(full_text,'') <> '' AND content_hash IS DISTINCT FROM indexed_hash") +``` +and expose it in the returned summary dict: `"stale_embedding_case_law": stale_embedding_case_law`. + +- [ ] **Step 2: Smoke-import + commit** + +Run: `cd ~/legal-ai/mcp-server && .venv/bin/python -c "from legal_mcp.services import metrics; print('clean')"` +```bash +cd ~/legal-ai +git add mcp-server/src/legal_mcp/services/metrics.py +git commit -m "feat(reindex): health-check stale_embedding_case_law count (GAP-09, FU-3)" +``` + +--- + +## Task 5: MCP tool `precedent_reindex` + +**Files:** Modify `mcp-server/src/legal_mcp/tools/precedent_library.py`, `mcp-server/src/legal_mcp/server.py` + +- [ ] **Step 1: Add the tool function in precedent_library.py (mirror `precedent_extract_metadata`)** + +READ `precedent_extract_metadata` (tools/precedent_library.py ~205-216) for the `_ok`/`_err`/UUID pattern. Add: + +```python +async def precedent_reindex(case_law_id: str) -> str: + """re-chunk + re-embed פסיקה קיימת מה-full_text השמור (FU-3/GAP-09). + + לתיקון drift של embeddings או אחרי שינוי-תוכן. אינו מריץ OCR/LLM — רק + chunking + voyage embeddings. idempotent (מוחק ובונה chunks מחדש). + """ + try: + cid = UUID(case_law_id) + except ValueError: + return _err("case_law_id לא תקין") + try: + from legal_mcp.services import ingest + result = await ingest.reindex_case_law(cid) + except Exception as e: + return _err(str(e)) + return _ok(result) +``` + +- [ ] **Step 2: Register in server.py (mirror the precedent tools' `@mcp.tool()` registration)** + +READ server.py — find where `precedent_extract_metadata` (or another `precedent_*` tool) is registered with `@mcp.tool()` and delegated to `tools.precedent_library`. Add an equivalent registration for `precedent_reindex` following the identical pattern (decorator + delegation + the same import style). Report the exact registration block you added. + +- [ ] **Step 3: Smoke-import + commit** + +Run: `cd ~/legal-ai/mcp-server && .venv/bin/python -c "from legal_mcp.tools import precedent_library; import legal_mcp.server; print('clean')"` +```bash +cd ~/legal-ai +git add mcp-server/src/legal_mcp/tools/precedent_library.py mcp-server/src/legal_mcp/server.py +git commit -m "feat(reindex): precedent_reindex MCP tool (GAP-09, FU-3)" +``` + +--- + +## Task 6: Backfill + full suite + DB smoke + lint + TaskMaster + +- [ ] **Step 1: Full offline suite** + +Run: `cd ~/legal-ai/mcp-server && .venv/bin/python -m pytest tests/ -q` +Expected: all pass (FU-1/2a/7 + new FU-3). If a pre-existing test that calls `ingest_document` breaks because `mark_indexed` isn't stubbed, fix that fixture to stub `db.mark_indexed` (same pattern as the FU-2a `recompute_searchable` fixture fix). Report. + +- [ ] **Step 2: DB smoke + backfill (real Postgres — applies V23, runs backfill)** + +```bash +cd ~/legal-ai && set -a && source ~/.env 2>/dev/null && set +a +cd mcp-server && .venv/bin/python -c " +import asyncio +from legal_mcp.services import db +async def main(): + await db.get_pool() # applies V23 + pool = await db.get_pool() + async with pool.acquire() as c: + cols = await c.fetchval(\"SELECT count(*) FROM information_schema.columns WHERE table_name='case_law' AND column_name IN ('content_hash','indexed_hash')\") + print('V23 columns present:', cols, '(expect 2)') + res = await db.recompute_content_hashes() + print('backfill:', res) + stale = await db.list_stale_case_law() + print('stale after backfill:', len(stale)) +asyncio.run(main()) +" 2>&1 | grep -vE 'INFO|WARNING|httpx|deprecat|command not found|\^\^\^' | tail -5 +``` +Expected: `V23 columns present: 2`, backfill updated ~129, `stale after backfill:` a small number (rows with text but no chunks, e.g. cited_only). Report the stale count. + +- [ ] **Step 3: Lint** + +Run: `cd ~/legal-ai/mcp-server && .venv/bin/python -m ruff check src/legal_mcp/services/db.py src/legal_mcp/services/ingest.py 2>/dev/null; echo "exit=$?"` +Expected: clean or "ruff not available". + +- [ ] **Step 4: TaskMaster** — controller marks #61 + subtask 61.1 done (61.2 already cancelled), verifies via MCP. + +--- + +## Self-Review Notes + +- **GAP-09** → content_hash detection (Task 2) + reindex_case_law (Task 3) + drift health-check (Task 4) + MCP tool (Task 5). +- **No re-OCR:** reindex uses stored `full_text` only (Task 3) — honors feedback_no_reocr_retrofit. +- **Backfill is hash-only** (Task 6 Step 2) — no re-embed, no API cost; existing vectors untouched. +- **#61.2 closed** (not-applicable, in the spec commit) — no multimodal backfill task here. +- **Scope:** `case_law` only — `create_case`/`cases` table NOT touched (Task 2 Step 4). +- **Type consistency:** `_content_hash(text)->str`, `mark_indexed(case_law_id)`, `reindex_case_law(id)->{chunks,reindexed}`, `list_stale_case_law()`, `recompute_content_hashes()->{updated}` — names identical across tasks + tests. +- **Param-count risk** (Task 2 Step 3): the FU-2a upsert SQL must get exactly one new placeholder + one new arg per function; verified at runtime by the Task 6 DB smoke (a mismatch raises immediately).