Skip to content

fix(sheet): apply AUTO header merge context on the right axis - #1202

Open
Sean-Walker0 wants to merge 1 commit into
apache:mainfrom
Sean-Walker0:fix/auto-header-merge-context
Open

Sean-Walker0 wants to merge 1 commit into
apache:mainfrom
Sean-Walker0:fix/auto-header-merge-context

Conversation

@Sean-Walker0

Copy link
Copy Markdown
Contributor

Purpose of the pull request

The AUTO header merge strategy validates merge context on the wrong axis in ExcelWriteHeadProperty.headCellRangeList, producing two wrong merges:

  1. The case reported in [Bug] Merge error in dynamic header #666 still merges. Writing that issue's exact dynamic head on current main still produces the C2:D2 merged region it complains about: same-name cells (head31/head31) sitting in columns under different parents (head30 vs head40) are joined horizontally, because the horizontal run has no context check at all. feat: introduce header merge strategy for Excel writing #674 added canMergeVertically for this report, but that only guards the vertical path.
  2. Equal cells below the first header row no longer merge vertically. The AUTO-only canMergeVertically compares the cell above a vertical run with a cell inside the run (row1-1 vs row2-1 where row2-1 is a run member). A vertical run starting at row >= 1 can only exist when the cell above differs from it (otherwise the run was already consumed from the earlier row), so the comparison is provably always false there. AUTO consequently never merges equal cells under the same parent below row 0 — a regression against the pre-strategy automatic merge and against VERTICAL_ONLY, which both merge them.

Found by code audit of the merge algorithm; both effects verified end-to-end before this change (issue #666's head yields C2:D2; head columns [G,A,A] [G,B,y] [G,B,z] yield no A2:A3).

What's changed?

  • Under AUTO, a horizontal run may only extend across cells whose parent labels (the cell above each column, row j-1) are equal — this is the context check the horizontal path was missing, and it fixes the [Bug] Merge error in dynamic header #666 case (head30 vs head40 differ, no merge) while keeping same-parent runs merged (B/B under a merged G run still merge).
  • The wrong-axis canMergeVertically check is removed from the vertical path (the method became dead code), restoring automatic vertical merging of equal cells under the same parent (A2:A3 merges again).

VERTICAL_ONLY, HORIZONTAL_ONLY and FULL_RECTANGLE are untouched.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Verifying this change

AutoHeaderMergeContextTest (both directions):

  • autoDoesNotMergeSameNameCellsUnderDifferentParents — the [Bug] Merge error in dynamic header #666 head; red before (same-name cells under different parents must not merge ==> expected: <false> but was: <true>), green after; also pins that first-row runs (head40/head40) still merge;

  • autoMergesVerticalRunsBelowTheFirstHeaderRow — red before (equal cells under the same parent must merge vertically ==> expected: <true> but was: <false>), green after; also pins same-parent horizontal merges (B/B under G).

  • Existing HeaderMergeStrategyTest: all 5 cases still pass.

  • ./mvnw clean package -B -Dmaven.test.skip=false -pl fesod-common,fesod-shaded,fesod-sheet: 950 tests, 0 failures, 0 errors.

  • ./mvnw -pl fesod-sheet spotless:check: clean.

Notes

@Sean-Walker0
Sean-Walker0 force-pushed the fix/auto-header-merge-context branch 2 times, most recently from 87ea95b to 2f07984 Compare October 14, 2026 12:13
The AUTO merge strategy in ExcelWriteHeadProperty validates merge
context on the wrong axis, with two visible effects:

- Horizontal runs still join same-name cells that live under
  different parent groups. This is what issue apache#666 reported for a
  dynamic head (head30/head31 next to head40/head31): the reported
  C2:D2 merge still happens on current main.
- The AUTO-only canMergeVertically check compares the cell above a
  vertical run with a cell inside the run. A vertical run starting at
  row >= 1 only exists when the cell above differs from the run (an
  equal cell above would have consumed the run from the earlier row),
  so the comparison is provably false there and AUTO never merges
  equal cells below the first header row anymore - a regression
  against both the pre-strategy automatic merge and VERTICAL_ONLY.

Fix both by validating context where it belongs: under AUTO, a
horizontal run may only join cells whose parent labels (the cell
above each column) are equal, and vertical runs no longer carry the
wrong-axis check, so equal cells under the same parent merge again.

Verified with AutoHeaderMergeContextTest (both cases red before the
change, green after) plus the full fesod-sheet suite (950 tests) and
spotless:check.
@Sean-Walker0
Sean-Walker0 force-pushed the fix/auto-header-merge-context branch from 2f07984 to ea07bde Compare October 15, 2026 06:45

This branch has not been deployed

No deployments
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.

1 participant