From 6cc100f9f82c5426f3512a0ccd005f9d542ade31 Mon Sep 17 00:00:00 2001 From: Chaim Date: Mon, 15 Jun 2026 04:21:15 +0000 Subject: [PATCH] =?UTF-8?q?fix(halacha):=20rate-limit=20refusal=20?= =?UTF-8?q?=E2=89=A0=20empty=20answer=20=E2=80=94=20=D7=9C=D7=90=20checkpo?= =?UTF-8?q?int=20chunk=20=D7=91=D7=9B=D7=A9=D7=9C=20(#144)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit תיקון-ליבה (b): כש-claude CLI מחזיר exit=0 עם הודעת-מגבלה/שגיאה כ-result, query זיהה אותה כהצלחה → _extract_chunk קיבל []/non-list וסימן chunk כ-done-ריק; resume דילג עליו לתמיד → תת-חילוץ קבוע (3→1→0). עכשיו is_error/_looks_like_limit_notice הופכים אותה לכשל-חולף → retry → raise → chunk נשאר un-checkpointed → resume משחזר (כך force-delete כבר לא הרסני-לצמיתות). + churn-detect במתזמר (Δdone<0 / Δhal<-2 → אזהרה+churn_ok ב-JSON). + scripts/reconcile_under_extracted_halacha.py — שחזור completed-עם-0-הלכות-ו≥3 מקטעי-נימוק (dry-run הראה 15 מועמדים); נתיב-הזמנה קנוני (G2), שמרני (לא remand). הערה: אטומיות-מלאה (staging_run_id) נדחתה — PR #257 מיתן את ה-trigger, ו-(b)+resume מונעים אובדן-קבוע (force-delete מתאושש דרך resume). בדיקות: test_claude_session_limit_notice. כל 354 עוברות. guards נקיים. Invariants: G1, INV-G3/X16 (checkpoint=הושלם-באמת), INV-G4 (churn לא-שקט), G12. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/legal_mcp/services/claude_session.py | 47 +++++++++- .../tests/test_claude_session_limit_notice.py | 40 ++++++++ scripts/SCRIPTS.md | 1 + scripts/halacha_drain_supervisor.py | 13 ++- scripts/reconcile_under_extracted_halacha.py | 93 +++++++++++++++++++ 5 files changed, 190 insertions(+), 4 deletions(-) create mode 100644 mcp-server/tests/test_claude_session_limit_notice.py create mode 100644 scripts/reconcile_under_extracted_halacha.py diff --git a/mcp-server/src/legal_mcp/services/claude_session.py b/mcp-server/src/legal_mcp/services/claude_session.py index 668d230..76d9146 100644 --- a/mcp-server/src/legal_mcp/services/claude_session.py +++ b/mcp-server/src/legal_mcp/services/claude_session.py @@ -60,6 +60,31 @@ _SESSION_MARKER_EXACT = frozenset({"AI_AGENT", "CLAUDE_EFFORT"}) MAX_RETRIES = 3 RETRY_BACKOFF_BASE = 5 # seconds; sleep = base * attempt_number +# Phrases the CLI emits as the "result" of an exit-0 run that actually hit a +# usage/rate limit (a refusal NOTICE, not a real answer). Matched only against a +# result that is NOT structured output (doesn't start with [ or {), so a genuine +# JSON extraction containing these words as content is never mis-flagged. +_LIMIT_NOTICE_MARKERS = ( + "usage limit", + "rate limit", + "limit reached", + "limit will reset", + "try again later", +) + + +def _looks_like_limit_notice(data: dict) -> bool: + """True if an exit-0 ``result`` is really a usage/rate-limit refusal.""" + result = data.get("result") + if not isinstance(result, str): + return False + stripped = result.lstrip() + # Structured output (JSON array/object) is a real answer, never a notice. + if stripped.startswith(("[", "{")): + return False + low = result.lower() + return any(m in low for m in _LIMIT_NOTICE_MARKERS) + def _clean_subprocess_env() -> dict[str, str]: """Copy the current env minus Claude Code session markers. @@ -187,11 +212,27 @@ async def query( try: data = json.loads(stdout) if isinstance(data, dict) and "result" in data: - return data["result"] - return stdout + # A usage/rate-limit hit can exit 0 with a refusal NOTICE + # as the "result" (is_error / error subtype). Returning it + # as success makes callers treat a throttled run as a real + # empty answer — e.g. the halacha extractor then checkpoints + # the chunk as done-with-0-halachot and a resume skips it + # forever (#138/#144 silent under-extraction). Treat it as a + # transient failure → retry, and raise if it persists so the + # chunk stays un-checkpointed for a real resume. + if data.get("is_error") or _looks_like_limit_notice(data): + last_err = ( + f"error result (subtype={data.get('subtype')}): " + f"{str(data.get('result',''))[:200]}" + ) + else: + return data["result"] + else: + return stdout except json.JSONDecodeError: return stdout - last_err = "empty response" + else: + last_err = "empty response" # Transient failure — retry with linear backoff unless this was the last try. if attempt < MAX_RETRIES: diff --git a/mcp-server/tests/test_claude_session_limit_notice.py b/mcp-server/tests/test_claude_session_limit_notice.py new file mode 100644 index 0000000..51d2d9c --- /dev/null +++ b/mcp-server/tests/test_claude_session_limit_notice.py @@ -0,0 +1,40 @@ +"""Regression test for #144 — a usage/rate-limit refusal returned with exit 0 +must NOT be treated as a real (empty) answer. + +``_looks_like_limit_notice`` recognises a prose refusal in the CLI's exit-0 +``result`` so ``query`` raises instead of returning it — keeping the halacha +extractor from checkpointing the chunk as done-with-0-halachot (silent +under-extraction). A genuine JSON result (array/object) is never mis-flagged. +""" + +from __future__ import annotations + +from legal_mcp.services import claude_session as cs + + +def test_detects_usage_limit_prose(): + assert cs._looks_like_limit_notice( + {"result": "Claude usage limit reached. Try again later."}) + assert cs._looks_like_limit_notice( + {"result": "You've hit the rate limit for this model."}) + + +def test_json_array_result_is_not_a_notice(): + # A real halacha extraction — must pass through as a valid answer. + assert not cs._looks_like_limit_notice( + {"result": '[{"rule_statement": "the usage limit of a permit ..."}]'}) + + +def test_json_object_result_is_not_a_notice(): + assert not cs._looks_like_limit_notice( + {"result": '{"summary": "rate limit doctrine"}'}) + + +def test_plain_answer_without_markers_is_not_a_notice(): + assert not cs._looks_like_limit_notice({"result": "אין הלכות בקטע זה."}) + + +def test_missing_or_nonstring_result(): + assert not cs._looks_like_limit_notice({}) + assert not cs._looks_like_limit_notice({"result": None}) + assert not cs._looks_like_limit_notice({"result": 42}) diff --git a/scripts/SCRIPTS.md b/scripts/SCRIPTS.md index 4d18da5..ca31d07 100644 --- a/scripts/SCRIPTS.md +++ b/scripts/SCRIPTS.md @@ -33,6 +33,7 @@ | `drain_metadata_queue.py` | python | **ריקון תור חילוץ-המטא של הפסיקה** — `process_pending_extractions(kind='metadata')` ב-batches עד ריק. רץ על **Gemini Flash** (structured JSON, `gemini_session`) — מהיר ואמין, במקום ה-claude CLI ה-agentic שפגע ב-`error_max_turns`. no-op מהיר כשריק. הרצה ידנית: `mcp-server/.venv/bin/python scripts/drain_metadata_queue.py [batch]`. | דרך `legal-metadata-drain.config.cjs` (pm2 cron) | | `legal-metadata-drain.config.cjs` | pm2/js | **תזמון כל 15 דק' של `drain_metadata_queue.py`** (cron `*/15 * * * *`, `METADATA_DRAIN_CRON` לעקיפה) — מונע סתימה של תור חילוץ-המטא ב-/precedents. דורש `GEMINI_API_KEY` ב-`~/.env`. התקנה: `pm2 start scripts/legal-metadata-drain.config.cjs && pm2 save`. | pm2 cron (host-side) | | `reconcile_metadata_status.py` | python | **נרמול `metadata_extraction_status` תקוע (G1)** — שורות עם ברירת-המחדל `'pending'` שאינן בצנרת-Gemini נערמות כ-backlog-רפאים שהדריינר (סורק `*_requested_at IS NOT NULL`) לעולם לא מנקה ומנפח את מונה "ממתין" ב-/operations. מיישב כל שורה למצב-אמת במקור: `internal_committee`→`completed` (מטא דטרמיניסטי, מחוץ ל-Gemini), `external_upload` מלא→`completed`, `external_upload` עם טקסט וחסר שם/תקציר→חותם `requested_at` (הדריינר יטפל), `cited_only` (אין טקסט)→`skipped`. **מכסה את שני התורים (#140):** אותו `cited_only→skipped` מוחל גם על `halacha_extraction_status` (תור-תאום, G2). אידמפוטנטי. תיקון-המקור הנלווה ב-`db.create_internal_committee_decision` + מסנן `EXTRACTION_ELIGIBLE_PREDICATE` ב-`list_pending_extraction_requests`. הרצה: `mcp-server/.venv/bin/python scripts/reconcile_metadata_status.py`. | חד-פעמי / re-runnable כהגנת-drift | +| `reconcile_under_extracted_halacha.py` | python | **#144 — שחזור פסיקה תת-מחולצת** שהושלמה אך עם 0 הלכות למרות ≥3 מקטעי-נימוק (legal_analysis/ruling/conclusion) — חתימת ה-checkpoint-הריק שנוצרה לפני תיקון limit-notice ב-claude_session. מאפס checkpoints + `request_halacha_extraction` (נתיב קנוני, G2) → הדריינר מחלץ מחדש. שמרני (≥3 מקטעים → לא מטפל ב-remand לגיטימי חסר-הלכה; אפס אובדן כי 0 הלכות ממילא). מחריג cited_only. אידמפוטנטי, dry-run כברירת-מחדל / `--apply`. הרצה: `HOME=/home/chaim mcp-server/.venv/bin/python scripts/reconcile_under_extracted_halacha.py --apply`. | חד-פעמי / re-runnable | | `backfill_plans_registry.py` | python | **ייבוא מרשם-התכניות (V38) מקורפוס-ההחלטות** — סורק `data/cases/*/drafts/decision.md` + `data/training/cmp/*.md`, מאתר פסקאות-תוקף ("פורסמה למתן תוקף"), מחלץ רשומת-תכנית מובנית (`plans_extractor`, claude CLI מקומי) ועושה `upsert_plan(review_status='pending_review')` עם provenance. ה-SSOT לזהות+תוקף של תכנית, פעם-אחת במקום גזירה-מחדש מהשומות בכל תיק (G2). idempotent על plan_number מנורמל (G1/G3). `--dry-run` (ברירת-מחדל, כלום לא נכתב) / `--apply` / `--glob` (תת-קבוצה). אחרי הרצה: אישור-יו"ר ב-`plan_review`/תור-האישור (G10). הרץ: `mcp-server/.venv/bin/python scripts/backfill_plans_registry.py`. | ידני (חד-פעמי + לפי-צורך כשנוספות החלטות) | | `backfill_precedent_citations.py` | python | **#145** — backfill ל-`citation_formatted` (מראה-מקום) ברשומות `case_law` ריקות, באמצעות `db.format_precedent_citation` הדטרמיניסטי (X1 §3 / INV-ID2 — שדה-תצוגה נגזר, לא מעוצב ע"י LLM ש-הפיל אותו, #145). שני מעברים לכל שורה: (1) **ללא-LLM** — הרכבה מהשדות השמורים (ממלא שורות-ועדה עם parties+docket+date); (2) **LLM** — אם (1) נמנע ויש full_text, מריץ את מחלץ-המטא (extract_and_apply) שמחלץ רכיבים (parties, citation_prefix) ואז מרכיב — זה ממלא את 171 פסקי-בתי-המשפט מהכותרת. שורות בלי רובריקה (אין צדדים) נשארות ריקות ומדווחות, לא מנוחשות (INV-AH). idempotent — רק שדה ריק (G3). `--apply` / `--limit N` / `--no-llm`. הרץ: `HOME=/home/chaim mcp-server/.venv/bin/python scripts/backfill_precedent_citations.py`. | ידני (חד-פעמי + לפי-צורך) | | `auto-sync-cases.sh` | bash | סנכרון תיקי ערר ל-Gitea — רץ כל דקה | `* * * * *` (cron) | diff --git a/scripts/halacha_drain_supervisor.py b/scripts/halacha_drain_supervisor.py index 70dbf05..f89c07c 100644 --- a/scripts/halacha_drain_supervisor.py +++ b/scripts/halacha_drain_supervisor.py @@ -527,6 +527,16 @@ def tick(): if not staging_ok: notes.append("⚠️ תיקים הושלמו אך checkpoints לא התקדמו — לבדוק staging.") + # Churn guard (#144): a precedent regressing completed→pending (Δdone<0) or a + # net loss of halachot beyond consolidation noise (Δhal<-2) means a re-extract + # is DEGRADING the corpus (the 3→1 incident). Surface it loudly — never silent. + churn_ok = not (d_done < 0 or d_hal < -2) + if not churn_ok: + notes.append( + f"⚠️ churn: Δהלכות={d_hal} Δתיקים={d_done} — חילוץ-מחדש מדרדר " + "(completed→pending / אובדן-הלכות); לבדוק לפני המשך." + ) + if mode in ("ratelimited", "weekly_exhausted") and cd_dt: next_wake = max(300, min(int((cd_dt - now).total_seconds()) + 180, 3600)) elif mode in ("done", "idle_off_window"): @@ -562,7 +572,8 @@ def tick(): "halachot_total": hal, "checkpointed_chunks": ck, "delta_halachot": d_hal, "delta_checkpoints": d_ck, "delta_done": d_done, "pm2": status, "progress_age_sec": int(progress_age), - "staging_ok": staging_ok, "rate_limited": in_cooldown, "cooldown_until": cooldown_until, + "staging_ok": staging_ok, "churn_ok": churn_ok, + "rate_limited": in_cooldown, "cooldown_until": cooldown_until, }, ensure_ascii=False)) diff --git a/scripts/reconcile_under_extracted_halacha.py b/scripts/reconcile_under_extracted_halacha.py new file mode 100644 index 0000000..5aa9984 --- /dev/null +++ b/scripts/reconcile_under_extracted_halacha.py @@ -0,0 +1,93 @@ +"""Recover halacha precedents that completed extraction but are under-extracted (#144). + +Before the claude_session limit-notice fix, a chunk that hit a usage/rate limit +could exit 0 with a refusal NOTICE as its "result" → the extractor read it as an +empty answer and checkpointed the chunk as done-with-0-halachot. A ``completed`` +precedent could thus carry substantial reasoning yet ZERO holdings, and a resume +would skip the empty-checkpointed chunks forever. + +This finds the strongly-degraded survivors — ``status='completed'``, +``source_kind <> 'cited_only'``, with >= MIN_REASONING_CHUNKS reasoning chunks +(legal_analysis / ruling / conclusion) yet 0 halachot — and re-queues them for a +clean re-extraction: clear their chunk checkpoints, set status 'pending', stamp +``halacha_extraction_requested_at`` so the nightly drain re-runs them. + +CONSERVATIVE by design: requires >= 3 reasoning chunks so a genuine remand (a +1-2 chunk ruling that just sends the case back, legitimately holding-free) is +NOT churned into a pointless re-extraction loop. Nothing is lost (these have 0 +halachot already). Chair-approved/published rows are untouched (there are none +on a 0-halachot precedent, but the drain's reset preserves them regardless). + +Idempotent / re-runnable. Dry-run by default; pass ``--apply`` to write. +Host-only (reads POSTGRES_URL from ~/.env). Run: + HOME=/home/chaim mcp-server/.venv/bin/python scripts/reconcile_under_extracted_halacha.py [--apply] +""" + +from __future__ import annotations + +import asyncio +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "mcp-server", "src")) + +from legal_mcp.services import db + +MIN_REASONING_CHUNKS = 3 +_REASONING = ("legal_analysis", "ruling", "conclusion") + +_CANDIDATES_SQL = """ +SELECT cl.id, cl.case_number, + (SELECT count(*) FROM precedent_chunks pc + WHERE pc.case_law_id = cl.id + AND pc.section_type = ANY($1::text[])) AS reasoning_chunks +FROM case_law cl +WHERE cl.halacha_extraction_status = 'completed' + AND cl.source_kind <> 'cited_only' + AND (SELECT count(*) FROM halachot h WHERE h.case_law_id = cl.id) = 0 + AND (SELECT count(*) FROM precedent_chunks pc + WHERE pc.case_law_id = cl.id + AND pc.section_type = ANY($1::text[])) >= $2 +ORDER BY reasoning_chunks DESC +""" + + +async def main(apply: bool) -> int: + pool = await db.get_pool() + rows = await pool.fetch(_CANDIDATES_SQL, list(_REASONING), MIN_REASONING_CHUNKS) + + if not rows: + print("no under-extracted completed precedents found — nothing to do") + return 0 + + print(f"under-extracted candidates (completed, 0 halachot, >= " + f"{MIN_REASONING_CHUNKS} reasoning chunks): {len(rows)}") + for r in rows: + print(f" {r['case_number']:<22} reasoning_chunks={r['reasoning_chunks']} id={r['id']}") + + if not apply: + print("\n(dry-run — pass --apply to re-queue these for clean re-extraction)") + return 0 + + requeued = 0 + for r in rows: + cid = r["id"] + async with pool.acquire() as conn: + async with conn.transaction(): + # Clear checkpoints so the resume re-extracts every chunk (these + # carry 0 halachot, so there is nothing to preserve/lose). + await conn.execute( + "UPDATE precedent_chunks SET halacha_extracted_at = NULL " + "WHERE case_law_id = $1", cid, + ) + # Status→pending + stamp requested_at via the canonical request path so + # the drain picks it up (G2: same enqueue path as everything else). + await db.request_halacha_extraction(cid) + requeued += 1 + + print(f"\n✓ re-queued {requeued} precedent(s) — the halacha drain will re-extract them") + return 0 + + +if __name__ == "__main__": + sys.exit(asyncio.run(main("--apply" in sys.argv))) -- 2.49.1