refactor(engine): extract RowSlots helper for weights/children guard (#85)

Polish pass on PR-7.3's defence-in-depth IllegalArgumentException added
in v1.6.5 (commit f868499). The guard lived inline as duplicated code
at two engine call sites — LayoutCompiler#distributeRowSlotWidths and
NodeDefinitionSupport#measureRow — with no direct test. The path is
unreachable through public API (RowNode canonical constructor blocks
the mismatched state), but unreachable code that lacks coverage rots:
the next refactor can silently delete either guard with no test going
red.

Changes:

- New com.demcha.compose.document.layout.RowSlots (package-private)
  with a single helper, validateWeightsMatchChildren(weights, n).
  Error message unchanged from PR-7.3.
- Both call sites replaced with the helper call.
- New RowSlotsTest with 4 tests: happy path, more-weights-than-children,
  fewer-weights-than-children, and a message-quality check that the
  error tells the caller how to fix it.
- GraphCompose.DocumentBuilder#pageBackgrounds(...) Javadoc now spells
  out the empty-list-clears semantics in prose, not just in @param.
  Inner comment in DocumentBuilder.create() shortened — the rationale
  moved up to the public Javadoc where it belongs.

No behaviour change. Suite: 1023 tests, 0 failures, 0 errors (~47s).

Surfaced by retrospective senior-review of PR-7.3 via the new
graphcompose-senior-review skill.

Author: DemchaAV

CHANGELOG.md

@@ -22,6 +22,20 @@ JitPack continue to resolve through the existing coordinates.
   land in `target/japicmp/`. JitPack repository is scoped to the
   `japicmp` profile, so downstream consumers do not inherit it.
 
+### Engine internals (no behaviour change)
+
+- **`RowSlots` helper extracted** from `LayoutCompiler` and
+  `NodeDefinitionSupport`. The defence-in-depth `IllegalArgumentException`
+  guard added in v1.6.5 (PR-7.3) for the row weights / children size
+  mismatch lived as duplicated inline code at both engine call sites
+  with no direct test — a future refactor could have silently deleted
+  either copy. The validation now lives in
+  `com.demcha.compose.document.layout.RowSlots#validateWeightsMatchChildren`
+  (package-private), with `RowSlotsTest` driving it directly. Error
+  message is unchanged. `GraphCompose.DocumentBuilder#pageBackgrounds(...)`
+  Javadoc now spells out the empty-list-clears semantics in prose, not
+  only in the `@param` line.
+
 ## v1.6.5 — 2026-05-30
 
 ### Templates v2

src/main/java/com/demcha/compose/GraphCompose.java

@@ -251,6 +251,11 @@ public DocumentBuilder pageBackground(java.awt.Color color) {
          * {@link com.demcha.compose.document.api.DocumentSession#pageBackgrounds}
          * for the full semantics.
          *
+         * <p>Calling this method with an empty list is an <b>explicit clear</b>:
+         * it overrides any earlier {@link #pageBackground(com.demcha.compose.document.style.DocumentColor)}
+         * call on the same builder and emits no page-background fragments.
+         * Passing {@code null} has the same effect.</p>
+         *
          * @param fills ordered fills, or {@code null}/empty to clear
          * @return this builder
          */
@@ -381,9 +386,8 @@ public DocumentSession create() {
                     markdown,
                     guideLines);
             if (pageBackgrounds != null) {
-                // Explicit pageBackgrounds() call wins — even an empty
-                // list is an intentional clear that should override any
-                // earlier pageBackground(color) on the same builder.
+                // Explicit pageBackgrounds() call wins over a prior
+                // pageBackground(color). Empty list = clear; see builder Javadoc.
                 session.pageBackgrounds(pageBackgrounds);
             } else if (pageBackground != null) {
                 session.pageBackground(pageBackground);

src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java

@@ -746,11 +746,7 @@ private static double[] distributeRowSlotWidths(List<DocumentNode> children,
             }
             return slots;
         }
-        if (weights.size() != n) {
-            throw new IllegalArgumentException(
-                    "Row weights size (" + weights.size() + ") must match children size (" + n
-                            + "). Pass exactly " + n + " weight(s) or leave weights empty for an even split.");
-        }
+        RowSlots.validateWeightsMatchChildren(weights, n);
         double total = 0.0;
         for (double w : weights) {
             total += w;

src/main/java/com/demcha/compose/document/layout/NodeDefinitionSupport.java

@@ -324,11 +324,7 @@ public static MeasureResult measureRow(RowNode node,
                 slotWidths[i] = share;
             }
         } else {
-            if (node.weights().size() != n) {
-                throw new IllegalArgumentException(
-                        "Row weights size (" + node.weights().size() + ") must match children size (" + n
-                                + "). Pass exactly " + n + " weight(s) or leave weights empty for an even split.");
-            }
+            RowSlots.validateWeightsMatchChildren(node.weights(), n);
             double total = 0.0;
             for (Double w : node.weights()) {
                 total += w;

src/main/java/com/demcha/compose/document/layout/RowSlots.java

@@ -0,0 +1,44 @@
+package com.demcha.compose.document.layout;
+
+import java.util.List;
+
+/**
+ * Shared validation helpers for row weight / children distribution code.
+ *
+ * <p>Centralises the {@link IllegalArgumentException} contract used by both
+ * {@link LayoutCompiler#distributeRowSlotWidths(List, List, double, double) compile-phase}
+ * and {@link NodeDefinitionSupport#measureRow measure-phase} row distribution.
+ * The {@code RowNode} canonical constructor already rejects a mismatched
+ * weights list at construction time; these helpers are defence-in-depth
+ * for any path that bypasses the constructor (reflection-based
+ * deserialization, framework proxies, etc.) and arrives at the engine
+ * with an inconsistent {@code (weights, children)} pair.</p>
+ *
+ * <p>Package-private intentionally — engine surface, not public API.</p>
+ *
+ * @author Artem Demchyshyn
+ */
+final class RowSlots {
+
+    private RowSlots() {
+        // Utility class, no instantiation.
+    }
+
+    /**
+     * Asserts that an explicit {@code weights} list matches the row's
+     * children count. Callers must skip this check when {@code weights}
+     * is null or empty — the even-split fallback applies there instead.
+     *
+     * @param weights non-null, non-empty weights list
+     * @param childCount number of row children
+     * @throws IllegalArgumentException if {@code weights.size() != childCount}
+     */
+    static void validateWeightsMatchChildren(List<Double> weights, int childCount) {
+        if (weights.size() != childCount) {
+            throw new IllegalArgumentException(
+                    "Row weights size (" + weights.size() + ") must match children size ("
+                            + childCount + "). Pass exactly " + childCount
+                            + " weight(s) or leave weights empty for an even split.");
+        }
+    }
+}

src/test/java/com/demcha/compose/document/layout/RowSlotsTest.java

@@ -0,0 +1,61 @@
+package com.demcha.compose.document.layout;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Engine-level contract for the row weights / children-count guard
+ * shared by {@link LayoutCompiler} and {@link NodeDefinitionSupport}.
+ *
+ * <p>The {@code RowNode} canonical constructor already rejects the
+ * mismatched state via {@link RowBuilder}, so under normal authoring the
+ * helper is unreachable. The helper exists for defence-in-depth paths
+ * that bypass the constructor (reflection-based deserialization, etc.);
+ * these tests pin the contract so a future refactor cannot silently
+ * delete the guard at either call site.</p>
+ */
+class RowSlotsTest {
+
+    @Test
+    void matchingSizesPassWithoutThrowing() {
+        assertThatCode(() -> RowSlots.validateWeightsMatchChildren(List.of(1.0, 2.0, 3.0), 3))
+                .doesNotThrowAnyException();
+    }
+
+    @Test
+    void moreWeightsThanChildrenIsRejectedWithBothSizesNamed() {
+        assertThatThrownBy(() -> RowSlots.validateWeightsMatchChildren(List.of(1.0, 2.0, 3.0), 2))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessageContaining("weights")
+                .hasMessageContaining("children")
+                .hasMessageContaining("(3)")
+                .hasMessageContaining("(2)");
+    }
+
+    @Test
+    void fewerWeightsThanChildrenIsRejectedWithBothSizesNamed() {
+        assertThatThrownBy(() -> RowSlots.validateWeightsMatchChildren(List.of(1.0, 2.0), 5))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessageContaining("(2)")
+                .hasMessageContaining("(5)");
+    }
+
+    @Test
+    void errorMessageHintsAtTheFix() {
+        // The senior-review bar for engine exception messages: name the
+        // values AND tell the caller how to fix it. Asserting the verb
+        // is enough — the exact wording is implementation detail.
+        assertThatThrownBy(() -> RowSlots.validateWeightsMatchChildren(List.of(1.0), 4))
+                .isInstanceOf(IllegalArgumentException.class)
+                .extracting(Throwable::getMessage)
+                .satisfies(msg -> {
+                    String s = (String) msg;
+                    assertThat(s).containsAnyOf("Pass", "Provide", "Use");
+                });
+    }
+}