codeant-ai-for-open-source[bot] commented on code in PR #31477:
URL: https://github.com/apache/superset/pull/31477#discussion_r3465073114
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:
##########
@@ -251,6 +252,7 @@ export default function transformProps(
columnLabels,
metricLabel,
secondaryMetricLabel,
+ !showNullValues,
Review Comment:
**Suggestion:** `showNullValues` is read without a default and then
inverted. For saved/legacy charts where this new control is absent from
`formData`, the value is `undefined`, so `!showNullValues` becomes `true` and
null nodes are filtered unexpectedly. Default this field to `true` when
destructuring to preserve existing behavior. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Legacy sunburst charts unexpectedly hide null hierarchy entries.
- ⚠️ Users see different data after upgrade without changing controls.
- ⚠️ Tooltips and totals exclude rows previously visible as <Null>.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In
`superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:158-191`,
note that `transformProps()` destructures `showNullValues` from `formData`
without a
default: `const { ..., showTotal, showNullValues, sliceId } = formData;`
(line 189).
2. In the same file at lines 250-255, `treeBuilder` is called with
`!showNullValues` as
the fifth argument: `const treeData = treeBuilder(data, columnLabels,
metricLabel,
secondaryMetricLabel, !showNullValues);`, so when `showNullValues` is
`undefined`,
`filterNullNames` becomes `true`.
3. In
`superset-frontend/plugins/plugin-chart-echarts/src/utils/treeBuilder.ts:34-40`
and
`62-82`, `treeBuilder()` receives `filterNullNames` and, when it is `true`,
filters
children via `const validChildren = filterNullNames ? children.filter(child
=> child.name
!== null) : children;`, so passing `true` hides null-name nodes.
4. Call `transformProps()` with a `chartProps` object whose `formData` omits
`showNullValues` (mimicking a legacy saved Sunburst chart created before
`show_null_values` existed): `showNullValues` evaluates to `undefined`,
`!showNullValues`
becomes `true`, and null hierarchy entries are filtered out even though the
new UI control
in
`superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/controlPanel.tsx:97-104`
defaults `show_null_values` to `true` (meaning "show nulls"), resulting in
opposite
behavior for such charts.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=89413eebd6794507a7cc7df5efcb2c5d&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=89413eebd6794507a7cc7df5efcb2c5d&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:** 189:255
**Comment:**
*Api Mismatch: `showNullValues` is read without a default and then
inverted. For saved/legacy charts where this new control is absent from
`formData`, the value is `undefined`, so `!showNullValues` becomes `true` and
null nodes are filtered unexpectedly. Default this field to `true` when
destructuring to preserve existing 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%2F31477&comment_hash=e67468edf7cb3eaeb877c63da3421f7a0789d0128df1121fac0ce2f4ad9f881c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F31477&comment_hash=e67468edf7cb3eaeb877c63da3421f7a0789d0128df1121fac0ce2f4ad9f881c&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/treeBuilder.ts:
##########
@@ -74,9 +76,12 @@ export function treeBuilder(
0,
)
: metricValue;
+ const validChildren = filterNullNames
+ ? children.filter(child => child.name !== null)
+ : children;
Review Comment:
**Suggestion:** The null filtering only removes children in the recursive
branch, so it does not filter null names when the hierarchy has a single level
(or at the root node level with no parent to filter it). This leaves `<Null>`
entries visible in valid input shapes. Apply filtering to the current level's
result as well, not only to `children`. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Sunburst with one hierarchy still shows <Null> nodes.
- ⚠️ Top-level null categories remain despite "hide nulls" enabled.
- ⚠️ Users see inconsistent null handling across hierarchy levels.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In
`superset-frontend/plugins/plugin-chart-echarts/src/utils/treeBuilder.ts:34-40`,
`treeBuilder()` accepts `filterNullNames?: boolean` and in the multi-level
branch (lines
62-88) computes `children = treeBuilder(...)` and then `const validChildren =
filterNullNames ? children.filter(child => child.name !== null) : children;`.
2. In the single-level branch at lines 47-60 (`if (!restGroupby.length)`),
the function
pushes leaf items with `name` taken from `curData[key][0][curGroupBy]!`
without consulting
`filterNullNames` or applying any filter, so names equal to `null` at this
level are never
removed.
3. In the multi-level branch at lines 82-88, the code pushes the parent node
as
`result.push({ name, children: validChildren, value: metricValue,
secondaryValue, groupBy:
curGroupBy });` while never filtering out the parent itself when `name` is
`null`, only
its `children`.
4. From
`superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:249-255`,
when a user disables nulls via the new control (`showNullValues` false) the
call
`treeBuilder(..., !showNullValues)` passes `filterNullNames = true`; with
data where the
hierarchy has a single level or top-level `columns` entries containing
`null`, the
resulting tree still includes nodes whose `name` is `null`, confirming that
root/single-level `<Null>` entries are not filtered.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a3333897a9fc4ff4b6601ada2fb12135&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=a3333897a9fc4ff4b6601ada2fb12135&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/treeBuilder.ts
**Line:** 79:81
**Comment:**
*Incomplete Implementation: The null filtering only removes children in
the recursive branch, so it does not filter null names when the hierarchy has a
single level (or at the root node level with no parent to filter it). This
leaves `<Null>` entries visible in valid input shapes. Apply filtering to the
current level's result as well, not only to `children`.
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%2F31477&comment_hash=9b254dd48d5e8fedf6c170ea0ef4b8edb26d0f270813f31d67d0681959b6a7cb&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F31477&comment_hash=9b254dd48d5e8fedf6c170ea0ef4b8edb26d0f270813f31d67d0681959b6a7cb&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
