From 2ff9a3dda2a10ba2365b765280049fcc78f2e247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Cl=C3=A9rice?= Date: Thu, 17 Sep 2026 18:56:14 +0200 Subject: [PATCH] Support milestone citeStructures nested under a non-traversing parent A citeStructure such as match="/TEI/text/body/div" with a child matching ".//milestone[@unit='section']" was unusable: the same @n value occurs in every chapter, and the parent path is absolute, so nothing scoped the child to its own parent. - normalize_xpath: drop the "." step a ".//" child match produces, which made reconstruct_doc copy the parent twice (
). - reverse_ancestor: that same "." step generated the invalid "[ancestor::]" (XPST0003). - _get_sibling_xpath: bind $end with head(), several siblings could match the boundary xpath and "<<" rejects a sequence (XPTY0004). - get_passage: always prefer a concrete boundary node over a tag/attribute xpath fragment, which otherwise matched a later chapter's section and over-copied. _prune_at_node prunes at a node, and _treat_siblings excludes the sibling that contains the boundary from the "<<" copy (it starts before it) to copy it pruned instead. - get_next_unit_boundary: a unit with no next sibling fell through to COPY_UNTIL_END, returning the rest of the document (157KB for one section). It now stops at the next unit of the same kind, clamped to the end of its own parent so a section never reaches into the next chapter. - find_refs / _resolve_groups: a child match written "//x" joined to ".///x"; treat it as ".//x", like generate_xpath already does. - find_refs: raise a located error on an empty citation value instead of "Reference '' does not match the expected format." further down. --- dapytains/tei/citeStructure.py | 70 +++++++++++- dapytains/tei/document.py | 105 ++++++++++++++++-- tests/tei/div_section_milestones.xml | 17 +++ .../div_section_milestones_double_slash.xml | 17 +++ tests/tei/div_section_milestones_empty_n.xml | 17 +++ tests/test_tei.py | 59 ++++++++++ 6 files changed, 268 insertions(+), 17 deletions(-) create mode 100644 tests/tei/div_section_milestones.xml create mode 100644 tests/tei/div_section_milestones_double_slash.xml create mode 100644 tests/tei/div_section_milestones_empty_n.xml diff --git a/dapytains/tei/citeStructure.py b/dapytains/tei/citeStructure.py index c00aae4..abe4514 100644 --- a/dapytains/tei/citeStructure.py +++ b/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( [ diff --git a/dapytains/tei/document.py b/dapytains/tei/document.py index db24b09..03ea948 100644 --- a/dapytains/tei/document.py +++ b/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

): 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 diff --git a/tests/tei/div_section_milestones.xml b/tests/tei/div_section_milestones.xml new file mode 100644 index 0000000..bfad0f8 --- /dev/null +++ b/tests/tei/div_section_milestones.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + +

One one.One two.

+

Before.Two one.Two two.

+ + + diff --git a/tests/tei/div_section_milestones_double_slash.xml b/tests/tei/div_section_milestones_double_slash.xml new file mode 100644 index 0000000..8a0faee --- /dev/null +++ b/tests/tei/div_section_milestones_double_slash.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + +

One one.One two.

+

Before.Two one.Two two.

+ + + diff --git a/tests/tei/div_section_milestones_empty_n.xml b/tests/tei/div_section_milestones_empty_n.xml new file mode 100644 index 0000000..69cbba0 --- /dev/null +++ b/tests/tei/div_section_milestones_empty_n.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + +

One one.One two.

+

Before.Two one.Two two.

+ + + diff --git a/tests/test_tei.py b/tests/test_tei.py index 61ce0d8..2114b4f 100644 --- a/tests/test_tei.py +++ b/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 '): + 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) == ( + '\n' + ' \n' + '

One one.

\n' + ' \n' + ' \n' + '' + ) + # 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) == ( + '\n' + ' \n' + '

One two.

\n' + ' \n' + ' \n' + '' + ) + assert tostring(doc.get_passage("2.1"), encoding=str) == ( + '\n' + ' \n' + '

Two one.

\n' + ' \n' + ' \n' + '' + ) + + +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()