From 7f4bb6b547d788d6323f0295a9d2e9a60181da89 Mon Sep 17 00:00:00 2001 From: hellices Date: Thu, 6 Aug 2026 23:44:08 +0900 Subject: [PATCH 1/5] perf: absorb column widths instead of re-measuring every row Textual computes column widths from `on_idle` by rebuilding every new row's renderables and measuring each cell. Seeding a 50,000-row view therefore cost 700,000 renderable constructions and 700,000 measurements after the rows were already in the table -- ~74% of a 5s freeze, and the same bill on every rebuild path (kind switch, sort change, custom view, and the reorder fallback). ResourceTable already holds exactly those cells (`_emitted`, #208) and already has `_cell_width()`, written to match DataTable's measurement. Fold the emitted cells into `column.content_width` directly and hand `_update_dimensions` only the rows that were not accounted for, so the superclass still measures anything added outside `show()` and still recomputes the virtual size. An ASCII cell's display width is exactly its length, and both markup parsing and newline truncation can only shorten it, so `len(raw) <= width and raw.isascii()` proves a cell cannot widen its column without measuring it -- which settles almost every cell. Cold build, same fixture: 1,000 rows 142.5 ms -> 111.3 ms 10,000 rows 1010.4 ms -> 387.6 ms 50,000 rows 5017.1 ms -> 1507.0 ms (3.3x) Resulting widths are identical to the superclass result, pinned by a test that compares against a control DataTable seeded with the same cells, including CJK and markup-bearing values. Fixes #210 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/korvid/ui/widgets/resource_table.py | 63 +++++++++- tests/ui/test_table_column_widths.py | 153 ++++++++++++++++++++++++ 2 files changed, 214 insertions(+), 2 deletions(-) create mode 100644 tests/ui/test_table_column_widths.py diff --git a/src/korvid/ui/widgets/resource_table.py b/src/korvid/ui/widgets/resource_table.py index 1bd5d5aa..72676a90 100644 --- a/src/korvid/ui/widgets/resource_table.py +++ b/src/korvid/ui/widgets/resource_table.py @@ -2,7 +2,7 @@ from __future__ import annotations -from collections.abc import Callable +from collections.abc import Callable, Iterable from datetime import UTC, datetime from typing import Final, cast @@ -10,7 +10,7 @@ from rich.text import Text from textual.coordinate import Coordinate from textual.widgets import DataTable -from textual.widgets.data_table import RowDoesNotExist +from textual.widgets.data_table import RowDoesNotExist, RowKey from korvid.core.config import ViewConfig from korvid.core.sorting import SortSpec, sort_rows @@ -347,6 +347,10 @@ class ResourceTable(DataTable[str | Text]): _last_all_namespaces: bool | None = None _last_sort: SortSpec | None = None _active_view: ViewConfig | None = None + #: Set once this widget has folded the rows it emitted into the column + #: widths itself; consumed by the next `_update_dimensions`. Declared at + #: class level so the hook is safe before `on_mount` has run. + _widths_absorbed: bool = False def on_mount(self) -> None: self.cursor_type = "row" @@ -459,6 +463,7 @@ def show( for key, cells in pending: self.add_row(*cells, key=key) self._emitted = dict(pending) + self._absorb_widths(cells for _, cells in pending) if restore is not None: # A background refresh must not scroll the cursor back into # view — the viewport restore below keeps the user's position. @@ -537,9 +542,11 @@ def _apply_in_place(self, pending: list[tuple[str, list[str | Text]]]) -> bool: for key in doomed: self.remove_row(key) columns = self.ordered_columns + fresh: list[list[str | Text]] = [] for key, cells in pending: if key not in current_set: self.add_row(*cells, key=key) + fresh.append(cells) continue old_cells = self._emitted.get(key) if old_cells is cells: @@ -551,8 +558,60 @@ def _apply_in_place(self, pending: list[tuple[str, list[str | Text]]]) -> bool: grew = _cell_width(new_cell) > column.content_width self.update_cell(key, column.key, new_cell, update_width=grew) self._emitted = dict(pending) + if fresh: + self._absorb_widths(fresh) return True + def _absorb_widths(self, rows: Iterable[list[str | Text]]) -> None: + """Grow the column widths from cells this widget is holding anyway. + + `DataTable` derives column widths from `on_idle`, and to do so it + rebuilds every new row's renderables and measures each cell — 700,000 + renderable constructions and measurements to seed a 50,000-row view, + which is most of the freeze when a large kind first loads (issue + #210). The cells are already in hand here, so the same fourteen + integers are computed directly and `_update_dimensions` is told to + skip the rows they came from. + + The `len(raw) <= width and raw.isascii()` guard settles the + overwhelming majority of cells without measuring: an ASCII string's + display width is exactly its length, and both markup parsing and the + newline truncation `_cell_width` performs can only shorten it — so a + cell that short cannot widen its column no matter how it renders. + """ + columns = self.ordered_columns + widths = [column.content_width for column in columns] + limit = len(widths) + for cells in rows: + for index, cell in enumerate(cells): + if index >= limit: + break + raw = cell.plain if isinstance(cell, Text) else cell + if len(raw) <= widths[index] and raw.isascii(): + continue + width = _cell_width(cell) + if width > widths[index]: + widths[index] = width + for column, width in zip(columns, widths, strict=True): + column.content_width = width + self._widths_absorbed = True + + def _update_dimensions(self, new_rows: Iterable[RowKey]) -> None: + """Measure only the rows whose widths were not absorbed above. + + The superclass still recomputes the virtual size and handles anything + this widget did not emit (a row added directly, or added after the + absorption and before this idle pass). Should a future Textual rename + the hook, this override simply stops being called and the widget falls + back to today's slower-but-correct measuring pass. + """ + if not self._widths_absorbed: + super()._update_dimensions(new_rows) + return + self._widths_absorbed = False + absorbed = self._emitted + super()._update_dimensions([key for key in new_rows if key.value not in absorbed]) + def _prune_memo(self, pending: list[tuple[str, list[str | Text]]]) -> None: """Drop memo entries for rows no longer rendered. diff --git a/tests/ui/test_table_column_widths.py b/tests/ui/test_table_column_widths.py new file mode 100644 index 00000000..44133ac2 --- /dev/null +++ b/tests/ui/test_table_column_widths.py @@ -0,0 +1,153 @@ +"""Column-width absorption (issue #210). + +Textual recomputes column widths from `on_idle` by re-deriving every new +row's renderables and measuring each cell — 700,000 measurements to seed a +50,000-row view, and ~74% of the freeze when a large kind first loads. + +`ResourceTable` already holds the exact cells it emitted, so it folds their +widths into the columns itself and hands the superclass only the rows it did +not account for. These tests pin both halves: that the measuring pass is +skipped, and that the widths it produces are indistinguishable from the ones +Textual would have measured. +""" + +from __future__ import annotations + +from typing import Any + +from textual.app import App, ComposeResult +from textual.widgets import DataTable + +from korvid.core.store import Summary +from korvid.k8s.models import PodSummary +from korvid.ui.widgets.resource_table import ResourceTable + + +class _TableApp(App[None]): + """Bare host for a `ResourceTable`, so a test drives `show()` directly.""" + + def compose(self) -> ComposeResult: + yield ResourceTable() + + +def _pods(names: list[str]) -> list[Summary]: + return [ + PodSummary( + name=name, + namespace="default", + phase="Running", + ready="1/1", + restarts=0, + node=None, + qos="-", + ) + for name in names + ] + + +def _spy_row_renderables(table: DataTable[Any]) -> list[int]: + """Record every row whose renderables are rebuilt. + + Textual also rebuilds renderables to *paint*, so only distinct data rows + matter: painting touches the handful on screen, measuring touches all of + them. Index -1 is the header row. + """ + seen: list[int] = [] + original = table._get_row_renderables + + def spy(row_index: int) -> Any: + seen.append(row_index) + return original(row_index) + + table._get_row_renderables = spy # type: ignore[method-assign] # test spy + return seen + + +def _widths(table: DataTable[Any]) -> list[int]: + return [column.content_width for column in table.ordered_columns] + + +async def test_seeding_does_not_rebuild_every_row_to_measure_it() -> None: + """Seeding a view must not cost one renderable rebuild per row.""" + app = _TableApp() + async with app.run_test(size=(120, 12)) as pilot: + table = app.query_one(ResourceTable) + await pilot.pause() + seen = _spy_row_renderables(table) + rows = _pods([f"pod-{i:04d}" for i in range(400)]) + table.show("pods", rows, all_namespaces=False, pattern="") + await pilot.pause() + assert table._require_update_dimensions is False, "dimension pass did not run" + touched = {index for index in seen if index >= 0} + assert len(touched) < 100, f"rebuilt {len(touched)} of 400 rows" + + +async def test_absorbed_widths_match_what_textual_would_measure() -> None: + """Widths must be identical to the superclass result for every cell shape.""" + names = [ + "a", + "pod-with-a-fairly-long-generated-name-0001", + "파드-매우-긴-한글-이름", + "[bold]not-markup[/bold]", + "trailing", + ] + app = _TableApp() + async with app.run_test(size=(120, 12)) as pilot: + table = app.query_one(ResourceTable) + await pilot.pause() + table.show("pods", _pods(names), all_namespaces=True, pattern="") + await pilot.pause() + + control: DataTable[Any] = DataTable() + await app.mount(control) + await pilot.pause() + control.add_columns(*[column.label for column in table.ordered_columns]) + for key, cells in table._emitted.items(): + control.add_row(*cells, key=key) + await pilot.pause() + + assert _widths(table) == _widths(control) + + +async def test_rows_added_outside_show_are_still_measured() -> None: + """A row this widget did not emit must fall through to the superclass.""" + app = _TableApp() + async with app.run_test(size=(120, 12)) as pilot: + table = app.query_one(ResourceTable) + await pilot.pause() + table.show("pods", _pods(["short"]), all_namespaces=False, pattern="") + await pilot.pause() + long_value = "a-value-added-without-going-through-show" + table.add_row(long_value, key="extra") + await pilot.pause() + assert table.ordered_columns[0].content_width >= len(long_value) + + +async def test_unabsorbed_row_added_before_the_dimension_pass_is_measured() -> None: + """Absorbing one batch must not suppress measurement of rows added after + it but before the idle pass runs.""" + app = _TableApp() + async with app.run_test(size=(120, 12)) as pilot: + table = app.query_one(ResourceTable) + await pilot.pause() + long_value = "a-value-added-between-show-and-idle-00000" + table.show("pods", _pods(["short"]), all_namespaces=False, pattern="") + table.add_row(long_value, key="extra") + await pilot.pause() + assert table.ordered_columns[0].content_width >= len(long_value) + + +async def test_row_appearing_in_place_widens_its_column() -> None: + """A new row appended by the in-place diff still grows the column.""" + app = _TableApp() + async with app.run_test(size=(120, 12)) as pilot: + table = app.query_one(ResourceTable) + await pilot.pause() + table.show("pods", _pods(["aaa"]), all_namespaces=False, pattern="") + await pilot.pause() + narrow = table.ordered_columns[0].content_width + appeared = "zzz-a-much-longer-pod-name-than-before" + table.show("pods", _pods(["aaa", appeared]), all_namespaces=False, pattern="") + await pilot.pause() + assert table.ordered_columns[0].content_width > narrow + assert table.ordered_columns[0].content_width >= len(appeared) From 940a94045adf58f891449ed08c3092194753be00 Mon Sep 17 00:00:00 2001 From: hellices Date: Fri, 7 Aug 2026 00:18:42 +0900 Subject: [PATCH 2/5] fix: never queue width updates, so absorption cannot cause a rescan Review catch on #211: `_update_column_widths` drains queued cells *before* the dimension pass, and re-measures every cell in a column whenever the queued value looks narrower than the column. Absorbing a freshly appended row's widths synchronously could make that true for a cell queued earlier in the same repaint -- reintroducing the very O(total rows) measuring pass this change removes. Stop queuing entirely: every in-place cell update now passes update_width=False, and the widths of the rows that actually changed (appended or patched) are absorbed directly. `_absorb_widths` requests the dimension pass itself when a column genuinely grew, since update_width=False no longer schedules one. `_update_column_widths` can now never run from this widget, so the rescan is unreachable rather than merely unlikely. Extract `_patch_row` to keep `_apply_in_place` under the complexity gate. Test: `test_widening_new_row_does_not_trigger_a_full_column_rescan` spies on `get_column`, which only the rescan branch reaches. It fails before this commit (1 column rescanned) and passes after. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/korvid/ui/widgets/resource_table.py | 57 +++++++++++++-------- tests/ui/test_table_column_widths.py | 66 ++++++++++++++++++++++++- 2 files changed, 100 insertions(+), 23 deletions(-) diff --git a/src/korvid/ui/widgets/resource_table.py b/src/korvid/ui/widgets/resource_table.py index 72676a90..d40fd911 100644 --- a/src/korvid/ui/widgets/resource_table.py +++ b/src/korvid/ui/widgets/resource_table.py @@ -10,7 +10,7 @@ from rich.text import Text from textual.coordinate import Coordinate from textual.widgets import DataTable -from textual.widgets.data_table import RowDoesNotExist, RowKey +from textual.widgets.data_table import Column, RowDoesNotExist, RowKey from korvid.core.config import ViewConfig from korvid.core.sorting import SortSpec, sort_rows @@ -529,11 +529,15 @@ def _apply_in_place(self, pending: list[tuple[str, list[str | Text]]]) -> bool: settled by one identity check. `get_row` remains the fallback whenever `_emitted` has no record of a row. - Width updates are requested only for cells wider than their column: - `update_width=True` is not grow-only — a narrower replacement rescans - and *shrinks* the column, shifting the layout this path must keep - still. The trade-off is that a column stays at its widest-seen size - until the next rebuild. + Width updates are never handed to Textual's queue. `update_width=True` + is not grow-only: it defers the cell to `_update_column_widths`, which + runs *before* the dimension pass and re-measures every cell in the + column the moment the queued value looks narrower than the column — + both when the replacement genuinely shrank, and when another row in + the same repaint had already widened that column. Widths are absorbed + below instead, from the rows that actually changed. The trade-off is + unchanged: a column stays at its widest-seen size until the next + rebuild. """ plan = self._in_place_plan(pending) if plan is None: @@ -542,26 +546,32 @@ def _apply_in_place(self, pending: list[tuple[str, list[str | Text]]]) -> bool: for key in doomed: self.remove_row(key) columns = self.ordered_columns - fresh: list[list[str | Text]] = [] + touched: list[list[str | Text]] = [] for key, cells in pending: if key not in current_set: self.add_row(*cells, key=key) - fresh.append(cells) - continue - old_cells = self._emitted.get(key) - if old_cells is cells: - continue # memo hit: same list object, nothing can have changed - if old_cells is None: - old_cells = self.get_row(key) - for column, old_cell, new_cell in zip(columns, old_cells, cells, strict=True): - if not _cells_equal(old_cell, new_cell): - grew = _cell_width(new_cell) > column.content_width - self.update_cell(key, column.key, new_cell, update_width=grew) + touched.append(cells) + elif self._patch_row(key, cells, columns): + touched.append(cells) self._emitted = dict(pending) - if fresh: - self._absorb_widths(fresh) + if touched: + self._absorb_widths(touched) return True + def _patch_row(self, key: str, cells: list[str | Text], columns: list[Column]) -> bool: + """Update an existing row's changed cells; True when any cell moved.""" + old_cells = self._emitted.get(key) + if old_cells is cells: + return False # memo hit: same list object, nothing can have changed + if old_cells is None: + old_cells = self.get_row(key) + changed = False + for column, old_cell, new_cell in zip(columns, old_cells, cells, strict=True): + if not _cells_equal(old_cell, new_cell): + self.update_cell(key, column.key, new_cell, update_width=False) + changed = True + return changed + def _absorb_widths(self, rows: Iterable[list[str | Text]]) -> None: """Grow the column widths from cells this widget is holding anyway. @@ -593,7 +603,12 @@ def _absorb_widths(self, rows: Iterable[list[str | Text]]) -> None: if width > widths[index]: widths[index] = width for column, width in zip(columns, widths, strict=True): - column.content_width = width + if width > column.content_width: + column.content_width = width + # A wider column means a wider table; the superclass + # recomputes the virtual size from the dimension pass, which + # `update_cell(update_width=False)` does not schedule. + self._require_update_dimensions = True self._widths_absorbed = True def _update_dimensions(self, new_rows: Iterable[RowKey]) -> None: diff --git a/tests/ui/test_table_column_widths.py b/tests/ui/test_table_column_widths.py index 44133ac2..7143caab 100644 --- a/tests/ui/test_table_column_widths.py +++ b/tests/ui/test_table_column_widths.py @@ -31,17 +31,21 @@ def compose(self) -> ComposeResult: def _pods(names: list[str]) -> list[Summary]: + return _pods_with_phase([(name, "Running") for name in names]) + + +def _pods_with_phase(rows: list[tuple[str, str]]) -> list[Summary]: return [ PodSummary( name=name, namespace="default", - phase="Running", + phase=phase, ready="1/1", restarts=0, node=None, qos="-", ) - for name in names + for name, phase in rows ] @@ -151,3 +155,61 @@ async def test_row_appearing_in_place_widens_its_column() -> None: await pilot.pause() assert table.ordered_columns[0].content_width > narrow assert table.ordered_columns[0].content_width >= len(appeared) + + +def _spy_column_rescan(table: DataTable[Any]) -> list[Any]: + """Record full-column width rescans. + + `_update_column_widths` reads a whole column back out — and measures every + cell in it — only when a queued cell looks *narrower* than the column it + sits in. `get_column` is that read, so any call means the O(total rows) + rescan this widget exists to avoid has just run. + """ + seen: list[Any] = [] + original = table.get_column + + def spy(column_key: Any) -> Any: + seen.append(column_key) + return original(column_key) + + table.get_column = spy # type: ignore[method-assign] # test spy + return seen + + +async def test_widening_new_row_does_not_trigger_a_full_column_rescan() -> None: + """A repaint that both widens a column and changes an existing cell must + not make Textual re-measure the whole column. + + Textual drains queued cell updates *before* the dimension pass. If the + column has already been widened by then, the queued cell reads as a + shrink and every cell in the column is measured again — reintroducing the + very cost this widget avoids. + """ + app = _TableApp() + async with app.run_test(size=(120, 12)) as pilot: + table = app.query_one(ResourceTable) + await pilot.pause() + table.show( + "pods", + _pods_with_phase([("alpha", "Running"), ("beta", "Running")]), + all_namespaces=False, + pattern="", + ) + await pilot.pause() + rescans = _spy_column_rescan(table) + table.show( + "pods", + _pods_with_phase( + [ + ("alpha", "Running"), + ("beta", "CrashLoop"), + ("zeta", "ContainerCreating"), + ] + ), + all_namespaces=False, + pattern="", + ) + await pilot.pause() + assert rescans == [], f"rescanned {len(rescans)} column(s)" + status = table.ordered_columns[2] + assert status.content_width >= len("ContainerCreating") From 057d88f14cc600d9af1a50685d92b1b1a9bf27ea Mon Sep 17 00:00:00 2001 From: hellices Date: Fri, 7 Aug 2026 11:57:00 +0900 Subject: [PATCH 3/5] fix: fail safe when an absorbed row carries a label or auto height Width absorption only accounts for cell widths, but Textual's dimension pass also sizes the row-label column and computes auto-height rows. Excluding an emitted row unconditionally would leave a future `height=None` row zero rows tall and a labelled row's label column unsized. Only skip rows confirmed unlabelled and non-auto-height. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/korvid/ui/widgets/resource_table.py | 18 ++++++++++++- tests/ui/test_table_column_widths.py | 34 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/korvid/ui/widgets/resource_table.py b/src/korvid/ui/widgets/resource_table.py index d40fd911..f84ac27b 100644 --- a/src/korvid/ui/widgets/resource_table.py +++ b/src/korvid/ui/widgets/resource_table.py @@ -625,7 +625,23 @@ def _update_dimensions(self, new_rows: Iterable[RowKey]) -> None: return self._widths_absorbed = False absorbed = self._emitted - super()._update_dimensions([key for key in new_rows if key.value not in absorbed]) + super()._update_dimensions( + [key for key in new_rows if not self._is_absorbed(key, absorbed)] + ) + + def _is_absorbed(self, key: RowKey, absorbed: dict[str, list[str | Text]]) -> bool: + """Whether `_absorb_widths` fully accounted for the row behind *key*. + + Absorption only folds in cell *widths*. The superclass pass also sizes + the row-label column and computes auto-height rows, so a row carrying + either must still reach it — today this widget emits neither, but the + fast path must fail safe rather than silently drop them if that + changes. + """ + if key.value not in absorbed: + return False + row = self.rows.get(key) + return row is not None and row.label is None and not row.auto_height def _prune_memo(self, pending: list[tuple[str, list[str | Text]]]) -> None: """Drop memo entries for rows no longer rendered. diff --git a/tests/ui/test_table_column_widths.py b/tests/ui/test_table_column_widths.py index 7143caab..bfedb951 100644 --- a/tests/ui/test_table_column_widths.py +++ b/tests/ui/test_table_column_widths.py @@ -15,6 +15,7 @@ from typing import Any +from rich.text import Text from textual.app import App, ComposeResult from textual.widgets import DataTable @@ -213,3 +214,36 @@ async def test_widening_new_row_does_not_trigger_a_full_column_rescan() -> None: assert rescans == [], f"rescanned {len(rescans)} column(s)" status = table.ordered_columns[2] assert status.content_width >= len("ContainerCreating") + + +async def test_absorption_never_skips_a_labelled_or_auto_height_row() -> None: + """Width absorption must not swallow the rest of the dimension pass. + + Textual's `_update_dimensions` also assigns auto-height rows their height + and widens the row-label column. Absorption only accounts for cell + *widths*, so a row carrying either of those must still reach the + superclass — otherwise a future `height=None` row would render zero rows + tall and a labelled row would never size its label column. + """ + app = _TableApp() + async with app.run_test(size=(120, 12)) as pilot: + table = app.query_one(ResourceTable) + await pilot.pause() + table.show( + "pods", + _pods(["alpha", "beta"]), + all_namespaces=False, + pattern="", + ) + await pilot.pause() + + first, second = table.ordered_rows + first.auto_height = True + first.height = 0 + second.label = Text("a-very-long-row-label") + + table._widths_absorbed = True + table._update_dimensions([first.key, second.key]) + + assert first.height > 0, "auto-height row was skipped and stayed 0 tall" + assert table._label_column.content_width >= len("a-very-long-row-label") From 73e5dd758fb935b2624b01a711c675b2a80926b0 Mon Sep 17 00:00:00 2001 From: hellices Date: Fri, 7 Aug 2026 12:33:59 +0900 Subject: [PATCH 4/5] fix: track absorbed row keys instead of a sticky absorbed flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A repaint that absorbed widths but grew no column scheduled no dimension pass, so the boolean "widths were absorbed" state survived to whatever pass ran next. That pass then filtered out every key in `_emitted` — including a row removed and re-added since, whose wider content never reached the column. Absorption now records the exact keys it accounted for and the dimension pass consumes them, so a row it never measured can never be skipped. `remove_row` and `clear` drop the record for rows that go away, closing the remove-then-re-add window. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/korvid/ui/widgets/resource_table.py | 73 ++++++++++++++++--------- tests/ui/test_table_column_widths.py | 39 ++++++++++++- 2 files changed, 85 insertions(+), 27 deletions(-) diff --git a/src/korvid/ui/widgets/resource_table.py b/src/korvid/ui/widgets/resource_table.py index f84ac27b..03735564 100644 --- a/src/korvid/ui/widgets/resource_table.py +++ b/src/korvid/ui/widgets/resource_table.py @@ -3,8 +3,9 @@ from __future__ import annotations from collections.abc import Callable, Iterable +from collections.abc import Set as AbstractSet from datetime import UTC, datetime -from typing import Final, cast +from typing import Final, Self, cast from rich.cells import cell_len from rich.text import Text @@ -347,10 +348,17 @@ class ResourceTable(DataTable[str | Text]): _last_all_namespaces: bool | None = None _last_sort: SortSpec | None = None _active_view: ViewConfig | None = None - #: Set once this widget has folded the rows it emitted into the column - #: widths itself; consumed by the next `_update_dimensions`. Declared at - #: class level so the hook is safe before `on_mount` has run. - _widths_absorbed: bool = False + #: Row keys whose widths this widget folded into the columns itself, + #: pending consumption by the next `_update_dimensions`. Created lazily so + #: the hook is safe before `on_mount` has run. + _absorbed_keys: set[str] | None = None + + @property + def _absorbed(self) -> set[str]: + keys = self._absorbed_keys + if keys is None: + keys = self._absorbed_keys = set() + return keys def on_mount(self) -> None: self.cursor_type = "row" @@ -463,7 +471,7 @@ def show( for key, cells in pending: self.add_row(*cells, key=key) self._emitted = dict(pending) - self._absorb_widths(cells for _, cells in pending) + self._absorb_widths(pending) if restore is not None: # A background refresh must not scroll the cursor back into # view — the viewport restore below keeps the user's position. @@ -546,13 +554,13 @@ def _apply_in_place(self, pending: list[tuple[str, list[str | Text]]]) -> bool: for key in doomed: self.remove_row(key) columns = self.ordered_columns - touched: list[list[str | Text]] = [] + touched: list[tuple[str, list[str | Text]]] = [] for key, cells in pending: if key not in current_set: self.add_row(*cells, key=key) - touched.append(cells) + touched.append((key, cells)) elif self._patch_row(key, cells, columns): - touched.append(cells) + touched.append((key, cells)) self._emitted = dict(pending) if touched: self._absorb_widths(touched) @@ -572,7 +580,7 @@ def _patch_row(self, key: str, cells: list[str | Text], columns: list[Column]) - changed = True return changed - def _absorb_widths(self, rows: Iterable[list[str | Text]]) -> None: + def _absorb_widths(self, rows: Iterable[tuple[str, list[str | Text]]]) -> None: """Grow the column widths from cells this widget is holding anyway. `DataTable` derives column widths from `on_idle`, and to do so it @@ -592,7 +600,9 @@ def _absorb_widths(self, rows: Iterable[list[str | Text]]) -> None: columns = self.ordered_columns widths = [column.content_width for column in columns] limit = len(widths) - for cells in rows: + absorbed = self._absorbed + for key, cells in rows: + absorbed.add(key) for index, cell in enumerate(cells): if index >= limit: break @@ -609,27 +619,38 @@ def _absorb_widths(self, rows: Iterable[list[str | Text]]) -> None: # recomputes the virtual size from the dimension pass, which # `update_cell(update_width=False)` does not schedule. self._require_update_dimensions = True - self._widths_absorbed = True + + def remove_row(self, row_key: RowKey | str) -> None: + """Forget the absorption record for a row that is going away. + + A key removed and re-added before the next dimension pass carries + content this widget never measured, so it must not stay on the skip + list. + """ + self._absorbed.discard(row_key.value if isinstance(row_key, RowKey) else row_key) + super().remove_row(row_key) + + def clear(self, columns: bool = False) -> Self: + """Drop the absorption record along with the rows it described.""" + self._absorbed.clear() + return super().clear(columns=columns) def _update_dimensions(self, new_rows: Iterable[RowKey]) -> None: - """Measure only the rows whose widths were not absorbed above. + """Measure only the rows whose widths were absorbed above. The superclass still recomputes the virtual size and handles anything - this widget did not emit (a row added directly, or added after the - absorption and before this idle pass). Should a future Textual rename - the hook, this override simply stops being called and the widget falls - back to today's slower-but-correct measuring pass. + this widget did not account for (a row added directly, or added after + the absorption and before this idle pass). Should a future Textual + rename the hook, this override simply stops being called and the + widget falls back to today's slower-but-correct measuring pass. """ - if not self._widths_absorbed: - super()._update_dimensions(new_rows) - return - self._widths_absorbed = False - absorbed = self._emitted - super()._update_dimensions( - [key for key in new_rows if not self._is_absorbed(key, absorbed)] - ) + absorbed = self._absorbed_keys + if absorbed: + new_rows = [key for key in new_rows if not self._is_absorbed(key, absorbed)] + absorbed.clear() + super()._update_dimensions(new_rows) - def _is_absorbed(self, key: RowKey, absorbed: dict[str, list[str | Text]]) -> bool: + def _is_absorbed(self, key: RowKey, absorbed: AbstractSet[str]) -> bool: """Whether `_absorb_widths` fully accounted for the row behind *key*. Absorption only folds in cell *widths*. The superclass pass also sizes diff --git a/tests/ui/test_table_column_widths.py b/tests/ui/test_table_column_widths.py index bfedb951..81a7408d 100644 --- a/tests/ui/test_table_column_widths.py +++ b/tests/ui/test_table_column_widths.py @@ -242,8 +242,45 @@ async def test_absorption_never_skips_a_labelled_or_auto_height_row() -> None: first.height = 0 second.label = Text("a-very-long-row-label") - table._widths_absorbed = True + table._absorbed.update(key for key in (first.key.value, second.key.value) if key) table._update_dimensions([first.key, second.key]) assert first.height > 0, "auto-height row was skipped and stayed 0 tall" assert table._label_column.content_width >= len("a-very-long-row-label") + + +async def test_a_repaint_that_widens_nothing_leaves_no_stale_skip() -> None: + """Absorption must not license skipping a row it never accounted for. + + When a repaint absorbs widths but grows no column, no dimension pass is + scheduled. If the "widths were absorbed" state survives to whatever pass + runs next, that pass filters out rows this widget merely *emitted* once — + including a row re-added since, whose wider content then never reaches + the column. + """ + app = _TableApp() + async with app.run_test(size=(120, 12)) as pilot: + table = app.query_one(ResourceTable) + await pilot.pause() + table.show("pods", _pods(["alpha", "beta"]), all_namespaces=False, pattern="") + await pilot.pause() + + # A repaint that changes a cell but widens nothing: absorption runs, + # no column grows, so nothing schedules a dimension pass. + table.show( + "pods", + _pods_with_phase([("alpha", "Pending"), ("beta", "Running")]), + all_namespaces=False, + pattern="", + ) + + name_column = table.ordered_columns[1] + wide = "a" * (name_column.content_width + 30) + key = next(iter(table._emitted)) + table.remove_row(key) + table.add_row(*([wide] * len(table.ordered_columns)), height=1, key=key) + await pilot.pause() + + assert name_column.content_width >= len(wide), ( + f"column stayed {name_column.content_width} wide, needed {len(wide)}" + ) From be35399680158bab87deb180102ef64ed5a121a8 Mon Sep 17 00:00:00 2001 From: hellices Date: Fri, 7 Aug 2026 12:54:00 +0900 Subject: [PATCH 5/5] docs: correct the inverted _update_dimensions docstring Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/korvid/ui/widgets/resource_table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/korvid/ui/widgets/resource_table.py b/src/korvid/ui/widgets/resource_table.py index 03735564..df37d86d 100644 --- a/src/korvid/ui/widgets/resource_table.py +++ b/src/korvid/ui/widgets/resource_table.py @@ -636,7 +636,7 @@ def clear(self, columns: bool = False) -> Self: return super().clear(columns=columns) def _update_dimensions(self, new_rows: Iterable[RowKey]) -> None: - """Measure only the rows whose widths were absorbed above. + """Measure only the rows whose widths were not absorbed above. The superclass still recomputes the virtual size and handles anything this widget did not account for (a row added directly, or added after