diff --git a/.claude/agents/legal-analyst.md b/.claude/agents/legal-analyst.md index 0b13820..90f277d 100644 --- a/.claude/agents/legal-analyst.md +++ b/.claude/agents/legal-analyst.md @@ -143,6 +143,8 @@ tools: **טענות סף** (אם קיימות): חוסר סמכות, שיהוי, התיישנות, אי-מיצוי הליכים, חוסר יריבות, מעשה בית דין — הצג כל אחת עם עמדת שני הצדדים. לכל טענת סף הוסף **עמדת ועדת הערר** (שדה ריק ליו"ר). אם אין — כתוב: "לא זוהו טענות סף." +**מבנה השדות — זהה לזה של הסוגיות:** כל טענת סף היא H3 משלה, והשדות בתוכה נכתבים בתחילת שורה (`**טענה (claim):**`) — **לא** כפריטי רשימה (`- **טענה (claim):**`) ולא בכותרת מעוטרת. השדה `**עמדת ועדת הערר:**` נכתב בשורה נפרדת והערך מתחתיו. זהו השדה שדפנה עורכת ישירות מה-UI — סטייה מהפורמט מסתירה אותו ואת יתר השדות של אותה טענה מהמסך. + **תקן ביקורת**: ציין את תקן הביקורת של הוועדה בתיק זה — "הוועדה מפעילה שיקול דעת תכנוני עצמאי" (ברישוי) או "הוועדה בוחנת את תקינות השומה המכרעת" (בהיטל השבחה) או תקן אחר לפי סוג ההליך. **מפת דרכים**: לאחר זיהוי טענות הסף ולפני הדיון בסוגיות — כתוב פסקת מפה: "X שאלות עומדות להכרעה: (1)...; (2)...; (3)..." — כדי שהקורא ידע מראש מה לצפות. @@ -347,7 +349,18 @@ wakeup ל-CEO עם `payload.issueId=$PAPERCLIP_TASK_ID` ו-`reason="מנתח מ 1. ... ## 5. טענות סף -[אם קיימות — כולל שאלות משפטיות + עמדת ועדת הערר לכל טענה] +[אם אין — "לא זוהו טענות סף." אם יש — טענה אחת לכל H3, במבנה הזה:] + +### ס-1 — [כותרת הטענה] ([מי מעלה]) + +**טענה (claim):** ... +**תשובה (response):** ... +**שאלה משפטית:** ... + +**עמדת ועדת הערר:** +[ימולא ע"י יו"ר הוועדה] + +### ס-2 — ... **תקן ביקורת:** [שיקול דעת עצמאי / בחינת תקינות השומה / אחר] diff --git a/mcp-server/src/legal_mcp/services/research_md.py b/mcp-server/src/legal_mcp/services/research_md.py index 49287d0..79a0b4b 100644 --- a/mcp-server/src/legal_mcp/services/research_md.py +++ b/mcp-server/src/legal_mcp/services/research_md.py @@ -43,8 +43,18 @@ SUBSECTION_RE = re.compile(r"^###\s+(.+?)$", re.MULTILINE) # Matches "**LABEL:**" field markers — handles both inline and block variants: # "**עמדת המבקשת:** Some text on same line" # "**שאלות משפטיות:**\n1. First question" +# and both the bare and the list-item form, because the analyst agent writes +# threshold claims as a bullet list ("- **עמדת ועדת הערר:**") while it writes +# issues bare. Group 1 is the list marker (or None), group 2 is the label. # The label itself must not contain ** or newlines. -FIELD_LABEL_RE = re.compile(r"^\*\*([^\n*]+?):\*\*[ \t]*", re.MULTILINE) +FIELD_LABEL_RE = re.compile( + r"^([ \t]*(?:[-*+]|\d+[.)])[ \t]+)?\*\*([^\n*]+?):\*\*[ \t]*", + re.MULTILINE, +) + +# Terminators that end a field's content even without a following field label: +# a heading, or a horizontal rule closing the subsection. +FIELD_TERMINATOR_RE = re.compile(r"^(?:#{2,}[ \t]|[ \t]*---[ \t]*$)", re.MULTILINE) # Matches the case number in the H1 CASE_NUMBER_RE = re.compile(r"#\s*ניתוח.*?ערר\s+([\d/\-]+)", re.MULTILINE) @@ -53,6 +63,17 @@ CASE_NUMBER_RE = re.compile(r"#\s*ניתוח.*?ערר\s+([\d/\-]+)", re.MULTILIN DATE_RE = re.compile(r"^תאריך:\s*(.+?)\s*$", re.MULTILINE) +def _is_chair_label(label: str) -> bool: + """Is this field label the chair-position field? + + Matches on prefix, not equality, because the analyst sometimes decorates the + label with a parenthetical — "עמדת ועדת הערר (הכוונת יו"ר 24.6)". Requiring + equality made the reader treat those as ordinary fields and the writer append + a second, duplicate block instead of updating the existing one. + """ + return label.strip().startswith(CHAIR_POSITION_LABEL) + + def _is_placeholder(text: str) -> bool: """Check if a field value is one of the placeholder strings (empty).""" stripped = text.strip() @@ -135,7 +156,7 @@ def _extract_fields(text: str) -> list[dict]: fields = [] for i, m in enumerate(matches): - label = m.group(1).strip() + label = m.group(2).strip() content_start = m.end() content_end = matches[i + 1].start() if i + 1 < len(matches) else len(text) content = text[content_start:content_end].strip() @@ -164,11 +185,17 @@ def _build_subsection_dict( parts = title.split(": ", 1) display_title = parts[1] if len(parts) > 1 else title + # Only the *first* chair-position field is the editable one — the same one + # update_chair_position writes to. A subsection that carries more than one + # (e.g. an H3 covering "סוגיות 4–6") keeps the extras as ordinary fields + # rather than silently reading back a different field than the one saved. chair_position = "" + chair_seen = False regular_fields = [] for f in fields: - if f["label"] == CHAIR_POSITION_LABEL: + if not chair_seen and _is_chair_label(f["label"]): chair_position = _normalize_chair_position(f["content"]) + chair_seen = True else: regular_fields.append(f) @@ -311,6 +338,46 @@ def _find_subsection_by_id( return None +def _split_trailing_rule(body: str) -> tuple[str, str]: + """Split a subsection body into (content, trailing "---" separator). + + Returns ("", "") when the subsection has no closing rule. + """ + m = re.search(r"\n[ \t]*---[ \t]*\s*\Z", body) + if not m: + return body, "" + return body[: m.start()], body[m.start() :] + + +def _chair_field_span(body: str) -> tuple[int, int] | None: + """Locate the chair-position field's *content* range in a subsection body. + + Returns (content_start, content_end) — the slice the chair's text occupies, + excluding the "**LABEL:**" marker itself so the marker line (and any list + bullet in front of it) survives an update untouched. Returns None when the + subsection has no chair-position field yet. + + Field boundaries come from FIELD_LABEL_RE — the same definition the reader + uses — so what update writes is exactly what parse reads back (G2: one + definition of a field, not two that can drift). + """ + matches = list(FIELD_LABEL_RE.finditer(body)) + for i, m in enumerate(matches): + if not _is_chair_label(m.group(2)): + continue + content_start = m.end() + if i + 1 < len(matches): + content_end = matches[i + 1].start() + else: + content_end = len(body) + # A heading or closing rule ends the field even without a next label. + term = FIELD_TERMINATOR_RE.search(body, content_start, content_end) + if term: + content_end = term.start() + return content_start, content_end + return None + + def update_chair_position( file_path: Path, section_id: str, new_text: str ) -> dict[str, Any]: @@ -330,40 +397,62 @@ def update_chair_position( _abs_start, _abs_end, subsection_body = found - # Find the "**עמדת ועדת הערר:**" label within this subsection - label_pattern = re.compile( - r"(\*\*" + re.escape(CHAIR_POSITION_LABEL) + r":\*\*)\s*\n?([^*]*?)(?=\n\*\*|\n##|\n---|\Z)", - re.DOTALL, - ) - m = label_pattern.search(subsection_body) - if not m: - # Label not present — append it at the end of the subsection - # (just before the trailing --- if any) - new_block = f"\n\n**{CHAIR_POSITION_LABEL}:**\n{new_text.strip()}\n" - new_subsection = subsection_body.rstrip() + new_block - new_content = content[:_abs_start] + new_subsection + content[_abs_end:] + span = _chair_field_span(subsection_body) + body_text = new_text.strip() or CHAIR_POSITION_PLACEHOLDERS[0] + + if span is None: + # Label not present — append it at the end of the subsection, + # before a trailing horizontal rule if there is one. + head, tail = _split_trailing_rule(subsection_body) + new_block = f"\n\n**{CHAIR_POSITION_LABEL}:**\n{body_text}\n" + new_subsection = head.rstrip() + new_block + tail else: - # Replace the existing content of the chair_position field - replacement = f"{m.group(1)}\n{new_text.strip() if new_text.strip() else CHAIR_POSITION_PLACEHOLDERS[0]}\n" + # Replace the existing content of the chair_position field, keeping the + # label line exactly as written (including any list marker) so the file + # structure the analyst produced is preserved. + content_start, content_end = span new_subsection = ( - subsection_body[: m.start()] + replacement + subsection_body[m.end():] + subsection_body[:content_start].rstrip("\r\n \t") + + f"\n{body_text}\n" + + subsection_body[content_end:] ) - new_content = content[:_abs_start] + new_subsection + content[_abs_end:] + + new_content = content[:_abs_start] + new_subsection + content[_abs_end:] # Atomic write tmp_path = file_path.with_suffix(file_path.suffix + ".tmp") tmp_path.write_text(new_content, encoding="utf-8") # noqa: STG1 — atomic .tmp; in-place edit, S3 re-sync in Phase-2 read-wiring os.replace(tmp_path, file_path) - preview = new_text.strip()[:120] + # Read-after-write: a position the parser cannot read back is not saved, + # however cleanly the write itself succeeded. Reporting success here is what + # let a whole class of format drift hide behind a green "נשמר" in the UI. + stored = _stored_chair_position(file_path, section_id) + expected = _normalize_chair_position(new_text) + if stored != expected: + raise RuntimeError( + f"העמדה נכתבה ל-{file_path.name} אך לא נקראה בחזרה עבור {section_id} " + f"— ככל הנראה מבנה השדה בקובץ חורג מהתבנית הצפויה" + ) + return { "saved": True, "section_id": section_id, - "preview": preview, + "position": stored, + "preview": stored[:120], "timestamp": datetime.now(IL_TZ).isoformat(), } +def _stored_chair_position(file_path: Path, section_id: str) -> str: + """Re-parse the file and return the chair position now stored for a section.""" + parsed = parse(file_path) + for item in parsed.get("threshold_claims", []) + parsed.get("issues", []): + if item["id"] == section_id: + return item.get("chair_position", "") or "" + return "" + + # ── Chair directions extraction (for downstream agents) ───────── diff --git a/mcp-server/tests/test_research_md_chair_position.py b/mcp-server/tests/test_research_md_chair_position.py new file mode 100644 index 0000000..b9ba3eb --- /dev/null +++ b/mcp-server/tests/test_research_md_chair_position.py @@ -0,0 +1,237 @@ +"""Chair-position round-trip in analysis-and-research.md. + +Regression cover for the reader/writer asymmetry that made "עמדת ועדת הערר" +appear to save on threshold claims and then vanish on refresh: the writer +matched the label anywhere on a line, the reader only at the start of one, and +the analyst agent writes threshold claims as a bullet list +("- **עמדת ועדת הערר:**") while it writes issues flush-left. + +The invariant these tests pin down: whatever update_chair_position writes, +parse() must read back — for every label form that appears in the corpus. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from legal_mcp.services import research_md + + +BULLETED = """# ניתוח משפטי — ערר 1069-04-26 +תאריך: 2026-08-02 + +## 5. טענות סף + +### ס-1 — אי-מילוי תנאי ההפקדה +- **טענה (claim):** התכנית אושרה מבלי שמולאו תנאי ההפקדה. +- **שאלה משפטית:** האם הסטת התנאים פוגמת בחוקיות? +- **עמדת ועדת הערר:** [ימולא ע"י יו"ר הוועדה] + +### ס-2 — זכות עמידה +- **טענה (claim):** לעוררים אין אינטרס מוגן. +- **עמדת ועדת הערר:** [ימולא ע"י יו"ר הוועדה] + +## 6. סוגיות להכרעה + +### סוגיה 1: סמכות לפי 62א(א)(11) + +**ניתוח:** +- הכלל החל: ... + +**עמדת ועדת הערר:** [ימולא ע"י יו"ר הוועדה] + +--- +""" + + +@pytest.fixture() +def analysis_file(tmp_path: Path) -> Path: + path = tmp_path / "analysis-and-research.md" + path.write_text(BULLETED, encoding="utf-8") + return path + + +def _positions(path: Path) -> dict[str, str]: + parsed = research_md.parse(path) + items = parsed["threshold_claims"] + parsed["issues"] + return {item["id"]: item["chair_position"] for item in items} + + +# ── the reported bug ──────────────────────────────────────────── + + +def test_bulleted_threshold_position_survives_a_reload(analysis_file: Path) -> None: + """The bug as chaim hit it: saved on a threshold claim, gone on refresh.""" + research_md.update_chair_position(analysis_file, "threshold_1", "עמדתנו היא שהטענה נדחית.") + + assert _positions(analysis_file)["threshold_1"] == "עמדתנו היא שהטענה נדחית." + + +def test_flush_left_issue_position_still_round_trips(analysis_file: Path) -> None: + """The surface that already worked must keep working.""" + research_md.update_chair_position(analysis_file, "issue_1", "יש לקבל את הערר בנקודה זו.") + + assert _positions(analysis_file)["issue_1"] == "יש לקבל את הערר בנקודה זו." + + +def test_bulleted_threshold_fields_are_parsed_at_all(analysis_file: Path) -> None: + """Bulleted labels were invisible to the reader — the whole claim looked empty.""" + threshold = research_md.parse(analysis_file)["threshold_claims"] + + labels = [f["label"] for f in threshold[0]["fields"]] + assert "טענה (claim)" in labels + assert "שאלה משפטית" in labels + # the chair field is surfaced separately, never as a regular field + assert not any(research_md._is_chair_label(label) for label in labels) + + +# ── writer/reader agreement on content boundaries ─────────────── + + +def test_position_containing_markdown_is_not_truncated(analysis_file: Path) -> None: + """The old writer stopped the field at the first '*' and swallowed the rest.""" + text = 'העמדה כוללת **הדגשה** וגם קו --- באמצע\nושורה שנייה.' + research_md.update_chair_position(analysis_file, "issue_1", text) + + assert _positions(analysis_file)["issue_1"] == text + + +def test_update_does_not_touch_sibling_subsections(analysis_file: Path) -> None: + research_md.update_chair_position(analysis_file, "threshold_1", "ראשונה") + research_md.update_chair_position(analysis_file, "threshold_2", "שנייה") + + positions = _positions(analysis_file) + assert positions["threshold_1"] == "ראשונה" + assert positions["threshold_2"] == "שנייה" + assert positions["issue_1"] == "" + + +def test_closing_rule_and_bullet_marker_are_preserved(analysis_file: Path) -> None: + """An update must not restructure the file the analyst produced.""" + research_md.update_chair_position(analysis_file, "threshold_1", "עמדה") + research_md.update_chair_position(analysis_file, "issue_1", "עמדה") + content = analysis_file.read_text(encoding="utf-8") + + assert "- **עמדת ועדת הערר:**" in content + assert content.rstrip().endswith("---") + + +def test_clearing_a_position_restores_the_placeholder(analysis_file: Path) -> None: + research_md.update_chair_position(analysis_file, "threshold_1", "עמדה") + research_md.update_chair_position(analysis_file, "threshold_1", "") + + assert _positions(analysis_file)["threshold_1"] == "" + assert research_md.CHAIR_POSITION_PLACEHOLDERS[0] in analysis_file.read_text( + encoding="utf-8" + ) + + +def test_repeated_saves_do_not_accumulate_blocks(analysis_file: Path) -> None: + for text in ("ראשון", "שני", "שלישי"): + research_md.update_chair_position(analysis_file, "threshold_1", text) + + content = analysis_file.read_text(encoding="utf-8") + assert content.count(f"**{research_md.CHAIR_POSITION_LABEL}:**") == 3 + assert _positions(analysis_file)["threshold_1"] == "שלישי" + + +# ── decorated and duplicated labels ───────────────────────────── + + +def test_decorated_label_is_updated_not_duplicated(tmp_path: Path) -> None: + """'עמדת ועדת הערר (הכוונת יו"ר 24.6)' appears in the corpus (8125-09-24).""" + path = tmp_path / "a.md" + path.write_text( + "## 6. סוגיות להכרעה\n\n" + "### סוגיה 1: כותרת\n\n" + '**עמדת ועדת הערר (הכוונת יו"ר 24.6):** [ימולא ע"י יו"ר הוועדה]\n', + encoding="utf-8", + ) + + research_md.update_chair_position(path, "issue_1", "עמדה מעודכנת") + + content = path.read_text(encoding="utf-8") + assert content.count(research_md.CHAIR_POSITION_LABEL) == 1 + assert _positions(path)["issue_1"] == "עמדה מעודכנת" + + +def test_duplicate_labels_read_back_the_one_that_was_written(tmp_path: Path) -> None: + """An H3 covering several issues (1033-02-25) must not read a sibling's text.""" + path = tmp_path / "a.md" + path.write_text( + "## 6. סוגיות להכרעה\n\n" + "### סוגיות 4–6: מקובצות\n\n" + "**עמדת ועדת הערר:** ראשונה\n\n" + "**עמדת ועדת הערר:** אחרונה\n", + encoding="utf-8", + ) + + research_md.update_chair_position(path, "issue_1", "העמדה הנכונה") + + assert _positions(path)["issue_1"] == "העמדה הנכונה" + + +# ── read-after-write guard ────────────────────────────────────── + + +def test_update_returns_the_persisted_value(analysis_file: Path) -> None: + result = research_md.update_chair_position(analysis_file, "threshold_1", " עמדה ") + + assert result["saved"] is True + assert result["position"] == "עמדה" + + +def test_a_write_the_parser_cannot_read_back_is_reported_as_failure( + analysis_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Regression guard: the bug hid behind a success response for months.""" + monkeypatch.setattr( + research_md, "_stored_chair_position", lambda *_args: "משהו אחר" + ) + + with pytest.raises(RuntimeError, match="לא נקראה בחזרה"): + research_md.update_chair_position(analysis_file, "threshold_1", "עמדה") + + +# ── downstream consumers see the recovered positions ──────────── + + +def test_chair_directions_count_bulleted_positions(analysis_file: Path) -> None: + """extract_chair_directions feeds legal-writer; it under-reported before.""" + research_md.update_chair_position(analysis_file, "threshold_1", "עמדה") + + directions = research_md.extract_chair_directions(analysis_file) + assert directions["filled_count"] == 1 + assert directions["status"] == "partial" + filled = [t for t in directions["threshold_claims"] if t["direction"]] + assert [t["id"] for t in filled] == ["threshold_1"] + + +# ── the real corpus ───────────────────────────────────────────── + + +CORPUS = sorted( + Path("/home/chaim/legal-ai/data/cases").glob( + "*/documents/research/analysis-and-research.md" + ) +) + + +@pytest.mark.skipif(not CORPUS, reason="case corpus not present on this host") +@pytest.mark.parametrize("source", CORPUS, ids=lambda p: p.parts[-4]) +def test_every_corpus_subsection_round_trips(source: Path, tmp_path: Path) -> None: + sentinel = "עמדת-בדיקה — **הדגשה** ו---קו\nושורה שנייה." + before = _positions(source) + + for section_id in before: + working = tmp_path / f"{section_id}.md" + working.write_text(source.read_text(encoding="utf-8"), encoding="utf-8") + + research_md.update_chair_position(working, section_id, sentinel) + after = _positions(working) + + assert after[section_id] == sentinel, f"{source.parts[-4]} {section_id}" + untouched = {k: v for k, v in after.items() if k != section_id} + assert untouched == {k: v for k, v in before.items() if k != section_id} diff --git a/web-ui/src/components/compose/chair-editor.tsx b/web-ui/src/components/compose/chair-editor.tsx index 5ce0b6c..e2ced86 100644 --- a/web-ui/src/components/compose/chair-editor.tsx +++ b/web-ui/src/components/compose/chair-editor.tsx @@ -45,8 +45,10 @@ export function ChairEditor({ if (trimmed === lastSaved.current.trim()) return; setState({ kind: "saving" }); try { - await mutate.mutateAsync({ sectionId, position: trimmed }); - lastSaved.current = trimmed; + const res = await mutate.mutateAsync({ sectionId, position: trimmed }); + /* Track what the backend read back, not what we sent — "✓ נשמר" must + mean "persisted and re-readable", or a later blur skips the save. */ + lastSaved.current = res?.position ?? trimmed; setState({ kind: "saved", at: new Date() }); } catch (e) { setState({ diff --git a/web-ui/src/lib/api/research.ts b/web-ui/src/lib/api/research.ts index 3ad24b7..a71f019 100644 --- a/web-ui/src/lib/api/research.ts +++ b/web-ui/src/lib/api/research.ts @@ -59,20 +59,31 @@ export function useResearchAnalysis(caseNumber: string | undefined) { }); } +export type SaveChairPositionResult = { + saved: boolean; + section_id: string; + /** What the backend read back out of the file after writing — the truth. */ + position: string; + timestamp?: string; +}; + export function useSaveChairPosition(caseNumber: string | undefined) { const qc = useQueryClient(); return useMutation({ mutationFn: async (vars: { sectionId: string; position: string }) => - apiRequest( + apiRequest( `/api/cases/${caseNumber}/research/analysis/chair-position`, { method: "PATCH", body: { section_id: vars.sectionId, position: vars.position }, }, ), - onSuccess: (_res, vars) => { + onSuccess: (res, vars) => { /* Locally patch the cached analysis so other consumers stay in sync - without an immediate refetch that would steal focus from the editor. */ + without an immediate refetch that would steal focus from the editor. + Cache the value the server read back, never the value we sent — the + two diverged silently while the parser could not see bulleted fields. */ + const persisted = res?.position ?? vars.position; qc.setQueryData( researchKeys.analysis(caseNumber ?? ""), (prev) => { @@ -80,7 +91,7 @@ export function useSaveChairPosition(caseNumber: string | undefined) { const patch = (arr?: ResearchSubsection[]) => arr?.map((s) => s.id === vars.sectionId - ? { ...s, chair_position: vars.position } + ? { ...s, chair_position: persisted } : s, ); return {