Skip to content

Fix too many bins protection for histogram distribution back end calls#1745

Merged
bobular merged 4 commits into
mainfrom
fix-too-many-bins-protection
Jun 8, 2026
Merged

Fix too many bins protection for histogram distribution back end calls#1745
bobular merged 4 commits into
mainfrom
fix-too-many-bins-protection

Conversation

@bobular

@bobular bobular commented Jun 8, 2026

Copy link
Copy Markdown
Member

The bug

In getSafeLowerBound (default-axis-range.ts), the "max bins" protection only triggered when rangeMin === rangeMax. The real-world year variable has rangeMin: 2000, rangeMax: 2004not equal — so the guard was skipped, the lower bound defaulted to 0, and a distribution request for 0..2004 with binWidth: 1 produced 2004 bins, exceeding the backend's 2000-bin limit.

The equality check was a red herring: the actual danger is purely "would anchoring the lower bound at 0 produce too many bins?" — which is rangeMax / binWidth > 2000, regardless of whether rangeMin equals rangeMax.

The fix

Dropped the rangeMin === rangeMax && condition. Now whenever anchoring at 0 would blow the bin budget, the lower bound falls back to rangeMin - binWidth (giving 1999..2004 → 5 bins for the year case). The ordinary case (small positive variable) still starts at 0.

No tests

Complex spaghetti logic can easily break (this bug being an example). The bin range code desperately needs refactoring but the only safe way to do this is build a wall of tests and make sure the refactor has no regressions. This is a big TODO but I thought it would be a good idea to start now. It turns out the test machinery had been neglected for some time, so we fixed that here (though the useAnalysis test has been ignored for now because there are jest transform problems with a transitive dependency to jquery - we already fixed one for d3)

The test

packages/libs/eda/src/lib/core/utils/default-axis-range.test.ts — two cases against numberDateDefaultAxisRange directly:

  1. The year variable (2000–2004) must not exceed 2000 bins and must still cover the data. (This is the one that reproduced the bug.)
  2. An ordinary positive integer (5–100) still starts at 0 — regression guard.

@bobular bobular requested a review from aurreco-uga June 8, 2026 18:21

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bug fix! Everything else is test-related. See the PR description for why I added tests

@bobular bobular merged commit 35a84af into main Jun 8, 2026
1 check passed
@bobular bobular deleted the fix-too-many-bins-protection branch June 8, 2026 19:33
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