Skip to content

fix(partition): normalize ancestor-path array names to leaf values - #43731

Merged
rusackas merged 1 commit into
apache:masterfrom
fparodimoraes:fix/partition-chart-name-array-labels
Sep 4, 2026
Merged

rusackas merged 1 commit into
apache:masterfrom
fparodimoraes:fix/partition-chart-name-array-labels

Conversation

@fparodimoraes

Copy link
Copy Markdown
Contributor

SUMMARY

Fixes #43728.

PartitionViz.nest_values (and transformData.ts's equivalent frontend re-implementation for the v1 chart data API) return name as an array of the full ancestor path (e.g. ["a", "a.1", "a.1.1"]) for any node below the first grouping level, instead of a plain leaf string. Partition.ts's renderer was never updated to expect this, so every consumer of node.name -- the sort comparator, tooltip cell, on-chart segment label, and categorical color key -- treated it as a plain string. JS silently stringifies an array via Array.prototype.toString() wherever it's interpolated into a template literal, so a 3rd-level segment rendered as e.g. "a,a.1,a.1.1" instead of just "a.1.1".

This fix normalizes name to its own leaf value right where it's read from data, before any of those consumers run -- a single-point fix. It also widens the PartitionDataNode type and the PropTypes declarations to reflect the real string | string[] contract, instead of leaving them declared (and asserted) as string while actually receiving arrays.

Verified against a real 4-dimension Partition chart with 12,907 nodes: 0 arrays remained after the fix, and every node's displayed name matched the correct leaf value from the original payload. See #43728 for the full analysis, screenshots, and repro SQL.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

See #43728 for before/after screenshots from a real Superset instance using the exact repro SQL below.

TESTING INSTRUCTIONS

Minimal repro (from #43728) -- register as a virtual dataset, build a Partition Chart with metric SUM(val) and Levels = category, subcategory, sub_subcategory:

SELECT 'B' AS category, 'B1' AS subcategory, NULL AS sub_subcategory, 2 AS val
UNION ALL
SELECT 'A' AS category, 'A1' AS subcategory, 'A1a' AS sub_subcategory, 1 AS val
UNION ALL
SELECT 'A' AS category, 'A1' AS subcategory, 'A1b' AS sub_subcategory, 1 AS val

Before this fix, the 3rd-level segments are labeled A,A1,A1a / A,A1,A1b. After, they read A1a / A1b.

ADDITIONAL INFORMATION

@bito-code-review

bito-code-review Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #6caacc

Actionable Suggestions - 0
Additional Suggestions - 1
  • superset-frontend/plugins/plugin-chart-partition/src/Partition.ts - 1
    • Misleading comment location · Line 37-38
      The comment says `PartitionNode.name` is normalized in `init`, but `init` (lines 78-97) only computes layout (x/dx/y/dy). The name normalization actually happens in the `eachAfter` callback at lines 250-252. Pointing maintainers at the wrong function could send them hunting in the wrong place.
Review Details
  • Files reviewed - 1 · Commit Range: 3854755..3854755
    • superset-frontend/plugins/plugin-chart-partition/src/Partition.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers an incremental AI Review.

  • /review full - 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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

PartitionViz.nest_values / transformData return `name` as an array of
the full ancestor path (e.g. ["a", "a.1", "a.1.1"]) for any node below
the first grouping level, but Partition.ts's renderer never accounted
for that -- every consumer (the sort comparator, tooltip cell, on-chart
label, and categorical color key) treated `name` as a plain string, so
JS silently stringified the array via Array.prototype.toString(),
rendering the full comma-joined path instead of the node's own value.

Normalize `name` to its own leaf value right where it's read from
`data`, before any of those consumers run. Also widens the
PartitionDataNode type and PropTypes declarations to reflect the real
`string | string[]` contract.

Fixes apache#43728

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@fparodimoraes
fparodimoraes force-pushed the fix/partition-chart-name-array-labels branch from 3854755 to f2c8434 Compare August 31, 2026 19:02
@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.24%. Comparing base (5bffceb) to head (f2c8434).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...nd/plugins/plugin-chart-partition/src/Partition.ts 0.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #43731   +/-   ##
=======================================
  Coverage   79.24%   79.24%           
=======================================
  Files        2888     2888           
  Lines      166456   166458    +2     
  Branches    38532    38534    +2     
=======================================
+ Hits       131913   131916    +3     
+ Misses      32048    32047    -1     
  Partials     2495     2495           
Flag Coverage Δ
javascript 74.74% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bito-code-review

bito-code-review Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #539ca1

Actionable Suggestions - 0
Additional Suggestions - 1
  • superset-frontend/plugins/plugin-chart-partition/src/Partition.ts - 1
    • Inaccurate comment reference · Line 38-38
      The comment says `PartitionNode.name` is normalized in `init`, but `init` (lines 78-97) only computes `y/dy/x/dx` and never sets `name`. The normalization actually happens in `drawVis`'s `root.eachAfter` (lines 250-252). Please correct the reference so maintainers look in the right place.
Review Details
  • Files reviewed - 1 · Commit Range: f2c8434..f2c8434
    • superset-frontend/plugins/plugin-chart-partition/src/Partition.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers an incremental AI Review.

  • /review full - 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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@sadpandajoe
sadpandajoe requested a review from rusackas September 1, 2026 17:09

@rusackas rusackas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@rusackas
rusackas merged commit ff90a60 into apache:master Sep 4, 2026
80 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Legacy Partition chart: node labels/tooltips render as comma-joined ancestor arrays after #32290 (regression)

2 participants