perf(frontend): memoize ensemble chart statistics#45
Open
lewisjared wants to merge 2 commits into
Open
Conversation
Wraps the box-whisker grouping/quartile pipeline in EnsembleChart in a single useMemo, so it only recomputes when the data or grouping inputs change instead of on every render. Drops the window-level mousemove subscription that recalculated tooltip side on every pointer move anywhere on the page. The tooltip now derives its side from Recharts' own chart-local coordinate and viewBox, so the useMousePosition hook has no remaining consumers and is removed. Replaces the linear indexOf scan used to align series values by index value in createChartData with a precomputed Map per series, cutting the alignment step from effectively cubic to linear in the number of rows. Adds createChartData coverage for duplicate index values to lock in the first-occurrence semantics the Map lookup preserves from indexOf.
✅ Deploy Preview for climate-ref canceled.
|
📝 WalkthroughWalkthroughThe ensemble chart now consolidates memoized statistics preparation and positions tooltips using chart coordinates. The removed mouse-position hook eliminates window-level tracking. Series alignment uses per-series maps, with duplicate-index behaviour covered by a new test. ChangesChart Data Optimisations
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
lewisjared
marked this pull request as ready for review
July 20, 2026 09:55
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
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.
The ensemble chart recomputed its whole box-whisker pipeline on every render, and it subscribed to a window-level
mousemovelistener that set state on every pointer move anywhere on the page. Moving the mouse anywhere re-ran grouping, quartiles and sorting for every mounted chart.useMemo. The existingallGroupNamesandscalememos were keyed on the result of that pipeline, so they never hit either. They do now.useMousePositionAndWidthand picks the tooltip side from the chart-localcoordinateandviewBoxthat Recharts already passes to the tooltip renderer. The hook had no other consumer, so it is deleted. Behaviour shifts from "cursor on the right half of the window" to "cursor on the right half of the chart", which is what the original logic was approximating.indexOfscan increateChartDatawith one lookup map per series. Alignment was O(rows x series x index length). Ties keep the first occurrence, matchingindexOf.No behaviour change intended beyond the tooltip side becoming chart-relative. 221 frontend tests pass, including a new case pinning the duplicate-index first-occurrence semantics.
Summary by CodeRabbit
Performance
Bug Fixes
Tests