Repository navigation
fix(ui-components): correct histogram stats, persistence and redraw lifecycle - #1028
Merged
rx18-eng merged 1 commit intoOct 9, 2026
Merged
Conversation
…ifecycle The histogram panel added in HSF#843 has five defects in how it counts, persists and repaints values. - Out-of-range values inflated the stats. jsroot's TH1.Fill clamps a value outside [xmin, xmax] into the underflow/overflow cell, so it is never plotted, but the panel still counted it in "Entries" and folded it into "Mean". A value far outside the axis range skewed the mean badly while being invisible on the plot. Such values are now ignored. - localStorage grew without bound. Every value was appended to the saved array forever, so a long masterclass session eventually exceeded the quota, at which point setItem threw and persistence silently stopped. The stored history is now capped. - Restored data was trusted blindly. Values saved under a different axis range, or a hand-edited entry holding null or a string, were filled and counted as-is. Restore now revalidates each value, and the mean no longer divides by zero when nothing survives validation. - The export cancelled its own download. The object URL was revoked synchronously right after click(), which can abort the in-flight download. This matches the bug already documented and fixed in the saveFile helper: revoke on the next tick instead, and detach the temporary anchor. - Hiding the panel stranded a queued redraw. The pending timer handle was never cleared, so the next scheduleRedraw saw a non-null handle and returned early, permanently stopping live updates, while the stranded timer fired against a detached element. Adds a test for each defect; all six fail against the previous code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🚀 Preview deployed: http://phoenix-pr-1028.surge.sh Built from b1e7ebc. |
rx18-eng
approved these changes
Oct 9, 2026
This branch was successfully deployed
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.
Summary
The in-browser histogram panel added in #843 has five defects in how it counts, persists and repaints values. Each is fixed here with a regression test.
The bugs
1. Out-of-range values inflated the statistics.
jsroot's
TH1.Fillclamps a value outside[xmin, xmax]into the underflow or overflow cell, so it is never drawn. The panel still counted it inEntriesand folded it intoMean. With the default ATLAS config (xmin: 20,xmax: 120), a single stray value of 500 shifts the reported mean by a large margin while contributing nothing visible to the plot, so the displayed statistics disagree with the displayed histogram.2. localStorage grew without bound.
Every value was appended to the persisted array forever. A long masterclass session eventually exceeds the storage quota, at which point
setItemthrows, thecatchswallows it, and persistence silently stops working with no indication to the user. The stored history is now capped at 5000 values.3. Restored data was trusted blindly.
restoreFromStoragefilled and counted whatever it parsed. Values saved under a different axis range (the three shipped configs span1-5,20-120and0-200GeV and share a key when titles match), or a hand-edited entry holdingnullor a string, were accepted as-is. Restore now revalidates each value through the same range check. The mean also no longer divides by zero when nothing survives validation.4. The export cancelled its own download.
exportCSVrevoked the object URL synchronously right afterclick(), which can abort the in-flight download. This is the same bug already found and documented in thesaveFilehelper:The export now follows that pattern and detaches the temporary anchor.
5. Hiding the panel stranded a queued redraw.
The
showHistogramsetter cleareddrawnbut left the pending redraw timer armed and its handle non-null.scheduleRedrawreturns early whenever that handle is set, so after one hide during a pending redraw, live updates stop permanently for the life of the component. The stranded timer also fired a redraw against a detached element.Testing
Adds
histogram-panel-overlay.component.test.tswith one test per defect. All six fail against the previous code and pass with the fix. The fullphoenix-ngsuite is green: 227 passed, 0 failed.🤖 Generated with Claude Code