diff --git a/fnd/tui/match_navigator.py b/fnd/tui/match_navigator.py index 7bf42ff7..fba9cfa7 100644 --- a/fnd/tui/match_navigator.py +++ b/fnd/tui/match_navigator.py @@ -130,7 +130,7 @@ def offscreen_views(anchors: list[int], stops: list[int], top: int, vh: int) -> # prove only that two samples matched, and this cache has already been wrong # that way twice. # -# The read is `enumerate_stop_regions` over the pane subtree, so it scales with +# The read is `enumerate_stop_rows` over the pane subtree, so it scales with # the mounted STOPS, not just the chunks: 1.2ms warm at 13 chunks / 36 stops, # 2.6ms at 102 chunks / 882 stops (the densest in this corpus) and 3.5ms at 1592 # stops on a synthetic shape denser than anything in it, against a @@ -155,10 +155,10 @@ class MatchNavigator: nav scroll-settle window (region reads did — measured to stall the landing). It is cached; the footer reads the cache, so a focus change never re-walks the preview subtree (a perf contract the pane keeps). - * **Navigation** (``next``/``prev``) enumerates the match REGIONS fresh on + * **Navigation** (``next``/``prev``) enumerates the match stops fresh on the keypress — a snapshot goes stale as the lazy-mounted preview reflows (the reported bug: n did nothing because the snapshot was captured empty - before a deep table laid out). Region reads here are fine: they happen on + before a deep table laid out). Layout reads here are fine: they happen on a deliberate keypress, never during a cold-nav settle. * **The ▲a/▼b view arrows** answer the original awareness question — "does the result I'm on have matches I can't see?" Users navigate *between* @@ -166,7 +166,7 @@ class MatchNavigator: lower down in the *same* chunk; the arrows count how many screenfuls ("views") of the CURRENT result hold an off-screen match, above and below. Everything here is scoped to the current chunk (``anchor.focus_chunk_seq`` - and its widget extent) — never the whole file. The counts ARE region- + and its widget extent) — never the whole file. The counts ARE layout- derived, but the read is deferred to settle-safe moments only (after the mount settles, on a nav keypress, on a user scroll) and cached, so the border refresh stays layout-free. @@ -246,10 +246,10 @@ def _stops_within(self, root: Widget, spec: MatchSpec) -> int: layout is forced: one per matching table cell, one per non-table match block, one per matching plain line. - Deliberately per BLOCK where ``enumerate_stop_regions`` is per row: the + Deliberately per BLOCK where ``enumerate_stop_rows`` is per row: the rows need layout, and this runs on paths that must not force it. They go to zero together on a laid-out subtree only: a mounted block with no - geometry counts here and yields no region, so the hint can lead the keys + geometry counts here and yields no row, so the hint can lead the keys while a fill is hiding chunks. Shared by the preview-wide count and the current-chunk check so the two @@ -299,17 +299,12 @@ def _count_stops(self, pane: VerticalScroll) -> int: """Stops across the whole mounted preview — the footer-hint cache.""" return self._stops_within(pane, self._app._effective_match_spec) - def _region_stops(self, pane: VerticalScroll) -> list[int]: - """Match stops as content-space tops (reads regions — call only on a + def _content_stops(self, pane: VerticalScroll) -> list[int]: + """Match stops as content rows (forces layout, so call only on a deliberate navigation keypress, never during a cold-nav settle).""" - from fnd.tui.preview_scroll import enumerate_stop_regions + from fnd.tui.preview_scroll import enumerate_stop_rows - spec = self._app._effective_match_spec - if spec.is_empty: - return [] - base = pane.scrollable_content_region.offset.y - oy = pane.scroll_offset.y - return sorted(r.y - base + oy for r in enumerate_stop_regions(pane, spec)) + return enumerate_stop_rows(pane, self._app._effective_match_spec) def _current_chunk_extent(self, pane: VerticalScroll) -> tuple[int, int] | None: """Content-space ``[top, bottom)`` of the CURRENT result's chunk — the @@ -330,20 +325,15 @@ def _current_chunk_extent(self, pane: VerticalScroll) -> tuple[int, int] | None: cur = widgets.get(seq) if cur is None: return None - base = pane.scrollable_content_region.offset.y - oy = pane.scroll_offset.y def ctop(w: object) -> int | None: - region = getattr(w, "region", None) - if region is None or region.height <= 0: - return None - return region.y - base + oy + return self._chunk_top(pane, w) top = ctop(cur) if top is None: return 0, 0 # EMPTY, not None: None unscopes the caller (see docstring) # Bound on the next chunk that HAS laid out: a zero-height one leaks no - # stop (every arm of ``enumerate_stop_regions`` drops one), and for a + # stop (every arm of ``enumerate_stop_rows`` drops one), and for a # plain chunk ``chunk_widgets`` holds only its first LINE. laters = [ y for s2, w in widgets.items() if s2 > seq and (y := ctop(w)) is not None and y > top @@ -351,11 +341,22 @@ def ctop(w: object) -> int | None: bottom = min(laters) if laters else max(top + 1, pane.virtual_size.height) return top, bottom + @staticmethod + def _chunk_top(pane: VerticalScroll, widget: object) -> int | None: + """``widget``'s content row, or ``None`` while it is not laid out.""" + from textual.widget import Widget + + from fnd.tui.preview.match_row import layout_offset + + if not isinstance(widget, Widget): + return None + return layout_offset(widget, pane, into_ancestor=False) + def _chunk_stops(self, pane: VerticalScroll) -> list[int]: """The current result's match stops (content-space tops). An unknown extent (flat preview) falls back to every mounted stop — which on that substrate is none, so the keys are inert rather than unscoped.""" - stops = self._region_stops(pane) + stops = self._content_stops(pane) extent = self._current_chunk_extent(pane) if extent is None: return stops @@ -365,12 +366,12 @@ def _chunk_stops(self, pane: VerticalScroll) -> list[int]: def _offscreen_views(self, pane: VerticalScroll) -> tuple[int, int]: """Matching views above / below the viewport, scoped to the current result. ``(0, 0)`` when the chunk extent is unknown (no false signal on - flat previews). Reads regions — settle-safe callers only.""" + flat previews). Forces layout: settle-safe callers only.""" extent = self._current_chunk_extent(pane) if extent is None: return 0, 0 lo, hi = extent - stops = [y for y in self._region_stops(pane) if lo <= y < hi] + stops = [y for y in self._content_stops(pane) if lo <= y < hi] vh = pane.scrollable_content_region.height # Not pinned here: a landing may still be committing. _go pins it. home = self._home(lo) @@ -649,7 +650,7 @@ def _recount_if_empty(self) -> None: def _measure_offscreen(self) -> None: """Re-derive the cached ▲/▼ view counts (current result, above/below the - viewport) and refresh the border only when they changed. Reads regions — + viewport) and refresh the border only when they changed. Forces layout: call only when the scroll is settled (post-settle, a deliberate nav, or a user scroll).""" pane = self._pane() diff --git a/fnd/tui/preview/frozen.py b/fnd/tui/preview/frozen.py index b24bc486..3ed57b3a 100644 --- a/fnd/tui/preview/frozen.py +++ b/fnd/tui/preview/frozen.py @@ -131,7 +131,7 @@ def freeze(chunk: Widget, chunk_seq: int) -> FrozenChunk | None: cell_rows: dict[tuple[int, int, int], int] = {} if isinstance(chunk, FNDMarkdown): inner = chunk.first_match_block - # The chunk's own spec, so this asks exactly what enumerate_stop_regions + # The chunk's own spec, so this asks exactly what enumerate_stop_rows # asks of the live blocks — a block whose spans were cleared after it # registered would otherwise get a scanned row live and row 0 frozen. spec = chunk.match_spec diff --git a/fnd/tui/preview/match_row.py b/fnd/tui/preview/match_row.py index 95e50e60..082e471f 100644 --- a/fnd/tui/preview/match_row.py +++ b/fnd/tui/preview/match_row.py @@ -156,7 +156,7 @@ def _match_rows(block: Widget, spec: MatchSpec | None) -> list[int]: offsets = _match_offsets(block, plain, spec) if not offsets: return [] - # Cached per block: ``enumerate_stop_regions`` asks every mounted match block + # Cached per block: ``enumerate_stop_rows`` asks every mounted match block # on every n/b press — 56 wrapped 4,000-char paragraphs cost 13.8ms cold and # 0.03ms after. Geometry is in the key, so a re-wrap recomputes. key = (content.width, content.height, len(plain), tuple(offsets)) diff --git a/fnd/tui/preview_scroll.py b/fnd/tui/preview_scroll.py index 481c4258..79253fc8 100644 --- a/fnd/tui/preview_scroll.py +++ b/fnd/tui/preview_scroll.py @@ -1070,15 +1070,16 @@ def _scroll_proxy_for(self, inner: Widget, *, chunk: FNDMarkdown) -> Widget: def locate(self) -> ViewportLocation | None: """The chunk whose top is nearest the viewport top, and the signed rows - between them. Only the boundary survives a re-wrap, so the nearest one - bounds the error: 13 rows out to 0, 60 rows into a 62-row chunk.""" + between them, in the layout frame ``scroll_to_location`` restores in. + Only the boundary survives a re-wrap, so the nearest one bounds the + error: 13 rows out to 0, 60 rows into a 62-row chunk.""" pane = self._host.preview_pane() - top = pane.scrollable_content_region.y + top = int(pane.scroll_offset.y) nearest: tuple[int, int] | None = None for seq, w in self._host.chunk_widgets.items(): - r = w.region + r = w.virtual_region if r.height <= 0: - continue # culled: NULL_REGION reads as y=0, not as a position + continue # culled: an empty region reads as y=0, not as a position offset = top - r.y if nearest is None or abs(offset) < abs(nearest[0]): nearest = (offset, seq) @@ -1210,33 +1211,35 @@ def _restore_structural( done() -def stop_region_for_cell(table: DataTable[Any], coord: Any) -> Region | None: - """Screen-space region of a DataTable cell, or ``None`` if unresolved - (rows unmounted / not sized). Mirrors ``StructuralScrollStrategy. - _anchor_region``: the full-height table has no internal scroll, but honour - its offset defensively.""" +def stop_row_in_table(table: DataTable[Any], coord: Any) -> int | None: + """A DataTable cell's row below the table's top, or ``None`` while its rows + are unsized. The full-height table has no internal scroll, but its offset is + honoured defensively, as ``StructuralScrollStrategy._anchor_region`` does.""" try: cell = table._get_cell_region(coord) # pyright: ignore[reportAttributeAccessIssue] except Exception: return None if cell.height == 0: return None - return cell.translate(table.region.offset - table.scroll_offset) + return cell.y - int(table.scroll_offset.y) -def enumerate_stop_regions(pane: VerticalScroll, spec: MatchSpec) -> list[Region]: - """Every match stop's screen-space region across the pane's mounted - chunks, sorted by content-space y. Tables expand to one region per matching - cell (``_fnd_match_coords``); other ``FNDMarkdown`` blocks contribute one - per row they paint a match on; plain (pdf/txt) chunks contribute one per - matching body line. +def enumerate_stop_rows(pane: VerticalScroll, spec: MatchSpec) -> list[int]: + """Every match stop's content row across the pane's mounted chunks, sorted. + Tables expand to one stop per matching cell (``_fnd_match_coords``); other + ``FNDMarkdown`` blocks contribute one per row they paint a match on; plain + (pdf/txt) chunks contribute one per matching body line. - Off-screen cells of a mounted table resolve fine — the table is one - full-height widget — so a big flashcards/glossary table is fully covered + Read from layout positions, never ``region``: a region lags a scroll until + the next render, and reading one off screen rebuilds the map at the live + offset, so regions in one pass need not share a frame. + + Off-screen cells of a mounted table resolve fine (the table is one + full-height widget), so a big flashcards or glossary table is fully covered once its chunk is mounted. Queries descendants (chunks live inside a ``PreviewContainer``), not just the pane's direct children. - A FROZEN chunk has no blocks to walk — that is the point of freezing — so its + A FROZEN chunk has no blocks to walk (that is the point of freezing), so its stops come from the rows recorded at capture time. Without this a frozen chunk contributes nothing, and its matches become unreachable by ``n``/``b`` and invisible to the off-screen markers: the failure is silent, which is @@ -1245,7 +1248,7 @@ def enumerate_stop_regions(pane: VerticalScroll, spec: MatchSpec) -> list[Region from fnd.render import text_has_any_match from fnd.tui.preview.frozen import FrozenChunkView - from fnd.tui.preview.match_row import region_at_row, rows_to_matches + from fnd.tui.preview.match_row import layout_offset, rows_to_matches from fnd.tui.widgets.markdown import ( FNDMarkdown, FNDMarkdownTableDT, @@ -1253,49 +1256,53 @@ def enumerate_stop_regions(pane: VerticalScroll, spec: MatchSpec) -> list[Region FNDMarkdownTH, ) - regions: list[Region] = [] + rows: list[int] = [] if spec.is_empty: - return regions + return rows for md in pane.query(FNDMarkdown): + md_top = layout_offset(md, pane, into_ancestor=False) + if md_top is None: + continue # Tables: query the DataTable directly for every matching cell. (The # table's TH/TD cells also self-register in match_blocks as phantom, - # never-mounted blocks — skip them below; the table owns their cells.) + # never-mounted blocks; skip them below, the table owns their cells.) for dt in md.query(DataTable): + dt_top = layout_offset(dt, md, into_ancestor=True) + if dt_top is None: + continue for coord in getattr(dt, "_fnd_match_coords", []): - r = stop_region_for_cell(dt, coord) - if r is not None: - regions.append(r) + cell = stop_row_in_table(dt, coord) + if cell is not None: + rows.append(md_top + dt_top + cell) # Non-table match blocks, on every row a match PAINTS on: a stop on the # block's top row sends n/b, and the ▲▼ markers that read this, to a row # with no match on it. for block in md.match_blocks: if isinstance(block, FNDMarkdownTableDT | FNDMarkdownTD | FNDMarkdownTH): continue - if block.region.height > 0: - regions.extend( - region_at_row(block.region, row) for row in rows_to_matches(block, spec) - ) + top = layout_offset(block, md, into_ancestor=True) + if top is not None: + rows.extend(md_top + top + row for row in rows_to_matches(block, spec)) # Frozen chunks: the stops were recorded as rows while the blocks still # existed, so they resolve by offset from the view's own top. A table cell's # row came from the same capture, which is why they need no special case - # here — unlike the live path above, where a cell has to be resolved against + # here, unlike the live path above, where a cell has to be resolved against # a DataTable that may not have laid its rows out yet. for view in pane.query(FrozenChunkView): - base = view.region - if base.height == 0: + top = layout_offset(view, pane, into_ancestor=False) + if top is None: continue - for row in view.frozen.stop_rows: - if 0 <= row < base.height: - regions.append(Region(base.x, base.y + row, base.width, 1)) + height = view.virtual_region.height + rows.extend(top + row for row in view.frozen.stop_rows if 0 <= row < height) # Plain (pdf/txt) chunks render one Static per body line; a matching line # is a stop. for line in pane.query("Static.chunk-line"): txt = getattr(line, "fnd_text", None) - if txt and text_has_any_match(txt, spec) and line.region.height > 0: - regions.append(line.region) - # Content-space y so the order is stable regardless of the live scroll. - regions.sort(key=lambda r: r.y + pane.scroll_offset.y) - return regions + if txt and text_has_any_match(txt, spec): + top = layout_offset(line, pane, into_ancestor=False) + if top is not None: + rows.append(top) + return sorted(rows) class FlatHost(Protocol): diff --git a/tests/_failure_state.py b/tests/_failure_state.py index 6865730e..b16b768a 100644 --- a/tests/_failure_state.py +++ b/tests/_failure_state.py @@ -149,7 +149,7 @@ def _match_nav_state(app: Any) -> str: # from these against the viewport, so a disagreement between where # the scroll landed and where the stops are is only readable here. ("chunk_stops", lambda: nav._chunk_stops(nav._pane())), - ("all_stops", lambda: nav._region_stops(nav._pane())), + ("all_stops", lambda: nav._content_stops(nav._pane())), ("chunk_extent", lambda: nav._current_chunk_extent(nav._pane())), ("viewport", lambda: _viewport(nav._pane())), ], diff --git a/tests/test_match_nav_cross_section.py b/tests/test_match_nav_cross_section.py index ad970c3d..49635d04 100644 --- a/tests/test_match_nav_cross_section.py +++ b/tests/test_match_nav_cross_section.py @@ -119,7 +119,7 @@ def _nav_pressing(sections: list[int], *, chunk_has_stop: bool, stops: list[int] ) taken: list[bool] = [] nav._pane = lambda: pane # type: ignore[assignment] - nav._region_stops = lambda _p: stops # type: ignore[assignment] + nav._content_stops = lambda _p: stops # type: ignore[assignment] nav._current_chunk_extent = lambda _p: (0, 100) # type: ignore[assignment] nav._select_section_row = lambda seq: (taken.append(seq), True)[1] # type: ignore[assignment] return nav, taken diff --git a/tests/test_match_nav_integration.py b/tests/test_match_nav_integration.py index 71d9097f..6e608632 100644 --- a/tests/test_match_nav_integration.py +++ b/tests/test_match_nav_integration.py @@ -298,7 +298,7 @@ async def test_n_never_scrolls_a_neighbours_match_under_the_current_result( # asserting it straight away tested how fast the runner mounts. await wait_until( pilot, - lambda: len(nav._region_stops(pane)) > len(nav._chunk_stops(pane)), + lambda: len(nav._content_stops(pane)) > len(nav._chunk_stops(pane)), timeout=30.0, message="the neighbouring result's match never mounted, so scope excludes nothing", ) @@ -550,6 +550,64 @@ async def test_a_hand_over_onto_the_row_the_cursor_already_holds_still_lands( ) +@pytest.fixture +def three_section_index(tmp_path: Path, tmp_index_dir: Path) -> Path: + """A long middle section, so the last section's final stop sits on the + content's bottom row and a scroll can push a misread one past it.""" + body = ( + "# Intro\n\nThe CRC intro.\n\n## Middle\n\nOne CRC in the middle.\n\n" + + "".join(f"filler paragraph line {i}.\n\n" for i in range(30)) + + "## End\n\nCRC CRC CRC checksum CRC.\n\nMore text.\n\nTrailing CRC line.\n" + ) + _write(tmp_path / "notes" / "Doc.md", body) + build_index(roots=[tmp_path / "notes"], index_dir=tmp_index_dir, collection="notes") + return tmp_index_dir + + +@pytest.mark.asyncio +@pytest.mark.parametrize("state", ["unrendered", "visible_map"]) +async def test_stops_hold_their_rows_while_a_scroll_awaits_its_render( + cfg: Config, three_section_index: Path, state: str +) -> None: + """The current chunk's stops are the same content rows mid-scroll as settled.""" + app = FNDApp(index_dir=three_section_index, config=cfg, collection="notes", initial_query="CRC") + async with app.run_test(size=(110, 24)) as pilot: + pane = app.query_one("#preview_pane", VerticalScroll) + nav = app._match_nav + await wait_until( + pilot, + lambda: ( + nav._current_chunk_extent(pane) not in (None, (0, 0)) + and bool(nav._chunk_stops(pane)) + and not app._preview_scroll.is_settling + ), + timeout=30.0, + message="the preview never landed on a section with stops", + ) + settled = nav._chunk_stops(pane) + bottom = pane.max_scroll_y + assert settled[-1] + 4 >= pane.virtual_size.height, ( + f"stops {settled} of {pane.virtual_size.height} rows: a 4-row misread must " + "cross the content bottom for this to discriminate" + ) + start, end = (bottom - 4, bottom) if state == "unrendered" else (bottom, bottom - 4) + pane.scroll_to(y=start, animate=False, immediate=True) + await wait_until( + pilot, + lambda: pane.scroll_offset.y == start and nav._chunk_stops(pane) == settled, + timeout=10.0, + message=f"the stops never read as settled at y={start}", + ) + if state == "visible_map": + # The render Textual makes after a scroll: visible widgets only. + app.screen._refresh_layout(scroll=True) + assert app.screen._compositor._visible_map is not None + + pane.scroll_to(y=end, animate=False, immediate=True) + + assert nav._chunk_stops(pane) == settled + + @pytest.mark.asyncio async def test_a_hand_over_into_a_collapsed_file_still_lands( cfg: Config, flashcards_index: Path diff --git a/tests/test_match_navigator.py b/tests/test_match_navigator.py index 8ada2f28..b486c183 100644 --- a/tests/test_match_navigator.py +++ b/tests/test_match_navigator.py @@ -72,7 +72,7 @@ def _nav(stops: list[int], vh: int = 20, virtual_height: int = 10**6) -> MatchNa # wide chunk extent so scoping keeps every stop (the app derives this from # the current result's widget; here the injected stops ARE the chunk). nav._pane = lambda: pane # type: ignore[assignment] - nav._region_stops = lambda _p: stops # type: ignore[assignment] + nav._content_stops = lambda _p: stops # type: ignore[assignment] # Extent from 0, so the offsets the navigator stores (measured from the # chunk's top) read as absolute content y in these tests. nav._current_chunk_extent = lambda _p: (0, 10**9) # type: ignore[assignment] @@ -177,7 +177,7 @@ def test_a_frozen_focused_chunk_still_reports_its_stops() -> None: """The footer hint must follow what ``n``/``b`` can actually reach. Serving a capture replaces the chunk's widget tree with a ``FrozenChunkView`` - and pops its match target. ``enumerate_stop_regions`` handles that view, so + and pops its match target. ``enumerate_stop_rows`` handles that view, so ``n``/``b`` keep working — but without a matching branch here the plain-chunk fallback reads the popped target, returns False, and the app hides the ``n/b Matches`` hint on a chunk where both keys work. @@ -269,6 +269,7 @@ def _extent_nav(widgets: dict[int, _StubWidget], seq: int, virtual_height: int = nav = MatchNavigator(FakeApp()) # type: ignore[arg-type] pane = FakePane(40, virtual_height) nav._pane = lambda: pane # type: ignore[assignment] + nav._chunk_top = lambda _p, w: w.region.y if w.region.height > 0 else None # type: ignore[assignment] nav._app = SimpleNamespace( # type: ignore[assignment] _preview_scroll=SimpleNamespace(anchor=SimpleNamespace(focus_chunk_seq=seq)), _preview=SimpleNamespace(chunk_widgets=widgets), @@ -284,7 +285,7 @@ def test_an_unlaid_chunk_scopes_to_nothing_not_to_everything() -> None: assert nav._current_chunk_extent(pane) == (0, 0) # type: ignore[arg-type] - nav._region_stops = lambda _p: [10, 200, 300] # type: ignore[assignment] + nav._content_stops = lambda _p: [10, 200, 300] # type: ignore[assignment] assert nav._chunk_stops(pane) == [], "an unlaid chunk handed back the whole preview" # type: ignore[arg-type] @@ -356,7 +357,7 @@ def test_a_reflow_under_the_walk_does_not_strand_the_landing() -> None: # Everything above the chunk grew by 5: the chunk and its stops move down. shifted = [y + 5 for y in stops] - nav._region_stops = lambda _p: shifted # type: ignore[assignment] + nav._content_stops = lambda _p: shifted # type: ignore[assignment] nav._current_chunk_extent = lambda _p: (5, 10**9) # type: ignore[assignment] nav._pane().scroll_offset = Offset(0, 59) # type: ignore[attr-defined] diff --git a/tests/test_match_stop_regions.py b/tests/test_match_stop_rows.py similarity index 87% rename from tests/test_match_stop_regions.py rename to tests/test_match_stop_rows.py index 3529e818..f8ec5fa1 100644 --- a/tests/test_match_stop_regions.py +++ b/tests/test_match_stop_rows.py @@ -1,4 +1,4 @@ -"""enumerate_stop_regions finds every match stop across mounted chunks, +"""enumerate_stop_rows finds every match stop across mounted chunks, including cells far below the fold of a table taller than the viewport.""" from __future__ import annotations @@ -8,7 +8,8 @@ from textual.containers import VerticalScroll from fnd.matching import MatchSpec -from fnd.tui.preview_scroll import enumerate_stop_regions +from fnd.tui.preview.match_row import layout_offset +from fnd.tui.preview_scroll import enumerate_stop_rows from fnd.tui.widgets.markdown import FNDMarkdown # A table ~4x the viewport height with CRC in row 32's answer and row 47's @@ -44,11 +45,10 @@ async def on_mount(self) -> None: await md.build_done.wait() await pilot.pause() pane = pilot.app.query_one("#preview_pane", VerticalScroll) - regions = enumerate_stop_regions(pane, MatchSpec.from_query("CRC")) - ys = [r.y for r in regions] + rows = enumerate_stop_rows(pane, MatchSpec.from_query("CRC")) # both CRC cells enumerated, card 32 above card 47, distinct positions - assert len(regions) == 2, ys - assert ys[0] < ys[1] + assert len(rows) == 2, rows + assert rows[0] < rows[1] # One source line wrapping over several screenfuls, its match late in it — a PDF @@ -87,11 +87,12 @@ async def on_mount(self) -> None: assert painted, "the match never painted" pane = pilot.app.query_one("#preview_pane", VerticalScroll) - regions = enumerate_stop_regions(pane, MatchSpec.from_query("quartzfin")) - assert len(regions) == 1, [r.y for r in regions] - assert regions[0].y == block.region.y + painted[0], ( - f"stop at y={regions[0].y} is the block's top ({block.region.y}), not the " - f"row its match paints on ({block.region.y + painted[0]})" + rows = enumerate_stop_rows(pane, MatchSpec.from_query("quartzfin")) + top = layout_offset(block, pane, into_ancestor=False) + assert top is not None + assert rows == [top + painted[0]], ( + f"stops {rows} are not the row its match paints on ({top + painted[0]}); " + f"the block's top is {top}" ) @@ -140,8 +141,10 @@ async def on_mount(self) -> None: assert len(painted) == 2, f"the fixture painted {len(painted)} matches" pane = pilot.app.query_one("#preview_pane", VerticalScroll) - regions = enumerate_stop_regions(pane, MatchSpec.from_query("quartzfin")) - assert [r.y - block.region.y for r in regions] == painted + rows = enumerate_stop_rows(pane, MatchSpec.from_query("quartzfin")) + top = layout_offset(block, pane, into_ancestor=False) + assert top is not None + assert [row - top for row in rows] == painted # A tab-indented fence: every line fits the pane as written, and every line @@ -194,5 +197,7 @@ async def on_mount(self) -> None: assert len(painted) == 2, f"the fixture painted {len(painted)} matches" pane = pilot.app.query_one("#preview_pane", VerticalScroll) - regions = enumerate_stop_regions(pane, MatchSpec.from_query("quartzfin")) - assert [r.y - block.region.y for r in regions] == painted + rows = enumerate_stop_rows(pane, MatchSpec.from_query("quartzfin")) + top = layout_offset(block, pane, into_ancestor=False) + assert top is not None + assert [row - top for row in rows] == painted diff --git a/tests/test_plain_chunk_fnd_text_type.py b/tests/test_plain_chunk_fnd_text_type.py index 3effb857..af63b894 100644 --- a/tests/test_plain_chunk_fnd_text_type.py +++ b/tests/test_plain_chunk_fnd_text_type.py @@ -3,7 +3,7 @@ ``_mount_plain_chunk`` renders each body line to a rich ``Text`` (for display) and stashed that same ``Text`` on ``line.fnd_text``. Match counting (``MatchNavigator._count_stops``) and stop-region scanning -(``enumerate_stop_regions``) then feed ``fnd_text`` to ``text_has_any_match``, +(``enumerate_stop_rows``) then feed ``fnd_text`` to ``text_has_any_match``, which runs ``DOC_WORD_RE.finditer`` over it — ``re`` needs a ``str``, so a rich ``Text`` raised ``TypeError: expected string or bytes-like object, got 'Text'`` and crashed the query (e.g. a wine PDF's "SPAIN" line while @@ -23,7 +23,7 @@ from fnd.matching import MatchSpec from fnd.query import FileChunk from fnd.tui import FNDApp -from fnd.tui.preview_scroll import enumerate_stop_regions +from fnd.tui.preview_scroll import enumerate_stop_rows def _plain_chunk() -> FileChunk: @@ -78,5 +78,4 @@ async def test_plain_chunk_lines_carry_str_fnd_text(min_app: FNDApp) -> None: assert isinstance(txt, str), f"fnd_text must be str, got {type(txt).__name__}" # The exact crash site: scanning stops over fnd_text must not raise. - regions = enumerate_stop_regions(pane, app._effective_match_spec) - assert len(regions) >= 1 + assert enumerate_stop_rows(pane, app._effective_match_spec) diff --git a/tests/test_preview_frozen_chunk.py b/tests/test_preview_frozen_chunk.py index c5d14f27..bb8b6122 100644 --- a/tests/test_preview_frozen_chunk.py +++ b/tests/test_preview_frozen_chunk.py @@ -17,6 +17,7 @@ from textual.app import App, ComposeResult from textual.containers import Container, VerticalScroll from textual.geometry import Size +from textual.widget import Widget from textual.widgets import DataTable from fnd.matching import MatchSpec @@ -70,6 +71,16 @@ async def _built(pilot) -> FNDMarkdown: # type: ignore[no-untyped-def] return md +def _rows_below(pane: VerticalScroll, chunk: Widget, spec: MatchSpec) -> list[int]: + """The pane's stop rows measured from ``chunk``'s top.""" + from fnd.tui.preview.match_row import layout_offset + from fnd.tui.preview_scroll import enumerate_stop_rows + + top = layout_offset(chunk, pane, into_ancestor=False) + assert top is not None, "the chunk is not laid out" + return [row - top for row in enumerate_stop_rows(pane, spec)] + + @pytest.mark.asyncio async def test_capture_holds_everything_the_tree_painted() -> None: app = _Host() @@ -289,12 +300,11 @@ async def test_a_chunk_that_scrolls_inside_itself_is_refused() -> None: async def test_a_frozen_chunk_still_contributes_its_match_stops() -> None: """Freezing must not make matches unreachable. - ``enumerate_stop_regions`` walks ``FNDMarkdown`` blocks, and a frozen chunk + ``enumerate_stop_rows`` walks ``FNDMarkdown`` blocks, and a frozen chunk is not one — so without explicit handling it contributes nothing and its matches drop out of n/b navigation and the off-screen markers. Nothing raises; the matches simply stop existing, which is why this is pinned. """ - from fnd.tui.preview_scroll import enumerate_stop_regions app = _Host() async with app.run_test(size=(90, 24)) as pilot: @@ -305,7 +315,7 @@ async def test_a_frozen_chunk_still_contributes_its_match_stops() -> None: # ROWS, not just a count: a count cannot see a stop sitting on a block's # top row instead of on its match, which is what sends n/b and the ▲▼ # markers to a row with nothing on it. - live_rows = sorted(r.y - md.region.y for r in enumerate_stop_regions(pane, spec)) + live_rows = _rows_below(pane, md, spec) assert live_rows, "fixture should have match stops while live" frozen = freeze(md, chunk_seq=7) @@ -320,7 +330,7 @@ async def test_a_frozen_chunk_still_contributes_its_match_stops() -> None: message="the live chunk never went away", ) - frozen_rows = sorted(r.y - view.region.y for r in enumerate_stop_regions(pane, spec)) + frozen_rows = _rows_below(pane, view, spec) assert frozen_rows == live_rows, ( f"stops at rows {live_rows} live but {frozen_rows} once frozen — " "n/b and the markers move when a chunk is captured" @@ -408,7 +418,6 @@ async def test_a_wrapped_chunk_keeps_its_stop_rows_through_a_capture() -> None: """The stop rows of ``_Host``'s chunk all sit on their blocks' first row, so that fixture cannot tell a block-top stop from a match-row one. This one wraps, which is where the two diverge.""" - from fnd.tui.preview_scroll import enumerate_stop_regions spec = MatchSpec.from_query("quartzfin") app = _WrappedHost() @@ -423,7 +432,7 @@ async def test_a_wrapped_chunk_keeps_its_stop_rows_through_a_capture() -> None: message="the chunk never laid out", ) pane = app.query_one("#pane", VerticalScroll) - live_rows = sorted(r.y - md.region.y for r in enumerate_stop_regions(pane, spec)) + live_rows = _rows_below(pane, md, spec) assert live_rows, "no match stops while live" assert len(live_rows) == 2, ( f"stops at {live_rows}: expected one stop per match. Either the live " @@ -447,7 +456,7 @@ async def test_a_wrapped_chunk_keeps_its_stop_rows_through_a_capture() -> None: message="the live chunk never went away", ) - frozen_rows = sorted(r.y - view.region.y for r in enumerate_stop_regions(pane, spec)) + frozen_rows = _rows_below(pane, view, spec) assert frozen_rows == live_rows, ( f"stops at rows {live_rows} live but {frozen_rows} once frozen" ) diff --git a/tests/test_preview_scroll_characterization.py b/tests/test_preview_scroll_characterization.py index 8b343940..e447a8cf 100644 --- a/tests/test_preview_scroll_characterization.py +++ b/tests/test_preview_scroll_characterization.py @@ -282,12 +282,12 @@ def _region() -> Region | None: return None # A frozen chunk has no child widgets to walk — that is what freezing # is — so its match resolves from the row recorded at capture time, the - # way ``enumerate_stop_regions`` resolves one. Walking children only + # way ``enumerate_stop_rows`` resolves one. Walking children only # made this probe answer None for good once the sweep reached the focus # chunk, and every wait gated on it then ran out its whole budget. if isinstance(chunk, FrozenChunkView): row = chunk.frozen.first_match_row - # Same bounds check as ``enumerate_stop_regions``: a clipped view + # Same bounds check as ``enumerate_stop_rows``: a clipped view # resolves nothing there, and a probe that answers anyway is # claiming a parity it does not have. if row is None or not (0 <= row < chunk.region.height): diff --git a/tests/test_preview_scroll_controller.py b/tests/test_preview_scroll_controller.py index ce2b4728..439d27fe 100644 --- a/tests/test_preview_scroll_controller.py +++ b/tests/test_preview_scroll_controller.py @@ -519,6 +519,17 @@ def test_structural_locate_ignores_culled_chunks() -> None: assert strat.locate() == ViewportLocation("structural", chunk_seq=2, offset=3) +def test_structural_locate_reads_layout_not_a_region_awaiting_its_render() -> None: + """A region lags a scroll, so it would place the viewport where it last rendered.""" + stale = _FakeWidget(Region(0, 0, 80, 10), virtual_region=Region(0, 10, 80, 10)) + pane = _FakePane(height=40) + pane.scroll_offset = Offset(0, 13) + host = _FakeHost(pane, chunk_widgets={2: stale}, match_targets={}) + strat = StructuralScrollStrategy(cast(StructuralHost, host)) + + assert strat.locate() == ViewportLocation("structural", chunk_seq=2, offset=3) + + def test_structural_scroll_to_location_scrolls_to_chunk_plus_offset() -> None: # virtual_region.y (content-space top) = 200; restore scrolls to top + the # captured 6-row in-chunk offset.