OCTO-11626

.claude/skills/gotchas.md

@@ -215,3 +215,13 @@ And: the samples decide the sensitivity. A plain-text sample reports "same" on a
 **Rule:** Spawn children with `sys.executable`, never a bare `python` — the interpreter running the script is the one guaranteed to exist. Any section that shells out gets an `except Exception` that degrades to a reported skip; a gate must never be able to cost the PR its report. Verify by running under `env -i PATH=/usr/bin:/bin`, which has `python3` but no `python`.
 
 **Applies to:** `check-last-pr`, any skill spawning a subprocess interpreter
+
+---
+
+## 18. `Caption.get_text()` strips, so it hides leading/trailing whitespace changes
+
+**What happened:** Reviewing PR #436 (OCTO-11623, first-line NBSP indentation), a probe compared `caption.get_text()` before and after a DFXP round trip and showed the first line's NBSPs as lost — an apparent defect in the very thing the PR fixed. `Caption.get_text()` (`pycaption/base.py`) ends in `.strip()`, and Python's `str.strip()` counts U+00A0 as whitespace. The nodes were correct.
+
+**Rule:** Any check about indentation, NBSP, or edge whitespace must compare `caption.nodes` (or `get_text_nodes()`), never `get_text()`. The behavior gate's `text_of()` uses `get_text()` too, so it is blind to this whole class of change — its "no flow changed" says nothing about whitespace PRs. Its built-in samples also carry no NBSP indentation; add one when a PR touches whitespace handling.
+
+**Applies to:** `check-last-pr`

docs/changelog.rst

@@ -9,6 +9,9 @@ Changelog
   - Fix ``DFXPReader`` and ``SAMIReader`` stripping non-breaking spaces at
     the start of a line along with the markup's pretty-print whitespace, so
     line indentation now survives a round trip.
+  - ``SCCReader`` line-length errors now end with ``(continues on the row
+    without a row change)`` when a line ran long because the next line's
+    text stayed on the same row with no PAC or row change of its own.
 
 2.3.12
 ^^^^^^

pycaption/scc/reader.py

@@ -112,6 +112,10 @@
 
 _TIMECODE_RE = re.compile(r"\d{2}:\d{2}:\d{2}[:;](\d{1,2})")
 
+# Appended to a too-long line whose text kept going on the row with no PAC or
+# row change of its own. Matched downstream, so the wording is part of the API.
+ROW_CONTINUATION_SUFFIX = " (continues on the row without a row change)"
+
 
 class SCCReader(BaseReader):
     """Converts a given unicode string to a CaptionSet.
@@ -243,30 +247,42 @@ def _validate_frame_numbers(self):
 
     def _validate_line_lengths(self):
         """Raise CaptionLineLengthError if any line exceeds 32 characters."""
-        violations = [
-            f"around {cap.format_start()} - {self._describe_line_length(line)}"
-            for cap in (c.to_real_caption() for c in self.caption_stash._collection)
-            for line in cap.get_text().split("\n")
-            if len(line) > 32
-        ]
+        violations = []
+        for precap in self.caption_stash._collection:
+            cap = precap.to_real_caption()
+            raw = "".join(cap.get_text_nodes())
+            # get_text() strips the caption, dropping any lines it opens with
+            skipped = raw[: len(raw) - len(raw.lstrip())].count("\n")
+            for index, line in enumerate(cap.get_text().split("\n")):
+                if len(line) > 32:
+                    description = self._describe_line_length(
+                        line, (index + skipped) in precap.continued_rows
+                    )
+                    violations.append(f"around {cap.format_start()} - {description}")
         if violations:
             raise CaptionLineLengthError(
                 "32 character limit for caption cue in scc file.\n"
                 "Lines longer than 32:\n" + "\n".join(violations)
             )
 
     @staticmethod
-    def _describe_line_length(line):
+    def _describe_line_length(line, continues_row=False):
         """Describe a line too long for the row, separating the text from the
-        indentation that places it, which can be what pushes it off the row.
+        indentation that places it, which can be what pushes it off the row,
+        and naming a missing PAC or row change when that is what joined it.
         """
         text = line.lstrip("\xa0")
         indent = len(line) - len(text)
         if not indent:
-            return f"{line} - Length {len(line)}"
-        return (
-            f"{text} - Length {len(text)} plus a {indent}-column indent = {len(line)}"
-        )
+            description = f"{line} - Length {len(line)}"
+        else:
+            description = (
+                f"{text} - Length {len(text)} plus a {indent}-column indent "
+                f"= {len(line)}"
+            )
+        if continues_row:
+            description += ROW_CONTINUATION_SUFFIX
+        return description
 
     @staticmethod
     def _fix_last_captions_without_ending(caption_list):
@@ -312,6 +328,7 @@ def _translate_line(self, line):
         parts = r.findall(line.lower())
 
         self.time_translator.start_at(parts[0][0])
+        self.position_tracker.start_burst()
         word_list = parts[0][2].split(" ")
 
         for idx, word in enumerate(word_list):

pycaption/scc/specialized_collections.py

@@ -53,6 +53,9 @@ def __init__(self, start=0, end=0):
         self.nodes = []
         self.style = {}
         self.layout_info = None
+        # Indices of the lines whose text continued on the row without a PAC
+        # or row change, for the line length check to name the cause
+        self.continued_rows = set()
 
     def to_real_caption(self):
         """Convert this mutable holder into an immutable Caption instance.
@@ -274,6 +277,7 @@ def create_and_store(
             _leftmost_line_starts(instructions) if caption_mode == "pop_on" else []
         )
         cue_index = 0
+        line_index = 0
         indenter = _LineIndenter(enabled=bool(leftmost_columns))
 
         for instruction in instructions:
@@ -289,8 +293,14 @@ def create_and_store(
                 self._still_editing.append(caption)
                 cue_index += 1
                 indenter.start_cue()
+                line_index = 0
                 continue
 
+            if instruction.is_explicit_break():
+                line_index += 1
+            elif instruction.continues_row:
+                caption.continued_rows.add(line_index)
+
             text = indenter.text_for(instruction, position)
             _append_caption_node(caption, instruction, text, position)
 
@@ -381,17 +391,39 @@ def add_chars(self, *chars):
             self._collection.append(node)
             self._position_tracer.acknowledge_position_changed()
 
+        # Where a continuation is recorded: on the overwrite marker if the text
+        # lands on columns a PAC pointed back over, so a row sent again in full
+        # can drop it without dropping one of text arriving later
+        continued = node
         overlap = self._position_tracer.consume_pending_overlap()
         if overlap and self._mark_overwrite(overlap, current_position):
+            continued = self._collection[-1]
             node = _InstructionNode.create_text(current_position)
             self._collection.append(node)
 
+        # The tracker is shared by the buffers, so the row it saw text on may
+        # be another buffer's: only this buffer's own text can be continued
+        if self._position_tracer.consume_row_continuation() and self._line_has_text():
+            continued.continues_row = True
+
         padding = self._position_tracer.consume_pending_padding()
         if padding:
             node.add_chars(" " * padding)
         node.add_chars(*chars)
         self._position_tracer.advance_cursor(len("".join(chars)))
 
+    def _line_has_text(self):
+        """Whether the line being written in this buffer already holds text
+
+        :rtype: bool
+        """
+        for node in reversed(self._collection):
+            if _ends_line(node):
+                return False
+            if node.is_text_node() and node.text:
+                return True
+        return False
+
     def _settle_line_start(self):
         """Record on the break opening this line the column it now starts at
 
@@ -645,7 +677,11 @@ def from_list(cls, stash_list, position_tracker):
         for idx, stash in enumerate(stash_list):
             # Resolved per stash: each is a row of its own, so an overwrite
             # is measured against its row rather than the rows joined together
-            new_collection.extend(_resolve_overwrites(stash._collection))
+            new_collection.extend(
+                _clear_continuations_of_fitting_rows(
+                    _resolve_overwrites(stash._collection)
+                )
+            )
 
             # use space to separate the stashes, but don't add final space
             if idx < len(stash_list) - 1:
@@ -878,6 +914,10 @@ def __init__(
         # only place a later line's own column survives
         self.line_start = line_start
         self.columns = columns
+        # For text, whether it continued a row already holding text, with no
+        # PAC or row change of its own; for an overwrite, whether the text
+        # after it did
+        self.continues_row = False
 
     def add_chars(self, *args):
         """This being a text node, add characters to it.
@@ -1099,7 +1139,10 @@ def _resolve_line_overwrites(line, line_start):
     """
     if not any(node.is_overwrite() for node in line):
         return line
-    line = [copy.copy(node) if node.is_text_node() else node for node in line]
+    line = [
+        copy.copy(node) if node.is_text_node() or node.is_overwrite() else node
+        for node in line
+    ]
     if line_start is None:
         line_start = next(
             (
@@ -1120,9 +1163,24 @@ def _resolve_line_overwrites(line, line_start):
                 arriving += len(node.text)
         if line_start + _line_length(line) > 32 and arriving >= marker.columns:
             _drop_last_columns(line[:index], marker.columns)
+            if not _line_length(line[:index]):
+                # The row was sent again in full: nothing is left of it for
+                # the text arriving to have continued
+                _clear_continuations(line[: index + 1])
     return _join_across_markers(line)
 
 
+def _clear_continuations(nodes):
+    """Clear the row continuation of the text and overwrite nodes, the line's
+    own copies
+
+    :type nodes: list[_InstructionNode]
+    """
+    for node in nodes:
+        if node.is_text_node() or node.is_overwrite():
+            node.continues_row = False
+
+
 def _join_across_markers(line):
     """Drop the overwrite markers, joining the text nodes each one split
 
@@ -1138,7 +1196,11 @@ def _join_across_markers(line):
     for node in line:
         if node.is_overwrite():
             after_marker = True
+            continued = node.continues_row
             continue
+        if after_marker and node.is_text_node():
+            # The text the marker opened, a copy, takes on its continuation
+            node.continues_row |= continued
         if (
             after_marker
             and node.is_text_node()
@@ -1147,12 +1209,38 @@ def _join_across_markers(line):
         ):
             if node.text:
                 joined[-1].add_chars(node.text)
+            joined[-1].continues_row |= node.continues_row
         else:
             joined.append(node)
         after_marker = False
     return joined
 
 
+def _clear_continuations_of_fitting_rows(collection):
+    """Clear the row continuation of every line that fits the row
+
+    Simulated roll-up joins its rows onto one line, which can run past column
+    32 although no row did. The continuation has to be judged on its own row,
+    before the join, or it would take the blame for a length it did not cause.
+
+    The nodes are cleared in place, not copied: the row's nodes are shared with
+    the stash it is re-read from, and a row that fits keeps fitting.
+
+    :type collection: list[_InstructionNode]
+    :rtype: list[_InstructionNode]
+    """
+    line = []
+    for node in collection + [None]:
+        if node is not None and not _ends_line(node):
+            line.append(node)
+            continue
+        if _line_length(line) <= 32:
+            for line_node in line:
+                line_node.continues_row = False
+        line = []
+    return collection
+
+
 def _drop_last_columns(line, columns):
     """Drop the last ``columns`` columns of text from the nodes of a line
 

pycaption/scc/state_machines.py

@@ -43,6 +43,10 @@ def __init__(self, positioning=None):
         self._pending_padding = 0
         # Columns a PAC pointed back over, not yet known to be an overwrite.
         self._pending_overlap = 0
+        # Text written next lands on a row that already holds text, reached
+        # with no PAC or row change of its own: a same-row PAC, or a later
+        # line of the file picking up where the row left off.
+        self._row_continued = False
 
     def update_positioning(
         self, positioning, column_jump_forces_reposition=False, is_offset=False
@@ -171,6 +175,7 @@ def _jump_rows_forward(self, positioning, row, col, column_jump_forces_repositio
             self._pending_overlap = 0
             self._pending_padding = 0
             self._line_has_text = False
+            self._row_continued = False
         # A skipped row, or a row+1 jump with a large paint-on column jump:
         # use repositioning (new cue)
         else:
@@ -218,6 +223,10 @@ def _continue_line(self, positioning, cursor, is_offset):
             self._last_column = new_col
             self._cursor_column = new_col
         else:
+            if not is_offset and self._line_has_text:
+                # A PAC on the row being written moves on along it instead of
+                # opening a row of its own
+                self._row_continued = True
             overlap = cursor - new_col
             if overlap < 0:
                 # The columns between the text and the PAC are blank
@@ -258,6 +267,7 @@ def _reposition(self, positioning):
         # and its text is no longer in front of the cursor to be overwritten
         self._pending_padding = 0
         self._pending_overlap = 0
+        self._row_continued = False
 
     def advance_cursor(self, count):
         """Notify the tracker that ``count`` columns of text were written.
@@ -295,6 +305,22 @@ def consume_pending_overlap(self):
         overlap, self._pending_overlap = self._pending_overlap, 0
         return overlap
 
+    def start_burst(self):
+        """Note that a new line of the file starts: text it writes before any
+        positioning command continues the row the previous line left off on.
+        """
+        if self._line_has_text:
+            self._row_continued = True
+
+    def consume_row_continuation(self):
+        """Return and clear whether the text arriving next continues a row
+        without a PAC or row change of its own.
+
+        :rtype: bool
+        """
+        continued, self._row_continued = self._row_continued, False
+        return continued
+
     def cancel_pending_overlap(self):
         """Drop the overlap, the column the PAC pointed at being taken by a
         command rather than by text.
@@ -361,6 +387,7 @@ def reset_for_new_caption(self):
         self._line_has_text = False
         self._pending_padding = 0
         self._pending_overlap = 0
+        self._row_continued = False
         # Reset positions to None so the next PAC sets position fresh
         self._positions = [None]
 
@@ -405,9 +432,19 @@ def update_positioning(
         :param positioning: a tuple of ints (row, col)
         :type positioning: tuple[int]
         """
+        # Text written before any PAC was placed on the default row
+        unplaced_row = (
+            self.default[0]
+            if self._line_has_text and not any(self._positions)
+            else None
+        )
         if positioning:
             self.default = positioning
 
         super().update_positioning(
             positioning, column_jump_forces_reposition, is_offset
         )
+        if not is_offset and positioning and positioning[0] == unplaced_row:
+            # The PAC names the row that text already took, so the text after
+            # it carries on along the row instead of opening one of its own
+            self._row_continued = True