feat(export): INV-EX4 single-template source + fix edit-path justification
מקור-תבנית-יחיד לכל מפיקי-מסמכי-ההחלטה: כולם מחילים סגנונות דרך skills/docx/decision_template.docx בלבד; אסור hand-setting של פונט/גודל. - docs/spec/06-export.md — INV-EX4 חדש (מקור-תבנית-יחיד, →G2) - docx_reviser: מסלול-העריכה (revise_draft/apply_user_edit) הכניס פסקאות עם jc=right קשיח + David/sz ידני → איבד יישור דו-צדדי של התבנית. עכשיו מחיל pStyle מן-הטמפלט (Normal/Heading 2/Quote) + jc=both קנוני, בלי rFonts/sz ידני (הגופן/גודל מגיעים מהסגנון). - שומר-CI test_docx_template_single_source — נכשל על בניית rFonts/sz במפיקים מחוץ לטמפלט (חריג מסומן # INV-EX4-ok ל-_mark_run_rtl, שמחזק את גופן-התבנית David כעקיפת באג-RTL של Word). - אומת: build_party_claims_summary_docx (סיכום-מנהלים, WS3) + export_decision (סופי/טיוטת-ביניים) כבר תבניתיים ותואמי-INV-EX4. - ארכוב scripts/exec_summary_1043.py (one-off; הוחלף ב-build_party_claims_summary_docx). - עדכון 2 טסטים שאימתו את ההתנהגות הישנה (rFonts על run מוכנס). Invariants: מקיים INV-EX4 (חדש), INV-G2 (מקור-יחיד, אין מסלול-סגנון מקביל), INV-EX1 (DOCX נגזר). נוגע ב-docx_reviser/docx_exporter/analysis_docx_exporter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -43,10 +43,16 @@ HEBREW_FONT = "David"
|
||||
|
||||
|
||||
def _mark_run_rtl(run) -> None:
|
||||
"""Force David font on all four slots, then add <w:rtl/>."""
|
||||
"""Force David font on all four slots, then add <w:rtl/>.
|
||||
|
||||
INV-EX4 note: this reinforces the template's OWN font (David, == the
|
||||
template's cs slot) to work around Word's RTL ascii-slot fallback to
|
||||
Times New Roman — it is NOT an ad-hoc font/size override. Marked
|
||||
INV-EX4-ok for the single-source guard for that reason.
|
||||
"""
|
||||
rPr = run._r.get_or_add_rPr()
|
||||
if rPr.find(qn("w:rFonts")) is None:
|
||||
fonts = OxmlElement("w:rFonts")
|
||||
fonts = OxmlElement("w:rFonts") # INV-EX4-ok: reinforces template font David
|
||||
fonts.set(qn("w:ascii"), HEBREW_FONT)
|
||||
fonts.set(qn("w:hAnsi"), HEBREW_FONT)
|
||||
fonts.set(qn("w:cs"), HEBREW_FONT)
|
||||
|
||||
@@ -199,27 +199,86 @@ def _find_enclosing_paragraph(element: etree._Element) -> etree._Element | None:
|
||||
return None
|
||||
|
||||
|
||||
# ── Template style resolution (INV-EX4) ────────────────────────────
|
||||
# Every inserted paragraph carries a <w:pStyle> pointing at one of the
|
||||
# template's named styles — font (David), size, and indent come from
|
||||
# styles.xml, NOT from hand-set rFonts/sz here. Single template = single
|
||||
# style source (INV-EX4 / G2). docx_exporter.py applies the same styles by
|
||||
# name via python-docx; here (raw lxml) we resolve name → styleId.
|
||||
|
||||
# StyleType → candidate style names in the template (case-insensitive).
|
||||
# Mirrors the names docx_exporter uses (paragraph.style = "Heading 2"/"Normal"/"Quote").
|
||||
_STYLE_NAME_CANDIDATES: dict[str, tuple[str, ...]] = {
|
||||
"body": ("Normal", "Default Paragraph Font", "Body Text"),
|
||||
"bold": ("Normal", "Body Text"),
|
||||
"heading": ("Heading 2", "heading 2"),
|
||||
"quote": ("Quote", "quote", "Intense Quote"),
|
||||
}
|
||||
|
||||
|
||||
def _resolve_style_ids(members: dict[str, bytes]) -> dict[str, str]:
|
||||
"""Map each StyleType → the template's w:styleId, read from styles.xml.
|
||||
|
||||
Returns {} when styles.xml is absent — callers then fall back to
|
||||
minimal inline emphasis (so a missing style never crashes rendering).
|
||||
"""
|
||||
raw = members.get("word/styles.xml")
|
||||
if not raw:
|
||||
return {}
|
||||
try:
|
||||
styles_tree = etree.fromstring(raw)
|
||||
except etree.XMLSyntaxError: # pragma: no cover - corrupt template
|
||||
return {}
|
||||
# Build a lowercase lookup over both the style's display name and its id.
|
||||
by_key: dict[str, str] = {}
|
||||
for style in styles_tree.iterfind(".//w:style", NSMAP):
|
||||
sid = style.get(_w("styleId"))
|
||||
if not sid:
|
||||
continue
|
||||
by_key.setdefault(sid.lower(), sid)
|
||||
name_el = style.find(_w("name"))
|
||||
if name_el is not None:
|
||||
nm = name_el.get(_w("val"))
|
||||
if nm:
|
||||
by_key.setdefault(nm.lower(), sid)
|
||||
resolved: dict[str, str] = {}
|
||||
for style_type, candidates in _STYLE_NAME_CANDIDATES.items():
|
||||
for cand in candidates:
|
||||
sid = by_key.get(cand.lower())
|
||||
if sid is not None:
|
||||
resolved[style_type] = sid
|
||||
break
|
||||
return resolved
|
||||
|
||||
|
||||
def _emphasis_fallback(style: StyleType, *, has_pstyle: bool) -> tuple[bool, bool]:
|
||||
"""(bold, italic) for a run. When the paragraph carries a resolved
|
||||
template pStyle, emphasis comes from the style (return no inline override),
|
||||
except 'bold' which is inline emphasis with no dedicated style. When no
|
||||
pStyle resolved, fall back to minimal inline emphasis so heading/quote
|
||||
stay visually distinct."""
|
||||
if style == "bold":
|
||||
return True, False
|
||||
if has_pstyle:
|
||||
return False, False
|
||||
return (style == "heading"), (style == "quote")
|
||||
|
||||
|
||||
# ── Paragraph builders ─────────────────────────────────────────────
|
||||
|
||||
|
||||
def _build_run(text: str, *, bold: bool = False, italic: bool = False,
|
||||
font: str = "David", size_half_pt: int | None = None) -> etree._Element:
|
||||
"""Build a w:r (run) element with RTL/David defaults and given text."""
|
||||
def _build_run(text: str, *, bold: bool = False, italic: bool = False) -> etree._Element:
|
||||
"""Build a w:r (run): RTL marker + optional bold/italic + text.
|
||||
|
||||
Font (David) and size are NOT set here — they derive from the paragraph's
|
||||
template style (INV-EX4). The <w:rtl/> marker is still required so Word
|
||||
picks the style's complex-script (cs=David) slot rather than the ascii
|
||||
(Times New Roman) slot for Hebrew (same reason as _mark_run_rtl in
|
||||
analysis_docx_exporter.py).
|
||||
"""
|
||||
r = etree.Element(_w("r"))
|
||||
rPr = etree.SubElement(r, _w("rPr"))
|
||||
|
||||
rFonts = etree.SubElement(rPr, _w("rFonts"))
|
||||
rFonts.set(_w("ascii"), font)
|
||||
rFonts.set(_w("hAnsi"), font)
|
||||
rFonts.set(_w("cs"), font)
|
||||
rFonts.set(_w("hint"), "cs")
|
||||
|
||||
if size_half_pt is not None:
|
||||
sz = etree.SubElement(rPr, _w("sz"))
|
||||
sz.set(_w("val"), str(size_half_pt))
|
||||
szCs = etree.SubElement(rPr, _w("szCs"))
|
||||
szCs.set(_w("val"), str(size_half_pt))
|
||||
|
||||
if bold:
|
||||
etree.SubElement(rPr, _w("b"))
|
||||
etree.SubElement(rPr, _w("bCs"))
|
||||
@@ -235,28 +294,36 @@ def _build_run(text: str, *, bold: bool = False, italic: bool = False,
|
||||
return r
|
||||
|
||||
|
||||
def _build_paragraph(text: str, *, style: StyleType = "body") -> etree._Element:
|
||||
"""Build a w:p (paragraph) with RTL + David + given text."""
|
||||
def _build_paragraph(text: str, *, style: StyleType = "body",
|
||||
style_id: str | None = None) -> etree._Element:
|
||||
"""Build a w:p (paragraph) carrying the template's named style.
|
||||
|
||||
The paragraph references `style_id` (resolved from the template's
|
||||
styles.xml) via <w:pStyle> — font/size/indent come from there, not from
|
||||
hand-set rFonts/sz (INV-EX4). The only explicit paragraph-level override
|
||||
is the canonical body alignment jc=both, matching docx_exporter's
|
||||
_set_paragraph_jc (which forces both over style-center headings).
|
||||
"""
|
||||
p = etree.Element(_w("p"))
|
||||
pPr = etree.SubElement(p, _w("pPr"))
|
||||
|
||||
if style_id:
|
||||
pStyle = etree.SubElement(pPr, _w("pStyle"))
|
||||
pStyle.set(_w("val"), style_id)
|
||||
|
||||
bidi = etree.SubElement(pPr, _w("bidi"))
|
||||
bidi.set(_w("val"), "1")
|
||||
|
||||
# Right alignment for body/RTL
|
||||
# Canonical body alignment (two-sided justification) — overrides any
|
||||
# style-center, exactly as docx_exporter._set_paragraph_jc does.
|
||||
jc = etree.SubElement(pPr, _w("jc"))
|
||||
jc.set(_w("val"), "right")
|
||||
jc.set(_w("val"), "both")
|
||||
|
||||
rPr_p = etree.SubElement(pPr, _w("rPr"))
|
||||
etree.SubElement(rPr_p, _w("rtl"))
|
||||
|
||||
bold = style in ("heading", "bold")
|
||||
italic = style == "quote"
|
||||
size = None
|
||||
if style == "heading":
|
||||
size = 28 # 14pt
|
||||
elif style == "quote":
|
||||
size = 22 # 11pt
|
||||
run = _build_run(text, bold=bold, italic=italic, size_half_pt=size)
|
||||
bold, italic = _emphasis_fallback(style, has_pstyle=bool(style_id))
|
||||
run = _build_run(text, bold=bold, italic=italic)
|
||||
p.append(run)
|
||||
return p
|
||||
|
||||
@@ -275,7 +342,7 @@ def _wrap_in_ins(elements: list[etree._Element], *, ins_id: int,
|
||||
|
||||
def _make_tracked_paragraph_insert(
|
||||
text: str, *, style: StyleType, ins_id: int, author: str, date_iso: str,
|
||||
mark_id: int | None = None,
|
||||
mark_id: int | None = None, style_id: str | None = None,
|
||||
) -> etree._Element:
|
||||
"""Build a whole tracked-inserted paragraph.
|
||||
|
||||
@@ -286,7 +353,7 @@ def _make_tracked_paragraph_insert(
|
||||
"""
|
||||
if mark_id is None:
|
||||
mark_id = ins_id
|
||||
p = _build_paragraph(text, style=style)
|
||||
p = _build_paragraph(text, style=style, style_id=style_id)
|
||||
pPr = p.find(_w("pPr"))
|
||||
assert pPr is not None
|
||||
rPr = pPr.find(_w("rPr"))
|
||||
@@ -339,6 +406,7 @@ def _apply_insert(
|
||||
ins_id: int,
|
||||
author: str,
|
||||
date_iso: str,
|
||||
style_ids: dict[str, str] | None = None,
|
||||
) -> RevisionResult:
|
||||
"""Apply insert_after / insert_before relative to a bookmark."""
|
||||
start, end = _find_bookmark(document_tree, revision.anchor_bookmark)
|
||||
@@ -362,6 +430,7 @@ def _apply_insert(
|
||||
revision.content, style=revision.style,
|
||||
ins_id=ins_id, mark_id=ins_id + 1,
|
||||
author=author, date_iso=date_iso,
|
||||
style_id=(style_ids or {}).get(revision.style),
|
||||
)
|
||||
|
||||
parent = enclosing_p.getparent()
|
||||
@@ -407,6 +476,7 @@ def _apply_replace(
|
||||
del_id: int,
|
||||
author: str,
|
||||
date_iso: str,
|
||||
style_ids: dict[str, str] | None = None,
|
||||
) -> RevisionResult:
|
||||
"""Replace = delete the existing paragraph + insert new one after it."""
|
||||
start, end = _find_bookmark(document_tree, revision.anchor_bookmark)
|
||||
@@ -428,6 +498,7 @@ def _apply_replace(
|
||||
revision.content, style=revision.style,
|
||||
ins_id=ins_id, mark_id=ins_id + 1,
|
||||
author=author, date_iso=date_iso,
|
||||
style_id=(style_ids or {}).get(revision.style),
|
||||
)
|
||||
idx = list(parent).index(enclosing_p)
|
||||
parent.insert(idx + 1, new_p)
|
||||
@@ -473,6 +544,10 @@ def apply_tracked_revisions(
|
||||
members, doc_tree, settings_tree = _load_docx_xml(source_path)
|
||||
_ensure_track_revisions(settings_tree)
|
||||
|
||||
# Resolve template style ids once (INV-EX4) — inserted paragraphs inherit
|
||||
# font/size/indent from these named styles instead of hand-set rFonts/sz.
|
||||
style_ids = _resolve_style_ids(members)
|
||||
|
||||
next_id = _next_revision_id(doc_tree)
|
||||
|
||||
batch = RevisionBatchResult()
|
||||
@@ -480,7 +555,8 @@ def apply_tracked_revisions(
|
||||
try:
|
||||
if rev.type in ("insert_after", "insert_before"):
|
||||
result = _apply_insert(doc_tree, rev, ins_id=next_id,
|
||||
author=author, date_iso=date_iso)
|
||||
author=author, date_iso=date_iso,
|
||||
style_ids=style_ids)
|
||||
# insert consumes 2 IDs: run-wrapper + paragraph-mark
|
||||
next_id += 2
|
||||
elif rev.type == "delete":
|
||||
@@ -490,7 +566,8 @@ def apply_tracked_revisions(
|
||||
elif rev.type == "replace":
|
||||
result = _apply_replace(doc_tree, rev,
|
||||
ins_id=next_id, del_id=next_id + 2,
|
||||
author=author, date_iso=date_iso)
|
||||
author=author, date_iso=date_iso,
|
||||
style_ids=style_ids)
|
||||
# replace consumes 3 IDs: ins-run, ins-mark, del
|
||||
next_id += 3
|
||||
else:
|
||||
|
||||
@@ -130,6 +130,16 @@ def _find_ins_with_runs(tree: etree._Element) -> etree._Element | None:
|
||||
return None
|
||||
|
||||
|
||||
def _find_enclosing_p(element: etree._Element) -> etree._Element | None:
|
||||
"""Walk up to the enclosing <w:p>."""
|
||||
cur = element
|
||||
while cur is not None:
|
||||
if cur.tag == _w("p"):
|
||||
return cur
|
||||
cur = cur.getparent()
|
||||
return None
|
||||
|
||||
|
||||
def test_insert_after_ins_has_author_and_date(sample_docx: Path, tmp_path: Path) -> None:
|
||||
out = tmp_path / "out.docx"
|
||||
rev = Revision(id="r1", type="insert_after",
|
||||
@@ -147,7 +157,10 @@ def test_insert_after_ins_has_author_and_date(sample_docx: Path, tmp_path: Path)
|
||||
assert date_str.endswith("Z") # ISO 8601 UTC
|
||||
|
||||
|
||||
def test_insert_after_uses_rtl_and_david(sample_docx: Path, tmp_path: Path) -> None:
|
||||
def test_insert_after_uses_rtl_and_template_style(sample_docx: Path, tmp_path: Path) -> None:
|
||||
"""INV-EX4: an inserted paragraph carries the template's named style via
|
||||
<w:pStyle> and an <w:rtl/> run marker — but does NOT hand-set rFonts/sz
|
||||
(font/size come from the style). jc is the canonical body 'both'."""
|
||||
out = tmp_path / "out.docx"
|
||||
rev = Revision(id="r1", type="insert_after",
|
||||
anchor_bookmark="block-alef", content="מוסף")
|
||||
@@ -162,10 +175,22 @@ def test_insert_after_uses_rtl_and_david(sample_docx: Path, tmp_path: Path) -> N
|
||||
assert run is not None
|
||||
rPr = run.find(_w("rPr"))
|
||||
assert rPr is not None
|
||||
# RTL marker preserved (so Word picks the style's cs=David slot)…
|
||||
assert rPr.find(_w("rtl")) is not None
|
||||
rFonts = rPr.find(_w("rFonts"))
|
||||
assert rFonts is not None
|
||||
assert rFonts.get(_w("ascii")) == "David"
|
||||
# …but NO hand-set font/size on the run (INV-EX4).
|
||||
assert rPr.find(_w("rFonts")) is None
|
||||
assert rPr.find(_w("sz")) is None
|
||||
assert rPr.find(_w("szCs")) is None
|
||||
|
||||
# The enclosing paragraph references a template style + canonical jc=both.
|
||||
para = _find_enclosing_p(ins)
|
||||
assert para is not None
|
||||
pPr = para.find(_w("pPr"))
|
||||
assert pPr is not None
|
||||
pStyle = pPr.find(_w("pStyle"))
|
||||
assert pStyle is not None and pStyle.get(_w("val")) # Normal → real styleId
|
||||
jc = pPr.find(_w("jc"))
|
||||
assert jc is not None and jc.get(_w("val")) == "both"
|
||||
|
||||
|
||||
# ── apply_tracked_revisions: insert_before ────────────────────────
|
||||
|
||||
76
mcp-server/tests/test_docx_template_single_source.py
Normal file
76
mcp-server/tests/test_docx_template_single_source.py
Normal file
@@ -0,0 +1,76 @@
|
||||
"""שומר INV-EX4 — מקור-תבנית-יחיד לכל מפיקי-מסמכי-ההחלטה.
|
||||
|
||||
כל מפיק של מסמך-Word הקשור-להחלטה חייב להחיל עיצוב דרך הסגנונות שמוגדרים
|
||||
בטמפלט היחיד (skills/docx/decision_template.docx), ולא לקבוע פונט/גודל ידנית
|
||||
(``rFonts`` / ``sz`` / ``szCs``) — אלה נגזרים מ-styles.xml של הטמפלט.
|
||||
|
||||
הטסט גורף את מודולי-המפיקים ונכשל אם נבנה אלמנט ``w:rFonts`` / ``w:sz`` /
|
||||
``w:szCs`` בקוד, אלא אם השורה נושאת את הסמן המפורש ``# INV-EX4-ok`` (חריג
|
||||
מתועד — למשל חיזוק גופן-התבנית עצמו David כעקיפת באג-RTL של Word).
|
||||
|
||||
ספ: docs/spec/06-export.md §INV-EX4.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from legal_mcp.services import (
|
||||
analysis_docx_exporter,
|
||||
docx_exporter,
|
||||
docx_reviser,
|
||||
docx_retrofit,
|
||||
)
|
||||
|
||||
# Every module that builds/edits a decision-related Word document.
|
||||
_PRODUCER_MODULES = [
|
||||
docx_exporter,
|
||||
analysis_docx_exporter,
|
||||
docx_reviser,
|
||||
docx_retrofit,
|
||||
]
|
||||
|
||||
# Construction (not lookup) of a hand-set font/size element:
|
||||
# OxmlElement("w:rFonts") (python-docx producers)
|
||||
# etree.SubElement(rPr, _w("sz")) (raw-lxml producer: docx_reviser)
|
||||
_HANDSET_RE = re.compile(
|
||||
r'OxmlElement\(\s*["\']w:(?:rFonts|sz|szCs)["\']'
|
||||
r'|SubElement\([^)]*_w\(\s*["\'](?:rFonts|sz|szCs)["\']'
|
||||
)
|
||||
|
||||
# Producers that build a document FROM SCRATCH must load the single template.
|
||||
# (docx_reviser/docx_retrofit operate on an already-templated existing doc.)
|
||||
_FROM_SCRATCH_MODULES = [docx_exporter, analysis_docx_exporter]
|
||||
|
||||
|
||||
def _module_path(module) -> Path:
|
||||
return Path(module.__file__)
|
||||
|
||||
|
||||
def test_no_handset_font_or_size_in_producers() -> None:
|
||||
"""INV-EX4: no hand-set rFonts/sz/szCs in any producer (only via the
|
||||
template's named styles), unless explicitly marked ``# INV-EX4-ok``."""
|
||||
violations: list[str] = []
|
||||
for module in _PRODUCER_MODULES:
|
||||
path = _module_path(module)
|
||||
for lineno, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1):
|
||||
if _HANDSET_RE.search(line) and "INV-EX4-ok" not in line:
|
||||
violations.append(f"{path.name}:{lineno}: {line.strip()}")
|
||||
assert not violations, (
|
||||
"INV-EX4 violation — hand-set font/size outside the template styles "
|
||||
"(use a named template style, or mark with # INV-EX4-ok):\n "
|
||||
+ "\n ".join(violations)
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("module", _FROM_SCRATCH_MODULES)
|
||||
def test_from_scratch_producers_load_the_single_template(module) -> None:
|
||||
"""INV-EX4: from-scratch producers reference the one template file."""
|
||||
source = _module_path(module).read_text(encoding="utf-8")
|
||||
assert "decision_template.docx" in source, (
|
||||
f"{_module_path(module).name} must load the single template "
|
||||
"skills/docx/decision_template.docx (INV-EX4)."
|
||||
)
|
||||
@@ -207,8 +207,9 @@ def test_multiple_revisions_all_tracked_independently(tmp_path: Path) -> None:
|
||||
|
||||
|
||||
def test_rtl_preserved_in_tracked_insertion(tmp_path: Path) -> None:
|
||||
"""Inserted paragraph must have bidi + rtl + David font so it renders
|
||||
correctly in Word alongside the user's content."""
|
||||
"""Inserted paragraph must have bidi + an <w:rtl/> run marker so Hebrew
|
||||
renders via the template's cs=David slot — but font/size come from the
|
||||
template's named paragraph style (INV-EX4), not hand-set rFonts/sz."""
|
||||
user_file = tmp_path / "user.docx"
|
||||
_make_user_edited_docx(user_file)
|
||||
docx_retrofit.retrofit_bookmarks(user_file, backup=False)
|
||||
@@ -229,9 +230,10 @@ def test_rtl_preserved_in_tracked_insertion(tmp_path: Path) -> None:
|
||||
if any("עברית RTL" in (t.text or "") for t in text_els):
|
||||
rPr = r.find(_w("rPr"))
|
||||
assert rPr is not None
|
||||
# RTL marker preserved…
|
||||
assert rPr.find(_w("rtl")) is not None
|
||||
rFonts = rPr.find(_w("rFonts"))
|
||||
assert rFonts is not None
|
||||
assert rFonts.get(_w("ascii")) == "David"
|
||||
# …but NO hand-set font/size on the run (INV-EX4).
|
||||
assert rPr.find(_w("rFonts")) is None
|
||||
assert rPr.find(_w("sz")) is None
|
||||
return
|
||||
pytest.fail("tracked insertion with 'עברית RTL' not found")
|
||||
|
||||
Reference in New Issue
Block a user