codeant-ai-for-open-source[bot] commented on code in PR #42933:
URL: https://github.com/apache/superset/pull/42933#discussion_r3743365101
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -318,18 +318,19 @@ export default function transformProps(
rebaseToPercentChange(forecastRebasedData, xAxisLabel || DTTM_ALIAS)
: forecastRebasedData;
const isHorizontal = orientation === OrientationType.Horizontal;
+ const extraMetricLabels = extractExtraMetrics(chartProps.rawFormData).map(
+ getMetricLabel,
+ );
Review Comment:
**Suggestion:** The labels are computed from the raw metric definitions, but
`rebasedData` has already been transformed by `rebaseForecastDatum`, which
replaces metric keys with their `verboseMap` names. When the sort-only metric
has a verbose label, `extraMetricLabels` contains the original label while the
row contains the verbose label, so the exclusion does not match and the
sort-only value is still included in stacked totals. Convert the extra labels
using the same key transformation as the data before passing them to both
helpers. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Only Total labels include hidden verbose sort metrics.
- ⚠️ Percentage threshold positioning is inflated.
- ⚠️ Verbose-label charts can render unintended sort series.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b6b1d1b6e30140e3929e29e1d82bc6a9&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=b6b1d1b6e30140e3929e29e1d82bc6a9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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/Timeseries/transformProps.ts
**Line:** 321:323
**Comment:**
*Api Mismatch: The labels are computed from the raw metric definitions,
but `rebasedData` has already been transformed by `rebaseForecastDatum`, which
replaces metric keys with their `verboseMap` names. When the sort-only metric
has a verbose label, `extraMetricLabels` contains the original label while the
row contains the verbose label, so the exclusion does not match and the
sort-only value is still included in stacked totals. Convert 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
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42933&comment_hash=88e783da5cb69ee3369a4c9cfc0a6e68f73be63e34e3ee8c293719ed548f1eb9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42933&comment_hash=88e783da5cb69ee3369a4c9cfc0a6e68f73be63e34e3ee8c293719ed548f1eb9&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -404,6 +405,9 @@ export function extractDataTotalValues(
if (legendState && !legendState[curr]) {
return prev;
}
+ if (extraMetricLabels?.includes(curr)) {
+ return prev;
+ }
Review Comment:
**Suggestion:** The comparison only matches the base metric label exactly,
but time-comparison post-processing produces keys such as `Sort__1 year ago`
(and other comparison-prefixed variants). Those columns are still included in
the reduction, so stacked totals and percentage thresholds remain inflated for
time-comparison charts even though the base sort-only metric is listed in
`extraMetricLabels`. Exclude the comparison-derived forms of each extra metric
as well. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Time-comparison stacked totals include hidden sort values.
- ⚠️ Only Total labels are incorrect for comparison series.
- ⚠️ Percentage threshold annotations use inflated totals.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=352eac29532c4d15bd03518200f3f851&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=352eac29532c4d15bd03518200f3f851&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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:** 408:410
**Comment:**
*Logic Error: The comparison only matches the base metric label
exactly, but time-comparison post-processing produces keys such as `Sort__1
year ago` (and other comparison-prefixed variants). Those columns are still
included in the reduction, so stacked totals and percentage thresholds remain
inflated for time-comparison charts even though the base sort-only metric is
listed in `extraMetricLabels`. Exclude the comparison-derived forms of each
extra metric as well.
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%2F42933&comment_hash=20e473803e99f960d0ebda5f8e472ee084a92e11028f95b42e70991d25b43774&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42933&comment_hash=20e473803e99f960d0ebda5f8e472ee084a92e11028f95b42e70991d25b43774&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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
