You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "tomerkl65 (via GitHub)" <gi...@apache.org> on 2026/05/26 17:29:30 UTC
[PR] New feature tk drilldown hierarchy [superset]
tomerkl65 opened a new pull request, #40449:
URL: https://github.com/apache/superset/pull/40449
### SUMMARY
Adds hierarchical drill-down support to ECharts-based chart plugins. Users can define a drill-down hierarchy by adding multiple columns to the X-Axis control. The first column is displayed initially; clicking a data point drills into the next level, scoped to the clicked value. A breadcrumb above the chart allows navigating back up.
**How it works:**
- X-Axis control is changed from single to multi-select (`multi: true` in `xAxisMixin`)
- `getXAxisColumn` and `isXAxisSet` are updated to handle array x_axis (using the first entry for the query)
- A new `DrillDownHost` wrapper component manages the drill state and re-fetches data for each level
- Click handlers in ECharts plugins (`EchartsTimeseries`, `allEventHandlers`) detect the hierarchy and call `onDrillDown` instead of emitting a cross-filter
- Cross-filter is also emitted alongside drill-down so other charts in the dashboard filter accordingly
- A `DrillDownBreadcrumb` component renders above the chart showing the navigation path
**Supported charts:** Bar, Pie, Funnel, Gauge, Radar, BoxPlot, Treemap, Sunburst (all charts declaring `Behavior.DrillBy`)
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before:** X-Axis accepts a single column. No drill-down on click.
**After:** X-Axis accepts multiple columns. Clicking a bar drills to the next level with a breadcrumb for navigation.
### TESTING INSTRUCTIONS
1. Create or edit a Bar Chart
2. In the X-Axis control, add 2+ columns (e.g. `country`, `region`)
3. Save and view in a dashboard
4. Click on a bar → chart drills to the next level showing values filtered to the clicked item
5. Breadcrumb appears above the chart (e.g. `country › USA`)
6. Click breadcrumb to navigate back up
7. Verify cross-filter is emitted (other charts in dashboard filter accordingly)
8. Verify that charts with a single X-Axis column continue to work normally
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
<img width="1776" height="994" alt="image" src="https://github.com/user-attachments/assets/b0983fa5-010e-4899-bbf5-c0711c29e5c2" />
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] New feature tk drilldown hierarchy [superset]
Posted by "tomerkl65 (via GitHub)" <gi...@apache.org>.
tomerkl65 closed pull request #40449: New feature tk drilldown hierarchy
URL: https://github.com/apache/superset/pull/40449
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4641384630
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4794355285
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4622454657
Approving workflows to run
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3534508661
##########
superset-frontend/src/components/Chart/DrillDown/DrillDownBreadcrumb.test.tsx:
##########
@@ -0,0 +1,143 @@
+/**
+ * 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 { render, screen, fireEvent } from 'spec/helpers/testing-library';
+import { DrillDownBreadcrumb } from './DrillDownBreadcrumb';
+import { DrillDownLevel } from './types';
+
+const hierarchy = ['country', 'region', 'city'];
+
+const drillStack: DrillDownLevel[] = [
+ {
+ column: 'country',
+ filters: [{ col: 'country', op: '==', val: 'USA' }],
+ label: 'USA',
+ },
+ {
+ column: 'region',
+ filters: [{ col: 'region', op: '==', val: 'Texas' }],
+ label: 'Texas',
+ },
+];
+
+test('breadcrumb returns null when drillStack is empty and no selectedLeaf', () => {
+ const { container } = render(
+ <DrillDownBreadcrumb
+ hierarchy={hierarchy}
+ drillStack={[]}
+ onJumpTo={jest.fn()}
+ />,
+ );
+
+ expect(container.firstChild).toBeNull();
+});
+
+test('breadcrumb renders hierarchy[0] and drill labels', () => {
+ render(
+ <DrillDownBreadcrumb
+ hierarchy={hierarchy}
+ drillStack={drillStack}
+ onJumpTo={jest.fn()}
+ />,
+ );
+
+ // The root column name should be rendered
+ expect(screen.getByText('country')).toBeInTheDocument();
+ // Drill labels should be rendered
+ expect(screen.getByText('USA')).toBeInTheDocument();
+ expect(screen.getByText('Texas')).toBeInTheDocument();
+});
+
+test('clicking a breadcrumb segment calls onJumpTo with correct depth', () => {
+ const onJumpTo = jest.fn();
+
+ render(
+ <DrillDownBreadcrumb
+ hierarchy={hierarchy}
+ drillStack={drillStack}
+ onJumpTo={onJumpTo}
+ />,
+ );
+
+ // Click the root segment (hierarchy[0]) — should call onJumpTo(0)
+ fireEvent.click(screen.getByText('country'));
+ expect(onJumpTo).toHaveBeenCalledWith(0);
+
+ // Click the first drill label 'USA' — should call onJumpTo(1)
+ fireEvent.click(screen.getByText('USA'));
+ expect(onJumpTo).toHaveBeenCalledWith(1);
+});
+
+test('keyboard Enter on breadcrumb calls onJumpTo', () => {
+ const onJumpTo = jest.fn();
+
+ render(
+ <DrillDownBreadcrumb
+ hierarchy={hierarchy}
+ drillStack={drillStack}
+ onJumpTo={onJumpTo}
+ />,
+ );
+
+ // Press Enter on the root segment
+ fireEvent.keyDown(screen.getByText('country'), { key: 'Enter' });
+ expect(onJumpTo).toHaveBeenCalledWith(0);
+
+ // Press Enter on the 'USA' segment
+ fireEvent.keyDown(screen.getByText('USA'), { key: 'Enter' });
+ expect(onJumpTo).toHaveBeenCalledWith(1);
+});
+
+test('selectedLeaf is rendered as non-clickable text', () => {
+ const onJumpTo = jest.fn();
+
+ render(
+ <DrillDownBreadcrumb
+ hierarchy={hierarchy}
+ drillStack={drillStack}
+ selectedLeaf="Austin"
+ onJumpTo={onJumpTo}
+ />,
+ );
+
+ expect(screen.getByText('Austin')).toBeInTheDocument();
+
+ // The leaf should not have role="button"
+ const leafElement = screen.getByText('Austin');
+ expect(leafElement).not.toHaveAttribute('role', 'button');
+});
+
+test('last drill label is not clickable when there is no selectedLeaf', () => {
+ const onJumpTo = jest.fn();
+
+ render(
+ <DrillDownBreadcrumb
+ hierarchy={hierarchy}
+ drillStack={drillStack}
+ onJumpTo={onJumpTo}
+ />,
+ );
+
+ // 'Texas' is the last item and no selectedLeaf, so it should not be clickable
+ const texasElement = screen.getByText('Texas');
+ expect(texasElement).not.toHaveAttribute('role', 'button');
+
+ fireEvent.click(texasElement);
+ // onJumpTo should only have been called for the root click test, not for Texas
Review Comment:
**Suggestion:** The assertion is correct, but the inline comment is factually wrong for this test case and can mislead future edits: this test never performs a root click, so saying the callback should only be called from a root click contradicts the actual setup. Update the comment to match the test’s behavior (no callback calls expected at all). [comment mismatch]
<details>
<summary><b>Severity Level:</b> Minor 🧹</summary>
```mdx
- ⚠️ Test documentation for drill breadcrumb behavior is misleading.
- ⚠️ Future maintainers may misinterpret expected callback usage.
- ⚠️ Confusion when updating drill-down click handling tests.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `superset-frontend/src/components/Chart/DrillDown/DrillDownBreadcrumb.test.tsx`
and locate the test `last drill label is not clickable when there is no selectedLeaf` at
lines 125–143 (as read from the repository).
2. Observe that within this test, `onJumpTo` is defined as `const onJumpTo = jest.fn();`
at line 126 and is passed to `DrillDownBreadcrumb` in the render call at lines 128–133.
3. Note that the only user interaction in this test is a click on the `Texas` breadcrumb
element at line 140, and the final assertion at line 142 is
`expect(onJumpTo).not.toHaveBeenCalled();`, confirming that `onJumpTo` is expected to have
zero calls in this test.
4. Compare this behavior with the inline comment at line 141: `// onJumpTo should only
have been called for the root click test, not for Texas`. Since this specific test never
performs a root click, the comment is factually incorrect and suggests a callback
invocation pattern (root click then Texas click) that does not exist here, which can
mislead future test edits.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cab3ed9b4fa648bcb4fbbe1dfdf945b4&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=cab3ed9b4fa648bcb4fbbe1dfdf945b4&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/src/components/Chart/DrillDown/DrillDownBreadcrumb.test.tsx
**Line:** 141:141
**Comment:**
*Comment Mismatch: The assertion is correct, but the inline comment is factually wrong for this test case and can mislead future edits: this test never performs a root click, so saying the callback should only be called from a root click contradicts the actual setup. Update the comment to match the test’s behavior (no callback calls expected at all).
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%2F40449&comment_hash=5c8e07b6ef05244dc1b3a5f4bf21cf03ced2e6245bca779e3dc996dad7e07316&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=5c8e07b6ef05244dc1b3a5f4bf21cf03ced2e6245bca779e3dc996dad7e07316&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] New feature tk drilldown hierarchy [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3305710192
##########
superset-frontend/plugins/plugin-chart-echarts/src/Gauge/transformProps.ts:
##########
@@ -377,6 +377,7 @@ export default function transformProps(
groupby,
selectedValues: filterState.selectedValues || [],
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** Passing `onDrillDown` into Gauge transformed props activates the drill-only click handler path in `allEventHandlers`, which does not apply a data mask. As a result, clicks drill but do not broadcast cross-filters to other charts when hierarchy is configured. Add cross-filter emission in the drill-down branch before exposing this hook. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Gauge drill-down no longer updates dashboard cross-filters.
- ⚠️ Other charts ignore Gauge click interactions under drill.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Gauge transform props, observe that hooks are destructured to include `onDrillDown`
at `superset-frontend/plugins/plugin-chart-echarts/src/Gauge/transformProps.ts:229`
(`const { setDataMask = () => {}, onContextMenu, onDrillDown } = hooks;`) and that
`onDrillDown` is returned to the renderer at lines 369-383, specifically line 380
(`onDrillDown,`).
2. See that `EchartsGauge` at
`superset-frontend/plugins/plugin-chart-echarts/src/Gauge/EchartsGauge.tsx:23-27`
constructs `eventHandlers = allEventHandlers(props);`, passing `setDataMask`,
`emitCrossFilters`, `groupby`, `labelMap`, `selectedValues`, `formData`, and the newly
wired `onDrillDown` into `allEventHandlers`.
3. Inspect `allEventHandlers` in
`superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts:152-215`, where
`onDrillDown` is read from `transformedProps` (lines 155-165), `hasDrillHierarchy` is set
based on `!!onDrillDown` (line 170), and `drillDownClickHandler` is created when
`hasDrillHierarchy && groupby.length > 0` (lines 172-195). In this case,
`eventHandlers.click` is set to `drillDownClickHandler` (lines 197-200).
4. Compare with `clickEventHandler` at lines 90-106 in the same file: it uses
`getCrossFilterDataMask(selectedValues, groupby, labelMap)` and calls `setDataMask` when
`emitCrossFilters` is true, emitting cross-filters. Because wiring `onDrillDown` for Gauge
causes `allEventHandlers` to choose `drillDownClickHandler` (which only calls
`onDrillDown` and never `setDataMask`) instead of `clickEventHandler`, Gauge clicks in a
dashboard with drill hierarchy configured will drill but no longer update the data mask,
so cross-filters are not propagated to other charts.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=38c2d1cbd2234731aaf1bc740ede6b84&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=38c2d1cbd2234731aaf1bc740ede6b84&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/Gauge/transformProps.ts
**Line:** 380:380
**Comment:**
*Logic Error: Passing `onDrillDown` into Gauge transformed props activates the drill-only click handler path in `allEventHandlers`, which does not apply a data mask. As a result, clicks drill but do not broadcast cross-filters to other charts when hierarchy is configured. Add cross-filter emission in the drill-down branch before exposing this hook.
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%2F40449&comment_hash=d72e781b94256132c101bf66bdbed64acc49275ce5b67ff954b153b2fb932bd4&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=d72e781b94256132c101bf66bdbed64acc49275ce5b67ff954b153b2fb932bd4&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/transformProps.ts:
##########
@@ -320,6 +320,7 @@ export default function transformProps(
groupby,
selectedValues,
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** Forwarding `onDrillDown` here switches BoxPlot clicks into the drill-down branch in `allEventHandlers`, which currently skips `setDataMask` updates. That means cross-filter is no longer emitted when drill hierarchy is enabled, so linked dashboard charts stop updating. Update the drill-down click path to also emit cross-filter (as done in Timeseries) before wiring this prop through. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ BoxPlot drill-down stops emitting dashboard cross-filters.
- ⚠️ Linked charts remain stale when clicking BoxPlot points.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure BoxPlot transform props to expose `onDrillDown` by returning it from
`transformProps` at
`superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/transformProps.ts:312-324`,
where the object includes `setDataMask`, `emitCrossFilters`, `labelMap`, `groupby`,
`selectedValues`, `onContextMenu`, and `onDrillDown` (line 323).
2. Observe that `EchartsBoxPlot` at
`superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/EchartsBoxPlot.tsx:23-28`
calls `allEventHandlers(props)` and passes the transformed props including `onDrillDown`,
`setDataMask`, `emitCrossFilters`, `groupby`, `labelMap`, and `selectedValues` into the
generic event handler factory.
3. Follow the click handling logic in `allEventHandlers` at
`superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts:152-215`, where
`onDrillDown` is read (lines 155-165), `hasDrillHierarchy` is computed as `!!onDrillDown`
(line 170), and when `hasDrillHierarchy && groupby.length > 0`, `eventHandlers.click` is
set to `drillDownClickHandler` (lines 172-195, 197-200) instead of `clickEventHandler`.
4. Note that `drillDownClickHandler` builds `drillFilters` and calls
`onDrillDown(drillFilters, label)` (lines 175-193) but never calls `clickEventHandler` or
`setDataMask`, while `clickEventHandler` (lines 90-106) is the only place that computes a
cross-filter data mask via `getCrossFilterDataMask(...)(name)` and calls `setDataMask`.
Therefore, once a drill hierarchy is configured and `onDrillDown` is non-null, BoxPlot
clicks invoke drill-down only and no longer emit cross-filters, leaving linked dashboard
charts unfiltered.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=929a6dea9ad348d7bc533d5bb701a198&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=929a6dea9ad348d7bc533d5bb701a198&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/BoxPlot/transformProps.ts
**Line:** 323:323
**Comment:**
*Logic Error: Forwarding `onDrillDown` here switches BoxPlot clicks into the drill-down branch in `allEventHandlers`, which currently skips `setDataMask` updates. That means cross-filter is no longer emitted when drill hierarchy is enabled, so linked dashboard charts stop updating. Update the drill-down click path to also emit cross-filter (as done in Timeseries) before wiring this prop through.
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%2F40449&comment_hash=b54d342886724e8c7a88099ee0c8296d3078892e64678c108d6eff9379710822&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=b54d342886724e8c7a88099ee0c8296d3078892e64678c108d6eff9379710822&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Radar/transformProps.ts:
##########
@@ -414,6 +414,7 @@ export default function transformProps(
groupby,
selectedValues,
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** This new `onDrillDown` passthrough makes Radar use the drill-down click path, but that path currently omits cross-filter updates. In dashboards with drill hierarchy enabled, Radar clicks will drill without filtering sibling charts. Align the drill-down handler with Timeseries behavior by emitting cross-filter and drill-down together. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Radar drill-down stops broadcasting cross-filters to dashboards.
- ⚠️ Radar clicks no longer synchronize with linked charts.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Radar transform props, verify that hooks are destructured to include `onDrillDown`
at `superset-frontend/plugins/plugin-chart-echarts/src/Radar/transformProps.ts:129`
(`const { setDataMask = () => {}, onContextMenu, onDrillDown } = hooks;`) and that
`onDrillDown` is propagated in the returned props at lines 406-420, specifically line 417
(`onDrillDown,`).
2. Confirm that `EchartsRadar` at
`superset-frontend/plugins/plugin-chart-echarts/src/Radar/EchartsRadar.tsx:23-27` calls
`const eventHandlers = allEventHandlers(props);`, which passes Radar's `groupby`,
`labelMap`, `selectedValues`, `setDataMask`, `emitCrossFilters`, `formData`,
`coltypeMapping`, and now `onDrillDown` into the shared `allEventHandlers` function.
3. Inspect `allEventHandlers` in
`superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts:152-215`: it
reads `onDrillDown` (lines 155-165), sets `hasDrillHierarchy = !!onDrillDown` (line 170),
and when `hasDrillHierarchy && groupby.length > 0` constructs `drillDownClickHandler` that
builds `drillFilters` and calls `onDrillDown(drillFilters, label)` (lines 172-193).
4. Note that in this branch, `eventHandlers.click` is assigned to `drillDownClickHandler`
(lines 197-200) and never calls `clickEventHandler`, while `clickEventHandler` (lines
90-106) is the only function that computes a cross-filter `dataMask` and calls
`setDataMask`. Thus, for Radar charts with a configured drill hierarchy (where
`onDrillDown` is provided by the host), clicks now trigger `onDrillDown` only and stop
emitting cross-filters, so sibling charts on the dashboard no longer filter in response to
Radar clicks.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bf906782130e4bde8924bee1678f70d3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=bf906782130e4bde8924bee1678f70d3&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/Radar/transformProps.ts
**Line:** 417:417
**Comment:**
*Logic Error: This new `onDrillDown` passthrough makes Radar use the drill-down click path, but that path currently omits cross-filter updates. In dashboards with drill hierarchy enabled, Radar clicks will drill without filtering sibling charts. Align the drill-down handler with Timeseries behavior by emitting cross-filter and drill-down together.
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%2F40449&comment_hash=29488d6e86519c4ad1c15208296b205cd3c62622c95a1450ce987e3e6f46ad7d&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=29488d6e86519c4ad1c15208296b205cd3c62622c95a1450ce987e3e6f46ad7d&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:
##########
@@ -404,6 +404,7 @@ export default function transformProps(
groupby,
selectedValues: filterState.selectedValues || [],
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** `onDrillDown` is now included in transformed props, but `EchartsSunburst` does not read or invoke it in its click handlers. This leaves Sunburst drill-down non-functional despite the new wiring, so the feature appears enabled in data flow but never executes. Wire `onDrillDown` into Sunburst click handling and build drill filters from the clicked tree path. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Sunburst drill-down never triggers despite onDrillDown wiring.
- ⚠️ Users get cross-filters only, no Sunburst hierarchy drill.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Sunburst transform props, verify that `hooks` are destructured to include
`onDrillDown` at
`superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:242`
(`const { setDataMask = () => {}, onContextMenu, onDrillDown } = hooks;`) and that
`onDrillDown` is returned to the renderer at lines 396-410, specifically line 407
(`onDrillDown,`).
2. Inspect `EchartsSunburst` at
`superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/EchartsSunburst.tsx:36-50`:
the component destructures `height`, `width`, `echartOptions`, `setDataMask`,
`selectedValues`, `formData`, `onContextMenu`, `refs`, `emitCrossFilters`, and
`coltypeMapping` from `props`, but does not read or destructure `onDrillDown` anywhere, so
the drill-down hook returned from `transformProps` is effectively ignored.
3. Examine Sunburst's click handling in the same file: `getCrossFilterDataMask` (lines
51-93) constructs a data mask from `treePathInfo`, and `handleChange` (lines 97-106) calls
`setDataMask(getCrossFilterDataMask(treePathInfo).dataMask)` when `emitCrossFilters` is
true. The `eventHandlers.click` implementation (lines 108-112) calls
`handleChange(treePathInfo)` but never calls `onDrillDown`, so clicks only update
cross-filters and never invoke the drill-down hook.
4. As a result, when a drill hierarchy is configured and the host provides `onDrillDown`
for Sunburst (demonstrated by transform props returning it at line 407), clicking on
Sunburst segments will continue to perform cross-filtering via `setDataMask` but will
never call `onDrillDown`, meaning the new drill-down feature (drill state and breadcrumb
updates managed by `DrillDownHost`) remains non-functional for Sunburst despite being
wired through transform props.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=dc5d731afb72459c96a1aa59f465125f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=dc5d731afb72459c96a1aa59f465125f&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/Sunburst/transformProps.ts
**Line:** 407:407
**Comment:**
*Incomplete Implementation: `onDrillDown` is now included in transformed props, but `EchartsSunburst` does not read or invoke it in its click handlers. This leaves Sunburst drill-down non-functional despite the new wiring, so the feature appears enabled in data flow but never executes. Wire `onDrillDown` into Sunburst click handling and build drill filters from the clicked tree path.
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%2F40449&comment_hash=e337247bdf4d60e98f9aad7e66caabd10e73f9b9b5ee5c42673bf95a81b054b8&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=e337247bdf4d60e98f9aad7e66caabd10e73f9b9b5ee5c42673bf95a81b054b8&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4548897530
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4546981570
<!-- Bito Reply -->
The PR comments file contains only the header row and no actual comments. There are no actionable suggestions or review comments to analyze in this PR.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3534536459
##########
superset-frontend/src/components/Chart/DrillDown/DrillDownHost.tsx:
##########
@@ -0,0 +1,225 @@
+/**
+ * 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 {
+ ComponentType,
+ useCallback,
+ useMemo,
+ useRef,
+ useState,
+ useEffect,
+} from 'react';
+import {
+ QueryData,
+ QueryFormData,
+ BinaryQueryObjectFilterClause,
+ FeatureFlag,
+ isFeatureEnabled,
+} from '@superset-ui/core';
+import { css } from '@apache-superset/core/theme';
+import { useDrillDownState } from './useDrillDownState';
+import { DrillDownBreadcrumb } from './DrillDownBreadcrumb';
+import type { ChartRendererProps } from '../ChartRenderer';
+
+/**
+ * Hook payload contract: chart plugins call `onDrillDown(filters, label)`
+ * via the chart's hooks bag when the user clicks a data point and a
+ * drill-down hierarchy is configured.
+ */
+export type OnDrillDownHook = (
+ filters: BinaryQueryObjectFilterClause[],
+ label: string,
+) => void;
+
+interface DrillDownHostProps extends ChartRendererProps {
+ /** The wrapped renderer component */
+ ChartRendererComponent: ComponentType<
+ ChartRendererProps & { onDrillDown?: OnDrillDownHook }
+ >;
+}
+
+/**
+ * Wraps `<ChartRenderer>` with drill-down behavior. When the chart's
+ * form_data declares a `drilldown_hierarchy`, this host:
+ *
+ * 1. Tracks how deep the user has drilled (a stack of levels)
+ * 2. Computes "effective" form_data for the current level (replacing the
+ * groupby and adding accumulated filters)
+ * 3. Re-fetches chart data for that level
+ * 4. Renders a breadcrumb above the chart for navigating back up
+ *
+ * If the chart has no hierarchy, this is a thin pass-through.
+ */
+export function DrillDownHost({
+ ChartRendererComponent,
+ ...rendererProps
+}: DrillDownHostProps) {
+ const { formData, queriesResponse } = rendererProps;
+
+ const {
+ isDrilling,
+ drillStack,
+ selectedLeaf,
+ hierarchy,
+ effectiveFormData,
+ effectiveQueriesResponse,
+ isLoading,
+ error,
+ hasHierarchy,
+ drillDown,
+ resetTo,
+ } = useDrillDownState({
+ formData,
+ baseQueriesResponse: queriesResponse,
+ });
+
+ const onDrillDown = useMemo<OnDrillDownHook | undefined>(() => {
+ if (!hasHierarchy) {
+ return undefined;
+ }
+ return (filters, label) => {
+ drillDown(filters, label);
+ };
+ }, [hasHierarchy, drillDown]);
+
+ const overlayProps = useMemo<Partial<ChartRendererProps>>(() => {
+ if (!isDrilling) {
+ // Force re-render when returning to base level
+ return { triggerRender: true };
+ }
+ return {
+ formData: effectiveFormData as QueryFormData,
+ queriesResponse: (effectiveQueriesResponse ?? null) as QueryData[] | null,
+ chartStatus: isLoading ? 'loading' : 'rendered',
+ latestQueryFormData: effectiveFormData,
+ chartIsStale: false,
+ triggerRender: true,
+ };
+ }, [isDrilling, effectiveFormData, effectiveQueriesResponse, isLoading]);
+
+ const handleResetTo = useCallback(
+ (depth: number) => {
+ resetTo(depth);
+ // Update cross-filter to match the level we're jumping to
+ if (rendererProps.actions?.updateDataMask) {
+ if (depth === 0) {
+ // Going back to root — clear cross-filter entirely
+ rendererProps.actions.updateDataMask(rendererProps.chartId, {
+ extraFormData: { filters: [] },
+ filterState: { value: null, selectedValues: null },
+ });
+ // While drilled, the dashboard may have skipped re-querying this
+ // chart's base when other charts' cross-filters changed (e.g. one
+ // was cleared), leaving the Redux base query result stale/filtered.
+ // Trigger a fresh base query so returning to the top level shows the
+ // full chart. Unlike refreshChart (which reuses latestQueryFormData),
+ // triggerQuery makes the chart re-run with its current dashboard
+ // form_data, which reflects the now-cleared filters.
+ rendererProps.actions.triggerQuery?.(true, rendererProps.chartId);
+ } else {
+ // Going to an intermediate level — rebuild accumulated filters
+ // from all levels up to the target depth (mirroring effectiveFormData)
+ const accumulatedFilters = drillStack.slice(0, depth).flatMap(level =>
+ level.filters.map(f => ({
+ col: f.col,
+ op: 'IN' as const,
+ val: [f.val] as (string | number | boolean)[],
+ })),
+ );
Review Comment:
**Suggestion:** When rebuilding cross-filters for breadcrumb jumps, null drill values are always converted into `IN [null]`. SQL filtering with `IN (NULL)` does not match null rows, so drilling on a null bucket and navigating via breadcrumb will produce incorrect filtering. Preserve null semantics by emitting an `IS NULL` filter when a drilled value is null (matching existing cross-filter behavior). [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Drill-down breadcrumb breaks filters for null dimension values.
- ⚠️ Dashboards show empty charts when drilling into nulls.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset-frontend/src/components/Chart/Chart.tsx:96-134`, the `Chart` component
renders `DrillDownHost` with `ChartRenderer` and passes `actions.updateDataMask` and
`chartId`, so any drill breadcrumb navigation will invoke `DrillDownHost`’s
`handleResetTo`.
2. In `superset-frontend/src/components/Chart/DrillDown/DrillDownHost.tsx:115-155`,
`handleResetTo` rebuilds cross-filters for intermediate breadcrumb levels by computing
`accumulatedFilters` from `drillStack.slice(0, depth)` and calling
`rendererProps.actions.updateDataMask(chartId, { extraFormData: { filters:
accumulatedFilters }, ... })`.
3. The drill stack filters are populated when plugins call `onDrillDown`: for generic
ECharts charts,
`superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts:175-207` builds
`drillFilters` with entries like `{ col: dimension, op: '==', val: values[i],
formattedVal: ... }` and calls `onDrillDown(drillFilters, label)`, so
`drillStack[level].filters` can contain `val === null` when the underlying dimension value
is null.
4. Cross-filter behavior for nulls uses `IS NULL`: in `eventHandlers.ts:61-81`,
`getCrossFilterDataMask` maps null-only selections to `{ col, op: 'IS NULL' }` (no `val`),
ensuring null buckets filter correctly, but `DrillDownHost.tsx:137-142` rebuilds filters
as `{ col: f.col, op: 'IN', val: [f.val] }` even when `f.val` is `null`, producing `IN
(NULL)` clauses that match no rows, so drilling into a null bucket and then jumping via
the breadcrumb yields an incorrect empty cross-filter instead of preserving the null
selection.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=83195b4801d942aabe7c356ed1e314c5&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=83195b4801d942aabe7c356ed1e314c5&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/src/components/Chart/DrillDown/DrillDownHost.tsx
**Line:** 137:143
**Comment:**
*Incorrect Condition Logic: When rebuilding cross-filters for breadcrumb jumps, null drill values are always converted into `IN [null]`. SQL filtering with `IN (NULL)` does not match null rows, so drilling on a null bucket and navigating via breadcrumb will produce incorrect filtering. Preserve null semantics by emitting an `IS NULL` filter when a drilled value is null (matching existing cross-filter behavior).
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%2F40449&comment_hash=483cb1d10c514a9989e25aa90fe373c5cbfff8b886166ed7a3453af59942f673&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=483cb1d10c514a9989e25aa90fe373c5cbfff8b886166ed7a3453af59942f673&reaction=dislike'>👎</a>
##########
superset-frontend/src/components/Chart/DrillDown/DrillDownHost.tsx:
##########
@@ -0,0 +1,225 @@
+/**
+ * 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 {
+ ComponentType,
+ useCallback,
+ useMemo,
+ useRef,
+ useState,
+ useEffect,
+} from 'react';
+import {
+ QueryData,
+ QueryFormData,
+ BinaryQueryObjectFilterClause,
+ FeatureFlag,
+ isFeatureEnabled,
+} from '@superset-ui/core';
+import { css } from '@apache-superset/core/theme';
+import { useDrillDownState } from './useDrillDownState';
+import { DrillDownBreadcrumb } from './DrillDownBreadcrumb';
+import type { ChartRendererProps } from '../ChartRenderer';
+
+/**
+ * Hook payload contract: chart plugins call `onDrillDown(filters, label)`
+ * via the chart's hooks bag when the user clicks a data point and a
+ * drill-down hierarchy is configured.
+ */
+export type OnDrillDownHook = (
+ filters: BinaryQueryObjectFilterClause[],
+ label: string,
+) => void;
+
+interface DrillDownHostProps extends ChartRendererProps {
+ /** The wrapped renderer component */
+ ChartRendererComponent: ComponentType<
+ ChartRendererProps & { onDrillDown?: OnDrillDownHook }
+ >;
+}
+
+/**
+ * Wraps `<ChartRenderer>` with drill-down behavior. When the chart's
+ * form_data declares a `drilldown_hierarchy`, this host:
+ *
+ * 1. Tracks how deep the user has drilled (a stack of levels)
+ * 2. Computes "effective" form_data for the current level (replacing the
+ * groupby and adding accumulated filters)
+ * 3. Re-fetches chart data for that level
+ * 4. Renders a breadcrumb above the chart for navigating back up
+ *
+ * If the chart has no hierarchy, this is a thin pass-through.
+ */
+export function DrillDownHost({
+ ChartRendererComponent,
+ ...rendererProps
+}: DrillDownHostProps) {
+ const { formData, queriesResponse } = rendererProps;
+
+ const {
+ isDrilling,
+ drillStack,
+ selectedLeaf,
+ hierarchy,
+ effectiveFormData,
+ effectiveQueriesResponse,
+ isLoading,
+ error,
+ hasHierarchy,
+ drillDown,
+ resetTo,
+ } = useDrillDownState({
+ formData,
+ baseQueriesResponse: queriesResponse,
+ });
+
+ const onDrillDown = useMemo<OnDrillDownHook | undefined>(() => {
+ if (!hasHierarchy) {
+ return undefined;
+ }
+ return (filters, label) => {
+ drillDown(filters, label);
+ };
+ }, [hasHierarchy, drillDown]);
+
+ const overlayProps = useMemo<Partial<ChartRendererProps>>(() => {
+ if (!isDrilling) {
+ // Force re-render when returning to base level
+ return { triggerRender: true };
+ }
+ return {
+ formData: effectiveFormData as QueryFormData,
+ queriesResponse: (effectiveQueriesResponse ?? null) as QueryData[] | null,
+ chartStatus: isLoading ? 'loading' : 'rendered',
+ latestQueryFormData: effectiveFormData,
+ chartIsStale: false,
+ triggerRender: true,
+ };
+ }, [isDrilling, effectiveFormData, effectiveQueriesResponse, isLoading]);
+
+ const handleResetTo = useCallback(
+ (depth: number) => {
+ resetTo(depth);
+ // Update cross-filter to match the level we're jumping to
+ if (rendererProps.actions?.updateDataMask) {
+ if (depth === 0) {
+ // Going back to root — clear cross-filter entirely
+ rendererProps.actions.updateDataMask(rendererProps.chartId, {
+ extraFormData: { filters: [] },
+ filterState: { value: null, selectedValues: null },
+ });
+ // While drilled, the dashboard may have skipped re-querying this
+ // chart's base when other charts' cross-filters changed (e.g. one
+ // was cleared), leaving the Redux base query result stale/filtered.
+ // Trigger a fresh base query so returning to the top level shows the
+ // full chart. Unlike refreshChart (which reuses latestQueryFormData),
+ // triggerQuery makes the chart re-run with its current dashboard
+ // form_data, which reflects the now-cleared filters.
+ rendererProps.actions.triggerQuery?.(true, rendererProps.chartId);
+ } else {
+ // Going to an intermediate level — rebuild accumulated filters
+ // from all levels up to the target depth (mirroring effectiveFormData)
+ const accumulatedFilters = drillStack.slice(0, depth).flatMap(level =>
+ level.filters.map(f => ({
+ col: f.col,
+ op: 'IN' as const,
+ val: [f.val] as (string | number | boolean)[],
+ })),
+ );
+ const labels = drillStack.slice(0, depth).map(l => l.label);
+ rendererProps.actions.updateDataMask(rendererProps.chartId, {
+ extraFormData: { filters: accumulatedFilters },
+ filterState: {
+ value: labels,
+ selectedValues: labels,
+ },
+ });
+ }
+ }
+ },
+ [resetTo, rendererProps.actions, rendererProps.chartId, drillStack],
+ );
+
+ const breadcrumbRef = useRef<HTMLDivElement>(null);
+ const [breadcrumbHeight, setBreadcrumbHeight] = useState(0);
+
+ useEffect(() => {
+ if (breadcrumbRef.current) {
+ setBreadcrumbHeight(breadcrumbRef.current.offsetHeight);
+ } else {
+ setBreadcrumbHeight(0);
+ }
+ }, [drillStack.length]);
+
+ if (!hasHierarchy || !isFeatureEnabled(FeatureFlag.DrillDownHierarchy)) {
+ return <ChartRendererComponent {...rendererProps} />;
+ }
+
+ // Reduce chart height by the breadcrumb height
+ const adjustedHeight =
+ rendererProps.height && breadcrumbHeight > 0
+ ? rendererProps.height - breadcrumbHeight
+ : rendererProps.height;
Review Comment:
**Suggestion:** The adjusted chart height can become negative when the breadcrumb is taller than the available chart height, which can lead to invalid chart sizing and render glitches. Clamp the computed height to a non-negative minimum before passing it to the renderer. [css layout issue]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Drill-down charts can receive negative heights in dashboards.
- ⚠️ Charts may render collapsed or visually corrupted.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset-frontend/src/components/Chart/Chart.tsx:96-134`, `Chart` renders a
`.slice_container` with `height={height}` and mounts `DrillDownHost` with the same
`height` prop passed through as `rendererProps.height` (e.g., a small dashboard row
height).
2. In `superset-frontend/src/components/Chart/DrillDown/DrillDownHost.tsx:158-167`, the
breadcrumb ref measures `breadcrumbHeight = breadcrumbRef.current.offsetHeight` whenever
the drill stack changes, which can be larger than a very small `rendererProps.height`.
3. The adjusted height is computed at `DrillDownHost.tsx:173-177` as `const adjustedHeight
= rendererProps.height && breadcrumbHeight > 0 ? rendererProps.height - breadcrumbHeight :
rendererProps.height;`, so if `rendererProps.height` is, for example, 20px while
`breadcrumbHeight` is 40px, `adjustedHeight` becomes `-20`.
4. That negative `adjustedHeight` is passed to `ChartRendererComponent` at
`DrillDownHost.tsx:216-219`, and `ChartRenderer` forwards `height={height}` directly to
`SuperChart` in `superset-frontend/src/components/Chart/ChartRenderer.tsx:271-279` without
clamping, so the rendered chart receives a negative height, leading to invalid sizing and
potential render glitches when the drill-down breadcrumb is taller than the available
chart area.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9e748b339b484571ae7c2076363d5ca7&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=9e748b339b484571ae7c2076363d5ca7&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/src/components/Chart/DrillDown/DrillDownHost.tsx
**Line:** 174:177
**Comment:**
*Css Layout Issue: The adjusted chart height can become negative when the breadcrumb is taller than the available chart height, which can lead to invalid chart sizing and render glitches. Clamp the computed height to a non-negative minimum before passing it to the renderer.
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%2F40449&comment_hash=2bd71a855a65f05e37ebd5e7fe649e272f45e3aa1fc218ce3f2b1a92956a3cf3&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=2bd71a855a65f05e37ebd5e7fe649e272f45e3aa1fc218ce3f2b1a92956a3cf3&reaction=dislike'>👎</a>
##########
superset-frontend/src/components/Chart/DrillDown/useDrillDownState.test.ts:
##########
@@ -0,0 +1,387 @@
+/**
+ * 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 { act, renderHook, waitFor } from '@testing-library/react';
+import { QueryFormData } from '@superset-ui/core';
+import { useDrillDownState, clearDrillDownState } from './useDrillDownState';
+
+jest.mock('src/components/Chart/chartAction', () => ({
+ getChartDataRequest: jest.fn(() =>
+ Promise.resolve({ response: {}, json: { result: [{ data: [] }] } }),
+ ),
+ handleChartDataResponse: jest.fn(() =>
+ Promise.resolve([{ data: [{ col1: 'val1' }] }]),
+ ),
+}));
+
+jest.mock('src/explore/exploreUtils', () => ({
+ getQuerySettings: jest.fn(() => [false]),
+}));
+
+jest.mock('src/utils/simpleFilterToAdhoc', () => ({
+ simpleFilterToAdhoc: jest.fn(filter => ({
+ expressionType: 'SIMPLE',
+ clause: 'WHERE',
+ operator: filter.op,
+ subject: filter.col,
+ comparator: filter.val,
+ })),
+}));
+
+const baseFormData: QueryFormData = {
+ datasource: '1__table',
+ viz_type: 'echarts_timeseries_bar',
+ slice_id: 42,
+ x_axis: 'country',
+ groupby: [],
+ adhoc_filters: [],
+};
+
+beforeEach(() => {
+ // Drill state persists in a module-level store keyed by chart id; clear it
+ // so each test starts from a clean slate (tests reuse slice_id 42).
+ clearDrillDownState();
+});
+
+test('hierarchy is empty when x_axis is a single string with no drilldown_hierarchy', () => {
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData: baseFormData,
+ baseQueriesResponse: [{ data: [] }],
+ }),
+ );
+
+ expect(result.current.hierarchy).toEqual([]);
+ expect(result.current.hasHierarchy).toBe(false);
+});
+
+test('hierarchy is built from x_axis array with 2+ items', () => {
+ const formData = {
+ ...baseFormData,
+ x_axis: 'country',
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ };
Review Comment:
**Suggestion:** The test title claims hierarchy is derived from an `x_axis` array, but the test data uses `x_axis` as a string and relies on `drilldown_hierarchy`; this is a misleading description that contradicts what is actually being tested. Rename the test description to reflect the real input source. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Minor 🧹</summary>
```mdx
- ⚠️ Test name misrepresents drilldown hierarchy configuration source.
- ⚠️ Developers may misunderstand supported hierarchy inputs.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset-frontend/src/components/Chart/DrillDown/useDrillDownState.ts:194-209`, the
`hierarchy` memo reads the drill columns exclusively from `formData.drilldown_hierarchy`
or `formData.drilldownHierarchy`, using `ensureIsArray(fd[HIERARCHY_FIELD] ??
fd[HIERARCHY_FIELD_CAMEL])`, and returns an empty array if there are not at least two
entries; there is no logic that derives hierarchy from `x_axis` arrays.
2. In `superset-frontend/src/components/Chart/DrillDown/useDrillDownState.test.ts:73-79`,
the test named `hierarchy is built from x_axis array with 2+ items` constructs `formData`
with `x_axis: 'country'` (a string, not an array) and `drilldown_hierarchy: ['country',
'region', 'city']`, then asserts `result.current.hierarchy` equals that drilldown array.
3. The next test `hierarchy is built from drilldown_hierarchy field` at
`useDrillDownState.test.ts:91-107` uses the same `drilldown_hierarchy` setup, so both
tests validate the same input source, while none cover an `x_axis` array scenario,
contradicting the “x_axis array” wording in the earlier test name.
4. As a result, the test title misleads readers into believing hierarchy can be derived
from an `x_axis` array, even though the implementation and test data rely solely on
`drilldown_hierarchy`, so renaming this test to reference `drilldown_hierarchy` would
better document the actual behavior without changing runtime semantics.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f51616955fbf45a69678716d66d3b1df&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=f51616955fbf45a69678716d66d3b1df&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/src/components/Chart/DrillDown/useDrillDownState.test.ts
**Line:** 73:78
**Comment:**
*Comment Mismatch: The test title claims hierarchy is derived from an `x_axis` array, but the test data uses `x_axis` as a string and relies on `drilldown_hierarchy`; this is a misleading description that contradicts what is actually being tested. Rename the test description to reflect the real input source.
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%2F40449&comment_hash=c8d6e57802d7924da1639501678139b31c0da4d4c23963c2e74fbc245f160953&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=c8d6e57802d7924da1639501678139b31c0da4d4c23963c2e74fbc245f160953&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3535745021
##########
superset-frontend/src/components/Chart/DrillDown/useDrillDownState.ts:
##########
@@ -0,0 +1,414 @@
+/**
+ * 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 {
+ useCallback,
+ useEffect,
+ useLayoutEffect,
+ useMemo,
+ useRef,
+ useState,
+} from 'react';
+import { t } from '@apache-superset/core/translation';
+import {
+ BinaryQueryObjectFilterClause,
+ ensureIsArray,
+ getClientErrorObject,
+ QueryData,
+ QueryFormData,
+} from '@superset-ui/core';
+import { simpleFilterToAdhoc } from 'src/utils/simpleFilterToAdhoc';
+import {
+ getChartDataRequest,
+ handleChartDataResponse,
+} from 'src/components/Chart/chartAction';
+import { getQuerySettings } from 'src/explore/exploreUtils';
+import { DrillDownLevel } from './types';
+
+/**
+ * The form-data field name that stores the ordered list of drill columns.
+ * The chart starts at hierarchy[0] and advances one level per click.
+ */
+const HIERARCHY_FIELD = 'drilldown_hierarchy';
+const HIERARCHY_FIELD_CAMEL = 'drilldownHierarchy';
+
+/**
+ * Default form-data field that holds the chart's grouping dimension.
+ * Most echarts plugins use 'groupby'; Sunburst uses 'columns'. The click
+ * handler can override this on a per-event basis.
+ */
+const DEFAULT_GROUPBY_FIELD = 'groupby';
+const DEFAULT_ADHOC_FILTERS_FIELD = 'adhoc_filters';
+
+/**
+ * Drill navigation is kept in a module-level store keyed by chart id so it
+ * survives incidental remounts of the chart component. On a dashboard, an
+ * unrelated filter change (e.g. removing another chart's cross-filter) can
+ * cause the grid to re-render and remount the chart, which would otherwise
+ * reset the local React state and make the breadcrumb vanish mid-drill,
+ * stranding the user with no way to navigate back up. The store is process
+ * memory only — a full page reload still starts fresh.
+ */
+interface StoredDrillState {
+ drillStack: DrillDownLevel[];
+ selectedLeaf?: string;
+}
+const drillStateStore = new Map<string | number, StoredDrillState>();
+
+/**
+ * Clear persisted drill state. Without arguments clears everything (used by
+ * tests to isolate cases); with a chart id clears just that chart.
+ */
+export function clearDrillDownState(chartKey?: string | number): void {
+ if (chartKey === undefined) {
+ drillStateStore.clear();
+ } else {
+ drillStateStore.delete(chartKey);
+ }
+}
+
+interface UseDrillDownStateArgs {
+ formData: QueryFormData;
+ /** Original chart data, shown when the drill stack is empty */
+ baseQueriesResponse?: QueryData[] | null;
+}
+
+interface UseDrillDownStateResult {
+ /** True if the user has drilled at least one level deep */
+ isDrilling: boolean;
+ /** The breadcrumb path showing where the user is in the hierarchy */
+ drillStack: DrillDownLevel[];
+ /** Value selected at the deepest level */
+ selectedLeaf?: string;
+ /** The computed hierarchy of column names */
+ hierarchy: string[];
+ /** form_data adjusted for the current drill level */
+ effectiveFormData: QueryFormData;
+ /** Chart data for the current drill level (or base data when not drilling) */
+ effectiveQueriesResponse: QueryData[] | null | undefined;
+ /** True while the next-level data is being fetched */
+ isLoading: boolean;
+ /** Error message if the drill query failed */
+ error?: string;
+ /** Whether the chart has a configured drill-down hierarchy */
+ hasHierarchy: boolean;
+ /** Whether there are more levels to drill into from the current state */
+ hasMoreLevels: boolean;
+ /**
+ * Push a new level onto the drill stack. Called from the chart's click
+ * handler with the filters that identify the clicked data point.
+ */
+ drillDown: (filters: BinaryQueryObjectFilterClause[], label: string) => void;
+ /** Truncate the drill stack to the given depth (0 = back to start) */
+ resetTo: (depth: number) => void;
+ /** Reset to base data (depth 0) */
+ reset: () => void;
+}
+
+/**
+ * Hook that manages a chart's drill-down state. Owns the drill stack,
+ * computes the effective form_data for the current level, fetches the
+ * data for that level, and exposes navigation helpers (drillDown / resetTo).
+ *
+ * The hook never mutates the upstream Redux store: closing or refreshing
+ * the dashboard wipes the drill state and restores the original chart.
+ */
+export function useDrillDownState({
+ formData,
+ baseQueriesResponse,
+}: UseDrillDownStateArgs): UseDrillDownStateResult {
+ const chartKey = formData.slice_id;
+
+ const [drillStack, setDrillStack] = useState<DrillDownLevel[]>(
+ () =>
+ (chartKey != null
+ ? drillStateStore.get(chartKey)?.drillStack
+ : undefined) ?? [],
+ );
+ const [selectedLeaf, setSelectedLeaf] = useState<string | undefined>(() =>
+ chartKey != null ? drillStateStore.get(chartKey)?.selectedLeaf : undefined,
+ );
+ const [drillData, setDrillData] = useState<QueryData[] | null>(null);
+ const [isLoading, setIsLoading] = useState(false);
+ const [error, setError] = useState<string | undefined>();
+
+ // Persist drill navigation synchronously so it survives remounts (see
+ // drillStateStore) without racing. Writing on a deferred effect would let a
+ // remount triggered by the same interaction (e.g. clearing a cross-filter)
+ // restore stale state before the effect runs, so the mutators below write
+ // through this helper immediately instead.
+ const persist = useCallback(
+ (stack: DrillDownLevel[], leaf: string | undefined) => {
+ if (chartKey == null) {
+ return;
+ }
+ if (stack.length === 0 && !leaf) {
+ drillStateStore.delete(chartKey);
+ } else {
+ drillStateStore.set(chartKey, {
+ drillStack: stack,
+ selectedLeaf: leaf,
+ });
+ }
+ },
+ [chartKey],
+ );
+
+ // Reset only when the chart is actually reconfigured (chart id or viz type
+ // changes) — e.g. the dashboard owner edited the chart and saved. A ref
+ // guard ensures the initial mount (which restores persisted state) does not
+ // wipe it, and that incidental re-renders from filter changes don't either.
+ // useLayoutEffect runs synchronously before paint so the stale drill state
+ // is cleared without a visible flash when the chart is reconfigured.
+ const configKey = `${formData.slice_id}__${formData.viz_type}`;
+ const prevConfigKeyRef = useRef(configKey);
+ useLayoutEffect(() => {
+ if (prevConfigKeyRef.current === configKey) {
+ return;
+ }
+ prevConfigKeyRef.current = configKey;
+ if (chartKey != null) {
+ drillStateStore.delete(chartKey);
+ }
+ setDrillStack([]);
+ setSelectedLeaf(undefined);
+ setDrillData(null);
+ setError(undefined);
+ }, [configKey, chartKey]);
+
+ const hierarchy = useMemo<string[]>(() => {
+ const fd = formData as Record<string, unknown>;
+
+ // The drill-down hierarchy is defined via the `drilldown_hierarchy` field
+ // on the chart's form_data — an ordered list of column names representing
+ // the levels to drill through. The first entry is the initial grouping
+ // column shown at the base level.
+ const drillLevels = ensureIsArray(
+ fd[HIERARCHY_FIELD] ?? fd[HIERARCHY_FIELD_CAMEL],
+ ) as string[];
+ if (drillLevels.length > 1) {
+ return drillLevels;
+ }
+
+ return [];
+ }, [formData]);
+
+ const hasHierarchy = hierarchy.length > 0;
+ const currentDepth = drillStack.length;
+ const hasMoreLevels = hasHierarchy && currentDepth < hierarchy.length - 1;
+
+ const effectiveFormData = useMemo<QueryFormData>(() => {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing test coverage</b></div>
<div id="fix">
The diff removes Option 1 (x_axis array path). The test at line 73 'hierarchy is built from x_axis array with 2+ items' passes `x_axis: 'country'` (single string) + `drilldown_hierarchy`, which does NOT exercise the removed `Array.isArray(xAxis) && xAxis.length > 1` branch. The removed x_axis array path now has zero test coverage.
</div>
</div>
<small><i>Code Review Run #a03d30</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4673189720
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] New feature tk drilldown hierarchy [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3305734363
##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/mixins.tsx:
##########
@@ -40,7 +40,7 @@ const getAxisLabel = (
export const xAxisMixin = {
label: (state: ControlPanelState) => getAxisLabel(state?.form_data).label,
- multi: false,
+ multi: true,
Review Comment:
**Suggestion:** Making `x_axis` multi-select at the shared mixin level affects every chart using this control, but several existing consumers still treat `x_axis` as a single value (for example scalar equality checks and single-column type checks). This creates inconsistent behavior in control logic and drill option filtering when multiple x-axis columns are selected. Restrict multi-select to drill-down-capable charts (or migrate all scalar consumers first). [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Advanced analytics controls hidden for multi-level x-axis charts.
- ⚠️ X-axis sort options miscomputed when x_axis holds arrays.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Explore view for any chart that uses the shared `x_axis` control, e.g. a
bar/timeseries chart whose control panel wiring maps `x_axis` to `dndXAxisControl`
(`superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx:509`).
2. Note that `dndXAxisControl` spreads `xAxisMixin`
(`superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:34-37`),
and `xAxisMixin` now sets `multi: true` (`mixins.tsx:41-58`), so `controls.x_axis.value`
can become an array when multiple x-axis columns are selected.
3. With such a chart, select multiple x-axis columns (e.g. a temporal column plus a
categorical column) so that `controls.x_axis.value` is an array, then observe
`displayTimeRelatedControls` in `displayTimeRelatedControls.ts:10-20` which reads `const
xAxisValue = xAxis?.value;` and passes it to `isAdhocColumn`/`isPhysicalColumn` assuming a
single `QueryFormColumn`; for an array both checks fail and the function returns `false`,
hiding advanced analytics/forecast sections whose `visibility` is
`displayTimeRelatedControls` (`advancedAnalytics.tsx:34`, `forecastInterval.tsx:36`).
4. In the same multi-x-axis configuration, observe that other control utilities also
assume a scalar x-axis value: `isSortable` in `isSortable.ts:8-21` and `xAxisSortControl`
in `customControls.tsx:24-47` both cast `controls?.x_axis?.value as QueryFormColumn` and
feed it into `getColumnLabel`/`checkColumnType`; when `value` is an array these casts
either miscompute options or break, so time-related visibility and sort controls behave
inconsistently whenever multiple x-axis columns are selected.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=98761c0a05a94a25b3a23e5481e5fe7b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=98761c0a05a94a25b3a23e5481e5fe7b&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/packages/superset-ui-chart-controls/src/shared-controls/mixins.tsx
**Line:** 43:43
**Comment:**
*Api Mismatch: Making `x_axis` multi-select at the shared mixin level affects every chart using this control, but several existing consumers still treat `x_axis` as a single value (for example scalar equality checks and single-column type checks). This creates inconsistent behavior in control logic and drill option filtering when multiple x-axis columns are selected. Restrict multi-select to drill-down-capable charts (or migrate all scalar consumers first).
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%2F40449&comment_hash=06e728382d652278546a7acf3b0da6b980bad79de635a34ac6d59f8682434d43&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=06e728382d652278546a7acf3b0da6b980bad79de635a34ac6d59f8682434d43&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Funnel/transformProps.ts:
##########
@@ -335,6 +335,7 @@ export default function transformProps(
groupby,
selectedValues,
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** Adding `onDrillDown` to transformed props switches click handling to the drill-down branch in `allEventHandlers`, which skips `setDataMask` updates. As a result, clicking a funnel segment no longer emits cross-filters to other dashboard charts. Keep drill-down, but also trigger the cross-filter data mask on click so dashboard-level filtering still works. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Funnel clicks stop cross-filtering other dashboard charts.
- ⚠️ Drill-down breadcrumbs and dashboard filters become inconsistent.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render a funnel chart via `EchartsFunnel`
(`superset-frontend/plugins/plugin-chart-echarts/src/Funnel/EchartsFunnel.tsx:5-20`),
which calls `allEventHandlers(props)` (`utils/eventHandlers.ts:152-217`) and passes the
resulting `eventHandlers` into the shared `Echart` component; `props` here are the
transformed props produced by `Funnel/transformProps.ts`.
2. In `transformProps` for Funnel
(`superset-frontend/plugins/plugin-chart-echarts/src/Funnel/transformProps.ts:88-102,
155-167, 68-82`), the hooks bag is destructured as `const { setDataMask = () => {},
onContextMenu, onDrillDown } = hooks;` and the returned object includes `setDataMask`,
`emitCrossFilters`, `labelMap`, `groupby`, `selectedValues`, `onContextMenu`, and
`onDrillDown` (line 79).
3. `allEventHandlers` (`utils/eventHandlers.ts:152-205`) reads `onDrillDown` from the
transformed props (line 164), computes `const hasDrillHierarchy = !!onDrillDown;` (line
170), and if `hasDrillHierarchy && groupby.length > 0` builds `drillDownClickHandler`
(lines 172-193) that assembles `drillFilters` and calls `onDrillDown(drillFilters, label)`
but never invokes `setDataMask`.
4. In the same function, the `click` handler is set to `drillDownClickHandler` when
present, otherwise to `clickEventHandler(...)` (lines 197-205), where `clickEventHandler`
(`eventHandlers.ts:90-106`) is the only code path that calls `setDataMask` and emits
cross-filters. When DrillDownHost wires `onDrillDown` into the hooks via `ChartRenderer`
(`ChartRenderer.tsx:16-22` and `DrillDownHost.tsx:23-30`), `hasDrillHierarchy` becomes
true, `click` is bound to `drillDownClickHandler`, and clicking a funnel segment no longer
updates the data mask, so no cross-filter is propagated to other charts on the dashboard.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=94b1445887104027aa36358f5c48a3ad&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=94b1445887104027aa36358f5c48a3ad&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/Funnel/transformProps.ts
**Line:** 338:338
**Comment:**
*Incomplete Implementation: Adding `onDrillDown` to transformed props switches click handling to the drill-down branch in `allEventHandlers`, which skips `setDataMask` updates. As a result, clicking a funnel segment no longer emits cross-filters to other dashboard charts. Keep drill-down, but also trigger the cross-filter data mask on click so dashboard-level filtering still works.
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%2F40449&comment_hash=cea0a1532e7cd8098c4124b5232dde879537dde55a7dc30afd9f1b32ff53f6e0&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=cea0a1532e7cd8098c4124b5232dde879537dde55a7dc30afd9f1b32ff53f6e0&reaction=dislike'>👎</a>
##########
superset-frontend/packages/superset-ui-core/src/query/getXAxis.ts:
##########
@@ -25,8 +25,13 @@ import {
Optional,
} from '@superset-ui/core';
-export const isXAxisSet = (formData: QueryFormData) =>
- isQueryFormColumn(formData.x_axis);
+export const isXAxisSet = (formData: QueryFormData) => {
+ const xAxis = formData.x_axis;
+ if (Array.isArray(xAxis)) {
+ return xAxis.length > 0 && isQueryFormColumn(xAxis[0]);
Review Comment:
**Suggestion:** This now treats array `x_axis` as set, but downstream query normalization still assumes `formData.x_axis` is a scalar column. In `normalizeTimeColumn`, comparisons against `formData.x_axis` fail when it is an array, so base-axis time metadata is not applied. Normalize by unwrapping the first x-axis column before downstream consumers use it. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Base-axis time metadata missing for multi-level x_axis.
- ⚠️ Time normalization skipped, affecting advanced analytics pipelines.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Build a chart that uses the shared `x_axis` control, then enable a drill hierarchy so
`formData.x_axis` becomes an array (multi-column) via the `xAxisMixin` multi-select
(`mixins.tsx:41-58`) and the DnD wiring in `sharedControls.tsx:509`.
2. When this chart executes, `buildQueryContext`
(`superset-frontend/packages/superset-ui-core/src/query/buildQueryContext.ts:35-61`)
constructs base query objects and checks `if (isXAxisSet(formData))` at line 59; the new
`isXAxisSet` implementation in `getXAxis.ts:28-34` treats an array `x_axis` as "set" by
checking only `xAxis[0]`.
3. Because `isXAxisSet(formData)` returns `true` for array-valued `x_axis`,
`buildQueryContext` maps each query through `normalizeTimeColumn(formData, query)`
(`buildQueryContext.ts:59-61`); inside `normalizeTimeColumn`
(`normalizeTimeColumn.ts:40-50`), `axisIdx` is computed by comparing each
`queryObject.columns` entry against `formData.x_axis`, but these comparisons call
`isPhysicalColumn(formData.x_axis)`/`isAdhocColumn(formData.x_axis)` assuming a scalar
`QueryFormColumn`, so when `formData.x_axis` is an array `axisIdx` never matches.
4. With `axisIdx` remaining `-1`, the normalization branch at
`normalizeTimeColumn.ts:51-77` is skipped and the function returns the original
`queryObject` (lines 80-81), meaning no column is annotated with `columnType: 'BASE_AXIS'`
or `timeGrain`; downstream consumers that rely on base-axis metadata (e.g. post-processing
that inspects `getXAxisLabel(formData)` in `sortOperator.ts:5-9`) lose this information
whenever `formData.x_axis` is an array rather than a single column.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=953d4f8bb5ba41e79d53eae8bed2cf4d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=953d4f8bb5ba41e79d53eae8bed2cf4d&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/packages/superset-ui-core/src/query/getXAxis.ts
**Line:** 30:31
**Comment:**
*Api Mismatch: This now treats array `x_axis` as set, but downstream query normalization still assumes `formData.x_axis` is a scalar column. In `normalizeTimeColumn`, comparisons against `formData.x_axis` fail when it is an array, so base-axis time metadata is not applied. Normalize by unwrapping the first x-axis column before downstream consumers use it.
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%2F40449&comment_hash=2d5f6d071d8f1f3a72a02b8028361d0330811ba9dad46698671b5014c3a088be&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=2d5f6d071d8f1f3a72a02b8028361d0330811ba9dad46698671b5014c3a088be&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts:
##########
@@ -495,6 +495,7 @@ export default function transformProps(
groupby,
selectedValues,
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** Passing `onDrillDown` into the pie transformed props makes `allEventHandlers` use the drill-down click path, which currently bypasses cross-filter updates (`setDataMask`). This causes pie clicks to stop propagating cross-filters to other charts when hierarchy is enabled. Emit cross-filter data alongside drill-down to preserve expected dashboard interactions. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Pie clicks no longer cross-filter linked dashboard charts.
- ⚠️ Drill-down pies show stale filters on sibling charts.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render a pie chart via `EchartsPie`
(`superset-frontend/plugins/plugin-chart-echarts/src/Pie/EchartsPie.tsx:5-20`), which
calls `allEventHandlers(props)` and passes the resulting handlers into `Echart`; as with
Funnel, `props` here are produced by `Pie/transformProps.ts`.
2. In `transformProps` for Pie
(`superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts:124-138,
246-260, 488-502`), the hooks bag is destructured as `const { setDataMask = () => {},
onContextMenu, onDrillDown } = hooks;` (line 259), and the returned transformed props
include `setDataMask`, `labelMap`, `groupby`, `selectedValues`, `onContextMenu`,
`onDrillDown`, `refs`, `emitCrossFilters`, and `coltypeMapping` (lines 488-502).
3. `allEventHandlers` in `utils/eventHandlers.ts:152-205` reads `onDrillDown` (line 164)
and, when it is defined and `groupby.length > 0`, constructs `drillDownClickHandler`
(lines 172-193) that computes drill filters and calls `onDrillDown` but does not call
`setDataMask`; it then assigns `click` to `drillDownClickHandler`, falling back to
`clickEventHandler(...)` only when no drill hierarchy is present (lines 197-205).
4. When DrillDownHost provides `onDrillDown` via the hooks getter in `ChartRenderer`
(`ChartRenderer.tsx:16-22`, `DrillDownHost.tsx:23-30`), pie charts with a configured drill
hierarchy end up using `drillDownClickHandler` for left-clicks, so clicks no longer
trigger `clickEventHandler` (`eventHandlers.ts:90-106`) and thus stop emitting
cross-filter data masks; as a result, pie segment clicks drill into the next level but do
not filter other dashboard charts.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bb4432d5657946efb30e8d1fef4e8a6a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=bb4432d5657946efb30e8d1fef4e8a6a&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/Pie/transformProps.ts
**Line:** 498:498
**Comment:**
*Incomplete Implementation: Passing `onDrillDown` into the pie transformed props makes `allEventHandlers` use the drill-down click path, which currently bypasses cross-filter updates (`setDataMask`). This causes pie clicks to stop propagating cross-filters to other charts when hierarchy is enabled. Emit cross-filter data alongside drill-down to preserve expected dashboard interactions.
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%2F40449&comment_hash=69cbcad93bb54d50c9aae08f3a75f4df80d481433c0a4951082d362c33f324c8&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=69cbcad93bb54d50c9aae08f3a75f4df80d481433c0a4951082d362c33f324c8&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4902061780
## EnxDev's Review Agent — apache/superset#40449 · HEAD 84b64b0
**Request changes** — supersedes my [prior review](https://github.com/apache/superset/pull/40449#pullrequestreview-4572719388) (HEAD 3de6b12). The risky items from last round are fixed, but the fix removed the only path that configured the hierarchy: as shipped, no control populates `drilldown_hierarchy`, so the feature is unreachable. Treemap/Sunburst are still advertised but not wired, and pre-commit CI is red.
**Fixed since last round** (thanks): dropped the global `x_axis` `multi: true`, removed the unrelated `superset/models/core.py` engine-listener change, gated behind the new `DRILL_DOWN_HIERARCHY` flag, fixed the hardcoded `2px` spacing (`theme.sizeUnit / 2`) and the inline `BinaryQueryObjectFilterClause` import.
### 🔴 Functional
- **`src/components/Chart/DrillDown/useDrillDownState.ts:2067`** · _High_ — the hook reads the hierarchy from `drilldown_hierarchy` / `drilldownHierarchy` form_data, but **no control in this diff writes that field** (code search: 0 references on master; no control panel file is touched). The prior `xAxisMixin multi:true` approach was reverted and nothing replaced it, so `hasHierarchy` is always false and the entire drill path is dead code for end users. The PR "How it works" still describes `xAxisMixin` / `getXAxisColumn` / `isXAxisSet` changes that aren't in the diff. **regression test:** an explore control test that sets a hierarchy via the UI and asserts `form_data.drilldown_hierarchy` is populated — impossible today because no control exists.
- **`Treemap/transformProps.ts`, `Sunburst/transformProps.ts` (absent)** · _High_ — PR body lists Treemap and Sunburst as supported, but neither `transformProps` is in the diff, so `onDrillDown` is never destructured/forwarded; they won't drill. Sunburst also groups on `columns`, which `effectiveFormData` can't target (it only swaps `groupby`/`x_axis`). Wire them or drop them from the supported list. (persists from prior review) **regression test:** click a Sunburst/Treemap segment with a hierarchy and assert `onDrillDown` fires.
- **`src/components/Chart/DrillDown/useDrillDownState.ts:2104`** · _Low_ — `effectiveFormData` checks `hasGroupby` first, so a bar/line chart configured with **both** an `x_axis` and a series `groupby` swaps `groupby` (the series breakdown) instead of `x_axis` when drilling. Confirm the intended target for charts that have both. **regression test:** drill a bar chart with `x_axis` + a non-empty `groupby` and assert `x_axis` is the field swapped.
### 🟡 Should-fix
- **`superset/config.py:809`** — new `DRILL_DOWN_HIERARCHY` flag added to `DEFAULT_FEATURE_FLAGS` and `featureFlags.ts`, but `docs/static/feature-flags.json` isn't synced. Keep them in sync.
- **CI** — `pre-commit (current)` is failing. Resolve before merge (likely lint/format).
- **`plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:246`** — re-implements the drill logic already in `eventHandlers.ts`; the two have diverged (only Timeseries handles the x-axis-only / `props.name` cases). Consolidate to avoid drift. (persists)
- **`packages/superset-ui-core/types/storybook-actions.d.ts`** — new ambient `declare module 'storybook/actions'` is unrelated to drill-down and looks like stray scope from a rebase. Drop it unless it's genuinely required by this PR.
- **PR description is stale** — "How it works" describes the reverted `xAxisMixin`/`getXAxisColumn`/`isXAxisSet` changes; update it to the `drilldown_hierarchy` approach so reviewers/users aren't misled.
- No `/docs` update for a new user-facing feature. (persists)
### 🔵 Nits
- `DrillDown/DrillDownBreadcrumb.tsx:75` — navigation still uses hand-rolled `<span role="button">`; prefer the core `Breadcrumb`/`Button` for a11y + consistency. (persists)
- PR title `feat: New feature tk drilldown hierarchy` passes pr-lint but reads as a placeholder (double spaces, "tk"); tidy to e.g. `feat(explore): drill-down hierarchy for ECharts charts`. (persists)
### 🙌 Praise
- `chartReducer.ts:226` — the guard against stale keyed actions on a removed chart is a clean, well-commented fix with a matching regression test.
- Strong test coverage across `useDrillDownState`, `DrillDownHost`, `DrillDownBreadcrumb`, and `eventHandlers`.
<!-- enxdev-review-agent:84b64b0 -->
_Reviewed by EnxDev's Review Agent — @EnxDev · HEAD 84b64b0._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3534487782
##########
superset-frontend/src/components/Chart/DrillDown/DrillDownHost.test.tsx:
##########
@@ -0,0 +1,230 @@
+/**
+ * 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 { ReactElement } from 'react';
+import {
+ render as rtlRender,
+ screen,
+ act,
+ fireEvent,
+} from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { ThemeProvider, supersetTheme } from '@apache-superset/core/theme';
+import { QueryFormData } from '@superset-ui/core';
+import { DrillDownHost } from './DrillDownHost';
+import { clearDrillDownState } from './useDrillDownState';
+import type { ChartRendererProps } from '../ChartRenderer';
+
+// Enable the drill-down feature flag for all tests in this file.
+jest.mock('@superset-ui/core', () => ({
+ ...jest.requireActual('@superset-ui/core'),
+ isFeatureEnabled: (flag: string) => flag === 'DRILL_DOWN_HIERARCHY',
+}));
+
+function render(ui: ReactElement) {
+ return rtlRender(ui, {
+ wrapper: ({ children }) => (
+ <ThemeProvider theme={supersetTheme}>{children}</ThemeProvider>
+ ),
+ });
+}
+
+beforeEach(() => {
+ clearDrillDownState();
+});
+
+jest.mock('src/components/Chart/chartAction', () => ({
+ getChartDataRequest: jest.fn(() =>
+ Promise.resolve({ response: {}, json: { result: [{ data: [] }] } }),
+ ),
+ handleChartDataResponse: jest.fn(() =>
+ Promise.resolve([{ data: [{ region: 'Texas' }] }]),
+ ),
+}));
+
+jest.mock('src/explore/exploreUtils', () => ({
+ getQuerySettings: jest.fn(() => [false]),
+}));
+
+jest.mock('src/utils/simpleFilterToAdhoc', () => ({
+ simpleFilterToAdhoc: jest.fn(filter => ({
+ expressionType: 'SIMPLE',
+ clause: 'WHERE',
+ operator: filter.op,
+ subject: filter.col,
+ comparator: filter.val,
+ })),
+}));
+
+// A simple mock ChartRenderer component that renders its props for inspection
+function MockChartRenderer(
+ props: ChartRendererProps & { onDrillDown?: unknown },
+) {
+ return (
+ <div data-test="mock-chart-renderer">
+ <span data-test="has-on-drill-down">
+ {props.onDrillDown ? 'yes' : 'no'}
+ </span>
+ <span data-test="form-data-x-axis">
+ {JSON.stringify((props.formData as Record<string, unknown>).x_axis)}
+ </span>
+ </div>
+ );
+}
+
+const baseFormData: QueryFormData = {
+ datasource: '1__table',
+ viz_type: 'echarts_timeseries_bar',
+ slice_id: 42,
+ x_axis: 'country',
+ groupby: [],
+ adhoc_filters: [],
+};
+
+const baseRendererProps: ChartRendererProps = {
+ formData: baseFormData,
+ vizType: 'echarts_timeseries_bar',
+ chartId: 1,
+ height: 400,
+ width: 600,
+ queriesResponse: [{ data: [{ country: 'USA' }] }],
+ actions: {} as ChartRendererProps['actions'],
+};
+
+test('DrillDownHost passes through to ChartRenderer when no hierarchy', () => {
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
Review Comment:
**Suggestion:** Replace the `any` cast with a concrete component type for `ChartRendererComponent` so the test uses explicit props typing. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The test file contains a direct `any` cast in new TypeScript code, which matches the custom rule prohibiting `any` types. This is a real violation in the final file state.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
.cursor/rules/dev-standard.mdc (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3f524f93e2094ba0866a392e4ba321e9&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=3f524f93e2094ba0866a392e4ba321e9&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/src/components/Chart/DrillDown/DrillDownHost.test.tsx
**Line:** 112:112
**Comment:**
*Custom Rule: Replace the `any` cast with a concrete component type for `ChartRendererComponent` so the test uses explicit props typing.
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%2F40449&comment_hash=d1fa622e091b6b303d5529cef6d3b7f3feb875b03e867942a35557bcab19f35d&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=d1fa622e091b6b303d5529cef6d3b7f3feb875b03e867942a35557bcab19f35d&reaction=dislike'>👎</a>
##########
superset-frontend/src/components/Chart/DrillDown/DrillDownHost.test.tsx:
##########
@@ -0,0 +1,230 @@
+/**
+ * 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 { ReactElement } from 'react';
+import {
+ render as rtlRender,
+ screen,
+ act,
+ fireEvent,
+} from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { ThemeProvider, supersetTheme } from '@apache-superset/core/theme';
+import { QueryFormData } from '@superset-ui/core';
+import { DrillDownHost } from './DrillDownHost';
+import { clearDrillDownState } from './useDrillDownState';
+import type { ChartRendererProps } from '../ChartRenderer';
+
+// Enable the drill-down feature flag for all tests in this file.
+jest.mock('@superset-ui/core', () => ({
+ ...jest.requireActual('@superset-ui/core'),
+ isFeatureEnabled: (flag: string) => flag === 'DRILL_DOWN_HIERARCHY',
+}));
+
+function render(ui: ReactElement) {
+ return rtlRender(ui, {
+ wrapper: ({ children }) => (
+ <ThemeProvider theme={supersetTheme}>{children}</ThemeProvider>
+ ),
+ });
+}
+
+beforeEach(() => {
+ clearDrillDownState();
+});
+
+jest.mock('src/components/Chart/chartAction', () => ({
+ getChartDataRequest: jest.fn(() =>
+ Promise.resolve({ response: {}, json: { result: [{ data: [] }] } }),
+ ),
+ handleChartDataResponse: jest.fn(() =>
+ Promise.resolve([{ data: [{ region: 'Texas' }] }]),
+ ),
+}));
+
+jest.mock('src/explore/exploreUtils', () => ({
+ getQuerySettings: jest.fn(() => [false]),
+}));
+
+jest.mock('src/utils/simpleFilterToAdhoc', () => ({
+ simpleFilterToAdhoc: jest.fn(filter => ({
+ expressionType: 'SIMPLE',
+ clause: 'WHERE',
+ operator: filter.op,
+ subject: filter.col,
+ comparator: filter.val,
+ })),
+}));
+
+// A simple mock ChartRenderer component that renders its props for inspection
+function MockChartRenderer(
+ props: ChartRendererProps & { onDrillDown?: unknown },
+) {
+ return (
+ <div data-test="mock-chart-renderer">
+ <span data-test="has-on-drill-down">
+ {props.onDrillDown ? 'yes' : 'no'}
+ </span>
+ <span data-test="form-data-x-axis">
+ {JSON.stringify((props.formData as Record<string, unknown>).x_axis)}
+ </span>
+ </div>
+ );
+}
+
+const baseFormData: QueryFormData = {
+ datasource: '1__table',
+ viz_type: 'echarts_timeseries_bar',
+ slice_id: 42,
+ x_axis: 'country',
+ groupby: [],
+ adhoc_filters: [],
+};
+
+const baseRendererProps: ChartRendererProps = {
+ formData: baseFormData,
+ vizType: 'echarts_timeseries_bar',
+ chartId: 1,
+ height: 400,
+ width: 600,
+ queriesResponse: [{ data: [{ country: 'USA' }] }],
+ actions: {} as ChartRendererProps['actions'],
+};
+
+test('DrillDownHost passes through to ChartRenderer when no hierarchy', () => {
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ />,
+ );
+
+ // Should render the chart
+ expect(screen.getByTestId('mock-chart-renderer')).toBeInTheDocument();
+ // No drill-down host wrapper when no hierarchy
+ expect(screen.queryByTestId('drill-down-host')).not.toBeInTheDocument();
+ // onDrillDown should not be provided
+ expect(screen.getByTestId('has-on-drill-down')).toHaveTextContent('no');
+});
+
+test('DrillDownHost shows breadcrumb wrapper when hierarchy exists', () => {
+ const formDataWithHierarchy: QueryFormData = {
+ ...baseFormData,
+ x_axis: 'country',
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ };
+
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ formData={formDataWithHierarchy}
+ />,
+ );
+
+ // Should render the drill-down host wrapper
+ expect(screen.getByTestId('drill-down-host')).toBeInTheDocument();
+ // Chart should still render
+ expect(screen.getByTestId('mock-chart-renderer')).toBeInTheDocument();
+});
+
+test('onDrillDown is provided when hierarchy exists', () => {
+ const formDataWithHierarchy: QueryFormData = {
+ ...baseFormData,
+ x_axis: 'country',
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ };
+
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ formData={formDataWithHierarchy}
+ />,
+ );
+
+ expect(screen.getByTestId('has-on-drill-down')).toHaveTextContent('yes');
+});
+
+test('DrillDownHost renders with drilldown_hierarchy field', () => {
+ const formDataWithHierarchy: QueryFormData = {
+ ...baseFormData,
+ x_axis: 'country',
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ };
+
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ formData={formDataWithHierarchy}
+ />,
+ );
+
+ expect(screen.getByTestId('drill-down-host')).toBeInTheDocument();
+ expect(screen.getByTestId('has-on-drill-down')).toHaveTextContent('yes');
+});
+
+test('returning to the top level clears the cross-filter and re-queries the base', async () => {
+ const updateDataMask = jest.fn();
+ const triggerQuery = jest.fn();
+ const formDataWithHierarchy: QueryFormData = {
+ ...baseFormData,
+ x_axis: 'country',
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ };
+
+ let capturedOnDrillDown:
+ | ((filters: unknown, label: string) => void)
+ | undefined;
+ function CaptureRenderer(
+ props: ChartRendererProps & {
+ onDrillDown?: (filters: unknown, label: string) => void;
+ },
+ ) {
+ capturedOnDrillDown = props.onDrillDown;
+ return <div data-test="mock-chart-renderer" />;
+ }
+
+ render(
+ <DrillDownHost
+ ChartRendererComponent={CaptureRenderer as any}
Review Comment:
**Suggestion:** Remove the `any` cast and type `CaptureRenderer` to match the expected `ChartRendererComponent` signature. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This new test code also uses an explicit `any` cast, so it violates the no-`any` rule. The suggestion accurately identifies a real issue present in the file.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
.cursor/rules/dev-standard.mdc (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=07fd90bd88174061b4ca34170b9b7ea9&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=07fd90bd88174061b4ca34170b9b7ea9&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/src/components/Chart/DrillDown/DrillDownHost.test.tsx
**Line:** 206:206
**Comment:**
*Custom Rule: Remove the `any` cast and type `CaptureRenderer` to match the expected `ChartRendererComponent` signature.
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%2F40449&comment_hash=8cf8cca8d5f418ada97265a0f466b75daad4290ae1fb573e419b849724e2297a&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=8cf8cca8d5f418ada97265a0f466b75daad4290ae1fb573e419b849724e2297a&reaction=dislike'>👎</a>
##########
superset-frontend/src/components/Chart/DrillDown/DrillDownHost.test.tsx:
##########
@@ -0,0 +1,230 @@
+/**
+ * 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 { ReactElement } from 'react';
+import {
+ render as rtlRender,
+ screen,
+ act,
+ fireEvent,
+} from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { ThemeProvider, supersetTheme } from '@apache-superset/core/theme';
+import { QueryFormData } from '@superset-ui/core';
+import { DrillDownHost } from './DrillDownHost';
+import { clearDrillDownState } from './useDrillDownState';
+import type { ChartRendererProps } from '../ChartRenderer';
+
+// Enable the drill-down feature flag for all tests in this file.
+jest.mock('@superset-ui/core', () => ({
+ ...jest.requireActual('@superset-ui/core'),
+ isFeatureEnabled: (flag: string) => flag === 'DRILL_DOWN_HIERARCHY',
+}));
+
+function render(ui: ReactElement) {
+ return rtlRender(ui, {
+ wrapper: ({ children }) => (
+ <ThemeProvider theme={supersetTheme}>{children}</ThemeProvider>
+ ),
+ });
+}
+
+beforeEach(() => {
+ clearDrillDownState();
+});
+
+jest.mock('src/components/Chart/chartAction', () => ({
+ getChartDataRequest: jest.fn(() =>
+ Promise.resolve({ response: {}, json: { result: [{ data: [] }] } }),
+ ),
+ handleChartDataResponse: jest.fn(() =>
+ Promise.resolve([{ data: [{ region: 'Texas' }] }]),
+ ),
+}));
+
+jest.mock('src/explore/exploreUtils', () => ({
+ getQuerySettings: jest.fn(() => [false]),
+}));
+
+jest.mock('src/utils/simpleFilterToAdhoc', () => ({
+ simpleFilterToAdhoc: jest.fn(filter => ({
+ expressionType: 'SIMPLE',
+ clause: 'WHERE',
+ operator: filter.op,
+ subject: filter.col,
+ comparator: filter.val,
+ })),
+}));
+
+// A simple mock ChartRenderer component that renders its props for inspection
+function MockChartRenderer(
+ props: ChartRendererProps & { onDrillDown?: unknown },
+) {
+ return (
+ <div data-test="mock-chart-renderer">
+ <span data-test="has-on-drill-down">
+ {props.onDrillDown ? 'yes' : 'no'}
+ </span>
+ <span data-test="form-data-x-axis">
+ {JSON.stringify((props.formData as Record<string, unknown>).x_axis)}
+ </span>
+ </div>
+ );
+}
+
+const baseFormData: QueryFormData = {
+ datasource: '1__table',
+ viz_type: 'echarts_timeseries_bar',
+ slice_id: 42,
+ x_axis: 'country',
+ groupby: [],
+ adhoc_filters: [],
+};
+
+const baseRendererProps: ChartRendererProps = {
+ formData: baseFormData,
+ vizType: 'echarts_timeseries_bar',
+ chartId: 1,
+ height: 400,
+ width: 600,
+ queriesResponse: [{ data: [{ country: 'USA' }] }],
+ actions: {} as ChartRendererProps['actions'],
+};
+
+test('DrillDownHost passes through to ChartRenderer when no hierarchy', () => {
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ />,
+ );
+
+ // Should render the chart
+ expect(screen.getByTestId('mock-chart-renderer')).toBeInTheDocument();
+ // No drill-down host wrapper when no hierarchy
+ expect(screen.queryByTestId('drill-down-host')).not.toBeInTheDocument();
+ // onDrillDown should not be provided
+ expect(screen.getByTestId('has-on-drill-down')).toHaveTextContent('no');
+});
+
+test('DrillDownHost shows breadcrumb wrapper when hierarchy exists', () => {
+ const formDataWithHierarchy: QueryFormData = {
+ ...baseFormData,
+ x_axis: 'country',
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ };
+
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ formData={formDataWithHierarchy}
+ />,
+ );
+
+ // Should render the drill-down host wrapper
+ expect(screen.getByTestId('drill-down-host')).toBeInTheDocument();
+ // Chart should still render
+ expect(screen.getByTestId('mock-chart-renderer')).toBeInTheDocument();
+});
+
+test('onDrillDown is provided when hierarchy exists', () => {
+ const formDataWithHierarchy: QueryFormData = {
+ ...baseFormData,
+ x_axis: 'country',
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ };
+
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ formData={formDataWithHierarchy}
+ />,
+ );
+
+ expect(screen.getByTestId('has-on-drill-down')).toHaveTextContent('yes');
+});
+
+test('DrillDownHost renders with drilldown_hierarchy field', () => {
+ const formDataWithHierarchy: QueryFormData = {
+ ...baseFormData,
+ x_axis: 'country',
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ };
+
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ formData={formDataWithHierarchy}
+ />,
+ );
+
+ expect(screen.getByTestId('drill-down-host')).toBeInTheDocument();
+ expect(screen.getByTestId('has-on-drill-down')).toHaveTextContent('yes');
+});
+
+test('returning to the top level clears the cross-filter and re-queries the base', async () => {
+ const updateDataMask = jest.fn();
+ const triggerQuery = jest.fn();
+ const formDataWithHierarchy: QueryFormData = {
+ ...baseFormData,
+ x_axis: 'country',
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ };
+
+ let capturedOnDrillDown:
+ | ((filters: unknown, label: string) => void)
+ | undefined;
+ function CaptureRenderer(
+ props: ChartRendererProps & {
+ onDrillDown?: (filters: unknown, label: string) => void;
+ },
+ ) {
+ capturedOnDrillDown = props.onDrillDown;
+ return <div data-test="mock-chart-renderer" />;
+ }
+
+ render(
+ <DrillDownHost
+ ChartRendererComponent={CaptureRenderer as any}
+ {...baseRendererProps}
+ formData={formDataWithHierarchy}
+ actions={{ updateDataMask, triggerQuery } as any}
Review Comment:
**Suggestion:** Type the `actions` mock object with the appropriate `ChartRendererProps['actions']` shape instead of casting to `any`. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The code uses `as any` in a changed TypeScript test file, which is exactly what the custom rule forbids. This is a verified violation.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
.cursor/rules/dev-standard.mdc (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7c945b31076547d1806e34d2aba69ddd&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=7c945b31076547d1806e34d2aba69ddd&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/src/components/Chart/DrillDown/DrillDownHost.test.tsx
**Line:** 209:209
**Comment:**
*Custom Rule: Type the `actions` mock object with the appropriate `ChartRendererProps['actions']` shape instead of casting to `any`.
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%2F40449&comment_hash=86ff9bff281fdd29f47723eede1549c4c7af9baf566181ee626090f4e7957b2f&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=86ff9bff281fdd29f47723eede1549c4c7af9baf566181ee626090f4e7957b2f&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature drilldown hierarchy [superset]
Posted by "tomerkl65 (via GitHub)" <gi...@apache.org>.
tomerkl65 closed pull request #40449: feat: New feature drilldown hierarchy
URL: https://github.com/apache/superset/pull/40449
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] New feature tk drilldown hierarchy [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4546924935
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 94ebe3dc4236375ee4bfb8d3aa0e24920aacdb64 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a15d87bae444b000923de8e |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40449--superset-docs-preview.netlify.app](https://deploy-preview-40449--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#q1YqKMrPLShRslJS0lFKKkrMS84Asv1Sy91SE0tKi1J1dUO8dXVdijJzclLyy_M8MlOLEouSMyqBqlNSC3LyKz1TgOrNEg1NUyzMkxJTTUxMkgwMDCyNjFNSLVKVagE) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40449--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3305830634
##########
superset-frontend/plugins/plugin-chart-echarts/src/Treemap/transformProps.ts:
##########
@@ -125,7 +125,7 @@ export default function transformProps(
currencyFormats = {},
currencyCodeColumn,
} = datasource;
- const { setDataMask = () => {}, onContextMenu } = hooks;
+ const { setDataMask = () => {}, onContextMenu, onDrillDown } = hooks;
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>onDrillDown not consumed in component</b></div>
<div id="fix">
The `onDrillDown` hook is extracted at line 128 and passed in the return object at line 320, but `EchartsTreemap.tsx` does not receive or consume this prop. The click event handler only emits cross-filters via `handleChange` — it never calls `onDrillDown`. This breaks drill-down functionality for the Treemap chart even when `drilldown_hierarchy` is configured.
</div>
</div>
<small><i>Code Review Run #7ccb16</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/components/Chart/DrillDown/DrillDownBreadcrumb.tsx:
##########
@@ -0,0 +1,116 @@
+/**
+ * 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 { useCallback } from 'react';
+import { css, useTheme } from '@apache-superset/core/theme';
+import { DrillDownLevel } from './types';
+
+interface DrillDownBreadcrumbProps {
+ /** Ordered list of column names from the chart's drill-down hierarchy */
+ hierarchy: string[];
+ /** The drill levels the user has navigated through */
+ drillStack: DrillDownLevel[];
+ /** Value selected at the deepest level (shown but not clickable) */
+ selectedLeaf?: string;
+ /** Reset the drill-down to the given depth (0 = back to the start) */
+ onJumpTo: (depth: number) => void;
+}
+
+/**
+ * Compact breadcrumb shown above the chart while the user is drilled into
+ * a hierarchy. Clicking any segment jumps back up the stack.
+ *
+ * country › USA › region › Texas
+ */
+export function DrillDownBreadcrumb({
+ hierarchy,
+ drillStack,
+ selectedLeaf,
+ onJumpTo,
+}: DrillDownBreadcrumbProps) {
+ const theme = useTheme();
+
+ const handleClick = useCallback(
+ (depth: number) => (e: React.MouseEvent) => {
+ e.preventDefault();
+ onJumpTo(depth);
+ },
+ [onJumpTo],
+ );
+
+ if (drillStack.length === 0 && !selectedLeaf) {
+ return null;
+ }
+
+ return (
+ <div
+ data-test="drill-down-breadcrumb"
+ css={css`
+ padding: ${theme.sizeUnit}px ${theme.sizeUnit * 2}px;
+ background: ${theme.colorBgContainer};
+ border-bottom: 1px solid ${theme.colorBorderSecondary};
+ font-size: ${theme.fontSizeSM}px;
+ display: flex;
+ align-items: center;
+ gap: ${theme.sizeUnit}px;
+ flex-wrap: wrap;
+ `}
+ >
+ <span
+ role="button"
+ tabIndex={0}
+ onClick={handleClick(0)}
+ css={css`
+ cursor: pointer;
+ color: ${theme.colorPrimary};
+ `}
+ >
+ {hierarchy[0] ?? ''}
+ </span>
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-20: Missing keyboard handler</b></div>
<div id="fix">
The breadcrumb segments have role="button" and tabIndex={0} but lack onKeyDown handlers, making them inaccessible via keyboard navigation. Users relying on keyboard cannot activate these interactive elements. (See also: [CWE-20](https://cwe.mitre.org/data/definitions/20.html))
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset-frontend/src/components/Chart/DrillDown/DrillDownBreadcrumb.tsx
+++ b/superset-frontend/src/components/Chart/DrillDown/DrillDownBreadcrumb.tsx
@@ -74,7 +74,10 @@ export function DrillDownBreadcrumb({
<span
role="button"
tabIndex={0}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') onJumpTo(0);
+ }}
onClick={handleClick(0)}
css={css`
cursor: pointer;
@@ -93,6 +96,9 @@ export function DrillDownBreadcrumb({
<span
role="button"
tabIndex={0}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') onJumpTo(index + 1);
+ }}
onClick={handleClick(index + 1)}
css={css`
cursor: pointer;
color: ${theme.colorPrimary};
`}
>
```
</div>
</details>
</div>
<small><i>Code Review Run #7ccb16</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -218,6 +219,87 @@ export default function EchartsTimeseries({
const eventHandlers: EventHandlers = {
click: props => {
+ // Drill-down takes priority over cross-filter when a hierarchy is configured.
+ // The DrillDownHost provides onDrillDown only when a hierarchy exists.
+ const hasDrillHierarchy = !!onDrillDown;
+
+ if (hasDrillHierarchy) {
+ if (clickTimer.current) {
+ clearTimeout(clickTimer.current);
+ }
+ clickTimer.current = setTimeout(() => {
+ const { seriesName: name } = props;
+ const drillFilters: BinaryQueryObjectFilterClause[] = [];
+ if (hasDimensions) {
+ const values = labelMap[name] ?? [];
+ groupby.forEach((dimension, i) => {
+ drillFilters.push({
+ col: dimension,
+ op: '==',
+ val: values[i],
+ formattedVal: String(values[i]),
+ });
+ });
+ // Also emit cross-filter so other charts in the dashboard filter too
+ if (emitCrossFilters) {
+ // Use direct setDataMask (not handleChange which toggles)
+ setDataMask({
+ extraFormData: {
+ filters: groupby.map((col, idx) => ({
+ col,
+ op: 'IN' as const,
+ val: [values[idx]] as (string | number | boolean)[],
+ })),
+ },
+ filterState: {
+ value: [values],
+ selectedValues: [name],
+ },
+ });
+ }
+ } else if (xAxis.type === AxisType.Category && props.data?.[0] != null) {
+ // Bar chart with x_axis only (no groupby dimensions)
+ drillFilters.push({
+ col: xAxis.label,
+ op: '==',
+ val: props.data[0],
+ formattedVal: String(props.data[0]),
+ });
+ // Also emit cross-filter by x-axis value
+ if (emitCrossFilters) {
+ setDataMask({
+ extraFormData: {
+ filters: [{
+ col: xAxis.label,
+ op: 'IN' as const,
+ val: [props.data[0]] as (string | number | boolean)[],
+ }],
+ },
+ filterState: {
+ value: [String(props.data[0])],
+ selectedValues: [String(props.data[0])],
+ },
+ });
+ }
+ } else if (props.name) {
+ // Fallback: use the event name (typically the x-axis label)
+ drillFilters.push({
+ col: xAxis.label,
+ op: '==',
+ val: props.name,
+ formattedVal: String(props.name),
+ });
+ }
+ if (drillFilters.length > 0) {
+ const label = drillFilters
+ .map(f => f.formattedVal ?? String(f.val))
+ .join(', ');
+ onDrillDown(drillFilters, label);
+ }
+ }, TIMER_DURATION);
+ return;
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing drill-down tests</b></div>
<div id="fix">
The new `onDrillDown` click handler (lines 226-300) with multiple branching logic and timer-based execution has no corresponding tests. Per [6262], tests must verify actual business logic — this click handler's complex branching (hasDimensions → categorical X-axis → props.name fallback) with cross-filter side effects requires test coverage to prevent regressions.
</div>
</div>
<small><i>Code Review Run #7ccb16</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/components/Chart/DrillDown/useDrillDownState.ts:
##########
@@ -0,0 +1,278 @@
+/**
+ * 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 { useCallback, useEffect, useMemo, useState } from 'react';
+import {
+ BinaryQueryObjectFilterClause,
+ ensureIsArray,
+ QueryData,
+ QueryFormData,
+} from '@superset-ui/core';
+import { simpleFilterToAdhoc } from 'src/utils/simpleFilterToAdhoc';
+import {
+ getChartDataRequest,
+ handleChartDataResponse,
+} from 'src/components/Chart/chartAction';
+import { getQuerySettings } from 'src/explore/exploreUtils';
+import { DrillDownLevel } from './types';
+
+/**
+ * The form-data field name that stores the ordered list of drill columns.
+ * The chart starts at hierarchy[0] and advances one level per click.
+ */
+const HIERARCHY_FIELD = 'drilldown_hierarchy';
+const HIERARCHY_FIELD_CAMEL = 'drilldownHierarchy';
+
+/**
+ * Default form-data field that holds the chart's grouping dimension.
+ * Most echarts plugins use 'groupby'; Sunburst uses 'columns'. The click
+ * handler can override this on a per-event basis.
+ */
+const DEFAULT_GROUPBY_FIELD = 'groupby';
+const DEFAULT_ADHOC_FILTERS_FIELD = 'adhoc_filters';
+
+interface UseDrillDownStateArgs {
+ formData: QueryFormData;
+ /** Original chart data, shown when the drill stack is empty */
+ baseQueriesResponse?: QueryData[] | null;
+}
+
+interface UseDrillDownStateResult {
+ /** True if the user has drilled at least one level deep */
+ isDrilling: boolean;
+ /** The breadcrumb path showing where the user is in the hierarchy */
+ drillStack: DrillDownLevel[];
+ /** Value selected at the deepest level */
+ selectedLeaf?: string;
+ /** The computed hierarchy of column names */
+ hierarchy: string[];
+ /** form_data adjusted for the current drill level */
+ effectiveFormData: QueryFormData;
+ /** Chart data for the current drill level (or base data when not drilling) */
+ effectiveQueriesResponse: QueryData[] | null | undefined;
+ /** True while the next-level data is being fetched */
+ isLoading: boolean;
+ /** Error message if the drill query failed */
+ error?: string;
+ /** Whether the chart has a configured drill-down hierarchy */
+ hasHierarchy: boolean;
+ /** Whether there are more levels to drill into from the current state */
+ hasMoreLevels: boolean;
+ /**
+ * Push a new level onto the drill stack. Called from the chart's click
+ * handler with the filters that identify the clicked data point.
+ */
+ drillDown: (
+ filters: BinaryQueryObjectFilterClause[],
+ label: string,
+ options?: { groupbyFieldName?: string; adhocFilterFieldName?: string },
+ ) => void;
+ /** Truncate the drill stack to the given depth (0 = back to start) */
+ resetTo: (depth: number) => void;
+ /** Reset to base data (depth 0) */
+ reset: () => void;
+}
+
+/**
+ * Hook that manages a chart's drill-down state. Owns the drill stack,
+ * computes the effective form_data for the current level, fetches the
+ * data for that level, and exposes navigation helpers (drillDown / resetTo).
+ *
+ * The hook never mutates the upstream Redux store: closing or refreshing
+ * the dashboard wipes the drill state and restores the original chart.
+ */
+export function useDrillDownState({
+ formData,
+ baseQueriesResponse,
+}: UseDrillDownStateArgs): UseDrillDownStateResult {
+ const [drillStack, setDrillStack] = useState<DrillDownLevel[]>([]);
+ const [selectedLeaf, setSelectedLeaf] = useState<string | undefined>();
+ const [drillData, setDrillData] = useState<QueryData[] | null>(null);
+ const [isLoading, setIsLoading] = useState(false);
+ const [error, setError] = useState<string | undefined>();
+
+ // Reset whenever the underlying form_data (chart configuration) changes —
+ // e.g. the dashboard owner edited the chart and saved.
+ useEffect(() => {
+ setDrillStack([]);
+ setSelectedLeaf(undefined);
+ setDrillData(null);
+ setError(undefined);
+ }, [formData.slice_id, formData.viz_type]);
+
+ const hierarchy = useMemo<string[]>(
+ () => {
+ const fd = formData as Record<string, unknown>;
+
+ // Option 1: x_axis is an array (multi-column, the new UX)
+ const xAxis = fd.x_axis ?? fd.xAxis;
+ if (Array.isArray(xAxis) && xAxis.length > 1) {
+ return xAxis as string[];
+ }
+
+ // Option 2: explicit drilldown_hierarchy field (legacy/separate control)
+ const drillLevels = ensureIsArray(fd[HIERARCHY_FIELD] ?? fd[HIERARCHY_FIELD_CAMEL]) as string[];
+ if (drillLevels.length > 0) {
+ const xAxisStr = (typeof xAxis === 'string' ? xAxis : undefined);
+ if (xAxisStr && !drillLevels.includes(xAxisStr)) {
+ return [xAxisStr, ...drillLevels];
+ }
+ return drillLevels;
+ }
+
+ return [];
+ },
+ [formData],
+ );
+
+ const hasHierarchy = hierarchy.length > 0;
+ const currentDepth = drillStack.length;
+ const hasMoreLevels = hasHierarchy && currentDepth < hierarchy.length - 1;
+
+ const effectiveFormData = useMemo<QueryFormData>(() => {
+ if (currentDepth === 0) {
+ return formData;
+ }
+ const nextColumn = hierarchy[currentDepth];
+
+ // Merge accumulated filters from every level into adhoc_filters.
+ const accumulatedFilters = drillStack.flatMap(level => level.filters);
+ const baseAdhoc = ensureIsArray(
+ (formData as Record<string, unknown>)[DEFAULT_ADHOC_FILTERS_FIELD],
+ );
+
+ const fdRecord = formData as Record<string, unknown>;
+
+ // Determine which field to swap with the next hierarchy level.
+ // Bar/Line/Area charts use x_axis as the primary dimension when groupby is empty.
+ const groupbyValue = fdRecord[DEFAULT_GROUPBY_FIELD];
+ const hasGroupby = Array.isArray(groupbyValue)
+ ? groupbyValue.length > 0
+ : !!groupbyValue;
+
+ const updated = { ...formData } as Record<string, unknown>;
+
+ if (hasGroupby) {
+ // Chart uses groupby — swap it
+ const isArrayGroupby = Array.isArray(groupbyValue);
+ updated[DEFAULT_GROUPBY_FIELD] = isArrayGroupby
+ ? [nextColumn]
+ : nextColumn;
+ } else if (fdRecord.x_axis !== undefined || fdRecord.xAxis !== undefined) {
+ // Chart uses x_axis (Bar/Line/Timeseries) — swap it
+ if (fdRecord.x_axis !== undefined) {
+ updated.x_axis = nextColumn;
+ }
+ if (fdRecord.xAxis !== undefined) {
+ updated.xAxis = nextColumn;
+ }
+ } else {
+ // Fallback: set groupby
+ updated[DEFAULT_GROUPBY_FIELD] = [nextColumn];
+ }
+
+ updated[DEFAULT_ADHOC_FILTERS_FIELD] = [
+ ...baseAdhoc,
+ ...accumulatedFilters.map(f => simpleFilterToAdhoc(f)),
+ ];
+
+ return updated as QueryFormData;
+ }, [formData, drillStack, currentDepth, hierarchy]);
+
+ // Fetch data whenever the user drills (stack changes and is non-empty).
+ useEffect(() => {
+ if (currentDepth === 0) {
+ setDrillData(null);
+ setError(undefined);
+ return undefined;
+ }
+
+ let cancelled = false;
+ setIsLoading(true);
+ setError(undefined);
+
+ const [useLegacyApi] = getQuerySettings(effectiveFormData);
+ getChartDataRequest({ formData: effectiveFormData })
+ .then(({ response, json }) =>
+ handleChartDataResponse(response, json, useLegacyApi),
+ )
+ .then(queriesResponse => {
+ if (!cancelled) {
+ setDrillData(queriesResponse as QueryData[]);
+ }
+ })
+ .catch(err => {
+ if (!cancelled) {
+ setError(err?.message || 'Failed to load drill-down data');
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing translation function wrapper</b></div>
<div id="fix">
User-facing error message should use translation function `t('Failed to load drill-down data')` for i18n support, consistent with DrillByModal.tsx:105 pattern in this same codebase.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset-frontend/src/components/Chart/DrillDown/useDrillDownState.ts
+++ b/superset-frontend/src/components/Chart/DrillDown/useDrillDownState.ts
@@ -17,6 +17,7 @@
* under the License.
*/
import { useCallback, useEffect, useMemo, useState } from 'react';
+import { t } from '@apache-superset/core/translation';
import {
BinaryQueryObjectFilterClause,
ensureIsArray,
@@ -218,7 +219,7 @@ export function useDrillDownState({
setError(err?.message || 'Failed to load drill-down data');
}
})
- .finally(() => {
+ .finally(() => {
if (!cancelled) {
setIsLoading(false);
}
});
@@ -233,7 +234,7 @@ export function useDrillDownState({
setError(err?.message || 'Failed to load drill-down data');
},
.catch(err => {
- if (!cancelled) {
- setError(err?.message || 'Failed to load drill-down data');
+ if (!cancelled) {
+ setError(err?.message || t('Failed to load drill-down data'));
}
})
.finally(() => {
```
</div>
</details>
</div>
<small><i>Code Review Run #7ccb16</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] New feature tk drilldown hierarchy [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3305709180
##########
superset-frontend/src/components/Chart/DrillDown/DrillDownHost.tsx:
##########
@@ -0,0 +1,209 @@
+/**
+ * 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 { ComponentType, useCallback, useMemo, useRef, useState, useEffect } from 'react';
+import {
+ QueryData,
+ QueryFormData,
+ BinaryQueryObjectFilterClause,
+} from '@superset-ui/core';
+import { css } from '@apache-superset/core/theme';
+import { useDrillDownState } from './useDrillDownState';
+import { DrillDownBreadcrumb } from './DrillDownBreadcrumb';
+import type { ChartRendererProps } from '../ChartRenderer';
+
+/**
+ * Hook payload contract: chart plugins call `onDrillDown(filters, label)`
+ * via the chart's hooks bag when the user clicks a data point and a
+ * drill-down hierarchy is configured.
+ */
+export type OnDrillDownHook = (
+ filters: BinaryQueryObjectFilterClause[],
+ label: string,
+) => void;
+
+interface DrillDownHostProps extends ChartRendererProps {
+ /** The wrapped renderer component */
+ ChartRendererComponent: ComponentType<
+ ChartRendererProps & { onDrillDown?: OnDrillDownHook }
+ >;
+}
+
+/**
+ * Wraps `<ChartRenderer>` with drill-down behavior. When the chart's
+ * form_data declares a `drilldown_hierarchy`, this host:
+ *
+ * 1. Tracks how deep the user has drilled (a stack of levels)
+ * 2. Computes "effective" form_data for the current level (replacing the
+ * groupby and adding accumulated filters)
+ * 3. Re-fetches chart data for that level
+ * 4. Renders a breadcrumb above the chart for navigating back up
+ *
+ * If the chart has no hierarchy, this is a thin pass-through.
+ */
+export function DrillDownHost({
+ ChartRendererComponent,
+ ...rendererProps
+}: DrillDownHostProps) {
+ const { formData, queriesResponse } = rendererProps;
+
+ const {
+ isDrilling,
+ drillStack,
+ selectedLeaf,
+ hierarchy,
+ effectiveFormData,
+ effectiveQueriesResponse,
+ isLoading,
+ error,
+ hasHierarchy,
+ drillDown,
+ resetTo,
+ } = useDrillDownState({
+ formData,
+ baseQueriesResponse: queriesResponse,
+ });
+
+ const onDrillDown = useMemo<OnDrillDownHook | undefined>(() => {
+ if (!hasHierarchy) {
+ return undefined;
+ }
+ return (filters, label) => {
+ drillDown(filters, label);
+ };
+ }, [hasHierarchy, drillDown]);
+
+ const overlayProps = useMemo<Partial<ChartRendererProps>>(() => {
+ if (!isDrilling) {
+ // Force re-render when returning to base level
+ return { triggerRender: true };
+ }
+ return {
+ formData: effectiveFormData as QueryFormData,
+ queriesResponse: (effectiveQueriesResponse ?? null) as
+ | QueryData[]
+ | null,
+ chartStatus: isLoading ? 'loading' : 'rendered',
+ latestQueryFormData: effectiveFormData,
+ chartIsStale: false,
+ triggerRender: true,
+ };
+ }, [isDrilling, effectiveFormData, effectiveQueriesResponse, isLoading]);
+
+ const handleResetTo = useCallback(
+ (depth: number) => {
+ resetTo(depth);
+ // Update cross-filter to match the level we're jumping to
+ if (rendererProps.actions?.updateDataMask) {
+ if (depth === 0) {
+ // Going back to root — clear cross-filter entirely
+ rendererProps.actions.updateDataMask(rendererProps.chartId, {
+ extraFormData: { filters: [] },
+ filterState: { value: null, selectedValues: null },
+ });
+ } else {
+ // Going to an intermediate level — set cross-filter to that level's filter
+ const targetLevel = drillStack[depth - 1];
+ if (targetLevel) {
+ const filters = targetLevel.filters.map(f => ({
+ col: f.col,
+ op: 'IN' as const,
+ val: [f.val] as (string | number | boolean)[],
+ }));
+ rendererProps.actions.updateDataMask(rendererProps.chartId, {
+ extraFormData: { filters },
+ filterState: {
+ value: filters.length ? [targetLevel.label] : null,
+ selectedValues: filters.length ? [targetLevel.label] : null,
+ },
Review Comment:
**🟠 Architect Review — HIGH**
Breadcrumb jump-to-depth updates the dashboard data mask using only the target level's filters (e.g., just the region), while the drilled chart's own queries use the accumulated filters from all ancestor levels (e.g., country + region). After multi-level drilling, this causes other charts' cross-filters to diverge from the drilled chart's actual state when navigating via the breadcrumb.
**Suggestion:** When handling breadcrumb navigation, rebuild the data mask filters from all DrillDownLevel entries up to the selected depth (mirroring effectiveFormData's accumulatedFilters), and set filterState to represent the full path, so the dashboard-wide cross-filter matches the drilled chart's query filters.
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=da7cc7c101f54b7599163d24a07cef1a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=da7cc7c101f54b7599163d24a07cef1a&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 an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** superset-frontend/src/components/Chart/DrillDown/DrillDownHost.tsx
**Line:** 120:133
**Comment:**
*HIGH: Breadcrumb jump-to-depth updates the dashboard data mask using only the target level's filters (e.g., just the region), while the drilled chart's own queries use the accumulated filters from all ancestor levels (e.g., country + region). After multi-level drilling, this causes other charts' cross-filters to diverge from the drilled chart's actual state when navigating via the breadcrumb.
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.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
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>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4902853604
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4624018381
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] New feature tk drilldown hierarchy [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3305723531
##########
superset-frontend/plugins/plugin-chart-echarts/src/Treemap/transformProps.ts:
##########
@@ -317,6 +317,7 @@ export default function transformProps(
groupby,
selectedValues: filterState.selectedValues || [],
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** This only forwards `onDrillDown` in transformed props, but Treemap click handling still does not invoke it, so Treemap never actually drills down despite being advertised as supported. Wire `onDrillDown` into Treemap click events (similar to other plugins) so user clicks can trigger hierarchy navigation. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Treemap charts never initiate drill-down on click.
- ⚠️ Treemap cannot participate in drill hierarchies.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Chart hooks, including `onDrillDown`, are exposed by `ChartRenderer` via its `hooks`
bag at `superset-frontend/src/components/Chart/ChartRenderer.tsx:265-271`, and
`DrillDownHost` only supplies a non-undefined `onDrillDown` when a hierarchy is configured
(see `superset-frontend/src/components/Chart/DrillDown/DrillDownHost.tsx:82-86`).
2. For Treemap, `transformProps` at
`superset-frontend/plugins/plugin-chart-echarts/src/Treemap/transformProps.ts:107-129`
destructures `hooks` as `const { setDataMask = () => {}, onContextMenu, onDrillDown } =
hooks;` and forwards `onDrillDown` in the returned props object at
`transformProps.ts:309-323`, specifically `onDrillDown,` on line 320 in the new hunk.
3. The Treemap renderer `EchartsTreemap` at
`superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx:34-47`
defines its parameter destructuring as `{ echartOptions, emitCrossFilters, groupby,
height, labelMap, onContextMenu, refs, setDataMask, selectedValues, width, formData,
coltypeMapping }` (no `onDrillDown`), and its `eventHandlers` at
`EchartsTreemap.tsx:112-156` only call `handleChange` to emit cross-filters and
`onContextMenu` for context menus; there is no reference to `onDrillDown`.
4. When a Treemap chart is used in a dashboard with a drilldown hierarchy configured (so
`onDrillDown` is provided through hooks) and the user clicks a node, the click handler at
`EchartsTreemap.tsx:112-116` runs `handleChange` and updates the data mask, but because
`onDrillDown` is never plumbed into `EchartsTreemap` or invoked from its `eventHandlers`,
no drill-down navigation occurs and the DrillDownHost state never changes, meaning Treemap
cannot participate in the advertised hierarchical drilldown feature.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6077b7cbc8384b7e8f5be9ef1fb69828&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6077b7cbc8384b7e8f5be9ef1fb69828&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/Treemap/transformProps.ts
**Line:** 320:320
**Comment:**
*Incomplete Implementation: This only forwards `onDrillDown` in transformed props, but Treemap click handling still does not invoke it, so Treemap never actually drills down despite being advertised as supported. Wire `onDrillDown` into Treemap click events (similar to other plugins) so user clicks can trigger hierarchy navigation.
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%2F40449&comment_hash=921f1f27436abc08f40e6779461edc57677ed96084c99e47f980b8ed5f38938d&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=921f1f27436abc08f40e6779461edc57677ed96084c99e47f980b8ed5f38938d&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts:
##########
@@ -161,10 +161,43 @@ export const allEventHandlers = (
selectedValues,
coltypeMapping,
formData,
+ onDrillDown,
} = transformedProps;
+
+ // When a drill-down hierarchy is configured, left-click drills instead of
+ // emitting a cross-filter. The DrillDownHost provides onDrillDown only when
+ // a hierarchy exists.
+ const hasDrillHierarchy = !!onDrillDown;
+
+ const drillDownClickHandler =
+ hasDrillHierarchy && groupby.length > 0
+ ? (e: { name: string }) => {
+ const values = labelMap[e.name];
+ if (!values) return;
+ const drillFilters: BinaryQueryObjectFilterClause[] = [];
+ groupby.forEach((dimension, i) => {
+ drillFilters.push({
+ col: dimension,
+ op: '==',
+ val: values[i],
+ formattedVal: formatSeriesName(values[i], {
+ timeFormatter: getTimeFormatter(formData.dateFormat),
+ numberFormatter: getNumberFormatter(formData.numberFormat),
+ coltype: coltypeMapping?.[getColumnLabel(dimension)],
+ }),
+ });
+ });
+ const label = drillFilters
+ .map(f => f.formattedVal ?? String(f.val))
+ .join(', ');
+ onDrillDown(drillFilters, label);
+ }
+ : undefined;
+
const eventHandlers: EventHandlers = {
- click:
- groupby.length > 0
+ click: drillDownClickHandler
Review Comment:
**Suggestion:** When `onDrillDown` is present, the click path bypasses `clickEventHandler` entirely, so no cross-filter data mask is emitted for Pie/Funnel/Gauge/Radar/BoxPlot clicks. This breaks dashboard synchronization during drill-down. Emit `setDataMask(getCrossFilterDataMask(...).dataMask)` inside the drill-down click handler (guarded by `emitCrossFilters`) before/alongside calling `onDrillDown`. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Pie/Funnel/Gauge/Radar/BoxPlot clicks stop cross-filtering.
- ⚠️ Dashboards lose synchronized filtering during drill-down.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Non-timeseries ECharts plugins like Pie, Funnel, Gauge, Radar, and BoxPlot all use the
shared `allEventHandlers` helper: for example `EchartsPie` at
`superset-frontend/plugins/plugin-chart-echarts/src/Pie/EchartsPie.tsx:23-28` computes
`const eventHandlers = allEventHandlers(props);` and passes them to `<Echart>`, as do
`EchartsFunnel`, `EchartsGauge`, `EchartsRadar`, and `EchartsBoxPlot`.
2. Each plugin's `transformProps` forwards drilldown and cross-filter hooks: e.g. Pie's
`transformProps` at
`superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts:124-139,259-243`
destructures `const { setDataMask = () => {}, onContextMenu, onDrillDown } = hooks;` and
returns `setDataMask`, `emitCrossFilters`, `labelMap`, `groupby`, `selectedValues`, and
`onDrillDown` in the transformed props (lines 229-243).
3. The shared `allEventHandlers` implementation at
`superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts:152-195` reads
`onDrillDown` and computes `hasDrillHierarchy = !!onDrillDown` at line 170; when
`hasDrillHierarchy && groupby.length > 0`, it defines `drillDownClickHandler` that builds
`drillFilters` from `groupby`/`labelMap` and calls `onDrillDown(drillFilters, label)`
(lines 172-193), but does not call `getCrossFilterDataMask` or `setDataMask`.
4. Because `eventHandlers.click` is defined at `eventHandlers.ts:197-206` as `click:
drillDownClickHandler ? drillDownClickHandler : groupby.length > 0 ?
clickEventHandler(...) : () => {};`, whenever a drill hierarchy is configured (so
`drillDownClickHandler` is truthy), clicks on Pie/Funnel/Gauge/Radar/BoxPlot charts go
exclusively through `drillDownClickHandler`; the `clickEventHandler` branch that emits
cross-filter data masks is bypassed, so `setDataMask` is never invoked on click and other
dashboard charts no longer receive cross-filters during drill-down, contradicting the
intended "emit cross-filter alongside drill-down" behavior described in the PR.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d3113fe14f324520871463289d4e54d6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d3113fe14f324520871463289d4e54d6&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/utils/eventHandlers.ts
**Line:** 198:206
**Comment:**
*Logic Error: When `onDrillDown` is present, the click path bypasses `clickEventHandler` entirely, so no cross-filter data mask is emitted for Pie/Funnel/Gauge/Radar/BoxPlot clicks. This breaks dashboard synchronization during drill-down. Emit `setDataMask(getCrossFilterDataMask(...).dataMask)` inside the drill-down click handler (guarded by `emitCrossFilters`) before/alongside calling `onDrillDown`.
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%2F40449&comment_hash=4716717e958ba1839b37429d234ce93e594f3b2a3a4de9ec9413fc3287121190&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=4716717e958ba1839b37429d234ce93e594f3b2a3a4de9ec9413fc3287121190&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4595104607
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40449?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `32.73543%` with `150 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 64.12%. Comparing base ([`c73106b`](https://app.codecov.io/gh/apache/superset/commit/c73106b7a285f54dc9fc80826881c42bbf26c17d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`df61640`](https://app.codecov.io/gh/apache/superset/commit/df616409d3585ba76f4aaf14d8e1f21f8684e904?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 67 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40449?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...rc/components/Chart/DrillDown/useDrillDownState.ts](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fcomponents%2FChart%2FDrillDown%2FuseDrillDownState.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnQvRHJpbGxEb3duL3VzZURyaWxsRG93blN0YXRlLnRz) | 40.20% | [58 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...components/Chart/DrillDown/DrillDownBreadcrumb.tsx](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fcomponents%2FChart%2FDrillDown%2FDrillDownBreadcrumb.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnQvRHJpbGxEb3duL0RyaWxsRG93bkJyZWFkY3J1bWIudHN4) | 0.00% | [25 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...chart-echarts/src/Timeseries/EchartsTimeseries.tsx](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2FTimeseries%2FEchartsTimeseries.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy9FY2hhcnRzVGltZXNlcmllcy50c3g=) | 8.00% | [23 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...d/src/components/Chart/DrillDown/DrillDownHost.tsx](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fcomponents%2FChart%2FDrillDown%2FDrillDownHost.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnQvRHJpbGxEb3duL0RyaWxsRG93bkhvc3QudHN4) | 43.90% | [23 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ns/plugin-chart-echarts/src/utils/eventHandlers.ts](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2Futils%2FeventHandlers.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvdXRpbHMvZXZlbnRIYW5kbGVycy50cw==) | 0.00% | [18 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...nd/packages/superset-ui-core/src/query/getXAxis.ts](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&filepath=superset-frontend%2Fpackages%2Fsuperset-ui-core%2Fsrc%2Fquery%2FgetXAxis.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvZ2V0WEF4aXMudHM=) | 77.77% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...et-frontend/src/components/Chart/ChartRenderer.tsx](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fcomponents%2FChart%2FChartRenderer.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnQvQ2hhcnRSZW5kZXJlci50c3g=) | 50.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40449?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
:x: Your project check has failed because the head coverage (99.95%) is below the target coverage (100.00%). You can increase the head coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #40449 +/- ##
==========================================
- Coverage 64.17% 64.12% -0.06%
==========================================
Files 2592 2595 +3
Lines 139299 139513 +214
Branches 32347 32418 +71
==========================================
+ Hits 89395 89459 +64
- Misses 48367 48517 +150
Partials 1537 1537
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/40449/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/40449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.23% <32.73%> (-0.11%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/40449?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4597867605
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3338097303
##########
superset-frontend/src/components/Chart/DrillDown/DrillDownBreadcrumb.test.tsx:
##########
@@ -0,0 +1,143 @@
+/**
+ * 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 { render, screen, fireEvent } from 'spec/helpers/testing-library';
+import { DrillDownBreadcrumb } from './DrillDownBreadcrumb';
+import { DrillDownLevel } from './types';
+
+const hierarchy = ['country', 'region', 'city'];
+
+const drillStack: DrillDownLevel[] = [
+ {
+ column: 'country',
+ filters: [{ col: 'country', op: '==', val: 'USA' }],
+ label: 'USA',
+ },
+ {
+ column: 'region',
+ filters: [{ col: 'region', op: '==', val: 'Texas' }],
+ label: 'Texas',
+ },
+];
+
+test('breadcrumb returns null when drillStack is empty and no selectedLeaf', () => {
+ const { container } = render(
+ <DrillDownBreadcrumb
+ hierarchy={hierarchy}
+ drillStack={[]}
+ onJumpTo={jest.fn()}
+ />,
+ );
+
+ expect(container.firstChild).toBeNull();
+});
+
+test('breadcrumb renders hierarchy[0] and drill labels', () => {
+ render(
+ <DrillDownBreadcrumb
+ hierarchy={hierarchy}
+ drillStack={drillStack}
+ onJumpTo={jest.fn()}
+ />,
+ );
+
+ // The root column name should be rendered
+ expect(screen.getByText('country')).toBeInTheDocument();
+ // Drill labels should be rendered
+ expect(screen.getByText('USA')).toBeInTheDocument();
+ expect(screen.getByText('Texas')).toBeInTheDocument();
+});
+
+test('clicking a breadcrumb segment calls onJumpTo with correct depth', () => {
+ const onJumpTo = jest.fn();
+
+ render(
+ <DrillDownBreadcrumb
+ hierarchy={hierarchy}
+ drillStack={drillStack}
+ onJumpTo={onJumpTo}
+ />,
+ );
+
+ // Click the root segment (hierarchy[0]) — should call onJumpTo(0)
+ fireEvent.click(screen.getByText('country'));
+ expect(onJumpTo).toHaveBeenCalledWith(0);
+
+ // Click the first drill label 'USA' — should call onJumpTo(1)
+ fireEvent.click(screen.getByText('USA'));
+ expect(onJumpTo).toHaveBeenCalledWith(1);
+});
+
+test('keyboard Enter on breadcrumb calls onJumpTo', () => {
+ const onJumpTo = jest.fn();
+
+ render(
+ <DrillDownBreadcrumb
+ hierarchy={hierarchy}
+ drillStack={drillStack}
+ onJumpTo={onJumpTo}
+ />,
+ );
+
+ // Press Enter on the root segment
+ fireEvent.keyDown(screen.getByText('country'), { key: 'Enter' });
+ expect(onJumpTo).toHaveBeenCalledWith(0);
+
+ // Press Enter on the 'USA' segment
+ fireEvent.keyDown(screen.getByText('USA'), { key: 'Enter' });
+ expect(onJumpTo).toHaveBeenCalledWith(1);
+});
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete keyboard accessibility coverage</b></div>
<div id="fix">
The keyboard test only covers Enter key but the component also handles Space key (DrillDownBreadcrumb.tsx:79,104). Add Space key coverage for accessibility compliance.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset-frontend/src/components/Chart/DrillDown/DrillDownBreadcrumb.test.tsx
+++ b/superset-frontend/src/components/Chart/DrillDown/DrillDownBreadcrumb.test.tsx
@@ -96,9 +96,15 @@ test('keyboard Enter on breadcrumb calls onJumpTo', () => {
// Press Enter on the root segment
fireEvent.keyDown(screen.getByText('country'), { key: 'Enter' });
expect(onJumpTo).toHaveBeenCalledWith(0);
+ onJumpTo.mockClear();
// Press Enter on the 'USA' segment
fireEvent.keyDown(screen.getByText('USA'), { key: 'Enter' });
expect(onJumpTo).toHaveBeenCalledWith(1);
+
+ // Press Space on the root segment - should also trigger onJumpTo
+ fireEvent.keyDown(screen.getByText('country'), { key: ' ' });
+ expect(onJumpTo).toHaveBeenCalledWith(0);
});
```
</div>
</details>
</div>
<small><i>Code Review Run #88ec4d</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/components/Chart/DrillDown/DrillDownHost.test.tsx:
##########
@@ -0,0 +1,161 @@
+/**
+ * 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 { ReactElement } from 'react';
+import { render as rtlRender, screen } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { ThemeProvider, supersetTheme } from '@apache-superset/core/theme';
+import { QueryFormData } from '@superset-ui/core';
+import { DrillDownHost } from './DrillDownHost';
+import type { ChartRendererProps } from '../ChartRenderer';
+
+function render(ui: ReactElement) {
+ return rtlRender(ui, {
+ wrapper: ({ children }) => (
+ <ThemeProvider theme={supersetTheme}>{children}</ThemeProvider>
+ ),
+ });
+}
+
+jest.mock('src/components/Chart/chartAction', () => ({
+ getChartDataRequest: jest.fn(() =>
+ Promise.resolve({ response: {}, json: { result: [{ data: [] }] } }),
+ ),
+ handleChartDataResponse: jest.fn(() =>
+ Promise.resolve([{ data: [{ region: 'Texas' }] }]),
+ ),
+}));
+
+jest.mock('src/explore/exploreUtils', () => ({
+ getQuerySettings: jest.fn(() => [false]),
+}));
+
+jest.mock('src/utils/simpleFilterToAdhoc', () => ({
+ simpleFilterToAdhoc: jest.fn(filter => ({
+ expressionType: 'SIMPLE',
+ clause: 'WHERE',
+ operator: filter.op,
+ subject: filter.col,
+ comparator: filter.val,
+ })),
+}));
+
+// A simple mock ChartRenderer component that renders its props for inspection
+function MockChartRenderer(props: ChartRendererProps & { onDrillDown?: unknown }) {
+ return (
+ <div data-test="mock-chart-renderer">
+ <span data-test="has-on-drill-down">
+ {props.onDrillDown ? 'yes' : 'no'}
+ </span>
+ <span data-test="form-data-x-axis">
+ {JSON.stringify((props.formData as Record<string, unknown>).x_axis)}
+ </span>
+ </div>
+ );
+}
+
+const baseFormData: QueryFormData = {
+ datasource: '1__table',
+ viz_type: 'echarts_timeseries_bar',
+ slice_id: 42,
+ x_axis: 'country',
+ groupby: [],
+ adhoc_filters: [],
+};
+
+const baseRendererProps: ChartRendererProps = {
+ formData: baseFormData,
+ vizType: 'echarts_timeseries_bar',
+ chartId: 1,
+ height: 400,
+ width: 600,
+ queriesResponse: [{ data: [{ country: 'USA' }] }],
+ actions: {} as ChartRendererProps['actions'],
+};
+
+test('DrillDownHost passes through to ChartRenderer when no hierarchy', () => {
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ />,
+ );
+
+ // Should render the chart
+ expect(screen.getByTestId('mock-chart-renderer')).toBeInTheDocument();
+ // No drill-down host wrapper when no hierarchy
+ expect(screen.queryByTestId('drill-down-host')).not.toBeInTheDocument();
+ // onDrillDown should not be provided
+ expect(screen.getByTestId('has-on-drill-down')).toHaveTextContent('no');
+});
+
+test('DrillDownHost shows breadcrumb wrapper when hierarchy exists', () => {
+ const formDataWithHierarchy: QueryFormData = {
+ ...baseFormData,
+ x_axis: ['country', 'region', 'city'],
+ };
+
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ formData={formDataWithHierarchy}
+ />,
+ );
+
+ // Should render the drill-down host wrapper
+ expect(screen.getByTestId('drill-down-host')).toBeInTheDocument();
+ // Chart should still render
+ expect(screen.getByTestId('mock-chart-renderer')).toBeInTheDocument();
+});
+
+test('onDrillDown is provided when hierarchy exists', () => {
+ const formDataWithHierarchy: QueryFormData = {
+ ...baseFormData,
+ x_axis: ['country', 'region', 'city'],
+ };
+
+ render(
+ <DrillDownHost
+ ChartRendererComponent={MockChartRenderer as any}
+ {...baseRendererProps}
+ formData={formDataWithHierarchy}
+ />,
+ );
+
+ expect(screen.getByTestId('has-on-drill-down')).toHaveTextContent('yes');
+});
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete test assertions</b></div>
<div id="fix">
Per rule [6262], tests should verify actual business logic, not just rendering. Tests at lines 91-142 only check that elements exist or props are present (`toBeInTheDocument()`, `toHaveTextContent('yes')`). They don't verify that drilling into a level correctly updates formData or that effectiveQueriesResponse is computed properly.
</div>
</div>
<small><i>Code Review Run #88ec4d</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/components/Chart/DrillDown/useDrillDownState.test.ts:
##########
@@ -0,0 +1,321 @@
+/**
+ * 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 { act, renderHook, waitFor } from '@testing-library/react';
+import { QueryFormData } from '@superset-ui/core';
+import { useDrillDownState } from './useDrillDownState';
+
+jest.mock('src/components/Chart/chartAction', () => ({
+ getChartDataRequest: jest.fn(() =>
+ Promise.resolve({ response: {}, json: { result: [{ data: [] }] } }),
+ ),
+ handleChartDataResponse: jest.fn(() =>
+ Promise.resolve([{ data: [{ col1: 'val1' }] }]),
+ ),
+}));
+
+jest.mock('src/explore/exploreUtils', () => ({
+ getQuerySettings: jest.fn(() => [false]),
+}));
+
+jest.mock('src/utils/simpleFilterToAdhoc', () => ({
+ simpleFilterToAdhoc: jest.fn(filter => ({
+ expressionType: 'SIMPLE',
+ clause: 'WHERE',
+ operator: filter.op,
+ subject: filter.col,
+ comparator: filter.val,
+ })),
+}));
+
+const baseFormData: QueryFormData = {
+ datasource: '1__table',
+ viz_type: 'echarts_timeseries_bar',
+ slice_id: 42,
+ x_axis: 'country',
+ groupby: [],
+ adhoc_filters: [],
+};
+
+test('hierarchy is empty when x_axis is a single string with no drilldown_hierarchy', () => {
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData: baseFormData,
+ baseQueriesResponse: [{ data: [] }],
+ }),
+ );
+
+ expect(result.current.hierarchy).toEqual([]);
+ expect(result.current.hasHierarchy).toBe(false);
+});
+
+test('hierarchy is built from x_axis array with 2+ items', () => {
+ const formData = {
+ ...baseFormData,
+ x_axis: ['country', 'region', 'city'],
+ };
+
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData,
+ baseQueriesResponse: [{ data: [] }],
+ }),
+ );
+
+ expect(result.current.hierarchy).toEqual(['country', 'region', 'city']);
+ expect(result.current.hasHierarchy).toBe(true);
+});
+
+test('hierarchy is built from drilldown_hierarchy field', () => {
+ const formData = {
+ ...baseFormData,
+ x_axis: 'country',
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ };
+
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData,
+ baseQueriesResponse: [{ data: [] }],
+ }),
+ );
+
+ expect(result.current.hierarchy).toEqual(['country', 'region', 'city']);
+ expect(result.current.hasHierarchy).toBe(true);
+});
+
+test('drillDown() pushes a level onto the stack', async () => {
+ const formData = {
+ ...baseFormData,
+ x_axis: ['country', 'region', 'city'],
+ };
+
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData,
+ baseQueriesResponse: [{ data: [] }],
+ }),
+ );
+
+ act(() => {
+ result.current.drillDown(
+ [{ col: 'country', op: '==', val: 'USA' }],
+ 'USA',
+ );
+ });
+
+ expect(result.current.drillStack).toHaveLength(1);
+ expect(result.current.drillStack[0]).toEqual({
+ column: 'country',
+ filters: [{ col: 'country', op: '==', val: 'USA' }],
+ label: 'USA',
+ });
+ expect(result.current.isDrilling).toBe(true);
+});
+
+test('drillDown() at last level sets selectedLeaf instead of pushing', () => {
+ const formData = {
+ ...baseFormData,
+ x_axis: ['country', 'region'],
+ };
+
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData,
+ baseQueriesResponse: [{ data: [] }],
+ }),
+ );
+
+ // Drill to depth 1 (only one more level available: 'region')
+ act(() => {
+ result.current.drillDown(
+ [{ col: 'country', op: '==', val: 'USA' }],
+ 'USA',
+ );
+ });
+
+ expect(result.current.drillStack).toHaveLength(1);
+
+ // Now we're at the last level; next drill should set selectedLeaf
+ act(() => {
+ result.current.drillDown(
+ [{ col: 'region', op: '==', val: 'Texas' }],
+ 'Texas',
+ );
+ });
+
+ expect(result.current.drillStack).toHaveLength(1);
+ expect(result.current.selectedLeaf).toBe('Texas');
+});
+
+test('resetTo(0) clears the stack', () => {
+ const formData = {
+ ...baseFormData,
+ x_axis: ['country', 'region', 'city'],
+ };
+
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData,
+ baseQueriesResponse: [{ data: [] }],
+ }),
+ );
+
+ act(() => {
+ result.current.drillDown(
+ [{ col: 'country', op: '==', val: 'USA' }],
+ 'USA',
+ );
+ });
+
+ expect(result.current.drillStack).toHaveLength(1);
+
+ act(() => {
+ result.current.resetTo(0);
+ });
+
+ expect(result.current.drillStack).toHaveLength(0);
+ expect(result.current.isDrilling).toBe(false);
+ expect(result.current.selectedLeaf).toBeUndefined();
+});
+
+test('effectiveFormData swaps x_axis when drilling', () => {
+ const formData = {
+ ...baseFormData,
+ x_axis: ['country', 'region', 'city'],
+ groupby: [],
+ };
+
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData,
+ baseQueriesResponse: [{ data: [] }],
+ }),
+ );
+
+ act(() => {
+ result.current.drillDown(
+ [{ col: 'country', op: '==', val: 'USA' }],
+ 'USA',
+ );
+ });
+
+ // After drilling one level, the effective x_axis should be 'region'
+ const effective = result.current.effectiveFormData as Record<string, unknown>;
+ expect(effective.x_axis).toBe('region');
+});
+
+test('effectiveFormData adds adhoc_filters from drill stack', () => {
+ const formData = {
+ ...baseFormData,
+ x_axis: ['country', 'region', 'city'],
+ groupby: [],
+ adhoc_filters: [{ expressionType: 'SIMPLE' as const, clause: 'WHERE' as const, subject: 'year', operator: '==' as const, comparator: '2023' }],
+ };
+
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData,
+ baseQueriesResponse: [{ data: [] }],
+ }),
+ );
+
+ act(() => {
+ result.current.drillDown(
+ [{ col: 'country', op: '==', val: 'USA' }],
+ 'USA',
+ );
+ });
+
+ const effective = result.current.effectiveFormData as Record<string, unknown>;
+ const adhocFilters = effective.adhoc_filters as unknown[];
+
+ // Should contain the original filter plus the drill filter
+ expect(adhocFilters).toHaveLength(2);
+ // The first should be the original filter
+ expect(adhocFilters[0]).toEqual(
+ expect.objectContaining({ subject: 'year' }),
+ );
+ // The second should be generated from the drill filter via simpleFilterToAdhoc
+ expect(adhocFilters[1]).toEqual(
+ expect.objectContaining({ subject: 'country', operator: '==', comparator: 'USA' }),
+ );
+});
+
+test('effectiveFormData swaps groupby when chart uses groupby', () => {
+ const formData = {
+ ...baseFormData,
+ x_axis: undefined,
+ groupby: ['country'],
+ drilldown_hierarchy: ['country', 'region', 'city'],
+ adhoc_filters: [],
+ };
+
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData,
+ baseQueriesResponse: [{ data: [] }],
+ }),
+ );
+
+ act(() => {
+ result.current.drillDown(
+ [{ col: 'country', op: '==', val: 'USA' }],
+ 'USA',
+ );
+ });
+
+ const effective = result.current.effectiveFormData as Record<string, unknown>;
+ expect(effective.groupby).toEqual(['region']);
+});
+
+test('fetches data when drilling and returns it as effectiveQueriesResponse', async () => {
+ const formData = {
+ ...baseFormData,
+ x_axis: ['country', 'region', 'city'],
+ groupby: [],
+ adhoc_filters: [],
+ };
+
+ const { result } = renderHook(() =>
+ useDrillDownState({
+ formData,
+ baseQueriesResponse: [{ data: [{ country: 'USA' }] }],
+ }),
+ );
+
+ // Before drilling, effectiveQueriesResponse is the base data
+ expect(result.current.effectiveQueriesResponse).toEqual([
+ { data: [{ country: 'USA' }] },
+ ]);
+
+ act(() => {
+ result.current.drillDown(
+ [{ col: 'country', op: '==', val: 'USA' }],
+ 'USA',
+ );
+ });
+
+ await waitFor(() => {
+ expect(result.current.isLoading).toBe(false);
+ });
+
+ expect(result.current.effectiveQueriesResponse).toEqual([
+ { data: [{ col1: 'val1' }] },
+ ]);
+});
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Add tests for uncovered paths</b></div>
<div id="fix">
Coverage gap: `reset()` (useDrillDownState.ts:258-261), `error` state on fetch failure, `hasMoreLevels` property, single-item `x_axis` array edge case, and formData-change reset are all untested. Add tests to cover these code paths per rule [6262].
</div>
</div>
<small><i>Code Review Run #88ec4d</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4622529059
<!-- translation-regression-bot -->
## ⚠️ Translation Regression Detected
A source change in this PR renamed or reworded strings, invalidating existing translations (they are now `#, fuzzy`) in `ar`, `ca`, `cs`, `de`, `fi`, `fr`, `it`, `ja`, `ko`, `lv`, `mi`, `nl`, `pt`, `ru`, `sk`, `sl`, `th`, `uk`. Please resolve the affected `.po` files before merging.
_Note: intentionally **deleting** a translatable string is not a regression and is not flagged here — only translations invalidated by a renamed/reworded source string are._
| Language | Fuzzy before | Fuzzy after | New |
|----------|-------------:|------------:|----:|
| `ar` | 1136 | 1137 | +1 |
| `ca` | 815 | 816 | +1 |
| `cs` | 787 | 788 | +1 |
| `de` | 997 | 998 | +1 |
| `fi` | 4824 | 4825 | +1 |
| `fr` | 1014 | 1015 | +1 |
| `it` | 1667 | 1668 | +1 |
| `ja` | 322 | 323 | +1 |
| `ko` | 1521 | 1522 | +1 |
| `lv` | 295 | 296 | +1 |
| `mi` | 807 | 808 | +1 |
| `nl` | 1143 | 1144 | +1 |
| `pt` | 1839 | 1840 | +1 |
| `ru` | 293 | 294 | +1 |
| `sk` | 774 | 775 | +1 |
| `sl` | 963 | 964 | +1 |
| `th` | 4824 | 4825 | +1 |
| `uk` | 293 | 294 | +1 |
### How to fix
**1. Install dependencies** (if not already set up):
```bash
pip install -r superset/translations/requirements.txt
sudo apt-get install gettext # or: brew install gettext
```
**2. Re-extract strings and sync `.po` files:**
```bash
./scripts/translations/babel_update.sh
```
This rewrites `superset/translations/messages.pot` from the current source files and merges the changes into every `.po` file. Strings whose `msgid` changed will be marked `#, fuzzy`.
**3. Resolve the fuzzy entries** in the affected language files (`ar`, `ca`, `cs`, `de`, `fi`, `fr`, `it`, `ja`, `ko`, `lv`, `mi`, `nl`, `pt`, `ru`, `sk`, `sl`, `th`, `uk`):
```bash
grep -n '#, fuzzy' superset/translations/<lang>/LC_MESSAGES/messages.po
```
For each fuzzy entry, either rewrite the `msgstr` to match the new string and remove the `#, fuzzy` line, or clear the `msgstr` to `""` if you cannot provide a translation.
**4. Commit your changes to the `.po` files.**
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4546897424
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4793027836
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3469835699
##########
superset-frontend/packages/superset-ui-core/src/components/Table/cell-renderers/ActionCell/fixtures.ts:
##########
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { action } from 'storybook/actions';
+import { action } from '@storybook/addon-actions';
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Wrong import path for Storybook v10</b></div>
<div id="fix">
The `import` on line 19 uses `@storybook/addon-actions`, which was deprecated and removed in Storybook v10. Change to `storybook/actions` to match the correct v10+ import path and the pattern used across 6 other story files in the codebase.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset-frontend/packages/superset-ui-core/src/components/Table/cell-renderers/ActionCell/fixtures.ts
+++ b/superset-frontend/packages/superset-ui-core/src/components/Table/cell-renderers/ActionCell/fixtures.ts
@@ -16,4 +16,4 @@
* under the License.
*/
- import { action } from '@storybook/addon-actions';
+ import { action } from 'storybook/actions';
import { ActionMenuItem } from './index';
```
</div>
</details>
</div>
<small><i>Code Review Run #b6ed0f</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: New feature tk drilldown hierarchy [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40449:
URL: https://github.com/apache/superset/pull/40449#issuecomment-4671567770
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org