fix(explore): stacked Timeseries Bar total excludes the sort-only metric by vjymisal0 · Pull Request #42881 · apache/superset

vjymisal0 · 2026-08-07T06:42:43Z

SUMMARY

On a stacked Timeseries Bar chart (echarts_timeseries_bar) with "Show Total"/"Only Total" enabled, the displayed stacked total was silently inflated when the chart also used a timeseries_limit_metric (the "Sort by" metric used to order/limit the x-axis) that isn't one of the chart's displayed metrics.

extraMetricLabels — which correctly excludes series-limit/sort-only metrics when building the actual rendered series via extractSeries — was computed in Timeseries/transformProps.ts after extractDataTotalValues ran, and was never passed into it. extractDataTotalValues summed every remaining numeric key in each row except the x-axis column, so the sort-only metric's value leaked into the total even though it never appeared in the legend.

Fix:

extractDataTotalValues (utils/series.ts) now accepts an optional extraMetricLabels list and excludes those columns the same way it already excludes the x-axis column.
transformProps.ts now computes extraMetricLabels once, above both call sites that need it (it was already being computed below, for extractSeries), instead of after extractDataTotalValues had already run without it.

This is easy to hit in practice: timeseries_limit_metric is the standard way to control x-axis category ordering when the natural order isn't alphabetical (e.g. binned/bucketed labels like 1-3d, 4-6d), which is precisely the situation where the sort metric is likely to differ from the displayed metrics.

Closes #42701

TESTING INSTRUCTIONS

Added two unit tests to test/utils/series.test.ts:

extractDataTotalValues excludes extraMetricLabels from the stacked total (#42701) — reproduces the bug on the unfixed path (verifies the pre-fix behavior with a comment) and asserts the fixed behavior with extraMetricLabels passed.
extractDataTotalValues still respects legendState alongside extraMetricLabels — confirms the existing legend-toggle exclusion still works together with the new one.

Ran locally:

npx jest plugins/plugin-chart-echarts/test/utils/series.test.ts
npx jest plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts
npx oxlint --config oxlint.json --quiet <changed files>

All pass (102 + 112 tests, oxlint clean). tsc --noEmit on the package reports pre-existing errors from unbuilt monorepo package type declarations and other unrelated files/lines (confirmed identical patterns exist elsewhere in the same untouched file); none are on the lines this PR changes.

Manual repro steps are in the linked issue (stacked bar chart, 2+ metrics, timeseries_limit_metric set to a non-displayed metric, "Only Total" enabled).

ADDITIONAL INFORMATION

Has associated issue: Timeseries Bar: stacked 'Only Total' sum includes the series-limit/sort metric's value, not just the displayed metrics #42701
Required feature flags:
Changes UI
Includes DB Migration
Introduces new feature or API
Removes existing feature or API

On a stacked Timeseries Bar chart with "Only Total"/"Show Total" enabled, the displayed stacked total silently included the timeseries_limit_metric value when that sort/limit metric differed from the chart's displayed metrics. The metric is correctly excluded from the rendered series/legend by extractSeries via extraMetricLabels, but extractDataTotalValues summed every remaining numeric column except the x-axis column, so the sort metric's value still leaked into the total. extractDataTotalValues now accepts an optional extraMetricLabels list and excludes those columns the same way it already excludes the x-axis column. transformProps.ts computes extraMetricLabels once, above both call sites that need it, instead of after extractDataTotalValues had already run without it. Closes #42701

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

bito-code-review · 2026-08-07T06:42:54Z

Code Review Agent Run #83afb7

Actionable Suggestions - 0

Review Details

Files reviewed - 3 · Commit Range: bbb8384..bbb8384
- superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
- superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
- superset-frontend/plugins/plugin-chart-echarts/test/utils/series.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-code-review · 2026-08-07T06:48:31Z

The flagged issue is correct. The extraMetricLabels are currently derived from raw metric labels, but rebasedData has already been processed through rebaseForecastDatum, which applies verboseMap to rename keys. Consequently, the exclusion set in extractDataTotalValues fails to match the verbose keys present in the data, causing sort-only metrics to be incorrectly included in the stacked totals.

To resolve this, you should map the extraMetricLabels using the same verboseMap before passing them to extractDataTotalValues. In transformProps.ts, update the extraction logic as follows:

  const extraMetricLabels = extractExtraMetrics(chartProps.rawFormData).map(
    (metric) => verboseMap[getMetricLabel(metric)] || getMetricLabel(metric),
  );

This ensures the exclusion set uses the same labels as the rebased data rows.

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

const extraMetricLabels = extractExtraMetrics(chartProps.rawFormData).map(
    (metric) => verboseMap[getMetricLabel(metric)] || getMetricLabel(metric),
  );

…luding rebasedData's keys have already been through rebaseForecastDatum, which renames a key to its verboseMap entry when one is configured for that metric. extraMetricLabels was still the raw metric label, so a sort-only metric with a verbose_name set would silently fail to match the exclusion set in extractDataTotalValues (and in extractSeries, which has the same requirement and was already using this variable). Added a regression test in transformProps.test.ts covering a verbose-named sort-only metric end to end. Thanks to the Bito AI review on this PR for catching this.

vjymisal0 · 2026-08-07T06:59:49Z

Good catch — pushed a fix. extraMetricLabels is now resolved through verboseMap the same way rebaseForecastDatum renames the data keys, using the same verboseMap[label] ?? label fallback pattern already used elsewhere in this file. Added a regression test in transformProps.test.ts covering a verbose-named sort-only metric end to end (fails without the fix: total comes out to 34 instead of 32; passes with it).

codecov · 2026-08-07T07:33:59Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.36%. Comparing base (38ba4a6) to head (3d2b7bc).
⚠️ Report is 32 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #42881      +/-   ##
==========================================
+ Coverage   57.10%   66.36%   +9.26%     
==========================================
  Files        2857     2857              
  Lines      161125   161047      -78     
  Branches    37060    37048      -12     
==========================================
+ Hits        92014   106883   +14869     
+ Misses      68259    52148   -16111     
- Partials      852     2016    +1164

Flag	Coverage Δ
javascript	`73.19% <100.00%> (-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.

bito-code-review · 2026-08-07T10:06:49Z

Code Review Agent Run #5a5350

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: bbb8384..516121d
- 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

sadpandajoe · 2026-08-07T17:17:25Z

@vjymisal0 thanks for the PR! Since there is a visual component to your change, can you add before and after screenshots of your fix into the PR description?

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:758

The test name references issue #42881, but this PR (and the new unit test in series.test.ts) is about #42701. Updating the reference avoids confusion when triaging failures or searching history.

  test('should exclude a verbose-named sort-only metric from the stacked total (#42881)', () => {

Fixes the oxfmt-frontend pre-commit CI failure (destructuring assignment exceeded the configured printWidth).

vjymisal0 · 2026-08-08T05:26:33Z

Pushed 3d2b7bc — ran oxfmt on series.ts to fix the oxfmt-frontend pre-commit CI failure (a destructuring assignment exceeded the configured printWidth). No logic change.

netlify · 2026-08-08T05:28:13Z

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	`3d2b7bc`
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6a76be0806f0770008543dbf
😎 Deploy Preview	https://deploy-preview-42881--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle 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 · 2026-08-08T08:28:21Z

Code Review Agent Run #f4bdf3

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: 516121d..3d2b7bc
- superset-frontend/plugins/plugin-chart-echarts/src/utils/series.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

rusackas

Thanks for chasing down the verboseMap edge case too, that's exactly the kind of thing that would've bitten someone later. Tests look solid (both the unit-level exclusion logic and a real end-to-end verboseMap repro) and CI's green. LGTM, merging!

apache#42881 excluded sort-only metrics from the "Only Total" label by exact name match. Time comparison post-processing emits a derived column per query metric named `<label>__<offset>`, and sort-only metrics are part of the query, so `Sort__1 year ago` still slipped into the total on time-comparison charts. The same inflated total feeds thresholdValues, so the percentage-threshold cutoff was off by the same amount. Match keys beginning with `<label>` + TIME_COMPARISON_SEPARATOR as well as the exact label. Keying on the separator keeps a distinct metric that merely shares a prefix (`SortTotal` vs `Sort`) counted in the total. Fixes apache#43068 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings August 7, 2026 06:42

pull-request-size Bot added the size/M label Aug 7, 2026

Copilot AI reviewed Aug 7, 2026

dosubot Bot added viz:charts:echarts Related to Echarts viz:charts:timeseries Related to Timeseries labels Aug 7, 2026

github-actions Bot added the plugins label Aug 7, 2026

vjymisal0 mentioned this pull request Aug 7, 2026

test(explore): zero-value stacked bar segment no longer overlaps its neighbor's label #42882

Merged

2 of 6 tasks

codeant-ai-for-open-source Bot reviewed Aug 7, 2026

View reviewed changes

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

pull-request-size Bot added size/L and removed size/M labels Aug 7, 2026

Merge branch 'master' into fix/stacked-total-excludes-sort-metric

e310fe5

sadpandajoe requested review from eschutho, justinpark and kgabryje and a lite review from Copilot August 7, 2026 17:17

Copilot started reviewing on behalf of sadpandajoe August 7, 2026 17:21

View session

Copilot AI reviewed Aug 7, 2026

View reviewed changes

style: run oxfmt on series.ts

3d2b7bc

Fixes the oxfmt-frontend pre-commit CI failure (destructuring assignment exceeded the configured printWidth).

rusackas approved these changes Aug 10, 2026

View reviewed changes

rusackas added the merge-if-green If approved and tests are green, please go ahead and merge it for me label Aug 10, 2026

rusackas merged commit 178d9c4 into apache:master Aug 10, 2026
79 checks passed

This was referenced Aug 10, 2026

fix(plugin-chart-echarts): exclude series limit metric from stacked total #42755

Closed

fix(plugin-chart-echarts): exclude extra metrics from stacked totals #42855

Closed

fix: exclude sort-only metrics from stacked total in timeseries bar chart #42933

Closed

This was referenced Aug 11, 2026

Stacked "Only Total" still includes the sort-only metric on time-comparison charts (follow-up to #42881) #43068

Open

fix(explore): exclude time-comparison sort metrics from stacked total #43116

Open

jan21deepak mentioned this pull request Aug 13, 2026

Stacked "Only Total" still includes the sort-only metric on time-comparison charts (follow-up to #42881) jan21deepak/superset#87

Open

bikash-barnwal mentioned this pull request Aug 14, 2026

fix(echarts): drop the sort metric's time-shift column from timeseries series #43175

Open

2 of 9 tasks

yichiwasabi mentioned this pull request Sep 13, 2026

Stacked "Only Total" still includes the sort-only metric on time-comparison charts (follow-up to #42881) max-shen-org/superset#39

Open

Conversation

vjymisal0 commented Aug 7, 2026

SUMMARY

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

bito-code-review Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #83afb7

Uh oh!

Uh oh!

bito-code-review Bot commented Aug 7, 2026

Uh oh!

vjymisal0 commented Aug 7, 2026

Uh oh!

codecov Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bito-code-review Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #5a5350

Uh oh!

sadpandajoe commented Aug 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

vjymisal0 commented Aug 8, 2026

Uh oh!

netlify Bot commented Aug 8, 2026

✅ Deploy Preview for superset-docs-preview ready!

Uh oh!

bito-code-review Bot commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #f4bdf3

Uh oh!

rusackas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bito-code-review Bot commented Aug 7, 2026 •

edited

Loading

codecov Bot commented Aug 7, 2026 •

edited

Loading

bito-code-review Bot commented Aug 7, 2026 •

edited

Loading

bito-code-review Bot commented Aug 8, 2026 •

edited

Loading