fix: style_cell and hover_template lost when sorting in descending order by ManasVardhan · Pull Request #8915 · marimo-team/marimo

ManasVardhan · 2026-03-29T00:47:26Z

Summary

Fixes #8847. style_cell styles (and hover_template texts) were lost when sorting a table in descending order.

Root Cause

_style_cells() and _hover_cells() generated row IDs assuming they correlated with the sort order:

Ascending: range(skip, skip + take)
Descending: range(total - 1 - skip, ..., -1)

This only worked when sorting by the index column. When sorting by any other column, the actual _marimo_row_id values on the displayed page are arbitrary and do not follow sequential or reversed-sequential patterns. The frontend looks up styles by _marimo_row_id, so the mismatched keys caused all styles to be silently discarded.

Fix

Introduces _get_page_row_ids() which reads the actual _marimo_row_id values from the sorted _searched_manager data for the current page. This ensures style/hover dict keys always match what the frontend uses for lookup, regardless of sort column or direction.

For tables without stable row IDs (list/dict data), positional indices are returned since both backend and frontend use positional indexing.

The descending parameter is removed from _style_cells() and _hover_cells() since it is no longer needed.

Additional fixes

Removes xfail from test_cell_search_df_hover_texts_sorted (hover with sorted data now works)
Fixes pre-existing test bug where sort was passed as a bare SortArgs instead of [SortArgs(...)]

Tests

test_cell_styles_descending_non_index_column: 4-row table sorted by Score (not index) descending. Verifies all row IDs present with correct style values.
test_cell_styles_sorted_by_value_with_pagination: 20-row table sorted by Value descending with page_size=5. Verifies page 0 contains the correct (non-sequential) row IDs.
All existing style/hover tests continue to pass.

When sorting by any column, _style_cells() and _hover_cells() generated sequential row IDs (or reversed sequential IDs for descending) that did not match the actual _marimo_row_id values of the displayed rows. This caused the frontend to discard all styles/hover texts because the dict keys did not match its row ID lookups. The fix introduces _get_page_row_ids() which reads actual _marimo_row_id values from the sorted searched manager data for the current page. This ensures style/hover dict keys always match what the frontend expects, regardless of sort column or direction. The descending parameter is removed from _style_cells() and _hover_cells() since it is no longer needed. Also fixes hover_template with sorted data (previously marked xfail) and fixes the pre-existing test bug where sort arg was passed as a single SortArgs instead of a list. Closes #8847

vercel · 2026-03-29T00:47:32Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
marimo-docs	Ready	Preview, Comment	Mar 31, 2026 9:24pm

github-actions · 2026-03-29T00:47:38Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

for more information, see https://pre-commit.ci

ManasVardhan · 2026-03-29T00:49:42Z

I have read the CLA Document and I hereby sign the CLA

Copilot

Pull request overview

Fixes a backend/frontend row-ID mismatch that caused style_cell styles and hover_template texts to disappear when sorting tables in descending order by non-index columns, by keying styles/hover text using the actual _marimo_row_id values for the current page.

Changes:

Add _get_page_row_ids() to derive per-page row IDs from the sorted/searched manager’s _marimo_row_id column.
Update _style_cells() and _hover_cells() to use _get_page_row_ids() and remove the descending parameter plumbing.
Extend and correct tests: add regression tests for descending sort + pagination, un-xfail the sorted-hover test, and fix sort arg shape.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`marimo/_plugins/ui/_impl/table.py`	Introduces `_get_page_row_ids()` and rewires style/hover computation to use true page row IDs rather than generated ranges.
`tests/_plugins/ui/_impl/test_table.py`	Adds regression tests for style behavior under descending non-index sorts/pagination; enables sorted-hover test and fixes `sort` argument format.

Copilot · 2026-03-30T17:47:06Z

+        if response.all_rows or response.error:
+            if self._has_stable_row_id:
+                try:
+                    all_ids = self._searched_manager.data[
+                        INDEX_COLUMN_NAME
+                    ].to_list()
+                    return all_ids[skip : skip + take]
+                except Exception:
+                    pass
+            return range(skip, skip + take)


_get_page_row_ids() materializes the entire _marimo_row_id column via to_list() and then slices it. For large tables this is an O(n) read (and potentially a full materialization) on every search page that computes styles/hover, which can be a significant performance regression compared to the prior O(1) range() behavior. Consider extracting IDs only for the requested page (e.g., take/page-slice the manager first, then read _marimo_row_id just for that slice), and avoid attempting a full-column read when response.error is set (which may occur in lazy/unknown-row-count cases).

Suggested change

if response.all_rows or response.error:

if self._has_stable_row_id:

try:

all_ids = self._searched_manager.data[

INDEX_COLUMN_NAME

].to_list()

return all_ids[skip : skip + take]

except Exception:

pass

return range(skip, skip + take)

# If there was an error computing row IDs (e.g. lazy/unknown-row-count),

# fall back to positional indexing without attempting to read from data.

if response.error:

return range(skip, skip + take)

if response.all_rows and self._has_stable_row_id:

try:

# Slice to just the requested page first, then read IDs to

# avoid materializing the entire index column for large tables.

page_data = self._searched_manager.data[skip : skip + take]

page_ids = page_data[INDEX_COLUMN_NAME].to_list()

return page_ids

except Exception:

# On any failure, fall back to positional indexing.

return range(skip, skip + take)

if response.all_rows:

# No stable row IDs: use positional indices.

return range(skip, skip + take)

# Filtered case: use the provided row IDs slice.

Copilot · 2026-03-30T17:47:07Z

+    create_dataframes(
+        {
+            "Name": ["alpha", "beta", "gamma", "delta"],
+            "Score": [1.0, 2.0, 3.0, 4.0],
+        },
+        exclude=NON_EAGER_LIBS,


This regression test intends to cover sorting by a non-index column where the sorted page’s _marimo_row_id order is not sequential/reversed-sequential. However, the chosen Score values [1.0, 2.0, 3.0, 4.0] are perfectly correlated with the original row IDs, so descending sort yields row IDs [3,2,1,0] which the old (buggy) “reverse range” logic would also generate—meaning the test may pass even without the fix. To ensure the test would fail on the pre-fix implementation, use non-monotonic scores (e.g., scores that produce an order like [1,3,0,2]) and assert the expected row ID keys accordingly.

mscolnick · 2026-03-31T15:00:10Z

@ManasVardhan it looks like tests are failing. mind taking a look?

- Optimize _get_page_row_ids to slice page data before reading IDs, avoiding full column materialization for large tables - Handle response.error separately to skip data reads in lazy/unknown cases - Add LOGGER.warning on fallback to positional indexing - Use non-monotonic scores in test to actually catch the original bug - Exclude non-eager libs from sorted hover test (no stable row IDs)

…omment Address review feedback: - Use take().as_frame() instead of direct .data slicing to handle lazy frame backends (polars LazyFrame, duckdb, ibis) that failed with 'Slicing is not supported on LazyFrame'. - Remove em dash from test comment.

vercel · 2026-03-31T20:59:10Z

Deployment failed with the following error:

Invalid request: `attribution.gitUser` should NOT have additional property `isBot`.

mscolnick · 2026-03-31T21:19:40Z

@ManasVardhan sorry for the back and forth. looks like lint it failing. lmk if you want to handle or want us to

TableManager base class does not define as_frame(), only NarwhalsTableManager does. Use getattr with fallback to .data so mypy does not complain about attr-defined on the base type.

ManasVardhan · 2026-03-31T21:35:30Z

hi @mscolnick , should be good now.

mscolnick · 2026-03-31T21:40:40Z

looks like a bad test that is irrelevant. this looks great, thank you!

github-actions · 2026-03-31T21:45:55Z

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.22.1-dev3

…der (marimo-team#8915) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Myles Scolnick <myles@marimo.io>

vercel Bot deployed to Preview March 29, 2026 00:48 View deployment

[pre-commit.ci] auto fixes from pre-commit.com hooks

ac13012

for more information, see https://pre-commit.ci

vercel Bot deployed to Preview March 29, 2026 00:50 View deployment

mscolnick requested a review from Copilot March 30, 2026 17:42

mscolnick mentioned this pull request Mar 30, 2026

fix: preserve style_cell mapping when table sorted descending #8912

Closed

Copilot started reviewing on behalf of mscolnick March 30, 2026 17:43 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

fix: add type annotation to satisfy mypy no-any-return

cf85261

vercel Bot deployed to Preview March 30, 2026 21:52 View deployment

fix: add cast to satisfy mypy no-any-return check

5b20f15

vercel Bot deployed to Preview March 30, 2026 21:57 View deployment

mscolnick previously approved these changes Mar 31, 2026

View reviewed changes

mscolnick added the bug Something isn't working label Mar 31, 2026

mscolnick reviewed Mar 31, 2026

View reviewed changes

Comment thread marimo/_plugins/ui/_impl/table.py

mscolnick dismissed their stale review via 7a52110 March 31, 2026 20:35

vercel Bot deployed to Preview March 31, 2026 20:37 View deployment

remove release files

2a02c11

vercel Bot deployed to Preview March 31, 2026 20:39 View deployment

mscolnick previously approved these changes Mar 31, 2026

View reviewed changes

ManasVardhan dismissed mscolnick’s stale review via a74788a March 31, 2026 20:59

mscolnick previously approved these changes Mar 31, 2026

View reviewed changes

fix: use getattr for as_frame to satisfy mypy attr-defined check

e251b7f

TableManager base class does not define as_frame(), only NarwhalsTableManager does. Use getattr with fallback to .data so mypy does not complain about attr-defined on the base type.

ManasVardhan dismissed mscolnick’s stale review via e251b7f March 31, 2026 21:23

vercel Bot deployed to Preview March 31, 2026 21:24 View deployment

mscolnick approved these changes Mar 31, 2026

View reviewed changes

mscolnick merged commit fce3c66 into marimo-team:main Mar 31, 2026
26 of 40 checks passed

-        if response.all_rows or response.error:
-            if self._has_stable_row_id:
-                try:
-                    all_ids = self._searched_manager.data[
-                        INDEX_COLUMN_NAME
-                    ].to_list()
-                    return all_ids[skip : skip + take]
-                except Exception:
-                    pass
-            return range(skip, skip + take)
+        # If there was an error computing row IDs (e.g. lazy/unknown-row-count),
+        # fall back to positional indexing without attempting to read from data.
+        if response.error:
+            return range(skip, skip + take)
+        if response.all_rows and self._has_stable_row_id:
+            try:
+                # Slice to just the requested page first, then read IDs to
+                # avoid materializing the entire index column for large tables.
+                page_data = self._searched_manager.data[skip : skip + take]
+                page_ids = page_data[INDEX_COLUMN_NAME].to_list()
+                return page_ids
+            except Exception:
+                # On any failure, fall back to positional indexing.
+                return range(skip, skip + take)
+        if response.all_rows:
+            # No stable row IDs: use positional indices.
+            return range(skip, skip + take)
+        # Filtered case: use the provided row IDs slice.

Uh oh!

Conversation

ManasVardhan commented Mar 29, 2026

Summary

Root Cause

Fix

Additional fixes

Tests

Uh oh!

vercel Bot commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ManasVardhan commented Mar 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

mscolnick commented Mar 31, 2026

Uh oh!

Uh oh!

vercel Bot commented Mar 31, 2026

Uh oh!

mscolnick commented Mar 31, 2026

Uh oh!

ManasVardhan commented Mar 31, 2026

Uh oh!

mscolnick commented Mar 31, 2026

Uh oh!

Uh oh!

github-actions Bot commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel Bot commented Mar 29, 2026 •

edited

Loading

github-actions Bot commented Mar 29, 2026 •

edited

Loading