From 32f252ff7cef60df1a1dcf3fdf8ca3ae4db32df1 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Thu, 14 May 2026 19:43:32 -0700 Subject: [PATCH] fix(mixed-timeseries): derive per-series metric from label map to stop duplicating first metric (#37921) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Query A (or B) on a Mixed Chart has multiple metrics plus at least one Group By dimension, the display-name builder in transformProps.ts always prepended metrics[0]'s label: the entryName.includes(metricPart) guard is false for every series belonging to metrics[1+], so those series were renamed to e.g. 'score_one, score_two, A' instead of 'score_two, A' — the 'first metric duplicated in legend/tooltip' symptom reported in #37921 (residual follow-up to #37055). Derive each series' metric from its label_map tuple ([metric, ...dimensions]) instead, matching what the per-series formatter lookup already does. Single-element tuples (no metric part) fall back to the first metric, preserving existing behavior for single-metric charts and showQueryIdentifiers naming. Applied to both the Query A and Query B paths. Includes the regression test originally landed red on this branch, verified red on master without the fix and green with it. Co-Authored-By: Claude Fable 5 --- .../src/MixedTimeseries/transformProps.ts | 36 ++++++--- .../MixedTimeseries/transformProps.test.ts | 79 +++++++++++++++++++ 2 files changed, 105 insertions(+), 10 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts index ee01fb6f89c..96b1a39897a 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts @@ -457,13 +457,23 @@ export default function transformProps( const seriesName = inverted[entryName] || entryName; const colorScaleKey = getOriginalSeries(seriesName, array); + const labelMapValues = rawLabelMap?.[seriesName]; + let displayName: string; if (groupby.length > 0) { - // When we have groupby, format as "metric, dimension" + // When we have groupby, format as "metric, dimension". Each series + // belongs to the metric recorded in its label-map tuple + // ([metric, ...dimensions]) — always using the first metric would + // prepend it to every other metric's series (#37921). Tuples without + // a metric part fall back to the first metric as before. + const metricDisplayName = + labelMapValues && labelMapValues.length > 1 + ? getMetricDisplayName(labelMapValues[0], verboseMap) + : MetricDisplayNameA; const metricPart: string = showQueryIdentifiers - ? `${MetricDisplayNameA} (Query A)` - : MetricDisplayNameA; + ? `${metricDisplayName} (Query A)` + : metricDisplayName; displayName = entryName.includes(metricPart) ? entryName : `${metricPart}, ${entryName}`; @@ -471,8 +481,6 @@ export default function transformProps( // When no groupby, format as just the entry name with optional query identifier displayName = showQueryIdentifiers ? `${entryName} (Query A)` : entryName; } - - const labelMapValues = rawLabelMap?.[seriesName]; if (labelMapValues) { displayLabelMap[displayName] = labelMapValues; } @@ -536,13 +544,23 @@ export default function transformProps( const seriesEntry = inverted[entryName] || entryName; const colorScaleKey = getOriginalSeries(seriesEntry, array); + const labelMapValuesB = rawLabelMapB?.[seriesEntry]; + let displayName: string; if (groupbyB.length > 0) { - // When we have groupby, format as "metric, dimension" + // When we have groupby, format as "metric, dimension". Each series + // belongs to the metric recorded in its label-map tuple + // ([metric, ...dimensions]) — always using the first metric would + // prepend it to every other metric's series (#37921). Tuples without + // a metric part fall back to the first metric as before. + const metricDisplayName = + labelMapValuesB && labelMapValuesB.length > 1 + ? getMetricDisplayName(labelMapValuesB[0], verboseMap) + : MetricDisplayNameB; const metricPart: string = showQueryIdentifiers - ? `${MetricDisplayNameB} (Query B)` - : MetricDisplayNameB; + ? `${metricDisplayName} (Query B)` + : metricDisplayName; displayName = entryName.includes(metricPart) ? entryName : `${metricPart}, ${entryName}`; @@ -550,8 +568,6 @@ export default function transformProps( // When no groupby, format as just the entry name with optional query identifier displayName = showQueryIdentifiers ? `${entryName} (Query B)` : entryName; } - - const labelMapValuesB = rawLabelMapB?.[seriesEntry]; if (labelMapValuesB) { displayLabelMapB[displayName] = labelMapValuesB; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts index 84c1765b83b..aa43548e654 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts +++ b/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'); + + // And explicitly: no series name should contain *both* metric names — + // that's the smoking gun for the duplication bug. + for (const name of queryASeriesNames) { + expect(name).not.toMatch(/score_one,\s+score_two/); + } +});