feat(layout): converge inline pipeline — multicol whole-in-column InlineFlow (slice 4 / I-multicol)#291
Merged
Merged
Conversation
…ineFlow (slice 4 / I-multicol) A multicol (`Column`) run that is whole in its column (skip_lines == 0, no fragment break) now persists an `InlineFlow`, fixing multi-line inline text in multi-column layout (legacy's single-linear-pass mis-rendered it). The column shift converges onto block layout's canonical `shift_descendants`, which already moves a persisted `InlineFlow` — so the converged column's inline text shifts to the column offset instead of repainting at column 0. - Gate (inline/mod.rs): split into a pre-pack `persist_candidate` (drives `flow_align`, optimistically includes `Column`) and a post-pack `persist_flow` refined to whole-in-column (`column_is_whole = skip_lines == 0 && break_after_line.is_none()`). Mid-IFC column break stays on legacy, deferred to the standalone fragment tree (Z) with box fragments (G11: one box/flow per entity — the column shift moves a run-start's whole subtree by one delta). - Multicol (lib.rs): delete the duplicate `shift_entity_and_descendants` (+ its `MAX_DESCENDANT_DEPTH`); `position_column_fragments` calls `elidex_layout_block::block::shift_descendants` (One-issue-one-way — a single subtree shifter, now InlineFlow-aware for columns too). No accumulate (whole-in-column = length-1 Vec) and no probe-gating flag: with no accumulate the final fill pass's `insert_one`-replace + unconditional `clear_inline_flows` deterministically overwrite/clear every balanced-fill probe flow. ecs + render unchanged. 7 new tests (4 gate unit + 3 multicol integration). cap-neutral. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…refinement form - inline/mod.rs: update the `fragmentation_type` field docstring (was stale at the I-paged "paged-scoped, Column gated until I-multicol" wording — this PR is that I-multicol; now documents whole-in-column Column persistence + mid-IFC→Z). - inline/mod.rs: express `persist_flow` as the refinement it is — `persist_candidate && (!frag_is_column || column_is_whole)` — instead of repeating the full disjunction (logically identical; makes the candidate→refine relation visible). Trim the duplicated `flow_align`-safety restatement. - multicol/lib.rs: trim the shifter comment's re-explanation of `shift_descendants` internals (they live at the callee docstring); keep the one-issue-one-way reuse rationale + the before/after correctness note. No behavior change (gate tests + 43 multicol + 303 layout-block all green; clippy clean). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR advances the render↔layout inline-pipeline convergence for multi-column (Column-fragmented) layout by persisting InlineFlow for whole-in-column inline formatting contexts and ensuring the column offset shift moves the persisted flow (not just LayoutBoxes). This addresses incorrect rendering of multi-line inline text in multicol when using the converged path.
Changes:
- Refines the
InlineFlowpersist gate to allow Column fragmentation only when the run is whole in its column (skip_lines == 0and no fragment break), while keeping mid-IFC column breaks on legacy. - Converges multicol column-shifting on the canonical
elidex_layout_block::block::shift_descendants, which shifts bothLayoutBoxand persistedInlineFlow. - Adds multicol integration tests covering flow persistence, probe overwrite-safety in balanced fill, and correct flow shifting to column offsets.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/layout/elidex-layout-multicol/src/tests.rs | Adds integration tests validating whole-in-column InlineFlow persistence and correct flow shifting with column offsets. |
| crates/layout/elidex-layout-multicol/src/lib.rs | Replaces the local LayoutBox-only shifter with the canonical shift_descendants to shift both boxes and persisted flows. |
| crates/layout/elidex-layout-block/src/inline/tests/inline_flow.rs | Updates/expands unit tests to cover multicol whole-in-column persistence and exclusion of continuation/truncation cases. |
| crates/layout/elidex-layout-block/src/inline/mod.rs | Splits the persist gate into pre-pack persist_candidate and post-pack refined persist_flow for whole-in-column multicol runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- `flow_inline_start` (multicol tests): assert single-line + non-empty runs before indexing `lines[0].runs[0]`, so a future regression fails with a clear message instead of an unhelpful out-of-bounds panic. - `multicol_column_run_shifted_to_column_offset`: tighten the column-1 assertion from `b_x > 250.0` to `(b_x - 300.0).abs() < 1.0` (and column 0 to `~0`) — the exact column offset (width 300, gap 0), so wrong offset math (e.g. 280) can't pass. Test-only. 43 multicol + clippy green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
send
added a commit
that referenced
this pull request
Jun 6, 2026
…2), not drop (#292) Balanced fill no longer drops overflow children: `column-count` is a balance target, not a hard layout cap. The definitive pass is uncapped (`u32::MAX`) so surplus content spills into overflow columns (CSS Multicol L1 §8.2), overwriting each child's column-0 height-probe `InlineFlow` (stranded as a ghost post-#291) with its real per-column flow; the search probe caps at `count + 1` (minimal feasibility cap, fixes the degenerate search). Adds a balance + small max-height regression test asserting flows land at their real column offsets. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
send
added a commit
that referenced
this pull request
Jun 6, 2026
… lines) (#293) `tests.rs` grew past the 1000-line convention (1214 lines after the #291 I-multicol whole-in-column tests + the #292 balanced-overflow regression). Split the single `mod tests` into a `tests/` directory module, matching the sibling layout crates (flex/grid/table `tests/mod.rs` house style): - mod.rs — shared imports + helpers + crate-root `#![allow(unused_must_use)]` - is_multicol.rs — `is_multicol` predicate tests (6) - geometry.rs — basic layout / geometry / width / gap / child positioning (17) - spanner.rs — `column-span` / spanner segmentation (7) - fill.rs — `column-fill` (auto/balance) + overflow columns (2) - inline_flow.rs — I-multicol `InlineFlow` persistence + column shift (4) Pure test-organization refactor: test bodies moved byte-for-byte, no production code change. Shared helpers live in the parent `mod tests`; submodules `use super::*`. The crate-root `#![allow(unused_must_use)]` cascades to every submodule. 36 tests preserved (identical names); 44/44 crate tests pass; clippy clean; `mise run ci` green. Each file is now ≤ 509 lines. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Summary
Slice 4 / I-multicol of the render↔layout inline-pipeline convergence (follows #282 I-paged). A multi-column (
Column-fragmented) run that is whole in its column now persists anInlineFlow, fixing multi-line inline text in multicol (legacy's single-linear-pass mis-rendered it), and the column shift now moves the persisted flow — not just theLayoutBox.Two production edits:
elidex-layout-block/src/inline/mod.rs) — split into a pre-packpersist_candidate(drivesflow_align, optimistically includesColumn) and a post-packpersist_flowrefined to whole-in-column:column_is_whole = skip_lines == 0 && break_after_line.is_none(). The optimistic candidate is safe becauseflow_aligngates onlyflow_linespopulation, neverentity_bounds/box geometry (the packer commits those unconditionally).elidex-layout-multicol/src/lib.rs) — delete the duplicateshift_entity_and_descendants(+MAX_DESCENDANT_DEPTH);position_column_fragmentscallselidex_layout_block::block::shift_descendants, the canonical project-wide subtree shifter, which already moves a persistedInlineFlow. One-issue-one-way: a single shifter, not a second InlineFlow-aware copy.Scope (whole-in-column; mid-IFC column break → deferred to Z), forced by G11. A paragraph that spans a column boundary needs ≥2 fragments at ≥2 column offsets, but the column shift moves a run-start's whole subtree by one delta and the containing box is already last-column-wins (
fill.rs:117, one box/flow per entity). So mid-IFC column break stays on legacy (no regression — broken today too) and converges with box fragments later, in the standalone fragment tree (Z), rather than shipping an inconsistent text-right/box-wrong partial fix.No accumulate (whole-in-column ⇒ length-1
InlineFlow.fragments) and no probe-gating flag: with no accumulate, the final balanced-fill pass'sinsert_one-replace + the unconditionalclear_inline_flowsdeterministically overwrite/clear every probe flow.elidex-ecs+ render are unchanged.Tests (7, cap-neutral)
elidex-layout-block):multicol_whole_run_now_persists(+ asserts byte-identical geometry vs the non-fragmented reference),gate_excludes_multicol_continuation,gate_excludes_multicol_truncation,multicol_nested_in_paged_stamps_page_generation.elidex-layout-multicol, reallayout_multicoldispatcher):multicol_column_run_shifted_to_column_offset(the shift crux — flow moves with its box to the column offset),multicol_balanced_persists_one_fragment_per_run(probe overwrite-safety),multicol_spanner_ifc_persists_nonfragmented.shift_descendants' own responsibility, tested inelidex-layout-block; multicol's vertical box shift is tested byvertical_rl_columns/vertical_lr_columns) — see the NOTE inmulticol/src/tests.rs.Verification
cargo fmt+mise run cigreen (3-OS check + clippy + nextest + doc + deny) · 303 layout-block + 43 multicol + 131 render (no churn) + 105 shell (no golden churn — the shifter convergence is behavior-neutral for all existing fixtures). Pre-push:/code-review(0 bugs) ·/elidex-review5-axis (0 CRIT / 0 IMP) ·/simplify+/review(docstring-drift + persist_flow refinement-form + comment trims applied).🤖 Generated with Claude Code