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

[gkhnelbstn]

SUMMARY

On a stacked Timeseries Bar/Area chart with "Show Total"/"Only Total" enabled, extractDataTotalValues (plugins/plugin-chart-echarts/src/utils/series.ts) summed every non x-axis column of each data row into the stacked total. This included the timeseries_limit_metric ("Sort by" metric), which is added to the query purely for sorting/limiting the x-axis and is correctly excluded from the rendered series/legend by extractSeries via extraMetricLabels. Since that column never appears as a legend entry, legendState could not exclude it either, so its value silently leaked into the displayed total.

This threads extraMetricLabels (already computed in Timeseries/transformProps.ts for extractSeries) into extractDataTotalValues and excludes those keys the same way the x-axis column is excluded.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A (data-only bug, see reproduction in linked issue)

TESTING INSTRUCTIONS

1. Create an echarts_timeseries_bar chart with 2 metrics A, B, Stacked, "Only Total" enabled.
2. Set timeseries_limit_metric ("Sort Series By") to a metric not among the displayed metrics, e.g. MIN(some_sequence_column).
3. Before the fix, the "Only Total" label includes the sort metric's value (e.g. 32 + 0 + 2 = 34) instead of the correct sum of only the displayed metrics (32 + 0 = 32).
4. Added unit tests in plugins/plugin-chart-echarts/test/utils/series.test.ts covering extractDataTotalValues with and without extraMetricLabels, combined with legendState and non-stacked mode.

ADDITIONAL INFORMATION

• Has associated issue: Fixes 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

[Copilot]

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

[bito-code-review]

Code Review Agent Run #333f74

Actionable Suggestions - 0

Review Details

• Files reviewed - 3 · Commit Range: d2e44f1..d2e44f1

  • 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

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[codeant-ai-for-open-source]

Suggestion: extraMetricLabels contains raw query metric labels, but rebasedData uses keys normalized by rebaseForecastDatum, including verboseMap display names and time-comparison suffixes. When the series-limit metric is verbose-mapped or time-compared, excludedKeys.has(curr) will not match its actual data key, so the supposedly hidden metric still contributes to totalStackedValues (and the same labels also fail to exclude it from extractSeries). Normalize the extra labels using the same key transformation as the data before passing them to both helpers. [api mismatch]

Severity Level: Major ⚠️

- ❌ Displayed stacked totals include hidden sort metrics.
- ❌ Hidden metrics can reappear in rendered series.
- ⚠️ Time-comparison totals use mismatched metric keys.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
**Line:** 307:309
**Comment:**
	*Api Mismatch: `extraMetricLabels` contains raw query metric labels, but `rebasedData` uses keys normalized by `rebaseForecastDatum`, including `verboseMap` display names and time-comparison suffixes. When the series-limit metric is verbose-mapped or time-compared, `excludedKeys.has(curr)` will not match its actual data key, so the supposedly hidden metric still contributes to `totalStackedValues` (and the same labels also fail to exclude it from `extractSeries`). Normalize the extra labels using the same key transformation as the data before passing them to both helpers.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

[bito-code-review]

The flagged issue is correct. The extraMetricLabels are currently passed as raw metric labels, but the data keys in rebasedData have been transformed (e.g., via verboseMap or time-comparison suffixes). Consequently, excludedKeys.has(curr) fails to match, causing hidden metrics to be incorrectly included in stacked totals.

To resolve this, you should normalize the extraMetricLabels using the same transformation logic applied to the data keys before passing them to extractDataTotalValues.

Would you like me to implement this normalization fix and check the rest of the PR comments for further issues?

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

const extraMetricLabels = extractExtraMetrics(chartProps.rawFormData).map(
    getMetricLabel,
  );
  // Normalize labels here using the same logic as rebasedData keys
  const normalizedExtraLabels = extraMetricLabels.map(label => verboseMap[label] ?? label);

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.59%. Comparing base (e66c7fc) to head (d2e44f1).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #42755   +/-   ##
=======================================
  Coverage   65.59%   65.59%           
=======================================
  Files        2819     2819           
  Lines      160166   160167    +1     
  Branches    36569    36570    +1     
=======================================
+ Hits       105059   105061    +2     
+ Misses      53059    53058    -1     
  Partials     2048     2048           

Flag	Coverage Δ
javascript	71.75% <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.