Support milestone citeStructures nested under a non-traversing parent

dapytains/tei/citeStructure.py

@@ -96,6 +96,12 @@ def get_children_cite_structures(elem: saxonlib.PyXdmNode, processor: saxonlib.P
     return []
 
 
+def _relative(prefix: str, xpath: str) -> str:
+    """ Join a "./" prefix with a child citeStructure match. A match may be written "//lb" as
+    well as ".//lb": the naive join gives the invalid ".///lb". """
+    return f"{prefix}{xpath}".replace(".///", ".//")
+
+
 class CiteStructureParser:
     """
 
@@ -277,7 +283,7 @@ def _resolve_groups(self, groups: List[tuple]) -> Optional[saxonlib.PyXdmNode]:
                 concrete_node = (self._milestone_window(concrete_node, boundary, formatted) or [None])[0]
             else:
                 local_proc = get_xpath_proc(concrete_node, processor=self.processor)
-                concrete_node = local_proc.evaluate_single(f"./{formatted}")
+                concrete_node = local_proc.evaluate_single(_relative("./", formatted))
             prev_structure = structure
         return concrete_node
 
@@ -339,6 +345,53 @@ def get_next_milestone_boundary(self, reference: str) -> Optional[saxonlib.PyXdm
         start_node = self._resolve_groups(groups[:-1])
         return self._milestone_boundary(start_node, parent_structure.match)
 
+    def _first_node_after(self, node: saxonlib.PyXdmNode) -> Optional[saxonlib.PyXdmNode]:
+        """ First node in document order after `node`'s whole subtree, i.e. the point where
+        `node` ends. """
+        xpath_proc = get_xpath_proc(self.doc_root, processor=self.processor)
+        xpath_proc.declare_variable("__node")
+        xpath_proc.set_parameter("__node", node)
+        return xpath_proc.evaluate_single("($__node/following::node())[1]")
+
+    def _earliest(self, *nodes: Optional[saxonlib.PyXdmNode]) -> Optional[saxonlib.PyXdmNode]:
+        """ The first of `nodes` in document order, ignoring the ones that are None. """
+        earliest = None
+        xpath_proc = get_xpath_proc(self.doc_root, processor=self.processor)
+        xpath_proc.declare_variable("__a")
+        xpath_proc.declare_variable("__b")
+        for node in nodes:
+            if node is None:
+                continue
+            if earliest is None:
+                earliest = node
+                continue
+            xpath_proc.set_parameter("__a", node)
+            xpath_proc.set_parameter("__b", earliest)
+            if xpath_proc.effective_boolean_value("$__a << $__b"):
+                earliest = node
+        return earliest
+
+    def get_next_unit_boundary(self, reference: str) -> Optional[saxonlib.PyXdmNode]:
+        """ Return the node that bounds `reference`'s deepest unit when that unit has no next
+        sibling inside its own parent: the next unit of the same kind in document order (e.g.
+        the first section of chapter 8 for the last section of chapter 7), clamped to the end
+        of `reference`'s own parent unit, so a passage never reaches into the following parent
+        (chapter 7's last section stops at the end of chapter 7, it does not pick up what
+        chapter 8 puts before its own first section). Without this bound, the passage would
+        bleed to the end of the document. Returns None when nothing follows `reference`. """
+        groups = self._parse_reference(reference)
+        key, _ = groups[-1]
+        start_node = self._resolve_groups(groups)
+        if start_node is None:
+            return None
+        boundary = self._milestone_boundary(start_node, self.structure_by_key[key].match)
+        if len(groups) < 2:
+            return boundary
+        parent_node = self._resolve_groups(groups[:-1])
+        if parent_node is None:
+            return boundary
+        return self._earliest(boundary, self._first_node_after(parent_node))
+
     def _dispatch(
             self,
             child_xpath: str,
@@ -387,9 +440,16 @@ def find_refs(
         else:
             xpath_prefix = "./" if unit else ""
             # .evaluate returns None instead of an empty list...
-            values = xpath_proc.evaluate(f"{xpath_prefix}{structure.xpath}") or []
-
-        for value in values:
+            values = xpath_proc.evaluate(_relative(xpath_prefix, structure.xpath)) or []
+
+        for idx, value in enumerate(values, start=1):
+            if not value.string_value:
+                raise ValueError(
+                    f"Empty citation value for unit '{structure.citeType}' "
+                    f"(match #{idx} of `{structure.xpath}`"
+                    + (f" inside `{unit.ref}`" if unit else "")
+                    + "): the cited node has no value, check the source document."
+                )
             child = CitableUnit(
                 citeType=structure.citeType,
                 ref=f"{prefix}{value.string_value}",
@@ -442,7 +502,7 @@ def find_refs_from_branches(
             if milestone_match is not None:
                 results = self._milestone_window(root, boundary, s.xpath)
             else:
-                results = xpath_proc.evaluate(f"{xpath_prefix}{s.xpath}")
+                results = xpath_proc.evaluate(_relative(xpath_prefix, s.xpath))
             if results is not None:
                 unsorted.extend(
                     [

dapytains/tei/document.py

@@ -137,7 +137,9 @@ def _get_sibling_xpath(node_xpath: str, prefix: str = ".", ancestor: str = "") -
     if node_xpath == "node()":
         return "./following-sibling::node()"
 
-    return (f"let $end := following-sibling::node()[{prefix}/descendant-or-self::{node_xpath}] "
+    # head(): several siblings can match the boundary xpath (e.g. every later chapter contains
+    # a section n="2"), and only the nearest one bounds the passage.
+    return (f"let $end := head(following-sibling::node()[{prefix}/descendant-or-self::{node_xpath}]) "
             f"return (./following-sibling::node() [. << $end])")
 
 
@@ -185,6 +187,34 @@ def _prune(node: saxonlib.PyXdmNode, milestone: str, processor: saxonlib.PySaxon
     return x
 
 
+def _prune_at_node(
+        node: saxonlib.PyXdmNode,
+        boundary: saxonlib.PyXdmNode,
+        processor: saxonlib.PySaxonProcessor
+) -> str:
+    """ Same as _prune, but cuts at a concrete boundary node rather than at the first node
+    matching a tag/attribute based xpath fragment: the same fragment (e.g. a section n="2")
+    usually occurs in many places in a document, while the node does not. """
+    xq = processor.new_xquery_processor()
+    xq.set_context(xdm_item=node)
+    xq.set_parameter("boundary", boundary)
+    query = """declare namespace output = 'http://www.w3.org/2010/xslt-xquery-serialization';
+declare default element namespace 'http://www.tei-c.org/ns/1.0';
+declare option output:omit-xml-declaration 'yes';
+declare variable $boundary external;
+declare function local:prune($node, $b) {
+  if ($node instance of element()) then
+    if (empty($node/descendant-or-self::node() intersect $b)) then $node
+    else element {name($node)} {
+      $node/@*,
+      for $child in $node/node()[. << $b] return local:prune($child, $b)
+    }
+  else $node
+};
+local:prune(., $boundary)"""
+    return xq.run_query_to_string(query_text=query)
+
+
 def copy_node(
         node: saxonlib.PyXdmNode,
         processor: saxonlib.PySaxonProcessor,
@@ -203,7 +233,9 @@ def copy_node(
     if include_children:
         # We simply go from the element as a string to an element as XML.
         # We need to workaround false indentation through this xQuery
-        if isinstance(remove_milestone, str):
+        if isinstance(remove_milestone, saxonlib.PyXdmNode):
+            element = _prune_at_node(node, remove_milestone, processor=processor)
+        elif isinstance(remove_milestone, str):
             element = _prune(node, remove_milestone, processor=processor)
         else:
             xq = processor.new_xquery_processor()
@@ -267,14 +299,20 @@ def normalize_xpath(xpath: List[str]) -> List[str]:
             new_xpath.append("/"+xpath[x])
         elif len(xpath[x]) > 0:
             new_xpath.append(xpath[x])
-    return new_xpath
+    # A "." step (from a child citeStructure matching ".//something") selects the node we are
+    # already on: keeping it would copy that node twice in the reconstructed tree.
+    return [step for step in new_xpath if step.strip("/") != "."]
 
 
 def reverse_ancestor(xpaths: List[str]) -> str:
+    strip = re.compile(r"^([./]+)")
+    # A step such as "." (produced by a citeStructure whose child match starts with ".//")
+    # carries no ancestor constraint: stripping it leaves nothing, and keeping it would
+    # generate the invalid "[ancestor::]".
+    xpaths = [stripped for stripped in (strip.sub("", xpath) for xpath in xpaths) if stripped]
     if not xpaths:
         return ""
-    strip = re.compile(r"^([./]+)")
-    here = f"[ancestor::{strip.sub('', xpaths[0])}{reverse_ancestor(xpaths[1:]) if len(xpaths) > 1 else ''}]"
+    here = f"[ancestor::{xpaths[0]}{reverse_ancestor(xpaths[1:]) if len(xpaths) > 1 else ''}]"
     return here
 
 
@@ -299,13 +337,40 @@ def _treat_siblings(
     if isinstance(xpath, saxonlib.PyXdmNode):
         xproc.declare_variable("__boundary")
         xproc.set_parameter("__boundary", xpath)
-        next_nodes = xpath_eval(xproc, "./following-sibling::node()[. << $__boundary]")
+        # "<<" compares start positions, so a sibling that *contains* the boundary also
+        # matches: it is excluded here and copied (pruned) below instead.
+        next_nodes = xpath_eval(
+            xproc,
+            "./following-sibling::node()[. << $__boundary]"
+            "[empty(descendant::node() intersect $__boundary)]"
+        )
         for node in next_nodes:
             if node.node_kind_str == "text":
                 if not last_node.tail:
                     last_node.tail = unescape(_get_text(node, ".", processor=processor))
             else:
                 last_node = copy_node(node, include_children=True, parent=last_node.getparent(), processor=processor)
+
+        # The boundary can sit inside a following sibling (e.g. the next section starts in the
+        # middle of the next <p>): that sibling is not "<< boundary", so it is copied here,
+        # pruned at the boundary.
+        sibling_with_data = xproc.evaluate_single(
+            "./following-sibling::node()[descendant::node() intersect $__boundary]"
+        )
+        if sibling_with_data is not None:
+            pruned = copy_node(
+                sibling_with_data,
+                include_children=True,
+                parent=last_node.getparent(),
+                remove_milestone=xpath,
+                processor=processor
+            )
+            # Nothing of that sibling belongs to the passage (the boundary is its very first
+            # content): drop the empty shell rather than emitting it.
+            if pruned is not None and not "".join(pruned.itertext()):
+                parent = pruned.getparent()
+                if parent is not None:
+                    parent.remove(pruned)
         return last_node
 
     loc_xpath = "node()" if xpath == COPY_UNTIL_END else xpath
@@ -733,13 +798,21 @@ def get_passage(
                 next_ref = self.get_next(tree, end)
                 if next_ref:
                     next_ref = next_ref.ref
-                    if self.citeStructure[tree].is_milestone_nested(next_ref):
-                        end_sibling = self.citeStructure[tree].resolve_node(next_ref)
-                    else:
+                    # A concrete node is always preferable to a tag/attribute based xpath
+                    # fragment: the fragment (e.g. `milestone[@unit='section'][@n='2']`) also
+                    # matches every other chapter's section 2, which would push the boundary
+                    # far beyond the end of the passage.
+                    end_sibling = self.citeStructure[tree].resolve_node(next_ref)
+                    if end_sibling is None:
                         next_ref_xpath = normalize_xpath(xpath_split(self.citeStructure[tree].generate_xpath(next_ref)))[-1]
                         end_sibling = next_ref_xpath.strip("/")
                 elif (milestone_boundary := self.citeStructure[tree].get_next_milestone_boundary(end)) is not None:
                     end_sibling = milestone_boundary
+                elif (unit_boundary := self.citeStructure[tree].get_next_unit_boundary(end)) is not None:
+                    # No next sibling inside the parent unit (e.g. last section of a chapter):
+                    # the passage still stops at the next unit of the same kind, not at the
+                    # end of the document.
+                    end_sibling = unit_boundary
                 else:
                     end_sibling = COPY_UNTIL_END
         else:
@@ -748,13 +821,21 @@ def get_passage(
                 next_ref = self.get_next(tree, start)
                 if next_ref:
                     next_ref = next_ref.ref
-                    if self.citeStructure[tree].is_milestone_nested(next_ref):
-                        start_sibling = self.citeStructure[tree].resolve_node(next_ref)
-                    else:
+                    # A concrete node is always preferable to a tag/attribute based xpath
+                    # fragment: the fragment (e.g. `milestone[@unit='section'][@n='2']`) also
+                    # matches every other chapter's section 2, which would push the boundary
+                    # far beyond the end of the passage.
+                    start_sibling = self.citeStructure[tree].resolve_node(next_ref)
+                    if start_sibling is None:
                         next_ref_xpath = normalize_xpath(xpath_split(self.citeStructure[tree].generate_xpath(next_ref)))[-1]
                         start_sibling = next_ref_xpath.strip("/")
                 elif (milestone_boundary := self.citeStructure[tree].get_next_milestone_boundary(start)) is not None:
                     start_sibling = milestone_boundary
+                elif (unit_boundary := self.citeStructure[tree].get_next_unit_boundary(start)) is not None:
+                    # No next sibling inside the parent unit (e.g. last section of a chapter):
+                    # the passage still stops at the next unit of the same kind, not at the
+                    # end of the document.
+                    start_sibling = unit_boundary
                 else:
                     start_sibling = COPY_UNTIL_END
 

tests/tei/div_section_milestones.xml

@@ -0,0 +1,17 @@
+<TEI xmlns="http://www.tei-c.org/ns/1.0">
+  <teiHeader>
+    <encodingDesc>
+      <refsDecl n="default">
+        <citeStructure match="/TEI/text/body/div" use="@n" unit="chapter">
+          <citeStructure match=".//milestone[@unit='section']" use="@n" unit="section" delim="."/>
+        </citeStructure>
+      </refsDecl>
+    </encodingDesc>
+  </teiHeader>
+  <text>
+    <body>
+      <div type="chapter" n="1"><p><milestone unit="section" n="1"/>One one.<milestone unit="section" n="2"/>One two.</p></div>
+      <div type="chapter" n="2"><p>Before.<milestone unit="section" n="1"/>Two one.<milestone unit="section" n="2"/>Two two.</p></div>
+    </body>
+  </text>
+</TEI>

tests/tei/div_section_milestones_double_slash.xml

@@ -0,0 +1,17 @@
+<TEI xmlns="http://www.tei-c.org/ns/1.0">
+  <teiHeader>
+    <encodingDesc>
+      <refsDecl n="default">
+        <citeStructure match="/TEI/text/body/div" use="@n" unit="chapter">
+          <citeStructure match="//milestone[@unit='section']" use="@n" unit="section" delim="."/>
+        </citeStructure>
+      </refsDecl>
+    </encodingDesc>
+  </teiHeader>
+  <text>
+    <body>
+      <div type="chapter" n="1"><p><milestone unit="section" n="1"/>One one.<milestone unit="section" n="2"/>One two.</p></div>
+      <div type="chapter" n="2"><p>Before.<milestone unit="section" n="1"/>Two one.<milestone unit="section" n="2"/>Two two.</p></div>
+    </body>
+  </text>
+</TEI>

tests/tei/div_section_milestones_empty_n.xml

@@ -0,0 +1,17 @@
+<TEI xmlns="http://www.tei-c.org/ns/1.0">
+  <teiHeader>
+    <encodingDesc>
+      <refsDecl n="default">
+        <citeStructure match="/TEI/text/body/div" use="@n" unit="chapter">
+          <citeStructure match=".//milestone[@unit='section']" use="@n" unit="section" delim="."/>
+        </citeStructure>
+      </refsDecl>
+    </encodingDesc>
+  </teiHeader>
+  <text>
+    <body>
+      <div type="chapter" n="1"><p><milestone unit="section" n="1"/>One one.<milestone unit="section" n="2"/>One two.</p></div>
+      <div type="chapter" n=""><p>Before.<milestone unit="section" n="1"/>Two one.<milestone unit="section" n="2"/>Two two.</p></div>
+    </body>
+  </text>
+</TEI>

tests/test_tei.py

@@ -531,3 +531,62 @@ def test_standoff_no_standoff():
     doc = Document(f"{local_dir}/base_tei.xml")
     result = tostring(doc.get_passage("Luke 1:1", include_standoff=True), encoding=str)
     assert '<standOff' not in result
+
+
+def test_section_milestones_inside_chapters():
+    """Test milestones cited under a non-milestone, non-traversing parent (a chapter <div>):
+    the same @n occurs in every chapter, so neither the passage nor its boundary may leak
+    into the following chapters"""
+    doc = Document(f"{local_dir}/div_section_milestones.xml")
+
+    refs = doc.get_reffs()
+    assert [(r.ref, [c.ref for c in r.children]) for r in refs] == [
+        ("1", ["1.1", "1.2"]),
+        ("2", ["2.1", "2.2"]),
+    ]
+
+    assert tostring(doc.get_passage("1.1"), encoding=str) == (
+        '<TEI xmlns="http://www.tei-c.org/ns/1.0"><text>\n'
+        '    <body>\n'
+        '      <div type="chapter" n="1"><p><milestone unit="section" n="1"/>One one.</p></div>\n'
+        '      </body>\n'
+        '  </text>\n'
+        '</TEI>'
+    )
+    # Last section of a chapter stops at the end of its own chapter: neither at the end of
+    # the document, nor at the next chapter's first section (chapter 2 opens with "Before.",
+    # which belongs to no section and must not be picked up here)
+    assert tostring(doc.get_passage("1.2"), encoding=str) == (
+        '<TEI xmlns="http://www.tei-c.org/ns/1.0"><text>\n'
+        '    <body>\n'
+        '      <div type="chapter" n="1"><p><milestone unit="section" n="2"/>One two.</p></div>\n'
+        '      </body>\n'
+        '  </text>\n'
+        '</TEI>'
+    )
+    assert tostring(doc.get_passage("2.1"), encoding=str) == (
+        '<TEI xmlns="http://www.tei-c.org/ns/1.0"><text>\n'
+        '    <body>\n'
+        '      <div type="chapter" n="2"><p><milestone unit="section" n="1"/>Two one.</p></div>\n'
+        '    </body>\n'
+        '  </text>\n'
+        '</TEI>'
+    )
+
+
+def test_section_milestones_double_slash_match():
+    """A child citeStructure written "//x" behaves like ".//x" """
+    doc = Document(f"{local_dir}/div_section_milestones_double_slash.xml")
+    assert _flat_refs(doc.get_reffs()) == ["1", "1.1", "1.2", "2", "2.1", "2.2"]
+    reference = Document(f"{local_dir}/div_section_milestones.xml")
+    for ref in ["1", "1.1", "1.2", "2.1", "2.2"]:
+        assert tostring(doc.get_passage(ref), encoding=str) == \
+               tostring(reference.get_passage(ref), encoding=str)
+
+
+def test_empty_citation_value_is_reported():
+    """An unusable (empty) citation value points at the offending unit instead of failing
+    later on with an unrelated message"""
+    doc = Document(f"{local_dir}/div_section_milestones_empty_n.xml")
+    with pytest.raises(ValueError, match="Empty citation value for unit 'chapter'"):
+        doc.get_reffs()