codeant-ai-for-open-source[bot] commented on code in PR #43369:
URL: https://github.com/apache/superset/pull/43369#discussion_r3825589393
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -861,12 +862,14 @@ export default function transformProps(
: params.value[0];
const forecastValue: any[] = richTooltip ? params : [params];
- const sortedKeys = extractTooltipKeys(
- forecastValue,
- // horizontal mode is not supported in mixed series chart
- 1,
- richTooltip,
- tooltipSortByMetric,
+ const sortedKeys = collapseForecastKeys(
+ extractTooltipKeys(
+ forecastValue,
+ // horizontal mode is not supported in mixed series chart
+ 1,
+ richTooltip,
+ tooltipSortByMetric,
+ ),
);
Review Comment:
**Suggestion:** The collapsed key no longer matches the raw display IDs
stored in `primarySeries` and `secondarySeries`. For a mixed chart containing
only forecast-suffixed series, a key such as `foo` is absent from both sets, so
the formatter always selects the secondary-axis formatter and number format,
even when the series belongs to the primary axis. Maintain the axis mapping
under the same collapsed keys used by the tooltip rows. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Primary-axis forecast metrics use secondary formatting.
- ⚠️ Currency and number formats may be incorrect.
- ⚠️ MixedTimeseries tooltip rows lose axis ownership.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<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/MixedTimeseries/transformProps.ts
**Line:** 865:873
**Comment:**
*Api Mismatch: The collapsed key no longer matches the raw display IDs
stored in `primarySeries` and `secondarySeries`. For a mixed chart containing
only forecast-suffixed series, a key such as `foo` is absent from both sets, so
the formatter always selects the secondary-axis formatter and number format,
even when the series belongs to the primary axis. Maintain the axis mapping
under the same collapsed keys used by the tooltip rows.
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%2F43369&comment_hash=2e11a5edf5535c62dd9627f34544db24410cc3eb884a3edb186c0173473bb56c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43369&comment_hash=2e11a5edf5535c62dd9627f34544db24410cc3eb884a3edb186c0173473bb56c&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -1392,11 +1393,13 @@ export default function transformProps(
const forecastValue: CallbackDataParams[] = richTooltip
? params
: [params];
- const sortedKeys = extractTooltipKeys(
- forecastValue,
- yIndex,
- richTooltip,
- tooltipSortByMetric,
+ const sortedKeys = collapseForecastKeys(
+ extractTooltipKeys(
+ forecastValue,
+ yIndex,
+ richTooltip,
+ tooltipSortByMetric,
+ ),
);
Review Comment:
**Suggestion:** When `tooltipSortByMetric` is enabled, `extractTooltipKeys`
sorts the individual forecast components by their raw values, then
`collapseForecastKeys` keeps whichever component appears first for each base
name. This means a lower or upper confidence-bound value can determine the
position of the entire metric instead of its observation value, changing the
metric ordering. Sort or collapse using the intended representative value
before deduplicating. [logic error]
<details>
<summary><b>Severity Level:</b> Minor 🧹</summary>
```mdx
- ⚠️ Sorted Timeseries tooltip metrics can move unexpectedly.
- ⚠️ Confidence bounds determine ordering instead of observations.
- ⚠️ Users see inconsistent metric order across forecast charts.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<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:** 1396:1403
**Comment:**
*Logic Error: When `tooltipSortByMetric` is enabled,
`extractTooltipKeys` sorts the individual forecast components by their raw
values, then `collapseForecastKeys` keeps whichever component appears first for
each base name. This means a lower or upper confidence-bound value can
determine the position of the entire metric instead of its observation value,
changing the metric ordering. Sort or collapse using the intended
representative value before deduplicating.
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%2F43369&comment_hash=b8fb0ed43ad6b5af5376f3d6a68744737a5953bb1fe74d17467a0fc1e2e7e3d6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43369&comment_hash=b8fb0ed43ad6b5af5376f3d6a68744737a5953bb1fe74d17467a0fc1e2e7e3d6&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]
