diff --git a/mcp-server/src/legal_mcp/services/analysis_docx_exporter.py b/mcp-server/src/legal_mcp/services/analysis_docx_exporter.py index c0b92cc..bc43f37 100644 --- a/mcp-server/src/legal_mcp/services/analysis_docx_exporter.py +++ b/mcp-server/src/legal_mcp/services/analysis_docx_exporter.py @@ -182,16 +182,21 @@ def _add_runs_with_inline_bold(paragraph: Paragraph, text: str) -> None: def _clear_body(doc: DocumentT) -> None: - """Remove every paragraph currently in the document body. + """Remove ALL body content (paragraphs, tables, stray bookmarks), + keeping only sectPr. - The template ships with example paragraphs ("רקע", "דיון והכרעה"…) - that we don't want in the output. Section properties (sectPr) are - kept so page size / margins / RTL / footer remain intact. + The template ships with example paragraphs ("רקע", "דיון והכרעה"…) *and + three sample tables* (header / panel / signatures). Removing only ``w:p`` + left the tables behind — including the signatures table, which then floated + to the top against the header table. Mirror the decision exporter's + ``_clear_body`` (G2 symmetry): drop everything except sectPr, which carries + page setup including bidi. """ body = doc.element.body - for p in list(body.findall(qn("w:p"))): - body.remove(p) - # Leave sectPr alone — it carries page setup including bidi. + sectPr_tag = qn("w:sectPr") + for child in list(body): + if child.tag != sectPr_tag: + body.remove(child) def _add_paragraph(doc: DocumentT, text: str, style: str) -> Paragraph: diff --git a/mcp-server/src/legal_mcp/services/docx_exporter.py b/mcp-server/src/legal_mcp/services/docx_exporter.py index 48d6c94..368a5ea 100644 --- a/mcp-server/src/legal_mcp/services/docx_exporter.py +++ b/mcp-server/src/legal_mcp/services/docx_exporter.py @@ -192,14 +192,29 @@ def _apply_list_numbering(paragraph, num_id: int) -> None: def _clear_body(doc) -> None: - """Remove all paragraphs in the document body while keeping sectPr. + """Remove ALL body content (paragraphs, tables, stray bookmarks) while + keeping only sectPr. - The template ships with sample paragraphs we don't want. Section - properties (page size, margins, bidi) stay intact. + The template ships with sample paragraphs *and three sample tables* + (institutional header, panel, and a **signatures** table at the end — + "ניתנה פה אחד" + יו"ר/מזכירה). Removing only ``w:p`` left those tables + behind: the leftover header/panel tables injected block-א–ד data that was + never extracted from the protocol (violates the empty-unless-extracted + rule), and the leftover signatures table collapsed up against the header + table at the **top** of the document (the chair-reported bug: חתימות צמודות + לבלוק-ד במקום בסוף). The decision is a derived artifact rebuilt purely from + ``decision_blocks`` (INV-EX1) — signatures come from block-yod-bet, rendered + last in block order — so the template's sample tables are stale scaffolding + and must go. Section properties (page size, margins, bidi) stay intact. """ body = doc.element.body - for p in list(body.findall(qn("w:p"))): - body.remove(p) + sectPr_tag = qn("w:sectPr") + for child in list(body): + # Keep section properties (page setup / bidi). Drop everything else: + # sample paragraphs (w:p), sample tables (w:tbl), and any stray + # bookmark markers the template left dangling between them. + if child.tag != sectPr_tag: + body.remove(child) # ── Bookmark helpers ────────────────────────────────────────────── diff --git a/mcp-server/tests/test_docx_exporter_bookmarks.py b/mcp-server/tests/test_docx_exporter_bookmarks.py index 7340aa6..7baea5d 100644 --- a/mcp-server/tests/test_docx_exporter_bookmarks.py +++ b/mcp-server/tests/test_docx_exporter_bookmarks.py @@ -14,7 +14,9 @@ from lxml import etree from legal_mcp.services.docx_exporter import ( _BOOKMARK_ID_START, HEBREW_FONT, + TEMPLATE_PATH, _add_styled_paragraph, + _clear_body, _insert_bookmark_end, _insert_bookmark_start, _mark_paragraph_rtl, @@ -168,6 +170,83 @@ def test_block_dalet_does_not_use_title_style() -> None: assert any("החלטה" in t for t in texts) +def test_block_dalet_is_heading_only_ignores_db_content() -> None: + """ת2 — block-ד is the 'החלטה' heading marker, NOT a data block. + + Whatever content sits in decision_blocks for block-dalet must be ignored; + the only non-empty text line produced is the literal heading 'החלטה'. + """ + doc = Document() + _write_block_to_docx( + doc, "block-dalet", title="כותרת", + content="נתון ישן שצריך להיות מתועלם\nשורה שנייה", + ) + text_lines = [p.text for p in doc.paragraphs if p.text.strip()] + assert text_lines == ["החלטה"], ( + f"block-dalet must render ONLY the heading 'החלטה', got {text_lines}" + ) + + +# ── _clear_body strips ALL scaffolding (ת1 + ת3 root cause) ──────── +# The template ships 3 sample tables (header / panel / signatures). Removing +# only left them behind: header/panel tables injected block-א–ד data +# never extracted from the protocol (ת1), and the signatures table floated to +# the top against the header (ת3 — חתימות צמודות לבלוק-ד במקום בסוף). The +# decision is rebuilt purely from decision_blocks (INV-EX1), so the template's +# sample tables are stale scaffolding and must be cleared. + + +def test_clear_body_removes_tables_and_leaves_only_sectpr() -> None: + doc = Document(str(TEMPLATE_PATH)) + body = doc.element.body + # Sanity: the template really ships sample tables (else this test is moot). + assert len(body.findall(qn("w:tbl"))) > 0, "template expected to ship sample tables" + + _clear_body(doc) + + remaining = [c.tag.split("}")[-1] for c in list(body)] + assert remaining == ["sectPr"], ( + f"_clear_body must leave only sectPr, got {remaining}" + ) + assert len(body.findall(qn("w:tbl"))) == 0, "no template tables may survive" + assert len(body.findall(qn("w:p"))) == 0, "no template paragraphs may survive" + + +def test_signatures_block_renders_after_body_not_at_top() -> None: + """ת3 — block-yod-bet (חתימות) renders LAST, after a cleared body. + + Regression for the bug where the leftover template signatures table sat at + the top against the header table. After the _clear_body fix, signatures + come only from block-yod-bet, which the export loop emits last in block + order. + """ + doc = Document(str(TEMPLATE_PATH)) + _clear_body(doc) + + # Emit a header block, a body block, then signatures last (canonical order). + _write_block_to_docx(doc, "block-alef", title="", content="מדינת ישראל") + _write_block_to_docx(doc, "block-vav", title="", content="רקע עובדתי\n1. המקרקעין.") + _write_block_to_docx( + doc, "block-yod-bet", title="", + content='ניתנה פה אחד היום.\nדפנה תמיר, עו"ד', + ) + + text_lines = [p.text for p in doc.paragraphs if p.text.strip()] + assert text_lines[0] == "מדינת ישראל", text_lines + # The signatures block (block-yod-bet) emits each line as its own paragraph, + # so its content must occupy the document's tail — after the body block. + body_idx = text_lines.index("רקע עובדתי") + sig_idx = next(i for i, t in enumerate(text_lines) if "ניתנה פה אחד" in t) + assert sig_idx > body_idx, ( + f"signatures must come AFTER the body, got tail={text_lines[-3:]}" + ) + assert any("דפנה תמיר" in t for t in text_lines[sig_idx:]), text_lines[sig_idx:] + # No template signatures table ("מזכירת ועדת ערר") leaked anywhere. + assert all( + "מזכירת ועדת ערר" not in (p.text or "") for p in doc.paragraphs + ), "template signatures-table text leaked into the body" + + # ── Heading overrides, numbered-list, dash strip ──────────────────