Skip to content

Fix stacking bug#403

Merged
teunbrand merged 1 commit intoposit-dev:mainfrom
teunbrand:fix-stack-sort-order
Apr 30, 2026
Merged

Fix stacking bug#403
teunbrand merged 1 commit intoposit-dev:mainfrom
teunbrand:fix-stack-sort-order

Conversation

@teunbrand
Copy link
Copy Markdown
Collaborator

This PR aims to fix #402.

Briefly, the partition_by is a HashMap, which has non-deterministic order. This made facets land in the wrong place in the sorting hierarchy, which led to treating every row as its own group. The fix is to put facet columns as the outer sorting columns.

The sort in apply_stack used [group_col, ...partition_by] as sort keys,
but partition_by order is non-deterministic (HashMap iteration). When
fill sorted before facet, rows from the same facet panel were
interleaved, causing compute_group_ids (which relies on adjacency) to
treat every row as its own group — making cumsum a no-op.

Fix: build the over/group columns first, use them as primary sort keys,
then append remaining partition_by columns after.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@teunbrand teunbrand marked this pull request as ready for review April 29, 2026 14:57
@teunbrand teunbrand requested a review from thomasp85 April 29, 2026 14:57
Copy link
Copy Markdown
Collaborator

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

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

Nice! LGTM

@teunbrand teunbrand merged commit 1a1c0fb into posit-dev:main Apr 30, 2026
2 checks passed
@teunbrand teunbrand deleted the fix-stack-sort-order branch April 30, 2026 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stacking bug

2 participants