codeant-ai-for-open-source[bot] commented on code in PR #42299:
URL: https://github.com/apache/superset/pull/42299#discussion_r3679678245
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -728,7 +728,9 @@ export default function transformProps(
nameGap: convertInteger(xAxisTitleMargin),
nameLocation: 'middle',
axisLabel: {
- hideOverlap: !(xAxisType === AxisType.Time && xAxisLabelRotation !==
0),
+ hideOverlap: showMaxLabel
+ ? false
+ : !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
Review Comment:
**Suggestion:** Mixed-timeseries charts now disable ECharts' overlap
suppression whenever `showMaxLabel` is active, including dense charts with many
combined ticks from both queries. This can render every intermediate label on
top of the others and make the x-axis unreadable; the endpoint should be
preserved without globally disabling density handling. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dense Mixed Timeseries x-axes become unreadable.
- ⚠️ Intermediate daily labels can overlap on narrow charts.
- ⚠️ Endpoint visibility is preserved at the cost of label density.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d0f4b3dacd5844a8afffa689903aba4d&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=d0f4b3dacd5844a8afffa689903aba4d&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/MixedTimeseries/transformProps.ts
**Line:** 731:733
**Comment:**
*Logic Error: Mixed-timeseries charts now disable ECharts' overlap
suppression whenever `showMaxLabel` is active, including dense charts with many
combined ticks from both queries. This can render every intermediate label on
top of the others and make the x-axis unreadable; the endpoint should be
preserved without globally disabling density handling.
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%2F42299&comment_hash=f9233bc1a3d32180b075bbb54b570353c91008956716023c0b2177450a481a27&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=f9233bc1a3d32180b075bbb54b570353c91008956716023c0b2177450a481a27&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -906,10 +906,12 @@ export default function transformProps(
// When rotation is applied on time axes, hideOverlap can
// aggressively hide the last label. Rotated labels already
// have less overlap, so disabling hideOverlap is safe.
- // At 0° rotation, keep hideOverlap to prevent long labels
- // from overlapping each other, with showMaxLabel to ensure
- // the last data point label stays visible (#37181).
- hideOverlap: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
+ // At 0° rotation, also disable hideOverlap when showMaxLabel
+ // is active so the forced boundary label is never suppressed
+ // by ECharts' overlap detection (#39899).
+ hideOverlap: showMaxLabel
+ ? false
+ : !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
Review Comment:
**Suggestion:** Setting `hideOverlap` to `false` for every non-rotated time
axis with a time grain disables ECharts' only label-density suppression. Dense
daily or monthly ranges on narrow charts will render all intermediate labels,
causing severe overlap and unreadable x-axis text. Preserve overlap suppression
for intermediate labels while ensuring the forced endpoint label remains
visible. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dense Timeseries x-axes can become unreadable.
- ⚠️ Daily and monthly labels collide on narrow charts.
- ⚠️ Label-density behavior regresses for common date ranges.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bfba196a0e114ad2a43620aa264ea2f0&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=bfba196a0e114ad2a43620aa264ea2f0&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:** 912:914
**Comment:**
*Logic Error: Setting `hideOverlap` to `false` for every non-rotated
time axis with a time grain disables ECharts' only label-density suppression.
Dense daily or monthly ranges on narrow charts will render all intermediate
labels, causing severe overlap and unreadable x-axis text. Preserve overlap
suppression for intermediate labels while ensuring the forced endpoint label
remains visible.
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%2F42299&comment_hash=60a575fe1c60b2c57cdd4d90599e58f5a7bcf264db7ff1344ef84305453abab9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=60a575fe1c60b2c57cdd4d90599e58f5a7bcf264db7ff1344ef84305453abab9&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and
Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain
overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as
TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ // Month grain (the dashboard override) should win, so the tooltip title
+ // reads "Jan 2021" rather than the Day-grain "2021-01-07".
+ expect(result).toContain('Jan');
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+test('tooltip time grain wiring: chart-level time grain drives the tooltip
when there is no dashboard override', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.YEAR,
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as
TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+const createTemporalMixedChartProps = (timeFormat: string) => {
+ const rows = [
+ { __timestamp: Date.UTC(2003, 4, 1), metric: 10 },
+ { __timestamp: Date.UTC(2004, 0, 1), metric: 20 },
+ { __timestamp: Date.UTC(2005, 4, 1), metric: 30 },
+ ];
+ const q = createTestQueryData(rows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+ return createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [q, q],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.MONTH,
+ xAxisTimeFormat: timeFormat,
+ },
+ queriesData: [q, q],
+ });
+};
+
+test('x-axis forces showMinLabel for time grains so the beginning date stays
visible (mixed)', () => {
+ const xAxis = transformProps(createTemporalMixedChartProps('smart_date'))
+ .echartOptions.xAxis as any;
+ expect(xAxis.axisLabel.showMinLabel).toBe(true);
+});
+
+test('x-axis dedup keeps the forced min label when the endpoints format
identically (mixed)', () => {
+ // May→May range renders "May" at both boundaries; the dedup must reset per
+ // ECharts pass so the forced min label survives the second pass.
+ const { formatter } = (
+ transformProps(createTemporalMixedChartProps('%b')).echartOptions
+ .xAxis as any
+ ).axisLabel;
+ const min = Date.UTC(2003, 4, 1);
+ const mid = Date.UTC(2004, 0, 1);
+ const max = Date.UTC(2005, 4, 1);
+
+ formatter(min);
+ formatter(mid);
+ formatter(max);
+
+ expect(formatter(min)).toBe('May');
+});
+
+test('#39899 - x-axis dates do not overlap and last label stays visible at 0°
rotation (mixed)', () => {
+ // When showMaxLabel is active on a time axis with 0° rotation,
+ // hideOverlap must be off so ECharts cannot suppress the forced
+ // max label (the end-of-axis date).
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ ],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['sum__num'],
+ metricsB: ['sum__num'],
+ groupby: [],
+ groupbyB: [],
+ xAxisLabelRotation: 0,
+ },
+ queriesData: [
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ ],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const { axisLabel } = echartOptions.xAxis as Record<string, any>;
+
+ expect(axisLabel.showMaxLabel).toBe(true);
+ expect(axisLabel.alignMaxLabel).toBe('right');
+ expect(axisLabel.hideOverlap).toBe(false);
+});
+
+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. Comparing the sorted array (rather than using
+ // separate toContain assertions) also catches a regression that emits
+ // duplicate series for the same name.
+ expect([...queryASeriesNames].sort()).toEqual(
+ ['score_one, A', 'score_one, B', 'score_two, A', 'score_two, B'].sort(),
+ );
Review Comment:
**Suggestion:** The expected series names encode a metric-specific naming
fix that is not present in the mixed-timeseries transform. Query A still
derives `metricPart` from `MetricDisplayNameA` (the first metric), so
`score_two, A` becomes `score_one, score_two, A`; the newly added expectation
will fail until the production series-name builder is changed or the test is
removed from this PR. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Mixed Timeseries transform tests fail in CI.
- ⚠️ Query A legend names contain incorrect metric prefixes.
- ⚠️ Tooltips expose cross-contaminated metric labels.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4b7e1b51fdc24bb2a4be5c627bffa307&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=4b7e1b51fdc24bb2a4be5c627bffa307&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:** 1334:1336
**Comment:**
*Api Mismatch: The expected series names encode a metric-specific
naming fix that is not present in the mixed-timeseries transform. Query A still
derives `metricPart` from `MetricDisplayNameA` (the first metric), so
`score_two, A` becomes `score_one, score_two, A`; the newly added expectation
will fail until the production series-name builder is changed or the test is
removed from this PR.
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%2F42299&comment_hash=19f346b4d43c3dcb3e1f028a47d5d344f09cbe429afca36fc775a436ee0e376c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=19f346b4d43c3dcb3e1f028a47d5d344f09cbe429afca36fc775a436ee0e376c&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and
Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain
overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as
TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ // Month grain (the dashboard override) should win, so the tooltip title
+ // reads "Jan 2021" rather than the Day-grain "2021-01-07".
+ expect(result).toContain('Jan');
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+test('tooltip time grain wiring: chart-level time grain drives the tooltip
when there is no dashboard override', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.YEAR,
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as
TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+const createTemporalMixedChartProps = (timeFormat: string) => {
+ const rows = [
+ { __timestamp: Date.UTC(2003, 4, 1), metric: 10 },
+ { __timestamp: Date.UTC(2004, 0, 1), metric: 20 },
+ { __timestamp: Date.UTC(2005, 4, 1), metric: 30 },
+ ];
+ const q = createTestQueryData(rows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+ return createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [q, q],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.MONTH,
+ xAxisTimeFormat: timeFormat,
+ },
+ queriesData: [q, q],
+ });
+};
+
+test('x-axis forces showMinLabel for time grains so the beginning date stays
visible (mixed)', () => {
+ const xAxis = transformProps(createTemporalMixedChartProps('smart_date'))
+ .echartOptions.xAxis as any;
+ expect(xAxis.axisLabel.showMinLabel).toBe(true);
+});
Review Comment:
**Suggestion:** The mixed-timeseries transform does not set
`axisLabel.showMinLabel`; it only sets `showMaxLabel` and `alignMaxLabel`.
Therefore this newly added assertion fails for the implementation in this
change and introduces a permanently failing test unless a corresponding
production assignment is added or the assertion is removed. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Mixed Timeseries transform tests fail in CI.
- ⚠️ Pull-request validation is blocked by a false contract.
- ⚠️ The test asserts unsupported minimum-label behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2b6aa0337bb246759d2441963749f32d&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=2b6aa0337bb246759d2441963749f32d&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:** 1141:1145
**Comment:**
*Api Mismatch: The mixed-timeseries transform does not set
`axisLabel.showMinLabel`; it only sets `showMaxLabel` and `alignMaxLabel`.
Therefore this newly added assertion fails for the implementation in this
change and introduces a permanently failing test unless a corresponding
production assignment is added or the assertion is removed.
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%2F42299&comment_hash=38feb0b656119d941ef8ea819077e392611a164e3e530ea41903df7701772009&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=38feb0b656119d941ef8ea819077e392611a164e3e530ea41903df7701772009&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and
Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain
overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
Review Comment:
**Suggestion:** This test asserts that `extraFormData.time_grain_sqla`
changes the tooltip grain, but the implementation constructs the tooltip
formatter solely from `tooltipTimeFormat`; it does not read `extraFormData` or
use `timeGrainSqla` for tooltip formatting. Consequently, the test either fails
because the formatter returns the raw timestamp or passes for an unrelated
default format without validating the claimed override. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Tooltip regression coverage does not test dashboard grain overrides.
- ⚠️ Future tooltip changes can pass this false-positive test.
- ⚠️ Mixed Timeseries tooltip behavior remains unverified.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eec731102e4b4a489dd4bc5a66c51c44&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=eec731102e4b4a489dd4bc5a66c51c44&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:** 1046:1049
**Comment:**
*Api Mismatch: This test asserts that `extraFormData.time_grain_sqla`
changes the tooltip grain, but the implementation constructs the tooltip
formatter solely from `tooltipTimeFormat`; it does not read `extraFormData` or
use `timeGrainSqla` for tooltip formatting. Consequently, the test either fails
because the formatter returns the raw timestamp or passes for an unrelated
default format without validating the claimed override.
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%2F42299&comment_hash=70a725e318d776fa1e7403e0c0d196073ea5698e11d1788f00a2afbc893eba15&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=70a725e318d776fa1e7403e0c0d196073ea5698e11d1788f00a2afbc893eba15&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and
Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain
overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as
TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ // Month grain (the dashboard override) should win, so the tooltip title
+ // reads "Jan 2021" rather than the Day-grain "2021-01-07".
+ expect(result).toContain('Jan');
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+test('tooltip time grain wiring: chart-level time grain drives the tooltip
when there is no dashboard override', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.YEAR,
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as
TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+const createTemporalMixedChartProps = (timeFormat: string) => {
+ const rows = [
+ { __timestamp: Date.UTC(2003, 4, 1), metric: 10 },
+ { __timestamp: Date.UTC(2004, 0, 1), metric: 20 },
+ { __timestamp: Date.UTC(2005, 4, 1), metric: 30 },
+ ];
+ const q = createTestQueryData(rows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+ return createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [q, q],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.MONTH,
+ xAxisTimeFormat: timeFormat,
+ },
+ queriesData: [q, q],
+ });
+};
+
+test('x-axis forces showMinLabel for time grains so the beginning date stays
visible (mixed)', () => {
+ const xAxis = transformProps(createTemporalMixedChartProps('smart_date'))
+ .echartOptions.xAxis as any;
+ expect(xAxis.axisLabel.showMinLabel).toBe(true);
+});
+
+test('x-axis dedup keeps the forced min label when the endpoints format
identically (mixed)', () => {
+ // May→May range renders "May" at both boundaries; the dedup must reset per
+ // ECharts pass so the forced min label survives the second pass.
+ const { formatter } = (
+ transformProps(createTemporalMixedChartProps('%b')).echartOptions
+ .xAxis as any
+ ).axisLabel;
+ const min = Date.UTC(2003, 4, 1);
+ const mid = Date.UTC(2004, 0, 1);
+ const max = Date.UTC(2005, 4, 1);
+
+ formatter(min);
+ formatter(mid);
+ formatter(max);
+
+ expect(formatter(min)).toBe('May');
+});
Review Comment:
**Suggestion:** `formatter` closes over a single `lastLabel` value and has
no reset mechanism between formatting passes. After formatting `min`, `mid`,
and `max`, the final `formatter(min)` call returns an empty string because the
previous `max` label is also `May`; this assertion therefore fails and assumes
lifecycle behavior the formatter does not provide. [stale reference]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Mixed Timeseries transform tests fail in CI.
- ⚠️ Formatter behavior is incorrectly tested across render passes.
- ⚠️ The regression test cannot model ECharts formatter lifecycle.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=072fafdea5d84410993dd1748514f19a&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=072fafdea5d84410993dd1748514f19a&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:** 1158:1163
**Comment:**
*Stale Reference: `formatter` closes over a single `lastLabel` value
and has no reset mechanism between formatting passes. After formatting `min`,
`mid`, and `max`, the final `formatter(min)` call returns an empty string
because the previous `max` label is also `May`; this assertion therefore fails
and assumes lifecycle behavior the formatter does not provide.
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%2F42299&comment_hash=261fd3369f67d83da6b537bfc87e540cd818283954313a1b2553a64a2d8d482d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=261fd3369f67d83da6b537bfc87e540cd818283954313a1b2553a64a2d8d482d&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]
