codeant-ai-for-open-source[bot] commented on code in PR #36214:
URL: https://github.com/apache/superset/pull/36214#discussion_r3658364489
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -535,6 +558,37 @@ export default function transformProps(
}
});
+ // ----- ensure series data are sorted naturally on the x-value -----
+ // Run after all series have been created so each series.data is complete.
+ series.forEach((s: SeriesOption) => {
+ const dataArr = (s as any).data;
+ if (!Array.isArray(dataArr) || dataArr.length <= 1) return;
+
+ (s as any).data = dataArr.sort((row1: any, row2: any) => {
+ // extract the raw x values (support both [x,y] and { x, y } shapes)
+ const rawX1 = Array.isArray(row1) ? row1[0] : row1?.x;
+ const rawX2 = Array.isArray(row2) ? row2[0] : row2?.x;
+
+ // If this chart's x-axis is temporal, coerce to timestamps (numbers)
for sorting.
+ // Fallback to original raw values if parsing fails.
+ const getComparableX = (raw: any) => {
+ if (xAxisType === AxisType.Time) {
+ // If it's already a number, use it. Otherwise try to coerce to Date
timestamp.
+ if (typeof raw === 'number' && isFinite(raw)) return raw;
+ const parsed = new Date(String(raw)).getTime();
+ return isFinite(parsed) ? parsed : String(raw);
+ }
+ return raw;
+ };
+
+ const x1 = getComparableX(rawX1);
+ const x2 = getComparableX(rawX2);
+
+ // naturalCompare already prefers numeric comparison when possible
+ return naturalCompare(x1, x2);
+ });
+ });
Review Comment:
**Suggestion:** Sorting only each series' data does not reorder the category
values supplied separately to the x-axis. When the x-axis has an explicit
`data` array in its original order, ECharts maps each category to that original
position, so the plotted points remain visually out of order despite the sorted
series arrays. Reorder the shared x-axis category data together with the series
data, or rely on a single consistently sorted source for both. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Category-axis points can align with incorrect labels.
- ❌ Numeric-like string Timeseries charts may still show misleading trends.
- ⚠️ Shared category axes can disagree with sorted series data.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render a non-temporal ECharts Timeseries chart through `transformProps()`
in
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts`,
using a
category dimension with values such as `"202401"`, `"202402"`, and
`"202410"` in an
unordered query result.
2. The code at lines 561-590 sorts each `series.data` array by its x-value,
changing the
point order to `"202401"`, `"202402"`, `"202410"`.
3. The x-axis category configuration is built separately later in the same
`transformProps()` execution; the code at lines 592-599 reads x-axis values
from the first
series, and any already-created explicit category `data` array retains its
original order.
4. ECharts renders category positions according to the explicit x-axis
category array
while consuming the reordered series points, so values and labels can be
associated with
the wrong category positions; sorting the category values and all series
data using the
same permutation is required.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ccdf77b021f74325a6ad6bfbf0e6ec2f&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=ccdf77b021f74325a6ad6bfbf0e6ec2f&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/Timeseries/transformProps.ts
**Line:** 561:590
**Comment:**
*Logic Error: Sorting only each series' data does not reorder the
category values supplied separately to the x-axis. When the x-axis has an
explicit `data` array in its original order, ECharts maps each category to that
original position, so the plotted points remain visually out of order despite
the sorted series arrays. Reorder the shared x-axis category data together with
the series data, or rely on a single consistently sorted source for both.
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%2F36214&comment_hash=fb6dffc1806b84c63dfbf9d70df1bb06f52c67db4288f50a94824a64545ddbd6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36214&comment_hash=fb6dffc1806b84c63dfbf9d70df1bb06f52c67db4288f50a94824a64545ddbd6&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]
