apache / apache/superset

Legacy Partition chart: icicle x-position algorithm mispositions descendants under unevenly-branched hierarchies (fresh repro for #10586)

Open
#43,727 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
74.8k
Forks
18.3k
Avg merge
2d 5h
Merged PRs (30d)
685

Description

### Bug description

The legacy Partition (icicle) chart's `init()` layout function (`superset-frontend/plugins/legacy-plugin-chart-partition/src/Partition.ts`) mispositions descendant nodes whenever the hierarchy branches unevenly — e.g. a category missing data for the deepest configured dimension while a sibling category has it. When that happens, descendant nodes render **outside their true parent's band**, so sibling categories visually bleed into each other, especially when zooming into a segment. See "Root cause" below for the exact mechanism.

This is the same symptom reported in #10586 ("Partition chart mixes up results") back in 2020, confirmed still present through versions up to 3.x in that thread, and closed as stale without a fix. A maintainer asked there for someone to file a fresh issue with current repro steps and root-cause detail — this is that issue.

We hit this on 6.1.0 after upgrading specifically to pick up the #32042 fix (PR #32290); it's a distinct, independent bug from #32042.

### 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`:

```sql
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
```

The key detail: category `B`'s only subcategory (`B1`) has no `sub_subcategory` value, so that branch terminates at depth 2 (this happens naturally whenever a combination doesn't exist in the underlying data — pandas' groupby drops rows with a NaN in a grouping column by default). Category `A`'s branch goes the full 3 levels deep. `B` also has to sort before `A` for the bug to surface (the chart's tie-break sorts descending-by-name for equal values, which is why `B`/`A` are named that way here rather than the reverse) — with these exact values it reproduces every time.

This is the JSON shape that SQL produces from `PartitionViz.get_payload()` (the frontend bug is fully reproducible from this JSON alone, independent of any backend):

```json
[
{
"name": "sum__val",
"val": 4,
"children": [
{ "name": "B", "val": 2, "children": [
{ "name": ["B", "B1"], "val": 2, "children": [] }
]},
{ "name": "A", "val": 2, "children": [
{ "name": ["A", "A1"], "val": 2, "children": [
{ "name": ["A", "A1", "A1a"], "val": 1, "children": [] },
{ "name": ["A", "A1", "A1b"], "val": 1, "children": [] }
]}
]}
]
}
]
```

Running `Partition.ts`'s `init()` on this tree produces:

| node | computed x range | parent's true x range | correct? |
|---|---|---|---|
| `B` | `[0, 0.5]` | `[0, 1]` | ✅ |
| `A` | `[0.5, 1.0]` | `[0, 1]` | ✅ |
| `["B","B1"]` | `[0, 0.5]` | `[0, 0.5]` | ✅ |
| `["A","A1"]` | `[0.5, 1.0]` | `[0.5, 1.0]` | ✅ |
| `["A","A1","A1b"]` | **`[0, 0.25]`** | `[0.5, 1.0]` | ❌ — rendered on top of `B`'s region |
| `["A","A1","A1a"]` | **`[0.25, 0.5]`** | `[0.5, 1.0]` | ❌ — rendered on top of `B`'s region |

`A1a`/`A1b` are computed to occupy `[0, 0.5]` — squarely inside `B`'s band, not their real parent `A1`'s band at `[0.5, 1.0]`. Visually, `A`'s deepest-level children appear to sit under `B`.

Screenshot from a real Superset instance (6.1.0), built from the exact SQL above (this instance already has a separate, unrelated label-array bug patched, filed as #43728):

Image

*Labels correctly read `A1a`/`A1b`, but the rectangles render under `B`'s column instead of `A`'s.*

After applying the suggested fix below:

Image

*Same chart, same data, with the suggested fix applied: `A1a`/`A1b` now render correctly nested under `A`/`A1`'s column.*

We also confirmed this against a real production dataset with a 4-dimension Partition chart (12,907 total nodes across the tree): **350 nodes** were positioned outside their true parent's bounds by the existing algorithm.

### Expected results

Every node's `[x, x+dx]` range should be fully contained within its parent's `[x, x+dx]` range, regardless of how unevenly the tree branches.

### Actual results

Nodes past an early-terminating sibling branch get positioned using a stale cumulative offset instead of their real parent's position, escaping their parent's band entirely.

### Root cause

`node.each()` (used by `init()`) walks the tree breadth-first, computing each node's `x` position with a single shared "previous node" pointer and a depth-equality heuristic:

```ts
n.x = prev.depth === n.parent.depth ? 0 : prev.x + prev.dx;
```

The `x` for a new depth's *very first* node is set to `0`, which is only correct because that first node is always a descendant of "whatever branch was visited first" one level up — **provided every branch reaches every depth**. If an earlier-visited branch terminates early (no children at some depth), the *actual* first node encountered at a deeper depth belongs to a *different, non-first* branch, but still gets the hard-coded `x = 0` reset — silently correct-looking math applied to the wrong anchor.

### Suggested fix

Replace the shared "previous node" heuristic with a per-parent running offset (e.g. a `Map`), so every child's `x` is always computed as `parent.x + offset-of-prior-siblings-within-that-parent` — correct regardless of tree shape, independent of traversal order quirks:

```ts
const offsets = new Map();
root.each((n: PartitionNode) => {
n.y = dy * n.depth;
n.dy = dy;
if (n.parent) {
const offset = offsets.get(n.parent) || 0;
n.x = n.parent.x + offset;
n.dx = (n.weight / n.parent.sum) * n.parent.dx;
offsets.set(n.parent, offset + n.dx);
} else {
n.x = 0;
n.dx = 1;
}
flat.push(n);
});
```

We verified this fix produces zero containment violations both on the minimal example above and on the 12,907-node real dataset mentioned earlier (executed directly in Node, not simulated).

We already have this patched locally (applied as a build-time transform on the compiled bundle, since we can't easily run a full frontend rebuild against our deployment) and are happy to turn it into a proper PR against `Partition.ts` if that's useful — let us know.

### Superset version

6.1.0 (also present in 5.0.0, 6.0.0, and per the linked #10586 thread, versions back to 2020)

### Python version

3.12

### Node version

Not applicable (frontend-only bug)

### Browser

Chrome

### Additional context

Related: #10586 (original report, went stale), #32042 / PR #32290 (a different, already-fixed backend bug in the same chart that we upgraded to pick up before noticing this one). Also see #43728 for a label-rendering regression in this same chart.

### Checklist

- [x] I have searched Superset docs and Slack and didn't find a solution to my problem.
- [x] I have searched the GitHub issue tracker and didn't find a similar **open** bug report (the closest match, #10586, was closed as stale).

Contributor guide

Open the contributing guide

Research direction

Start in superset-frontend/plugins/legacy-plugin-chart-partition/src/Partition.ts and read the Partition chart's init() layout function. Run the minimal JSON tree from the issue through the layout, then verify that every node's x range remains within its parent's range, including the uneven B/A hierarchy and the reported 12,907-node case.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
data-visualization, frontend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.