refactor(columns): resolved column-geometry foundation (SD-2629)#3633
Merged
Conversation
balanceSectionOnPage skipped every section with equalWidth=false plus explicit widths, so continuous newspaper sections declared as <w:cols w:num=N w:equalWidth=0> with equal <w:col w:w> children (the common case) never balanced and rendered single-column. Narrow the skip to GENUINELY-unequal widths: explicit widths that are all equal now balance like implicit equal columns. Genuinely-unequal widths still fill column-by-column (Word parity, unchanged). (SD-2324)
Per ECMA-376 §17.6.4, when columns are not equal width (w:equalWidth=0) the section-level w:cols/@w:space is ignored and the inter-column gap comes from each <w:col w:space>. extractColumns used the section space, over-spacing explicit columns so their widths scaled down to fit and diverged from Word (e.g. the 2002 ISDA sections). Use the per-column w:space for unequal columns; equal-width columns keep the section space. Advances SD-2629 for the uniform-spacing case. (SD-2324)
…ith-explicit-column
…2324) Equal-width sections (w:equalWidth="1" or omitted) now match Word: extraction drops child <w:col> widths and takes the gap from the section w:space (default 720), and normalizeColumnLayout honours per-column widths only when w:equalWidth="0". For explicit columns (w:equalWidth="0"), cap the count to min(w:num, valid child-width count) at the source, so a w:num larger than the provided <w:col> widths no longer creates surplus 1px phantom columns in the fill loop (which reads the raw count). A matching clamp in normalizeColumnLayout stays as a defensive net.
…ent w:cols (SD-2324) Adds three extraction unit tests for the landed column fix: count caps to the valid child-width count (four <w:col> but two usable w:w -> 2), equal mode takes the count from w:num (count 3, no children), and a section without <w:cols> yields no columnsPx.
…ndation) Additive, no behavior change: introduces `gaps?: number[]` (inter-column gaps, length count-1) on the ColumnLayout contract and teaches cloneColumnLayout to carry it. No producer emits it and no consumer reads it yet; geometry helpers, extraction, consumer migration, and the no-scale/per-gap semantic flip follow in subsequent steps (see tmp/SD-2629-state.md).
Additive, behavior-preserving. Adds ColumnGeometry and getColumnGeometry() (the resolved consumer API: content-relative x, width, gapAfter, separatorX), plus getColumnX/getColumnWidth/getColumnGapAfter/getColumnSeparatorPositions/ getColumnAtX (originX-explicit) and columnLayoutsEqual (gaps-aware). Geometry mirrors today's normalized widths + scalar gap; raw per-column `gaps` do not drive it until the step-4 semantic flip. normalizeColumnLayout output shape is unchanged, so no consumer or existing test is affected.
…lity (SD-2629) Barrel was missing the geometry helpers and columnLayoutsEqual, so the API added in 583b63e was unreachable for consumers. resolveColumnMode unifies the explicit/equal decision (explicit iff equalWidth===false AND usable widths) shared by extraction, normalization, and geometry. gaps added to every column equality/cache site in-place (behavior-preserving: gaps is undefined until extraction emits it in step 2).
…ecords (SD-2629)
Parse <w:col> as {width, space} records so a dropped or zero-width
column never desyncs spacing from its column, then slice widths and
gaps to the resolved count (gaps.length === count-1; the last column's
space is never a gap). Per-column w:col/@space defaults to 0 per
ECMA-376 §17.6.3, not parseColumnGap's 720tw section default. The
scalar gap is unchanged (first-gap fallback). This activates the
step-1 equality/cache sites, which now compare per-column gaps.
…raw w:space child (SD-2629) The scalar gap (the single-gap fallback read by consumers not yet on geometry) was taken from the first raw <w:col> declaring a w:space, which can be a dropped/zero-width column or a later one. Per the record model it must be the first VALID column's own space (=== gaps[0]). This removes the scalar-vs-gaps divergence before consumers migrate. Changes no existing test (every current explicit test has column 0's space equal to the first declared space); only the divergent case differs, now correct. Test added.
…at resolved count (SD-2629)
advanceColumn read the raw w:num while normalizeColumnLayout read the
widths-clamped count, so a direct layoutDocument({count:4, widths:[a,b]})
filled four columns while width math assumed two. Add resolveColumnCount
as the one count authority (min(num, usable widths)); normalize and the
paginator fill loop both read it. advanceColumn resolves from the same
state-aware columns it already fetches, so per-state config is honored.
The DOCX path is already capped at source by extraction; this guards
direct engine callers. Failing test added first (fragment x positions).
…lumnCount (SD-2629) The column break handler advanced into phantom columns on an explicit <w:br w:type="column"> the same way advanceColumn did (read raw w:num). Route it plus the balancing gates, the region force-page check, and the page/document column-metadata gates through resolveColumnCount, so every count DECISION uses the single authority. Behavior-preserving for the DOCX path (extraction caps count at source); hardens direct callers. Column-break test added. The change-detection comparisons stay raw for now (resolved-equality pass is separate).
…mns (SD-2629) page.columns and layout.columns gated on the resolved count but still cloned the raw config, so a direct count:4 / widths:[192,384] layout advertised columns.count===4 while rendering two. Add resolveColumnLayout (count clamped to resolveColumnCount; explicit widths/gaps sliced to that count; not scaled - that stays normalize's job) and use it for both metadata sites. Behavior-identical for the DOCX path (extraction caps at source) and every count<=widths case; only the direct count>widths metadata changes, toward what actually renders. Tests added.
…gion metadata (SD-2629) Two gaps in the metadata pass: (1) resolveColumnLayout sliced but kept widths/gaps in equal mode, yet the DOM painter treats any columns.widths as explicit, so equal-mode metadata carrying stray widths would misrender separators - now dropped when resolveColumnMode is not explicit. (2) page.columnRegions serialized raw region columns, and the renderer prefers columnRegions over page.columns, so a mid-page continuous-break region could still advertise phantom columns - now resolved at serialization. Tests added for both.
…idth reads them (SD-2629) Foundation for the columnX/width migration. getColumnGeometryForState derives geometry from a state's OWN columns + page size + margins, not the global latest-section values, so positioning an older page uses that page's geometry instead of whatever section is currently active. getCurrentColumnWidth now reads columnWidthForState. Behavior-preserving for the latest state (the common path) and constant margins (all tests); more correct once section margins/page size vary. The columnX re-derivation in placement helpers and the cross-package consumers migrate next, then the old paginator.columnX wrapper is removed.
…l active (SD-2629) getColumnGeometryForState fetched columns via getActiveColumnsForState, which falls back to the global latest-section columns when no mid-page boundary is active - so an older page would be positioned with whatever section is currently active, not its own. Now it uses the active boundary if one applies, else the page's creation-time snapshot (page.columns, the resolved metadata createPage stamps). Geometry-equivalent to the global for the latest state (the only state with consumers today), and correct for older pages, which the cross-package consumers will query next. createPage sets page.columns + margins at creation, so both are available during fill.
…ForState (SD-2629) The placement helpers (paragraph/drawing/image/table) now take columnX(state, index?) instead of columnX(index), and call it with their live state; the index.ts columnX alias points to columnXForState, which derives x from the page's own resolved geometry. columnWidth stays a fixed number (the step-4 boundary for per-span unequal widths). Behavior-preserving: for the latest state (the only state placement positions) columnXForState equals the old paginator.columnX. The old paginator.columnX is now unused by placement; it is removed in the final cleanup once the cross-package consumers migrate too.
…olumnX(state) (SD-2629) Two review cleanups before the renderer-separator migration. (1) The image relative-from-column width still read getColumnWidthAt of getCurrentColumns(); now columnWidthForState(state), behavior-preserving for the latest state and closing the same stale-state gap. (2) The two layout-drawing test mocks used columnIndex, but the helper now calls columnX(state), so they computed NaN at runtime; updated to take state. The constant table/paragraph mocks (() => 0) ignore args and stay correct. Note: the package build does not type-gate .test.ts (verified empirically), so vitest is what covers mock correctness.
…ived x math (SD-2629) getColumnSeparatorPositions now builds geometry via getColumnGeometry(normalizeColumnLayout(columns, contentWidth)) and reads the positions from the contracts getColumnSeparatorPositions(geometry, leftMargin), dropping the duplicate equal/explicit x math. Verified behavior-preserving: both old branches produce positions identical to the geometry, and the <= 1 width guard is kept (geometry.some(width <= 1)); the content-past-separator gate and the caller's withSeparator/count>1 gates are unchanged. Reads page.columns/columnRegions (resolved metadata), not raw activeColumns, so the count is already safe. Painter boundary preserved (contracts only). Isolated painter --noEmit typecheck hits pre-existing TS6305 (unbuilt dep dists), unrelated to this change; CI tsc -b is the gate.
…geometry migration (SD-2629) The geometry migration lost the legacy equal-mode skip: when the gap overflows the content area, availableWidth goes negative and normalize falls back to full-content-width columns (floored at 1), so the geometry width never trips the <=1 guard and phantom separators would draw outside the content area. Restore the pre-geometry equalWidth<=1 check for equal mode (explicit mode already matched via the geometry width guard). Added a renderer test (count:3, huge gap, fragments past the phantom positions so only the guard can suppress) to pin it.
… widths presence (SD-2629) The equal-mode narrow-column separator guard keyed on the presence of a widths array, which misclassifies a raw equalWidth:true config carrying stray widths as explicit and skips the guard. Key it on resolveColumnMode(columns) === 'equal' so such a config still takes the equalWidth<=1 skip. Behavior-identical for layout-produced metadata (equal-mode widths are already dropped) and all current tests; this hardens the raw-input path a consumer might pass.
…ns option (SD-2629) Placement now reads columnXForState, the renderer reads geometry, and footnotes/balancing keep their own local columnX, so paginator.columnX and the getCurrentColumns paginator option that only it consumed are dead. Remove both. index.ts's local getCurrentColumns stays (still used for page/document metadata and image width reads). Final behavior- preserving 3b-ii cleanup; layout-engine tsc clean.
…-2629) Unused after dropping the getCurrentColumns option (its only consumer); nothing imports it from paginator (layout-drawing imports its own from layout-image). Pure dead-code removal.
…t raw fields (SD-2629) Add columnRenderLayoutsEqual: a canonical render form (resolved mode + count, scalar gap, withSeparator, and explicit-mode sliced widths/gaps; ignores raw equalWidth and the surplus count/widths resolution discards). The engine region/cache sites (isColumnsChanging, isColumnConfigChanging, the normalized-columns cache key) now use it + resolveColumnCount for the reset-to-single check, so configs that render identically (num:4/widths [a,b] vs num:2/widths[a,b]; equalWidth:true vs omitted) no longer split into separate regions. ~Zero production impact (extraction caps at source). breaks.ts/signaturesEqual (adapter, re-exported, no internal caller) is left for a separate structural-vs-render classification. Tests added.
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
tupizz
approved these changes
Jun 5, 2026
Base automatically changed from
tadeu/sd-2324-feature-render-multi-columns-with-explicit-column
to
main
June 5, 2026 12:31
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
|
🎉 This PR is included in superdoc-cli v0.16.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc-sdk v1.15.0 |
Contributor
|
🎉 This PR is included in @superdoc-dev/mcp v0.11.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc v1.39.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in @superdoc-dev/react v1.10.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in vscode-ext v2.11.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unifies column handling onto one resolved model so every consumer agrees on how many columns exist and where each sits, instead of a fill path and a width path that could disagree. This is the behavior-preserving foundation for SD-2629 and does not change rendering; the visual fixes for unequal explicit widths and per-column gaps stack on top.
resolveColumnCount=min(num, usable widths)): fill, column breaks, balancing, and page/region metadata all resolve it the same way, so a section declaring more columns than it supplies widths no longer advances into phantom columns or advertises them.page.columns,layout.columns,columnRegions) is resolved so it never reports more columns than render.Verified:
check:typesclean repo-wide; a 484-doc layout-corpus comparison against the base shows zero geometry changes (only nondeterministic snapshot IDs and the newgapsmetadata differ).