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

[fparodimoraes]

Bug description

PR #32290 (fix for #32042) changed PartitionViz.nest_values (superset/viz.py) to return name as an array of the full ancestor path for any node at depth ≥ 2 (e.g. ["category_val", "subcategory_val", "sub_subcategory_val"]), instead of a plain string leaf label:

return [
    {
        "name": [*dims, i],   # was previously just `i`
        "val": dim_level[i],
        "children": self.nest_values(levels, level + 1, metric, dims + [i]),
    }
    for i in ...
]

That change was necessary and correct on the backend side, but the frontend renderer, superset-frontend/plugins/legacy-plugin-chart-partition/src/Partition.ts, was never updated to match. It still treats node.name as a plain string everywhere it's consumed:

n.name = n.data.name;          // now sometimes an array
...
root.sort((a, b) => {
  ...
  return b.name > a.name ? 1 : -1;   // array > array does string coercion
});
...
`<td>${n.name}</td>`           // tooltip cell — array gets Array.prototype.toString()'d
...
.text(d => d.disp ? `${d.name}: ${d.disp}` : d.name)   // on-chart segment label
...
d.color = colorFn(d.name, sliceId);   // categorical color key

Since JS silently stringifies an array via Array.prototype.toString() (comma-joined) wherever it's interpolated into a template literal, every node at depth ≥ 2 renders its entire ancestor chain instead of just its own value.

How to reproduce the bug

Minimal SQL reproduction (works against any engine — pure literals, no real table needed). Register this as a virtual dataset (SQL Lab -> "Save as dataset"), then build a Partition Chart on it 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

Look at the 3rd-level segments (children of A1).

Screenshot from a real Superset instance (6.1.0), built from the exact SQL above — note the positioning here is correct (this instance already has a separate, unrelated layout bug patched, filed as #43727), but the labels read the full ancestor path:

After applying the suggested fix below, the same chart, same data:

Expected results

A segment for sub_subcategory = "A1a" (under category = "A", subcategory = "A1") should be labeled A1a.

Actual results

The segment is labeled A,A1,A1a — the full comma-joined ancestor path. With more dimensions this gets long, repetitive, and at a glance looks like categories are duplicated or misattributed (it's easy to mistake this for the chart mis-nesting categories, since parent names appear to "leak" into every descendant's own label).

It also affects sibling sort order (comparing joined-path strings instead of each node's own value) and the color key (nodes are now colored by their full path rather than their own value, an unintended side effect of the same regression).

Root cause

PartitionDataNode.name is typed as string in Partition.ts, but the backend contract changed to sometimes send string[]. No consumer of node.name in this file was updated to handle both shapes.

Suggested fix

Normalize name to its own leaf value immediately after it's read from data, before the sort/tooltip/label/color code runs — a single-point fix that corrects all four consumers at once:

root.eachAfter(n => {
  n.disp = n.data.val;
  n.value = n.disp < 0 ? -n.disp : n.disp;
  n.weight = n.value;
  n.name = n.data.name;
  // ... existing date-node handling, unchanged ...
});
root.each(n => {
  if (Array.isArray(n.name)) n.name = n.name[n.name.length - 1];
});
root.sort((a, b) => { /* unchanged, now compares plain strings again */ });

We verified this normalization against a real 12,907-node production tree: zero arrays remained after the fix, and every node's displayed name matched the correct leaf value from the original backend payload.

We already have this patched locally (build-time transform on the compiled bundle) and are happy to turn it into a proper PR against Partition.ts if useful.

Superset version

6.1.0 (present in any version that includes PR #32290, i.e. 5.0.0+)

Python version

3.12

Node version

Not applicable (frontend-only bug)

Browser

Chrome

Additional context

Related: #32042 / PR #32290, which introduced this regression as a side effect of an otherwise-correct backend fix. Separately, we also found and reported an unrelated, pre-existing layout/positioning bug in the same chart (#43727) that produces a superficially similar "categories look mixed up" symptom — worth checking both if you're touching this component.

Checklist

• I have searched Superset docs and Slack and didn't find a solution to my problem.
• I have searched the GitHub issue tracker and didn't find a similar bug report.

[dosubot]

Hi @fparodimoraes! I'm Dosu and I'm helping the Apache Superset team.

Your analysis is thorough and lines up with the code. I can confirm the frontend renderer Partition.js (note: .js, not .ts as mentioned in the issue) treats n.name as a plain string in every consumer you identified:

• n.name = n.data.name; in the eachAfter pass [1]
• The sort comparator b.name > a.name ? 1 : -1 [2]
• The rich-tooltip cell <td>${n.name}</td> [3]
• The on-chart segment label d.disp ? ${d.name}: ${d.disp} : d.name [4]
• The categorical color key colorFn(d.name, sliceId) [5]

The PropTypes also declare name: PropTypes.string for both leaf and parent shapes [6], confirming the file was never updated to expect arrays.

Your proposed fix — adding an Array.isArray guard to extract the last element right after n.name = n.data.name and before any of the sort/tooltip/label/color consumers run — looks correct and appropriately minimal. It's a single normalization point that fixes all four consumers at once, and it won't conflict with the hasDateNode date-formatting logic that also mutates n.name later in the same eachAfter block [7].

One additional note: PR #41729 recently migrated the partition chart to the v1 chart data API by adding buildQuery.ts, transformData.ts, and updating transformProps.ts — but the actual rendering file Partition.js was left untouched. That PR's transformData.ts re-implements nest_values on the frontend side and appears to use the same dim-path-as-array convention for node names, so the same Array.isArray normalization in Partition.js would be needed regardless of whether the chart is served via the legacy or v1 API path. Worth verifying that in your PR.

Opening a PR with this fix would be very welcome — it's well-scoped and you've clearly validated it thoroughly.

To reply, just mention @dosu.

---

Docs are dead. Just use Dosu.

[fparodimoraes]

Thanks for digging into this, @dosu!

Two follow-ups:

1. Confirmed the .js vs .ts reference is just your knowledge base pointing at an older commit -- as of master, the plugin has moved (via feat(partition): migrate partition chart to v1 chart data API #41729) from legacy-plugin-chart-partition/src/Partition.ts to plugin-chart-partition/src/Partition.ts. I diffed the current file against the version this issue was originally filed against: identical except for an unrelated sanitizeHtml hardening change, so the bug and this fix apply unchanged at the new path.

2. On transformData.ts: confirmed its main aggregation path (not_time/agg_sum/agg_mean) does independently reproduce the same ancestor-path-array convention for deeper nodes, while its time-diff/factor path already extracts a plain leaf string. Since Partition.ts is the single shared renderer for both the legacy explore_json path and the new v1 API path, normalizing name there (rather than in transformData.ts) fixes both call sites with one change, and is a safe no-op wherever a plain string is already provided.

Opened #43731 against the current plugin-chart-partition/src/Partition.ts with this fix, plus updated the PartitionDataNode type and PropTypes declarations to reflect the real string | string[] contract.

[dosubot]

Good catch on both points, @fparodimoraes, and thanks for tracking down the current path. Minor clarification: #41729 was the v1 chart data API migration (adding buildQuery.ts / transformData.ts), while the actual directory rename from legacy-plugin-chart-partition to plugin-chart-partition happened in #41751 (the "drop the legacy- prefix" cleanup). Either way, the bug and fix apply unchanged at the new path, as you confirmed.

Your point 2 reasoning holds up: since Partition.ts is the single renderer shared by both the legacy explore_json payloads and the new transformData.ts v1-API output, and since the time-diff/factor path in transformData.ts already emits plain leaf strings while the sum/mean aggregation path independently reproduces the ancestor-path-array convention [1], normalizing at the renderer is the correct single-point fix rather than patching each producer separately.

I pulled up #43731: it matches everything discussed here — normalizing name to its leaf value right where it's read from data (before the sort/tooltip/label/color consumers run), plus widening PartitionDataNode and the PropTypes declarations from string to string | string[] to reflect the real contract. The repro SQL and before/after behavior (A,A1,A1a → A1a) in the PR description match your original report exactly.

This looks ready for a maintainer to review — nothing further from my side to add on the technical analysis.

To reply, just mention @dosu.

---

Docs are dead. Just use Dosu.