codeant-ai-for-open-source[bot] commented on code in PR #40146:
URL: https://github.com/apache/superset/pull/40146#discussion_r3651456256
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1161,3 +1161,82 @@ test('x-axis dedup keeps the forced min label when the
endpoints format identica
expect(formatter(min)).toBe('May');
});
+
+test('regression #37921: multi-metric Query A with groupby does not duplicate
first metric in series names', () => {
+ // Regression test for https://github.com/apache/superset/issues/37921
+ // ("Residual" follow-up to #37055).
+ //
+ // When Query A has multiple metrics + at least one Group By dimension,
+ // the display-name builder in transformProps.ts used to prepend the FIRST
+ // metric's display name to every series that didn't literally contain it:
+ // name: `${MetricDisplayNameA}, ${entryName}`
+ // For series belonging to the *second* metric, this produced a
+ // cross-contaminated label like `score_one, score_two, A` — the
+ // user-visible "first metric duplicated" symptom in the legend / tooltip.
+ // The fix derives each series' metric from its label-map tuple instead.
+ const multiMetricRows = [
+ {
+ 'score_one, A': 1,
+ 'score_one, B': 2,
+ 'score_two, A': 3,
+ 'score_two, B': 4,
+ ds: 599616000000,
+ },
+ {
+ 'score_one, A': 5,
+ 'score_one, B': 6,
+ 'score_two, A': 7,
+ 'score_two, B': 8,
+ ds: 599916000000,
+ },
+ ];
+ const multiMetricLabelMap = {
+ ds: ['ds'],
+ 'score_one, A': ['score_one', 'A'],
+ 'score_one, B': ['score_one', 'B'],
+ 'score_two, A': ['score_two', 'A'],
+ 'score_two, B': ['score_two', 'B'],
+ };
+
+ const queryAData = createTestQueryData(multiMetricRows, {
+ label_map: multiMetricLabelMap,
+ });
+ // Query B keeps the existing single-metric shape — the bug is on
+ // Query A's path so we just need a valid Query B alongside.
+ const queryBData = createTestQueryData(defaultQueryRows, {
+ label_map: defaultLabelMap,
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [queryAData, queryBData],
+ formData: {
+ ...formData,
+ metrics: ['score_one', 'score_two'],
+ groupby: ['category'],
+ },
+ queriesData: [queryAData, queryBData],
+ });
+ const transformed = transformProps(chartProps);
+
+ const queryASeriesNames = (transformed.echartOptions.series as any[])
+ .map((s: any) => String(s.name))
+ .filter((n: string) => n.includes('score_'));
+
+ // Each (metric, dim_value) combo from Query A should appear exactly once
+ // with the *correct* metric prefix — not the first-metric-prepended-to-
+ // everything-else form.
+ expect(queryASeriesNames).toContain('score_one, A');
+ expect(queryASeriesNames).toContain('score_one, B');
+ expect(queryASeriesNames).toContain('score_two, A');
+ expect(queryASeriesNames).toContain('score_two, B');
Review Comment:
**Suggestion:** The regression test claims that each metric/dimension
combination appears exactly once, but `toContain` only verifies presence and
does not detect duplicate series with the same correct name. As a result, a
regression that emits repeated `score_one, A` or `score_two, B` entries could
still pass. Assert the count of each expected name, and scope the collection to
Query A if Query B can produce matching names. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Regression test may miss duplicate legend or tooltip series.
- ⚠️ Query B names could affect the collected assertions.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the regression test `regression #37921: multi-metric Query A with
groupby does not
duplicate first metric in series names` in
`superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:1165`.
2. The test constructs multi-metric Query A data and invokes
`transformProps(chartProps)`
at `transformProps.test.ts:1223`, producing the Mixed Timeseries ECharts
series.
3. The test collects every matching series name at
`transformProps.test.ts:1225-1227`, but
the assertions at `transformProps.test.ts:1232-1235` only require each
expected name to
appear at least once.
4. If a future change in `MixedTimeseries/transformProps.ts` emits duplicate
`score_one,
A` or `score_two, B` series, all four `toContain` assertions can still pass;
counting each
expected name and scoping the collection to Query A would detect that
regression.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0dccc6ff045743dbb96c463e2ddcda7c&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=0dccc6ff045743dbb96c463e2ddcda7c&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/test/MixedTimeseries/transformProps.test.ts
**Line:** 1232:1235
**Comment:**
*Possible Bug: The regression test claims that each metric/dimension
combination appears exactly once, but `toContain` only verifies presence and
does not detect duplicate series with the same correct name. As a result, a
regression that emits repeated `score_one, A` or `score_two, B` entries could
still pass. Assert the count of each expected name, and scope the collection to
Query A if Query B can produce matching names.
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%2F40146&comment_hash=d50bc1ac47bd610892eb8c5879aaddd54f60efb37e820add17cab63b94e0661a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40146&comment_hash=d50bc1ac47bd610892eb8c5879aaddd54f60efb37e820add17cab63b94e0661a&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]
