Skip to content

fix(ui-components): correct histogram stats, persistence and redraw lifecycle - #1028

Merged
rx18-eng merged 1 commit into
HSF:mainfrom
Shivansh1205:fix/histogram-panel-stats-and-persistence
Oct 9, 2026
Merged

rx18-eng merged 1 commit into
HSF:mainfrom
Shivansh1205:fix/histogram-panel-stats-and-persistence

Conversation

@Shivansh1205

Copy link
Copy Markdown
Contributor

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.Fill clamps a value outside [xmin, xmax] into the underflow or overflow cell, so it is never drawn. The panel still counted it in Entries and folded it into Mean. 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 setItem throws, the catch swallows 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.
restoreFromStorage filled and counted whatever it parsed. Values saved under a different axis range (the three shipped configs span 1-5, 20-120 and 0-200 GeV and share a key when titles match), or a hand-edited entry holding null or 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.
exportCSV revoked the object URL synchronously right after click(), which can abort the in-flight download. This is the same bug already found and documented in the saveFile helper:

// The download is started asynchronously by the click above, so revoking
// synchronously here can cancel it. Release the URL on the next tick, once
// the browser has taken its own reference to the blob.
setTimeout(() => URL.revokeObjectURL(objectUrl));

The export now follows that pattern and detaches the temporary anchor.

5. Hiding the panel stranded a queued redraw.
The showHistogram setter cleared drawn but left the pending redraw timer armed and its handle non-null. scheduleRedraw returns 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.ts with one test per defect. All six fail against the previous code and pass with the fix. The full phoenix-ng suite is green: 227 passed, 0 failed.

🤖 Generated with Claude Code

…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>
@github-actions

Copy link
Copy Markdown

🚀 Preview deployed: http://phoenix-pr-1028.surge.sh

Built from b1e7ebc.

@rx18-eng
rx18-eng merged commit dcb7416 into HSF:main Oct 9, 2026
4 checks passed

This branch was successfully deployed

1 active deployment
pull-request — b1e7ebc6 Deployed Sep 13, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants