fix(bench): MUI + TanStack comparators publish post-filter row count#156
Merged
Conversation
B2 follow-up #5 residual. Since PR #140 surfaces comparator evidence in the H7/H8 filter hypotheses, the comparators' hardcoded `data-bench-result-row-count` (always dataset.rows.length = 3000) is now visibly misleading. - TanStack: publish `table.getRowModel().rows.length` (the post-filter, post-sort model) instead of `data.length` (full dataset). - MUI: track `gridFilteredTopLevelRowCountSelector` via a `filteredRowsSet` subscription instead of the static row state. Verified in Chromium (S2/hypothesis): both now report filter-metadata → 750, filter-text → 500 (matching pretable), and stay 3000 on sort. Adds jsdom regression tests asserting the post-filter count for each. AG Grid is intentionally NOT changed here: neither a synchronous getDisplayedRowCount() read nor onModelUpdated/onFilterChanged updates the count in Chromium (works in jsdom), and it stays exactly 3000 rather than dropping — suggesting setFilterModel may not reduce the client-side row model in the real bench. Tracked as a separate investigation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Vercel preview readyPreview: https://pretable-kfav9a5yk-cacheplane.vercel.app Updated automatically by the |
4 tasks
blove
added a commit
that referenced
this pull request
Jun 5, 2026
Completes B2 follow-up #5 residual #3 (MUI + TanStack landed in #156). AG Grid now reports filter-metadata → 750, filter-text → 500 (matching pretable) and stays 3000 on sort, verified in Chromium. Root cause: AG Grid renders its rows imperatively (outside React), so the bench settle loop records result_row_count the instant the filtered rows paint — but a normal setState re-render of the data-bench-result-row-count attribute lands a few frames later, after settle, so the bench captured the stale pre-filter count. (aria-rowcount confirmed AG Grid *does* filter correctly: 501/751.) MUI and TanStack didn't hit this because they're React-rendered — visible rows and the count update in the same commit. Fix: publish the count via onFilterChanged using flushSync, forcing the attribute update synchronously with the event so it lands inside the settle window. Also gate the interaction effect on a gridReady flag so a plan present at mount is applied once the grid API exists (and to make the behavior unit-testable). Adds a jsdom regression test for the published post-filter count. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
B2 follow-up #5 residual. Now that #140 surfaces comparator evidence inside the H7/H8 filter hypotheses, the comparators' hardcoded
data-bench-result-row-count(alwaysdataset.rows.length= 3000) is visibly misleading — the evidence array showed AG Grid/MUI/TanStack as "3000 rows" even after a filter reduced them. Pretable was already correct (it reports via telemetry override, not the DOM attribute).Changes
table.getRowModel().rows.length(the post-filter, post-sort row model) instead ofdata.length(full dataset).gridFilteredTopLevelRowCountSelectorvia afilteredRowsSetsubscription instead of the static row state.status === "running"→ 2 of 4 rows).Verification (Chromium, S2/hypothesis, n=2)
sortfilter-metadatafilter-textBefore this PR, tanstack/mui reported 3000 on every script. Now they match pretable's filtered count and correctly stay at 3000 for sort (no row reduction).
AG Grid intentionally excluded
AG Grid was investigated and reverted — its
data-bench-result-row-countstays at the full dataset size for now. Neither a synchronousgetDisplayedRowCount()read nor theonModelUpdated/onFilterChangedevents update the count in the Chromium bench (both work in jsdom). It stays exactly 3000 — not 0 — which suggests AG Grid's programmaticsetFilterModelmay not actually reduce the client-side row model in the real bench. If confirmed, that would also mean AG Grid's #5b filter comparator latencies measured a non-filtering re-render. Out of scope here; tracked as a separate investigation rather than shipping a Chromium-broken path with a false-confidence jsdom test.Gates
pnpm --filter @pretable/app-bench typecheckcleanpnpm --filter @pretable/app-bench lintcleanpnpm --filter @pretable/app-bench test— 61 passed (incl. 2 new)prettier --checkcleanresult_row_countis informational evidence (status verdicts remain pretable-only)🤖 Generated with Claude Code