fix: compute position stacking per-facet-panel instead of globally

[cpsievert]

Summary

Fixes #244.

Position stacking (cumulative sums) was computed globally across all facet panels, causing bars in later panels to "float" above 0 by stacking on top of cumulative values from earlier panels.

Before: Male panel bars start where female panel bars ended:

After: Each panel stacks independently from 0:

Query:

SELECT * FROM ggsql:penguins WHERE sex IS NOT NULL
VISUALISE island AS x, species AS fill
DRAW bar
FACET sex

Changes

• src/plot/layer/position/stack.rs: The .over() partition for cum_sum and sum now includes facet columns (extracted from layer.partition_by) in addition to the group column. This ensures stacking resets within each facet panel. Applies to all three stacking modes: Normal, Fill, and Center.
• src/execute/position.rs: New test test_stack_resets_per_facet_panel verifies that stacking starts from 0 in each facet panel.

Root cause

In apply_stack, the cumulative sum used .over([col(&group_col)]) — partitioning only by the x-axis position. Facet columns were not included, so the cumsum accumulated across all panels as if they were one.

Test plan

• New test test_stack_resets_per_facet_panel passes
• All 1221 existing tests pass
• No compiler warnings
• End-to-end verified with penguins dataset: all facet panels start from pos2end=0

🤖 Generated with Claude Code

[thomasp85]

Can you update this to always add facet variables to the grouping when stacking.

It might be that it makes sense to add these to partition_by early so it is there be default for both stat and position handling

[cpsievert]

add facet variables to the grouping when stacking

Done in ff2622f— stacking now always reads facet variables from spec.facet.layout.internal_facet_names() instead of filtering partition_by for facet-prefixed columns.

add these to partition_by early so it is there be default for both stat and position handling

Claude considered that but went with reading from spec.facet directly because stacking can't use all of partition_by uniformly. partition_by includes fill/color columns (via add_discrete_columns_to_partition_by), and stacking needs to stack across those groups, not within them. So the stacking over_cols need to be [group_col, facet_cols] specifically — not all of partition_by. Reading from spec.facet makes this intent explicit without needing to re-filter partition_by for facet columns.

(Facet columns do already end up in partition_by via add_discrete_columns_to_partition_by, which stat transforms consume through group_by — so that path is already covered.)

[cpsievert]

The latest commit (7c5011d) extends the facet-panel fix to dodge and jitter.

The problem: compute_group_indices() in dodge/jitter was called with all of layer.partition_by, which includes facet columns (added by add_discrete_columns_to_partition_by). This creates composite groups across facet panels — e.g., fill=["X","Y"] × facet=["F1","F2"] produces 4 groups instead of the correct 2 per panel. For dodge, this halves adjusted_width (0.225 instead of 0.45), making bars too narrow.

The fix: Added a shared non_facet_partition_cols() helper in position/mod.rs that filters facet columns out of partition_by using spec.facet.layout.internal_facet_names(). Both dodge and jitter call this before compute_group_indices().

This is the same pattern as the stacking fix (ff2622f) — position adjustments read facet info from spec.facet rather than relying on naming conventions in partition_by. Filed #254 to track the dodge/jitter issue separately.