Skip to content

fix(explore): pin and fix clipped segments in horizontal row-contribution stacked bar charts - #42610

Open
rusackas wants to merge 4 commits into
masterfrom
fix/issue-30914-followup
Open

fix(explore): pin and fix clipped segments in horizontal row-contribution stacked bar charts#42610
rusackas wants to merge 4 commits into
masterfrom
fix/issue-30914-followup

Conversation

@rusackas

Copy link
Copy Markdown
Member

SUMMARY

In a horizontal, stacked, row-contribution (100%) bar chart, some segments could vanish from the render entirely even though the underlying percentages were correct and summed to 100%.

The row-contribution axis max was hard-coded to exactly 1. Contribution shares are normalized so a stacked row is supposed to sum to 1, but floating point rounding can push the actual stacked total fractionally above 1 (e.g. 1.0000000000000002). When that happens, capping the axis max at exactly 1 causes echarts to clip the topmost stacked segment entirely on the swapped axis used for horizontal bars, instead of just rounding the pixel width. This lines up with the community's own workaround of nudging "Truncate Y Axis" max to 1.00001, which happened to sidestep the same edge case.

This PR pads the axis max up to the actual stacked total when it exceeds 1, so no segment gets clipped, while leaving the 0-1 default in place for the (much more common) case where the total is exactly 1 or below.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — this is a floating point edge case in axis-bounds calculation; see the reproduction and screenshots already on the issue.

TESTING INSTRUCTIONS

  1. Build a horizontal, stacked bar chart with contribution mode = Row over a dataset with several small groupby segments per category, using the SQL repro attached to Inconsistent behavior for stacked bar-chart #30914.
  2. Reload the chart a few times; before this fix, the same segment goes missing on certain categories on every load. After the fix, all segments render.
  3. pytest-equivalent for the frontend: npm run test -- plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts — new test should not clip small segments when row-contribution percentages float above 1 in horizontal stacked bar charts pins the regression.

ADDITIONAL INFORMATION

  • Has associated issue: Fixes Inconsistent behavior for stacked bar-chart #30914
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@bito-code-review

bito-code-review Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #a1b6e5

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: a0b6b96..a0b6b96
    • superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
    • superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot Bot added change:frontend Requires changing the frontend viz:charts:timeseries Related to Timeseries labels Jul 30, 2026
@bito-code-review

Copy link
Copy Markdown
Contributor

The flagged issue is correct. Using the spread operator (...) with Math.max on a large array can exceed the maximum number of arguments allowed by the JavaScript engine, leading to a RangeError. To resolve this, you should replace the spread operator with a reduce function to calculate the maximum value incrementally.

Here is the corrected implementation for the logic in superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:

      const stackedTotalMax = sortedTotalValues.reduce((max, v) => {
        if (typeof v === 'number' && !Number.isNaN(v)) {
          return Math.max(max, v);
        }
        return max;
      }, -Infinity);

I have checked the available PR comments, and there are no other review comments to address. Would you like me to proceed with any other tasks?

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts

const stackedTotalMax = sortedTotalValues.reduce((max, v) => {
        if (typeof v === 'number' && !Number.isNaN(v)) {
          return Math.max(max, v);
        }
        return max;
      }, -Infinity);

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.35294% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.60%. Comparing base (9a5cf76) to head (7f22d73).

Files with missing lines Patch % Lines
...gin-chart-echarts/src/Timeseries/transformProps.ts 82.35% 6 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #42610   +/-   ##
=======================================
  Coverage   65.59%   65.60%           
=======================================
  Files        2819     2819           
  Lines      160166   160199   +33     
  Branches    36569    36586   +17     
=======================================
+ Hits       105065   105093   +28     
- Misses      53053    53058    +5     
  Partials     2048     2048           
Flag Coverage Δ
javascript 71.76% <82.35%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts Outdated
@netlify

netlify Bot commented Jul 30, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 7f22d73
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a71b5bcc9c0a30008480b16
😎 Deploy Preview https://deploy-preview-42610--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@bito-code-review

bito-code-review Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #784aa2

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: a0b6b96..23cfc0e
    • superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
    • superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@pull-request-size pull-request-size Bot added size/L and removed size/M labels Aug 4, 2026
@bito-code-review

bito-code-review Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #c5df3f

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 23cfc0e..c3e69da
    • superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
    • superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

rusackas and others added 4 commits August 4, 2026 02:49
…tion stacked bar charts

Contribution percentages are normalized so a stacked row should sum to 1,
but floating point rounding can push the actual stacked total fractionally
above 1 (e.g. 1.0000000000000002). Hard-capping the row-contribution axis
max at exactly 1 in that case caused echarts to clip the topmost stacked
segment entirely on the axis swapped for horizontal orientation, instead of
just rounding the pixel width. Pad the axis max up to the actual stacked
total when it exceeds 1 so no segment gets clipped.

Fixes #30914

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`sortedTotalValues` holds pre-normalization row totals. For
contributionMode=row those are already ~1 (the case this PR targets),
but for an Expand ("100% stacked") chart they're the raw, un-normalized
sums, so padding the axis max against them stretched the chart out to
the raw total instead of the intended 0-1 range. Limit the padding to
row-contribution mode and keep the flat 1 max for Expand stacks.

Also swap the Math.max(...spread) for a reduce so a very large series
count can't blow the JS argument-spread limit.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sortedTotalValues summed every series value per row regardless of stack,
so with time_compare each independently-normalized comparison period
inflated the combined row total, pushing the axis max to ~N instead of
~1 and shrinking 100% bars accordingly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Series are sorted by name (descending) for stacking, so summing three
shares whose overflow is a single float ULP can round differently
depending on that order, making the test flaky. Widen the margin above
1 so the assertion holds regardless of summation order.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rusackas
rusackas force-pushed the fix/issue-30914-followup branch from 0e2185b to 7f22d73 Compare August 4, 2026 09:49
@bito-code-review

bito-code-review Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #55b55c

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 39c7074..7f22d73
    • superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
    • superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend plugins preset-io size/L viz:charts:timeseries Related to Timeseries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent behavior for stacked bar-chart

2 participants