From fc67b0bfd6414be2b7e5d14b94b342d5a6a1f02d Mon Sep 17 00:00:00 2001 From: Ben King Date: Wed, 9 Sep 2026 16:31:02 -0400 Subject: [PATCH 1/3] Fix marker loss and crash from unclosed character styles An unclosed character style is closed implicitly by the next paragraph marker. end_char consumed tokens through state.index even on an implicit close, so that paragraph marker was pulled into the block being closed and dropped along with the removed style. In a non-verse paragraph it also left the place markers handler holding a paragraph element marked for removal, which satisfied the early-out guard but produced nothing to place, raising IndexError. - end_char only consumes tokens when the style is explicitly closed, as end_note and end_sidebar already did - end_note mirrors start_note for duplicate verses, which end_char had been masking by consuming the end marker first - the place markers early-out guard is derived from a single _is_placeable predicate so it cannot disagree with element collection, plus an explicit check for nothing left to place - OTHER elements no longer count as content when locating end-of-verse paragraph markers, which was moving embeds past them - TEXT elements are read in full: a verse range matched by several rows has one token per row, and reading only the first dropped the rest of the translation Co-Authored-By: Claude Opus 5 (1M context) --- ...place_markers_usfm_update_block_handler.py | 48 +++-- machine/corpora/update_usfm_parser_handler.py | 24 ++- ...place_markers_usfm_update_block_handler.py | 170 ++++++++++++++++++ .../test_update_usfm_parser_handler.py | 50 ++++++ 4 files changed, 267 insertions(+), 25 deletions(-) diff --git a/machine/corpora/place_markers_usfm_update_block_handler.py b/machine/corpora/place_markers_usfm_update_block_handler.py index 287eb2d6..4ceddadc 100644 --- a/machine/corpora/place_markers_usfm_update_block_handler.py +++ b/machine/corpora/place_markers_usfm_update_block_handler.py @@ -21,6 +21,24 @@ class PlaceMarkersAlignmentInfo(TypedDict): style_behavior: UpdateUsfmMarkerBehavior +def _element_text(element: UsfmUpdateBlockElement) -> str: + # A TEXT element holds one token per row matched to the block, so a verse range matched by + # several rows has several tokens. Reading only the first drops the rest of the translation. + return "".join(t.to_usfm() for t in element.tokens) + + +def _is_placeable(element: UsfmUpdateBlockElement, alignment_info: PlaceMarkersAlignmentInfo) -> bool: + # An element marked for removal is never placed, so it must not keep the block from + # returning early below: the placement code would then have nothing left to place. + if element.marked_for_removal: + return False + if element.type == UsfmUpdateBlockElementType.PARAGRAPH: + return alignment_info["paragraph_behavior"] == UpdateUsfmMarkerBehavior.PRESERVE and len(element.tokens) == 1 + if element.type == UsfmUpdateBlockElementType.STYLE: + return alignment_info["style_behavior"] == UpdateUsfmMarkerBehavior.PRESERVE + return False + + class PlaceMarkersUsfmUpdateBlockHandler(UsfmUpdateBlockHandler): def __init__(self, *args): super().__init__(*args) @@ -38,20 +56,7 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: len(elements) == 0 or alignment_info["alignment"].row_count == 0 or alignment_info["alignment"].column_count == 0 - or not any( - ( - ( - e.type == UsfmUpdateBlockElementType.PARAGRAPH - and alignment_info["paragraph_behavior"] == UpdateUsfmMarkerBehavior.PRESERVE - and len(e.tokens) == 1 - ) - or ( - e.type == UsfmUpdateBlockElementType.STYLE - and alignment_info["style_behavior"] == UpdateUsfmMarkerBehavior.PRESERVE - ) - ) - for e in elements - ) + or not any(_is_placeable(e, alignment_info) for e in elements) ): return block @@ -74,7 +79,9 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: elements.pop(i) elif not ( element.type == UsfmUpdateBlockElementType.EMBED - or (element.type == UsfmUpdateBlockElementType.TEXT and len(element.tokens[0].to_usfm().strip()) == 0) + # OTHER elements are never transferred, so they must not count as content here + or element.type == UsfmUpdateBlockElementType.OTHER + or (element.type == UsfmUpdateBlockElementType.TEXT and len(_element_text(element).strip()) == 0) ): eob_empty_paras = False @@ -92,7 +99,7 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: for element in elements: if element.type == UsfmUpdateBlockElementType.TEXT: if element.marked_for_removal: - text = element.tokens[0].to_usfm() + text = _element_text(element) src_sent += text # Track seen tokens @@ -103,7 +110,7 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: if len(text.strip()) > 0: src_tok_idx += 1 else: - trg_sent += element.tokens[0].to_usfm() + trg_sent += _element_text(element) if element.marked_for_removal or ( element.type == UsfmUpdateBlockElementType.PARAGRAPH @@ -115,6 +122,8 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: elif element.type in [UsfmUpdateBlockElementType.PARAGRAPH, UsfmUpdateBlockElementType.STYLE]: to_place.append(element) adj_src_toks.append(src_tok_idx) + # OTHER elements (attributes, milestones) are intentionally dropped: they are tied to + # source text that no longer exists, so there is nowhere to transfer them to. if len(trg_sent.strip()) == 0: return block @@ -163,6 +172,11 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: to_insert.sort(key=lambda x: x[0]) to_insert += [(len(trg_sent), element) for element in embed_elements + end_elements] + # The guard above only predicts that something is placeable. If nothing survived + # element collection, leave the block untouched rather than rebuilding its text. + if len(to_insert) == 0: + return block + # Construct new text tokens to put between markers # and reincorporate headers and empty end-of-verse paragraph markers if to_insert[0][0] > 0: diff --git a/machine/corpora/update_usfm_parser_handler.py b/machine/corpora/update_usfm_parser_handler.py index fe405a64..61785c3e 100644 --- a/machine/corpora/update_usfm_parser_handler.py +++ b/machine/corpora/update_usfm_parser_handler.py @@ -235,7 +235,11 @@ def start_note(self, state: UsfmParserState, marker: str, caller: str, category: def end_note(self, state: UsfmParserState, marker: str, closed: bool) -> None: if closed: - self._collect_updatable_tokens(state) + # Mirror start_note: an embed in a duplicate verse is dropped, end marker included. + if self._duplicate_verse: + self._skip_updatable_tokens(state) + else: + self._collect_updatable_tokens(state) super().end_note(state, marker, closed) @@ -264,14 +268,18 @@ def end_char( attributes: Sequence[UsfmAttribute], closed: bool, ) -> None: - if self._current_text_type == ScriptureTextType.EMBED: - self._collect_updatable_tokens(state) - else: - self._replace_with_new_tokens(state) - if self._style_behavior == UpdateUsfmMarkerBehavior.STRIP: - self._skip_updatable_tokens(state) - else: + # An implicitly closed character style has no end marker of its own, so the token at + # state.index belongs to whatever closed it (e.g. the next paragraph marker). Leave it + # for the callback that handles it, as end_note and end_sidebar already do. + if closed: + if self._current_text_type == ScriptureTextType.EMBED: self._collect_updatable_tokens(state) + else: + self._replace_with_new_tokens(state) + if self._style_behavior == UpdateUsfmMarkerBehavior.STRIP: + self._skip_updatable_tokens(state) + else: + self._collect_updatable_tokens(state) super().end_char(state, marker, attributes, closed) diff --git a/tests/corpora/test_place_markers_usfm_update_block_handler.py b/tests/corpora/test_place_markers_usfm_update_block_handler.py index 41eff43a..961e1fc3 100644 --- a/tests/corpora/test_place_markers_usfm_update_block_handler.py +++ b/tests/corpora/test_place_markers_usfm_update_block_handler.py @@ -729,6 +729,176 @@ def test_adjustment_of_placed_paragraph_marker() -> None: assert_usfm_equals(target, result) +def test_unclosed_style_marker_in_non_verse_paragraph() -> None: + # An unclosed character style is closed implicitly by the next paragraph marker, which must + # not be pulled into the block being closed (and then dropped as part of the removed style). + source = "(A)" + pretranslation = "(A translated)" + align_info = PlaceMarkersAlignmentInfo( + source_tokens=[t for t in TOKENIZER.tokenize(source)], + translation_tokens=[t for t in TOKENIZER.tokenize(pretranslation)], + alignment=to_word_alignment_matrix("0-0 1-1 2-2"), + paragraph_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + style_behavior=UpdateUsfmMarkerBehavior.STRIP, + ) + rows = [ + UpdateUsfmRow(scr_ref("PSA 119:0/1:d"), str(pretranslation), metadata={"alignment_info": align_info}), + UpdateUsfmRow(scr_ref("PSA 119:1"), "New verse 1"), + ] + usfm = r"""\id PSA +\c 119 +\d \bd (A) +\q1 +\v 1 Verse 1 +""" + + target = update_usfm(rows, usfm, update_block_handlers=[PlaceMarkersUsfmUpdateBlockHandler()]) + result = r"""\id PSA +\c 119 +\d (A translated) +\q1 +\v 1 New verse 1 +""" + assert_usfm_equals(target, result) + + +def test_unmatched_end_marker() -> None: + # A stray end marker has no matching start marker, so it is marked for removal even when + # styles are preserved. It must not be mistaken for a marker that can be placed. + source = "Section header" + pretranslation = "New section header" + align_info = PlaceMarkersAlignmentInfo( + source_tokens=[t for t in TOKENIZER.tokenize(source)], + translation_tokens=[t for t in TOKENIZER.tokenize(pretranslation)], + alignment=to_word_alignment_matrix("0-1 1-2"), + paragraph_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + style_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + ) + rows = [ + UpdateUsfmRow(scr_ref("MAT 1:0/1:s"), str(pretranslation), metadata={"alignment_info": align_info}), + UpdateUsfmRow(scr_ref("MAT 1:1"), "New verse 1"), + ] + usfm = r"""\id MAT +\c 1 +\s Section header\it* +\p +\v 1 Verse 1 +""" + + target = update_usfm( + rows, + usfm, + style_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + update_block_handlers=[PlaceMarkersUsfmUpdateBlockHandler()], + ) + result = r"""\id MAT +\c 1 +\s New section header +\p +\v 1 New verse 1 +""" + assert_usfm_equals(target, result) + + +def test_marker_behavior_disagrees_with_alignment_info() -> None: + # The behaviors in the alignment info are supplied by the caller and can disagree with the + # ones the updater was built with. The markers are already stripped, so there is nothing to + # place and the block is left alone. + source = "Section header" + pretranslation = "New section header" + align_info = PlaceMarkersAlignmentInfo( + source_tokens=[t for t in TOKENIZER.tokenize(source)], + translation_tokens=[t for t in TOKENIZER.tokenize(pretranslation)], + alignment=to_word_alignment_matrix("0-1 1-2"), + paragraph_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + style_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + ) + rows = [ + UpdateUsfmRow(scr_ref("MAT 1:0/1:s"), str(pretranslation), metadata={"alignment_info": align_info}), + UpdateUsfmRow(scr_ref("MAT 1:1"), "New verse 1"), + ] + usfm = r"""\id MAT +\c 1 +\s Section \it header\it* +\p +\v 1 Verse 1 +""" + + target = update_usfm( + rows, + usfm, + style_behavior=UpdateUsfmMarkerBehavior.STRIP, + update_block_handlers=[PlaceMarkersUsfmUpdateBlockHandler()], + ) + result = r"""\id MAT +\c 1 +\s New section header +\p +\v 1 New verse 1 +""" + assert_usfm_equals(target, result) + + +def test_other_elements_do_not_affect_embed_placement() -> None: + # Attributes, milestones and the like are never transferred, so their presence must not + # change where anything else lands - here, an embed before end-of-verse paragraph markers. + source = "This is the first part. This is the second part." + pretranslation = "Esta es la primera parte. Esta es la segunda parte." + result = r"""\id MAT +\c 1 +\v 1 Esta es la primera parte. Esta es la segunda parte. \f + \ft Footnote\f* +\q1 +\q2 +""" + for milestone in ["", r" \ts-s\*"]: + align_info = PlaceMarkersAlignmentInfo( + source_tokens=[t for t in TOKENIZER.tokenize(source)], + translation_tokens=[t for t in TOKENIZER.tokenize(pretranslation)], + alignment=to_word_alignment_matrix("0-0 1-1 2-2 3-3 4-4 5-5 6-6 7-7 8-8 9-9 10-10"), + paragraph_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + style_behavior=UpdateUsfmMarkerBehavior.STRIP, + ) + rows = [UpdateUsfmRow(scr_ref("MAT 1:1"), str(pretranslation), metadata={"alignment_info": align_info})] + usfm = ( + "\\id MAT\n\\c 1\n" + "\\v 1 This is the first part. This is the second part.\\f + \\ft Footnote\\f*\n" + f"\\q1{milestone}\n\\q2\n" + ) + + target = update_usfm(rows, usfm, update_block_handlers=[PlaceMarkersUsfmUpdateBlockHandler()]) + assert_usfm_equals(target, result) + + +def test_verse_range_matched_by_multiple_rows() -> None: + # A verse range picks up one text token per matched row, so all of them have to be read. + source = "This is the first part. This is the second part." + pretranslation = "Esta es la primera parte. Esta es la segunda parte." + align_info = PlaceMarkersAlignmentInfo( + source_tokens=[t for t in TOKENIZER.tokenize(source)], + translation_tokens=[t for t in TOKENIZER.tokenize(pretranslation)], + alignment=to_word_alignment_matrix("0-0 1-1 2-2 3-3 4-4 5-5 6-6 7-7 8-8 9-9 10-10"), + paragraph_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + style_behavior=UpdateUsfmMarkerBehavior.STRIP, + ) + rows = [ + UpdateUsfmRow(scr_ref("MAT 1:1"), "Esta es la primera parte.", metadata={"alignment_info": align_info}), + UpdateUsfmRow(scr_ref("MAT 1:2"), "Esta es la segunda parte.", metadata={"alignment_info": align_info}), + ] + usfm = r"""\id MAT +\c 1 +\v 1-2 This is the first part. +\p This is the second part. +""" + + target = update_usfm(rows, usfm, update_block_handlers=[PlaceMarkersUsfmUpdateBlockHandler()]) + result = r"""\id MAT +\c 1 +\v 1-2 Esta es la primera parte. +\p Esta es la segunda parte. +""" + assert_usfm_equals(target, result) + + def scr_ref(*refs: str) -> List[ScriptureRef]: return [ScriptureRef.parse(ref) for ref in refs] diff --git a/tests/corpora/test_update_usfm_parser_handler.py b/tests/corpora/test_update_usfm_parser_handler.py index e59a717e..e07f6097 100644 --- a/tests/corpora/test_update_usfm_parser_handler.py +++ b/tests/corpora/test_update_usfm_parser_handler.py @@ -1648,6 +1648,56 @@ def test_filter_chapters_with_bad_chapter_reference() -> None: assert_usfm_equals(target, result) +def test_unclosed_style_marker_does_not_consume_next_paragraph_marker() -> None: + # An unclosed character style has no end marker of its own, so it is closed implicitly by + # the next paragraph marker. That marker belongs to the paragraph it starts, not to the + # style being closed, so it must survive. + rows = [ + UpdateUsfmRow(scr_ref("MAT 1:1"), "New verse 1"), + UpdateUsfmRow(scr_ref("MAT 1:2"), "New verse 2"), + ] + usfm = r"""\id MAT +\c 1 +\q1 +\v 1 Verse 1 \bd Selah +\b +\q1 +\v 2 Verse 2 +""" + + target = update_usfm(rows, usfm) + result = r"""\id MAT +\c 1 +\q1 +\v 1 New verse 1 +\b +\q1 +\v 2 New verse 2 +""" + assert_usfm_equals(target, result) + + # ...including when the unclosed style is in a non-verse paragraph + rows = [ + UpdateUsfmRow(scr_ref("MAT 1:0/1:d"), "New title"), + UpdateUsfmRow(scr_ref("MAT 1:1"), "New verse 1"), + ] + usfm = r"""\id MAT +\c 1 +\d \bd Title +\q1 +\v 1 Verse 1 +""" + + target = update_usfm(rows, usfm) + result = r"""\id MAT +\c 1 +\d New title +\q1 +\v 1 New verse 1 +""" + assert_usfm_equals(target, result) + + def scr_ref(*refs: str) -> List[ScriptureRef]: return [ScriptureRef.parse(ref) for ref in refs] From 12e53e5e3eb73f85c5b4a7ff1b4985d2b33ccde0 Mon Sep 17 00:00:00 2001 From: Ben King Date: Thu, 10 Sep 2026 16:29:43 -0400 Subject: [PATCH 2/3] Clean up Claude's changes --- machine/corpora/__init__.py | 8 ++--- .../paratext_project_text_updater_base.py | 8 ++--- ...place_markers_usfm_update_block_handler.py | 36 +++++-------------- machine/corpora/update_usfm_behavior.py | 12 +++++++ machine/corpora/update_usfm_parser_handler.py | 13 +------ machine/corpora/usfm_update_block_element.py | 15 ++++++++ 6 files changed, 40 insertions(+), 52 deletions(-) create mode 100644 machine/corpora/update_usfm_behavior.py diff --git a/machine/corpora/__init__.py b/machine/corpora/__init__.py index 52c034f6..1fc14d7a 100644 --- a/machine/corpora/__init__.py +++ b/machine/corpora/__init__.py @@ -58,12 +58,8 @@ normalize, unescape_spaces, ) -from .update_usfm_parser_handler import ( - UpdateUsfmMarkerBehavior, - UpdateUsfmParserHandler, - UpdateUsfmRow, - UpdateUsfmTextBehavior, -) +from .update_usfm_behavior import UpdateUsfmMarkerBehavior, UpdateUsfmTextBehavior +from .update_usfm_parser_handler import UpdateUsfmParserHandler, UpdateUsfmRow from .usfm_file_text import UsfmFileText from .usfm_file_text_corpus import UsfmFileTextCorpus from .usfm_memory_text import UsfmMemoryText diff --git a/machine/corpora/paratext_project_text_updater_base.py b/machine/corpora/paratext_project_text_updater_base.py index 7eee05fc..188f31c7 100644 --- a/machine/corpora/paratext_project_text_updater_base.py +++ b/machine/corpora/paratext_project_text_updater_base.py @@ -5,12 +5,8 @@ from .paratext_project_file_handler import ParatextProjectFileHandler from .paratext_project_settings import ParatextProjectSettings from .paratext_project_settings_parser_base import ParatextProjectSettingsParserBase -from .update_usfm_parser_handler import ( - UpdateUsfmMarkerBehavior, - UpdateUsfmParserHandler, - UpdateUsfmRow, - UpdateUsfmTextBehavior, -) +from .update_usfm_behavior import UpdateUsfmMarkerBehavior, UpdateUsfmTextBehavior +from .update_usfm_parser_handler import UpdateUsfmParserHandler, UpdateUsfmRow from .usfm_parser import parse_usfm from .usfm_token import UsfmTokenType from .usfm_tokenizer import UsfmToken, UsfmTokenizer diff --git a/machine/corpora/place_markers_usfm_update_block_handler.py b/machine/corpora/place_markers_usfm_update_block_handler.py index 4ceddadc..421d8b2b 100644 --- a/machine/corpora/place_markers_usfm_update_block_handler.py +++ b/machine/corpora/place_markers_usfm_update_block_handler.py @@ -4,7 +4,7 @@ from ..translation.word_alignment_matrix import WordAlignmentMatrix from .segment_boundary_adjuster import SegmentBoundaryAdjuster -from .update_usfm_parser_handler import UpdateUsfmMarkerBehavior +from .update_usfm_behavior import UpdateUsfmMarkerBehavior from .usfm_token import UsfmToken, UsfmTokenType from .usfm_update_block import UsfmUpdateBlock from .usfm_update_block_element import UsfmUpdateBlockElement, UsfmUpdateBlockElementType @@ -21,24 +21,6 @@ class PlaceMarkersAlignmentInfo(TypedDict): style_behavior: UpdateUsfmMarkerBehavior -def _element_text(element: UsfmUpdateBlockElement) -> str: - # A TEXT element holds one token per row matched to the block, so a verse range matched by - # several rows has several tokens. Reading only the first drops the rest of the translation. - return "".join(t.to_usfm() for t in element.tokens) - - -def _is_placeable(element: UsfmUpdateBlockElement, alignment_info: PlaceMarkersAlignmentInfo) -> bool: - # An element marked for removal is never placed, so it must not keep the block from - # returning early below: the placement code would then have nothing left to place. - if element.marked_for_removal: - return False - if element.type == UsfmUpdateBlockElementType.PARAGRAPH: - return alignment_info["paragraph_behavior"] == UpdateUsfmMarkerBehavior.PRESERVE and len(element.tokens) == 1 - if element.type == UsfmUpdateBlockElementType.STYLE: - return alignment_info["style_behavior"] == UpdateUsfmMarkerBehavior.PRESERVE - return False - - class PlaceMarkersUsfmUpdateBlockHandler(UsfmUpdateBlockHandler): def __init__(self, *args): super().__init__(*args) @@ -56,7 +38,9 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: len(elements) == 0 or alignment_info["alignment"].row_count == 0 or alignment_info["alignment"].column_count == 0 - or not any(_is_placeable(e, alignment_info) for e in elements) + or not any( + e.is_placeable(alignment_info["paragraph_behavior"], alignment_info["style_behavior"]) for e in elements + ) ): return block @@ -79,9 +63,8 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: elements.pop(i) elif not ( element.type == UsfmUpdateBlockElementType.EMBED - # OTHER elements are never transferred, so they must not count as content here or element.type == UsfmUpdateBlockElementType.OTHER - or (element.type == UsfmUpdateBlockElementType.TEXT and len(_element_text(element).strip()) == 0) + or (element.type == UsfmUpdateBlockElementType.TEXT and len(element.get_text().strip()) == 0) ): eob_empty_paras = False @@ -99,7 +82,7 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: for element in elements: if element.type == UsfmUpdateBlockElementType.TEXT: if element.marked_for_removal: - text = _element_text(element) + text = element.get_text() src_sent += text # Track seen tokens @@ -110,7 +93,7 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: if len(text.strip()) > 0: src_tok_idx += 1 else: - trg_sent += _element_text(element) + trg_sent += element.get_text() if element.marked_for_removal or ( element.type == UsfmUpdateBlockElementType.PARAGRAPH @@ -122,8 +105,6 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: elif element.type in [UsfmUpdateBlockElementType.PARAGRAPH, UsfmUpdateBlockElementType.STYLE]: to_place.append(element) adj_src_toks.append(src_tok_idx) - # OTHER elements (attributes, milestones) are intentionally dropped: they are tied to - # source text that no longer exists, so there is nowhere to transfer them to. if len(trg_sent.strip()) == 0: return block @@ -172,8 +153,7 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: to_insert.sort(key=lambda x: x[0]) to_insert += [(len(trg_sent), element) for element in embed_elements + end_elements] - # The guard above only predicts that something is placeable. If nothing survived - # element collection, leave the block untouched rather than rebuilding its text. + # In the case of unclosed markers, to_insert might be empty if len(to_insert) == 0: return block diff --git a/machine/corpora/update_usfm_behavior.py b/machine/corpora/update_usfm_behavior.py new file mode 100644 index 00000000..ed172cf4 --- /dev/null +++ b/machine/corpora/update_usfm_behavior.py @@ -0,0 +1,12 @@ +from enum import Enum, auto + + +class UpdateUsfmTextBehavior(Enum): + PREFER_EXISTING = auto() + PREFER_NEW = auto() + STRIP_EXISTING = auto() + + +class UpdateUsfmMarkerBehavior(Enum): + PRESERVE = auto() + STRIP = auto() diff --git a/machine/corpora/update_usfm_parser_handler.py b/machine/corpora/update_usfm_parser_handler.py index 61785c3e..3020773b 100644 --- a/machine/corpora/update_usfm_parser_handler.py +++ b/machine/corpora/update_usfm_parser_handler.py @@ -1,9 +1,9 @@ -from enum import Enum, auto from typing import Callable, Dict, Iterable, List, Optional, Sequence, Tuple, Union from ..scripture.verse_ref import IgnoreSegmentsVerseRef, VerseRef, Versification from .scripture_ref import ScriptureRef from .scripture_ref_usfm_parser_handler_base import ScriptureRefUsfmParserHandlerBase, ScriptureTextType +from .update_usfm_behavior import UpdateUsfmMarkerBehavior, UpdateUsfmTextBehavior from .usfm_parser_state import UsfmParserState from .usfm_stylesheet import UsfmStylesheet from .usfm_tag import UsfmTextType @@ -14,17 +14,6 @@ from .usfm_update_block_handler import UsfmUpdateBlockHandler, UsfmUpdateBlockHandlerError -class UpdateUsfmTextBehavior(Enum): - PREFER_EXISTING = auto() - PREFER_NEW = auto() - STRIP_EXISTING = auto() - - -class UpdateUsfmMarkerBehavior(Enum): - PRESERVE = auto() - STRIP = auto() - - class _RowInfo: def __init__(self, row_index: int): self.row_index = row_index diff --git a/machine/corpora/usfm_update_block_element.py b/machine/corpora/usfm_update_block_element.py index 7a4c01c6..045002ff 100644 --- a/machine/corpora/usfm_update_block_element.py +++ b/machine/corpora/usfm_update_block_element.py @@ -1,6 +1,7 @@ from dataclasses import dataclass from enum import Enum, auto +from .update_usfm_behavior import UpdateUsfmMarkerBehavior from .usfm_token import UsfmToken @@ -22,3 +23,17 @@ def get_tokens(self) -> list[UsfmToken]: if self.marked_for_removal: return [] return self.tokens.copy() + + def get_text(self) -> str: + return "".join(t.to_usfm() for t in self.tokens) + + def is_placeable( + self, paragraph_behavior: UpdateUsfmMarkerBehavior, style_behavior: UpdateUsfmMarkerBehavior + ) -> bool: + if self.marked_for_removal: + return False + if self.type == UsfmUpdateBlockElementType.PARAGRAPH: + return paragraph_behavior == UpdateUsfmMarkerBehavior.PRESERVE and len(self.tokens) == 1 + if self.type == UsfmUpdateBlockElementType.STYLE: + return style_behavior == UpdateUsfmMarkerBehavior.PRESERVE + return False From 9ddc6769219137c7efaba0141a6e25e5443bcb5b Mon Sep 17 00:00:00 2001 From: Ben King Date: Mon, 14 Sep 2026 15:20:01 -0400 Subject: [PATCH 3/3] Clarify name of multi-row test + comment --- tests/corpora/test_place_markers_usfm_update_block_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/corpora/test_place_markers_usfm_update_block_handler.py b/tests/corpora/test_place_markers_usfm_update_block_handler.py index 961e1fc3..b0d99a91 100644 --- a/tests/corpora/test_place_markers_usfm_update_block_handler.py +++ b/tests/corpora/test_place_markers_usfm_update_block_handler.py @@ -869,8 +869,8 @@ def test_other_elements_do_not_affect_embed_placement() -> None: assert_usfm_equals(target, result) -def test_verse_range_matched_by_multiple_rows() -> None: - # A verse range picks up one text token per matched row, so all of them have to be read. +def test_multiple_text_rows_in_verse_ranges_are_updated() -> None: + # Verse ranges contain multiple text rows, which must be processed as if they were a single row source = "This is the first part. This is the second part." pretranslation = "Esta es la primera parte. Esta es la segunda parte." align_info = PlaceMarkersAlignmentInfo(