codeant-ai-for-open-source[bot] commented on code in PR #40173:
URL: https://github.com/apache/superset/pull/40173#discussion_r3251041702
##########
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx:
##########
@@ -280,12 +280,37 @@ function Echart(
const notMerge = !isDashboardRefreshing;
chartRef.current?.dispatchAction({ type: 'hideTip' });
+ // setOption(notMerge:true) replaces the dataZoom config, dropping any
+ // range the user has engaged. Preserve it across the call.
+ const previousZoom = notMerge
+ ? (chartRef.current?.getOption() as any)?.dataZoom
+ : undefined;
chartRef.current?.setOption(themedEchartOptions, {
notMerge,
replaceMerge: notMerge ? undefined : ['series'],
// lazyUpdate defers render, causing tooltip crashes on stale shapes
(#39247)
lazyUpdate: false,
});
+ if (previousZoom?.length) {
+ const batch = previousZoom
+ .map((dz: any, dataZoomIndex: number) => ({
+ dataZoomIndex,
+ start: dz.start,
+ end: dz.end,
+ startValue: dz.startValue,
+ endValue: dz.endValue,
+ }))
+ .filter(
+ (b: any) =>
+ b.start !== undefined ||
+ b.end !== undefined ||
+ b.startValue !== undefined ||
+ b.endValue !== undefined,
+ );
Review Comment:
**Suggestion:** This filter treats any defined `start`/`end` as an engaged
zoom, but most timeseries options set default `start: 0` and `end: 100`, so
untouched charts still dispatch a `dataZoom` action on every re-render. That
adds unnecessary work and can trigger avoidable side effects; skip restore when
values are just the default range. [performance]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Extra dataZoom actions on untouched zoomable Timeseries charts.
- โ ๏ธ Unnecessary ECharts work during layout/theme-driven re-renders.
- โ ๏ธ Potential minor overhead for MixedTimeseries and Gantt charts.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Configure a zoomable ECharts Timeseries chart using the plugin at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:41-61`;
its `transformProps` builds `echartOptions.dataZoom` with a slider that has
default
`start` and `end` values (`Timeseries/transformProps.ts:1156-1165`), where
`start` and
`end` are taken from `TIMESERIES_CONSTANTS.dataZoomStart` and `dataZoomEnd`
(`src/constants.ts:44-45`, which are 0 and 100).
2. Render the chart so that `Echart` calls `setOption` with these options;
on first mount
`previousZoom` in `components/Echart.tsx:86-88` is undefined, so no restore
action is
dispatched yet, but the chart's internal option state now includes a
`dataZoom` array with
the default `start: 0` and `end: 100`.
3. Trigger a non-refresh re-render that changes the `echartOptions`
referenceโfor example,
a layout or theme change that causes `transformProps` to run again and
`EchartsTimeseries`
to pass new `echartOptions` into `Echart` (the shared effect in
`components/Echart.tsx:82-89` depends on `echartOptions` and re-runs
whenever they
change).
4. On this subsequent run, `notMerge` is `true` (`Echart.tsx:82`), so
`previousZoom` is
populated from `chartRef.current.getOption().dataZoom` and the restore logic
maps and
filters this array; because the default slider entry has defined `start` and
`end` fields,
it passes the filter condition at `components/Echart.tsx:103-109` (snippet
lines 303-309),
producing a non-empty `batch` and causing `dispatchAction({ type:
'dataZoom', batch })`
(`Echart.tsx:111-112`) even though the user has not changed the zoom,
leading to redundant
`dataZoom` actions on every such re-render.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2Fcomponents%2FEchart.tsx%0A%2A%2ALine%3A%2A%2A%20303%3A309%0A%2A%2AComment%3A%2A%2A%0A%09%2APerformance%3A%20This%20filter%20treats%20any%20defined%20%60start%60%2F%60end%60%20as%20an%20engaged%20zoom%2C%20but%20most%20timeseries%20options%20set%20default%20%60start%3A%200%60%20and%20%60end%3A%20100%60%2C%20so%20untouched%20charts%20still%20dispatch%20a%20%60dataZoom%60%20action%20on%20every%20re-render.%20That%20adds%20unnecessary%20work%20and%20can%20trigger%20avoidable%20side%20effects%3B%20skip%20restore%20when%20values%20are%20just%20the%20default%20range.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnc
e%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2Fcomponents%2FEchart.tsx%0A%2A%2ALine%3A%2A%2A%20303%3A309%0A%2A%2AComment%3A%2A%2A%0A%09%2APerformance%3A%20This%20filter%20treats%20any%20defined%20%60start%60%2F%60end%60%20as%20an%20engaged%20zoom%2C%20but%20most%20timeseries%20options%20set%20default%20%60start%3A%200%60%20and%20%60end%3A%20100%60%2C%20so%20untouched%20charts%20still%20dispatch%20a%20%60dataZoom%60%20action%20on%20every%20re-render.%20That%20adds%20unnecessary%20wo
rk%20and%20can%20trigger%20avoidable%20side%20effects%3B%20skip%20restore%20when%20values%20are%20just%20the%20default%20range.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(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/components/Echart.tsx
**Line:** 303:309
**Comment:**
*Performance: This filter treats any defined `start`/`end` as an
engaged zoom, but most timeseries options set default `start: 0` and `end:
100`, so untouched charts still dispatch a `dataZoom` action on every
re-render. That adds unnecessary work and can trigger avoidable side effects;
skip restore when values are just the default range.
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%2F40173&comment_hash=d29ce85287305aa80ec0b9322716232e6fddd6efd6d354be471641a06c0da6ee&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40173&comment_hash=d29ce85287305aa80ec0b9322716232e6fddd6efd6d354be471641a06c0da6ee&reaction=dislike'>๐</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx:
##########
@@ -280,12 +280,37 @@ function Echart(
const notMerge = !isDashboardRefreshing;
chartRef.current?.dispatchAction({ type: 'hideTip' });
+ // setOption(notMerge:true) replaces the dataZoom config, dropping any
+ // range the user has engaged. Preserve it across the call.
+ const previousZoom = notMerge
+ ? (chartRef.current?.getOption() as any)?.dataZoom
+ : undefined;
chartRef.current?.setOption(themedEchartOptions, {
notMerge,
replaceMerge: notMerge ? undefined : ['series'],
// lazyUpdate defers render, causing tooltip crashes on stale shapes
(#39247)
lazyUpdate: false,
});
+ if (previousZoom?.length) {
+ const batch = previousZoom
+ .map((dz: any, dataZoomIndex: number) => ({
+ dataZoomIndex,
+ start: dz.start,
+ end: dz.end,
+ startValue: dz.startValue,
+ endValue: dz.endValue,
+ }))
Review Comment:
**Suggestion:** The restore payload is keyed only by array position
(`dataZoomIndex`), so if the next `echartOptions` changes `dataZoom`
order/count (for example switching chart mode or toggling zoom config), ranges
can be reapplied to the wrong zoom component/axis. Use a stable identifier
(`id`/`dataZoomId`) and only restore entries that still exist in the new
option. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Zoom ranges may bind to wrong axis after config changes.
- โ ๏ธ Advanced custom EChart options can misalign zoom state.
- โ ๏ธ Confusing zoom behavior in Timeseries and MixedTimeseries charts.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Create a zoomable ECharts timeseries chart using the Superset ECharts
Timeseries plugin
(`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:41-61`),
which renders the shared `Echart` component with `echartOptions` produced by
`transformProps`
(`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:1156-1165`).
2. Configure advanced custom EChart options for this chart so that the
custom JSON
provides its own `dataZoom` array; in `transformProps` the custom options
are parsed and
merged, completely replacing base arrays such as `dataZoom` via
`mergeCustomEChartOptions`
(`Timeseries/transformProps.ts:1187-1203` and
`utils/mergeCustomEChartOptions.ts:34-74`).
3. With an initial `dataZoom` layout (e.g., slider at index 0, inside zooms
at indices 1
and 2), interact with the chart to zoom via the slider so that
`chartRef.current.getOption().dataZoom` reflects the current ranges; when
`echartOptions`
later change (e.g., user edits the custom options to reorder or change the
`dataZoom`
array), the `Echart` effect at `components/Echart.tsx:82-89` re-runs with
`notMerge =
!isDashboardRefreshing` (true in normal interactions) and captures the zoom
state into
`previousZoom`.
4. After calling `setOption`, `Echart` builds a restore payload by mapping
over
`previousZoom` and emitting `dataZoomIndex` based solely on array position
(`components/Echart.tsx:95-103` from the snippet at lines 295-302), then
dispatches a
`dataZoom` action with this batch (`Echart.tsx:111-112`); if the new
`echartOptions.dataZoom` has a different order or length than the previous
`getOption().dataZoom`, the same indices now refer to different zoom
components/axes, so
the saved ranges are applied to the wrong `dataZoom` elements.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2Fcomponents%2FEchart.tsx%0A%2A%2ALine%3A%2A%2A%20295%3A302%0A%2A%2AComment%3A%2A%2A%0A%09%2AApi%20Mismatch%3A%20The%20restore%20payload%20is%20keyed%20only%20by%20array%20position%20%28%60dataZoomIndex%60%29%2C%20so%20if%20the%20next%20%60echartOptions%60%20changes%20%60dataZoom%60%20order%2Fcount%20%28for%20example%20switching%20chart%20mode%20or%20toggling%20zoom%20config%29%2C%20ranges%20can%20be%20reapplied%20to%20the%20wrong%20zoom%20component%2Faxis.%20Use%20a%20stable%20identifier%20%28%60id%60%2F%60dataZoomId%60%29%20and%20only%20restore%20entries%20that%20still%20exist%20in%20the%20new%20option.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it
%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2Fcomponents%2FEchart.tsx%0A%2A%2ALine%3A%2A%2A%20295%3A302%0A%2A%2AComment%3A%2A%2A%0A%09%2AApi%20Mismatch%3A%20The%20restore%20payload%20is%20keyed%20only%20by%20array%20position%20%28%60dataZoomIndex%60%29%2C%20so%20if%20the%20next%20%60echartOptions%60%20changes%20%60dataZoom%60%20order%2Fcount%20%28for%20example%20switching%20chart%20mode%20or%20toggling%20zoom%20config%29%2C%20ranges%20ca
n%20be%20reapplied%20to%20the%20wrong%20zoom%20component%2Faxis.%20Use%20a%20stable%20identifier%20%28%60id%60%2F%60dataZoomId%60%29%20and%20only%20restore%20entries%20that%20still%20exist%20in%20the%20new%20option.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(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/components/Echart.tsx
**Line:** 295:302
**Comment:**
*Api Mismatch: The restore payload is keyed only by array position
(`dataZoomIndex`), so if the next `echartOptions` changes `dataZoom`
order/count (for example switching chart mode or toggling zoom config), ranges
can be reapplied to the wrong zoom component/axis. Use a stable identifier
(`id`/`dataZoomId`) and only restore entries that still exist in the new option.
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%2F40173&comment_hash=086dc2200335789ccd811d3b450dfd5624233239cdc106c8996f5e380b328de5&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40173&comment_hash=086dc2200335789ccd811d3b450dfd5624233239cdc106c8996f5e380b328de5&reaction=dislike'>๐</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/components/Echart.test.tsx:
##########
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import type { EChartsCoreOption } from 'echarts/core';
+import { render, waitFor } from 'spec/helpers/testing-library';
+
+const setOption = jest.fn();
+const on = jest.fn();
+const off = jest.fn();
+const resize = jest.fn();
+const dispose = jest.fn();
+const dispatchAction = jest.fn();
+const getOption = jest.fn();
+
+const mockInstance = {
+ setOption,
+ on,
+ off,
+ resize,
+ dispose,
+ dispatchAction,
+ getOption,
+ getZr: () => ({ on: jest.fn(), off: jest.fn() }),
+};
+
+jest.mock('echarts/core', () => ({
+ __esModule: true,
+ use: jest.fn(),
+ init: jest.fn(() => mockInstance),
+ registerLocale: jest.fn(),
+}));
+jest.mock('echarts/charts', () => ({}));
+jest.mock('echarts/renderers', () => ({}));
+jest.mock('echarts/components', () => ({}));
+jest.mock('echarts/features', () => ({}));
+
+// eslint-disable-next-line import/first
+import Echart from '../../src/components/Echart';
+
+const renderEchart = (echartOptions: EChartsCoreOption) => {
+ const refs = { divRef: undefined };
+ return render(
+ <Echart
+ width={400}
+ height={300}
+ echartOptions={echartOptions}
+ refs={refs}
+ />,
+ { useRedux: true, useTheme: true },
+ );
+};
+
+beforeEach(() => {
+ setOption.mockClear();
+ on.mockClear();
+ off.mockClear();
+ resize.mockClear();
+ dispatchAction.mockClear();
+ getOption.mockReset();
+});
+
+test('preserves user dataZoom range across setOption(notMerge)', async () => {
+ // After the user has zoomed, ECharts reports the current dataZoom range
+ // via getOption().dataZoom. We expect Echart to capture this before
+ // setOption replaces the option payload, then restore it via dispatchAction.
+ getOption.mockReturnValue({
+ dataZoom: [{ start: 12, end: 48 }],
+ });
+
+ const { rerender } = renderEchart({ xAxis: {}, series: [] });
+
+ // Trigger another setOption call by changing the echartOptions reference
+ rerender(
+ <Echart
+ width={400}
+ height={300}
+ echartOptions={{ xAxis: {}, series: [{ type: 'line' }] }}
+ refs={{ divRef: undefined }}
+ />,
+ );
+
+ await waitFor(() =>
+ expect(dispatchAction).toHaveBeenCalledWith(
+ expect.objectContaining({
+ type: 'dataZoom',
+ batch: [
+ expect.objectContaining({ dataZoomIndex: 0, start: 12, end: 48 }),
+ ],
+ }),
+ ),
+ );
+});
+
+test('does not restore when no prior zoom range exists', async () => {
+ // Fresh chart with no engaged zoom: dataZoom config has no start/end.
+ getOption.mockReturnValue({
+ dataZoom: [{ type: 'slider', show: true }],
+ });
Review Comment:
**Suggestion:** This fixture is not representative of real chart options in
this plugin, where slider dataZoom commonly includes default `start`/`end`
values; as written, the test can pass while production still restores default
ranges and dispatches unnecessary zoom actions. Update the mock to include
default bounds and assert they are not restored unless the user actually
changed them. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Test misses default-range restore on zoomable Timeseries charts.
- โ ๏ธ Regression coverage incomplete for Echart zoom preservation logic.
- โ ๏ธ Future changes to dataZoom restore may regress undetected.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Inspect the production timeseries configuration:
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:1156-1165`
shows that for zoomable charts, a slider `dataZoom` entry is created with
explicit default
`start` and `end` values from `TIMESERIES_CONSTANTS.dataZoomStart` and
`dataZoomEnd`
(`src/constants.ts:44-45`, set to 0 and 100), meaning a fresh chart already
has
`start`/`end` populated.
2. Compare this to the test `does not restore when no prior zoom range
exists` in
`superset-frontend/plugins/plugin-chart-echarts/test/components/Echart.test.tsx:109-132`,
where `getOption.mockReturnValue` is set to `{ dataZoom: [{ type: 'slider',
show: true }]
}` at lines 111-113, intentionally omitting `start` and `end`.
3. The implementation in `components/Echart.tsx:95-112` (new hunk lines
295-312) treats
any `dataZoom` entry with defined `start`, `end`, `startValue`, or
`endValue` as an
engaged zoom and restores it; default slider bounds in real charts satisfy
this condition,
but the test fixture's `dataZoom` object has none of these fields, so it
exercises the "no
bounds" path rather than the actual default-range path used by Timeseries,
MixedTimeseries, and Gantt charts.
4. As a result, the test can pass while the runtime behavior still restores
and dispatches
`dataZoom` actions for default `start: 0, end: 100` ranges (as described in
suggestion 2's
scenario), because the fixture is not representative of real
`getOption().dataZoom` in
zoomable charts; updating the mock to include default `start`/`end` and
asserting that no
`dataZoom` dispatch occurs for the untouched default range would more
accurately cover the
production behavior.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Ftest%2Fcomponents%2FEchart.test.tsx%0A%2A%2ALine%3A%2A%2A%20111%3A113%0A%2A%2AComment%3A%2A%2A%0A%09%2AIncomplete%20Implementation%3A%20This%20fixture%20is%20not%20representative%20of%20real%20chart%20options%20in%20this%20plugin%2C%20where%20slider%20dataZoom%20commonly%20includes%20default%20%60start%60%2F%60end%60%20values%3B%20as%20written%2C%20the%20test%20can%20pass%20while%20production%20still%20restores%20default%20ranges%20and%20dispatches%20unnecessary%20zoom%20actions.%20Update%20the%20mock%20to%20include%20default%20bounds%20and%20assert%20they%20are%20not%20restored%20unless%20the%20user%20actually%20changed%20them.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix
%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Ftest%2Fcomponents%2FEchart.test.tsx%0A%2A%2ALine%3A%2A%2A%20111%3A113%0A%2A%2AComment%3A%2A%2A%0A%09%2AIncomplete%20Implementation%3A%20This%20fixture%20is%20not%20representative%20of%20real%20chart%20options%20in%20this%20plugin%2C%20where%20slider%20dataZoom%20commonly%20includes%20default%20%60start%60%2F%60end%60%20values%3B%20as%20written%2C%20the%20test%20can%20pass%20whil
e%20production%20still%20restores%20default%20ranges%20and%20dispatches%20unnecessary%20zoom%20actions.%20Update%20the%20mock%20to%20include%20default%20bounds%20and%20assert%20they%20are%20not%20restored%20unless%20the%20user%20actually%20changed%20them.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(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/components/Echart.test.tsx
**Line:** 111:113
**Comment:**
*Incomplete Implementation: This fixture is not representative of real
chart options in this plugin, where slider dataZoom commonly includes default
`start`/`end` values; as written, the test can pass while production still
restores default ranges and dispatches unnecessary zoom actions. Update the
mock to include default bounds and assert they are not restored unless the user
actually changed them.
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%2F40173&comment_hash=cb389120761c6f8efbd6eb0b1d747acc4bb1d3e5ea52670f3061def4d79760c0&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40173&comment_hash=cb389120761c6f8efbd6eb0b1d747acc4bb1d3e5ea52670f3061def4d79760c0&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]
