You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "krishn1301 (via GitHub)" <gi...@apache.org> on 2026/08/06 18:39:42 UTC
[PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
krishn1301 opened a new pull request, #42855:
URL: https://github.com/apache/superset/pull/42855
### SUMMARY
Fixes #42701.
With **Stacked Style: Stack** and **Only Total** enabled, the total shown above each bar included the sort metric. In the reported case, metrics `A = 32` and `B = 0` with a `timeseries_limit_metric` of `2` displayed a total of **34** instead of **32**.
`extractDataTotalValues` sums every numeric column in each row:
```ts
Object.keys(data).reduce((prev, curr) => { ... prev + data[curr] ... })
```
A sort metric is present in the query result but is never rendered as a series, so it silently inflated the total — and the `thresholdValues` derived from that total, which decides which labels are shown.
This threads the extra metric labels into `extractDataTotalValues` and skips those columns when accumulating. The labels were already being computed in `transformProps.ts` via `extractExtraMetrics(...).map(getMetricLabel)`; that computation just happened *after* the `extractDataTotalValues` call, so it's moved above and passed in. The new option is optional, so other callers of `extractDataTotalValues` are unaffected.
### TESTING INSTRUCTIONS
```
cd superset-frontend
npm run test -- plugins/plugin-chart-echarts/test/utils/series.test.ts
```
Four tests were added under `extractDataTotalValues`, using the exact numbers from the issue:
- excludes extra metrics from the stacked total — `A=32, B=0, Sort=2` gives `[32]` (fails without this change)
- includes every metric when no `extraMetricLabels` are passed — the same row gives `[34]`
- derives `thresholdValues` from the total excluding extra metrics (fails without this change)
- respects `legendState` alongside the exclusion
Manually: build a Stacked Timeseries Bar chart with two metrics, set a **Sort By** metric that differs from both, enable **Only Total**, and confirm the total equals the sum of the visible segments.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #42701
- [x] Changes UI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on PR #42855:
URL: https://github.com/apache/superset/pull/42855#issuecomment-5219936068
@krishn1301 Glad to see your contribution to the project! Can you add before and after screenshots of your fix into the PR description?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42855:
URL: https://github.com/apache/superset/pull/42855#discussion_r3752383617
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -387,20 +387,33 @@ export function extractDataTotalValues(
percentageThreshold: number;
xAxisCol: string;
legendState?: LegendState;
+ extraMetricLabels?: string[];
},
): {
totalStackedValues: number[];
thresholdValues: number[];
} {
const totalStackedValues: number[] = [];
const thresholdValues: number[] = [];
- const { stack, percentageThreshold, xAxisCol, legendState } = opts;
+ const {
+ stack,
+ percentageThreshold,
+ xAxisCol,
+ legendState,
+ extraMetricLabels = [],
+ } = opts;
+ // Extra metrics, such as a sort metric, are present in the data but are not
+ // rendered as series, so they must not contribute to the stacked total.
+ const extraMetricLabelsSet = new Set(extraMetricLabels);
if (stack) {
data.forEach(datum => {
const values = Object.keys(datum).reduce((prev, curr) => {
if (curr === xAxisCol) {
return prev;
}
+ if (extraMetricLabelsSet.has(curr)) {
+ return prev;
+ }
Review Comment:
This uses the same extraMetricLabels matching that `sortAndFilterSeries` already does against the same rebased data (see how `extractSeries` filters series names), so it's consistent with how these metrics get excluded from series today. If verboseMap renaming ever breaks that match, it's a pre-existing issue there too, not something new in this fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42855:
URL: https://github.com/apache/superset/pull/42855#issuecomment-5245108152
Thanks @krishn1301 for the fix here! We merged #42881 instead, it additionally resolves verbose-named sort metrics hitting the same issue. Appreciate you digging into this, closing this one out in favor of that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #42855: fix(plugin-chart-echarts): exclude extra metrics from stacked totals
URL: https://github.com/apache/superset/pull/42855
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42855:
URL: https://github.com/apache/superset/pull/42855#discussion_r3737504355
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -387,20 +387,33 @@ export function extractDataTotalValues(
percentageThreshold: number;
xAxisCol: string;
legendState?: LegendState;
+ extraMetricLabels?: string[];
},
): {
totalStackedValues: number[];
thresholdValues: number[];
} {
const totalStackedValues: number[] = [];
const thresholdValues: number[] = [];
- const { stack, percentageThreshold, xAxisCol, legendState } = opts;
+ const {
+ stack,
+ percentageThreshold,
+ xAxisCol,
+ legendState,
+ extraMetricLabels = [],
+ } = opts;
+ // Extra metrics, such as a sort metric, are present in the data but are not
+ // rendered as series, so they must not contribute to the stacked total.
+ const extraMetricLabelsSet = new Set(extraMetricLabels);
if (stack) {
data.forEach(datum => {
const values = Object.keys(datum).reduce((prev, curr) => {
if (curr === xAxisCol) {
return prev;
}
+ if (extraMetricLabelsSet.has(curr)) {
+ return prev;
+ }
Review Comment:
**Suggestion:** The exclusion only compares the raw column key against `extraMetricLabels`, but `extractDataTotalValues` receives `rebasedData`, whose metric keys may be renamed through `verboseMap` or suffixed for time-comparison series. In those cases a hidden sort metric such as `Sort__1 day ago` or its verbose label is not matched and is still added to the stacked total and threshold, so the reported inflation persists for forecast/time-comparison charts. Normalize the extra labels to the same rendered key format, or match derived keys consistently with `extractSeries`. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Time-comparison stacked totals include hidden sort metrics.
- ❌ Verbose-mapped extra metrics inflate displayed totals.
- ⚠️ Incorrect totals affect threshold-based label visibility.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=988c2f1ad3e54353932b0e078fa1ef8c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=988c2f1ad3e54353932b0e078fa1ef8c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
**Line:** 414:416
**Comment:**
*Api Mismatch: The exclusion only compares the raw column key against `extraMetricLabels`, but `extractDataTotalValues` receives `rebasedData`, whose metric keys may be renamed through `verboseMap` or suffixed for time-comparison series. In those cases a hidden sort metric such as `Sort__1 day ago` or its verbose label is not matched and is still added to the stacked total and threshold, so the reported inflation persists for forecast/time-comparison charts. Normalize the extra labels to the same rendered key format, or match derived keys consistently with `extractSeries`.
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
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42855&comment_hash=4e87ab1ccad66cd4337f70fd566a1d8ee3f0ab791c32a73372856c941ad0bdf6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42855&comment_hash=4e87ab1ccad66cd4337f70fd566a1d8ee3f0ab791c32a73372856c941ad0bdf6&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42855:
URL: https://github.com/apache/superset/pull/42855#issuecomment-5219982713
<!-- Bito Reply -->
The flagged issue is correct. The current implementation only filters based on the raw metric key, which fails when the data has been transformed (e.g., via `verboseMap` or time-comparison suffixes). To resolve this, you should normalize the `extraMetricLabels` to match the rendered keys present in `rebasedData` before performing the exclusion check.
To implement this, you can update the `extractDataTotalValues` function to normalize the extra labels using the same logic applied to the series keys, or ensure the `extraMetricLabels` passed to the function already reflect the final rendered format used in the data records.
**superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts**
```
// Normalize extra labels to match the rendered keys in rebasedData
const normalizedExtraLabels = extraMetricLabels.map(label =>
// Apply the same transformation logic used for series keys here
getRenderedKey(label)
);
const extraMetricLabelsSet = new Set(normalizedExtraLabels);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42855:
URL: https://github.com/apache/superset/pull/42855#issuecomment-5220119906
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42855?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 66.37%. Comparing base ([`79c2cd3`](https://app.codecov.io/gh/apache/superset/commit/79c2cd367dc356115a2613c2c1dab3c945304391?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`88f649c`](https://app.codecov.io/gh/apache/superset/commit/88f649cf14703898f32f26aaaca12eb64e0b0b64?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 3 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42855 +/- ##
=======================================
Coverage 66.37% 66.37%
=======================================
Files 2857 2857
Lines 161048 161052 +4
Branches 37046 37048 +2
=======================================
+ Hits 106889 106893 +4
Misses 52143 52143
Partials 2016 2016
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42855/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/42855/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.19% <100.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42855?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42855:
URL: https://github.com/apache/superset/pull/42855#discussion_r3752386431
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -387,20 +387,33 @@ export function extractDataTotalValues(
percentageThreshold: number;
xAxisCol: string;
legendState?: LegendState;
+ extraMetricLabels?: string[];
},
): {
totalStackedValues: number[];
thresholdValues: number[];
} {
const totalStackedValues: number[] = [];
const thresholdValues: number[] = [];
- const { stack, percentageThreshold, xAxisCol, legendState } = opts;
+ const {
+ stack,
+ percentageThreshold,
+ xAxisCol,
+ legendState,
+ extraMetricLabels = [],
+ } = opts;
+ // Extra metrics, such as a sort metric, are present in the data but are not
+ // rendered as series, so they must not contribute to the stacked total.
+ const extraMetricLabelsSet = new Set(extraMetricLabels);
if (stack) {
data.forEach(datum => {
const values = Object.keys(datum).reduce((prev, curr) => {
if (curr === xAxisCol) {
return prev;
}
+ if (extraMetricLabelsSet.has(curr)) {
+ return prev;
+ }
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> When reviewing extraMetricLabels exclusions, treat matching behavior consistent with sortAndFilterSeries and extractSeries as intentional; do not flag pre-existing verboseMap or time-comparison matching limitations as regressions in this change.
**Applied to:**
- `superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42855:
URL: https://github.com/apache/superset/pull/42855#issuecomment-5208288630
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 2b657139e1cc22b71c543f5aaeacd9b58850de11 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a74d4efa4ce930008d23ee9 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-42855--superset-docs-preview.netlify.app](https://deploy-preview-42855--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctLCoAwDADRu2RtoGjxdwOPUZuIhdaUGkER7253s3jzQi6SssIM0MBa3OH32lu4UY74oIq6iHz7eBGfeEpRTKwl-MqJc5RnoTr0brBkeXPW89QZY0ZqO-YJvh8) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-42855--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): exclude extra metrics from stacked totals [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42855:
URL: https://github.com/apache/superset/pull/42855#issuecomment-5208274479
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org