diff --git a/docs/gallery/nested-hierarchy/README.md b/docs/gallery/nested-hierarchy/README.md index 956f35f..bfa44bd 100644 --- a/docs/gallery/nested-hierarchy/README.md +++ b/docs/gallery/nested-hierarchy/README.md @@ -35,6 +35,12 @@ independently, and packs the results into one combined canvas. "auto" routes any component containing a container node through the hierarchical algorithm regardless of its size, while the unrelated isolated sibling is packed alongside it through the shared containment bucket. +![Three-level nested container mixing connected pairs and singletons](auto-deep-nested-mixed-connectivity.svg) + +Every nested scope inherits "auto" without re-declaring it, and each one is independently re-classified there: the +connected pair routes through layered and the singleton is packed alongside it through containment, at every level of +nesting — not just the root. + ## Boundary and delegation ports The hierarchical engine's support for boundary (delegation) ports: a container may expose a named port carrying BOTH an diff --git a/docs/gallery/nested-hierarchy/auto-deep-nested-mixed-connectivity.svg b/docs/gallery/nested-hierarchy/auto-deep-nested-mixed-connectivity.svg new file mode 100644 index 0000000..3db8525 --- /dev/null +++ b/docs/gallery/nested-hierarchy/auto-deep-nested-mixed-connectivity.svg @@ -0,0 +1,53 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Outer + + Mid1 + + Mid2 + + MidSolo + + DeepContainer + + D1 + + D2 + + + DSolo + + + RootSolo + diff --git a/docs/gallery/nested-hierarchy/auto-nested-routes-hierarchical.svg b/docs/gallery/nested-hierarchy/auto-nested-routes-hierarchical.svg index cf47ae9..6796001 100644 --- a/docs/gallery/nested-hierarchy/auto-nested-routes-hierarchical.svg +++ b/docs/gallery/nested-hierarchy/auto-nested-routes-hierarchical.svg @@ -1,4 +1,4 @@ - + @@ -30,13 +30,13 @@ - - Group - - Inner1 - - Inner2 - - - Solo + + Group + + Inner1 + + Inner2 + + + Solo diff --git a/src/DemaConsulting.Rendering.Layout/AutoLayoutAlgorithm.cs b/src/DemaConsulting.Rendering.Layout/AutoLayoutAlgorithm.cs index 2250eb8..04dffc4 100644 --- a/src/DemaConsulting.Rendering.Layout/AutoLayoutAlgorithm.cs +++ b/src/DemaConsulting.Rendering.Layout/AutoLayoutAlgorithm.cs @@ -124,10 +124,35 @@ public sealed class AutoLayoutAlgorithm : LayoutAlgorithmBase /// private const double ComponentAspectRatio = 4.0 / 3.0; - private readonly HierarchicalLayoutAlgorithm _hierarchical = new(); + private readonly HierarchicalLayoutAlgorithm _hierarchical; private readonly LayeredLayoutAlgorithm _layered = new(); private readonly ContainmentLayoutAlgorithm _containment = new(); + /// + /// Initializes a new instance of the class. + /// + /// + /// The internal instance used to recurse into + /// container-routed groups is built with a registry that includes this engine itself under its + /// own , in addition to the bundled leaf algorithms. This is what makes + /// "auto" behave as documented for 's inheritance rules: a + /// nested container scope that does not re-declare its own algorithm inherits "auto" like any + /// other cascaded option value, and — because the recursion registry can resolve "auto" back to + /// this same instance — that scope is re-classified by this algorithm's own connectivity-based + /// routing, exactly as if a caller had selected "auto" for it directly. "Auto" is therefore + /// re-evaluated at every scope it cascades to, never resolved once and locked in as a fixed + /// concrete choice for descendant scopes. A raw + /// constructed independently of this class still has zero knowledge of "auto" and continues to + /// surface a resolution error for it, preserving that engine's documented independence. + /// + public AutoLayoutAlgorithm() + { + _hierarchical = new HierarchicalLayoutAlgorithm(new LayoutAlgorithmRegistry() + .Register(_layered) + .Register(_containment) + .Register(this)); + } + /// public override string Id => AlgorithmId; @@ -244,7 +269,12 @@ protected internal override LayoutTree ApplyCore(LayoutGraph graph, LayoutOption } // Fast path: exactly one group overall means nothing needs to be split — delegate straight to - // that group's algorithm on the original, unmodified graph. + // that group's algorithm on the original, unmodified graph. This is safe for every routed + // algorithm, including HierarchicalLayoutAlgorithm: when this graph's own cascaded options + // resolve CoreOptions.Algorithm to "auto" (the normal way a caller selects this engine), that + // same "auto" value survives into HierarchicalLayoutAlgorithm's own top-scope resolution, but + // its recursion registry (built in this class's constructor) resolves "auto" back to this same + // instance rather than throwing — re-running this algorithm's own classification for that scope. if (routedGroups.Count == 1 && singletons.Count == 0) { return routedGroups[0].Algorithm.ApplyCore(graph, options); @@ -256,19 +286,12 @@ protected internal override LayoutTree ApplyCore(LayoutGraph graph, LayoutOption } // Genuine multi-group case: capture the graph's own cascaded options once (so a graph-level - // override still applies to every split-off piece), split each group into its own freshly-built - // sub-graph, lay each out independently, and pack the results into one combined tree. - // - // The captured options must not keep carrying this graph's own CoreOptions.Algorithm value - // (typically "auto" itself, since that is how a caller selected this algorithm in the first - // place): each split-off group's leaf/hierarchical algorithm was already chosen by the routing - // rule above, but HierarchicalLayoutAlgorithm re-reads CoreOptions.Algorithm from its own - // effective options to resolve ITS OWN top scope's leaf algorithm, and "auto" is never a - // registered leaf identifier there. Resetting it to the layered default (the same default - // HierarchicalLayoutAlgorithm itself falls back to when nothing declares an override) restores - // the cascade to exactly what an ordinary caller not using "auto" would see. + // override, including "auto" itself, still applies to every split-off piece), split each group + // into its own freshly-built sub-graph, lay each out independently, and pack the results into one + // combined tree. A split-off piece routed to HierarchicalLayoutAlgorithm resolves "auto" the same + // way the fast path above does — back to this instance, via the recursion registry — so nested + // container scopes are re-classified rather than defaulting to a fixed leaf choice. var effective = graph.OverlayOnto(options); - effective.Set(CoreOptions.Algorithm, LayeredLayoutAlgorithm.AlgorithmId); var trees = new List(routedGroups.Count + (singletons.Count > 0 ? 1 : 0)); foreach (var (members, algorithm) in routedGroups) diff --git a/test/DemaConsulting.Rendering.Gallery/GalleryCatalog.cs b/test/DemaConsulting.Rendering.Gallery/GalleryCatalog.cs index 1454e2d..801ab4f 100644 --- a/test/DemaConsulting.Rendering.Gallery/GalleryCatalog.cs +++ b/test/DemaConsulting.Rendering.Gallery/GalleryCatalog.cs @@ -85,6 +85,8 @@ internal static class GalleryCatalog public const string HierarchicalNestedPng = "nested-hierarchy/hierarchical-nested.png"; public const string AutoNestedRoutesHierarchicalSvg = "nested-hierarchy/auto-nested-routes-hierarchical.svg"; + public const string AutoDeepNestedMixedConnectivitySvg = + "nested-hierarchy/auto-deep-nested-mixed-connectivity.svg"; public const string BoundaryPortsShowcaseHorizontalSvg = "nested-hierarchy/boundary-ports-showcase-horizontal.svg"; public const string BoundaryPortsShowcaseHorizontalPng = @@ -353,6 +355,13 @@ internal static class GalleryCatalog + "hierarchical algorithm regardless of its size, while the unrelated " + "isolated sibling is packed alongside it through the shared containment " + "bucket."), + new GalleryImage( + AutoDeepNestedMixedConnectivitySvg, + "Three-level nested container mixing connected pairs and singletons", + "Every nested scope inherits \"auto\" without re-declaring it, and each one " + + "is independently re-classified there: the connected pair routes through " + + "layered and the singleton is packed alongside it through containment, at " + + "every level of nesting — not just the root."), ]), new GallerySection( "Boundary and delegation ports", diff --git a/test/DemaConsulting.Rendering.Gallery/GalleryDiagrams.cs b/test/DemaConsulting.Rendering.Gallery/GalleryDiagrams.cs index 93e4024..1cd5103 100644 --- a/test/DemaConsulting.Rendering.Gallery/GalleryDiagrams.cs +++ b/test/DemaConsulting.Rendering.Gallery/GalleryDiagrams.cs @@ -1010,6 +1010,46 @@ public static LayoutGraph AutoNestedRoutesHierarchical() return graph; } + /// + /// A three-level-deep compound container whose intermediate and innermost scopes each mix a + /// connected pair with an unrelated singleton, none of them re-declaring + /// . Every nested scope inherits "auto" from the root + /// graph and must be independently re-classified there — proving "auto" is re-evaluated + /// at each level it cascades to (routing the connected pair through layered and the singleton + /// through containment at every scope), rather than being resolved once at the root and then + /// applied uniformly to every descendant scope. + /// + /// A graph with a two-level nested container plus an unrelated root-level singleton. + public static LayoutGraph AutoDeepNestedMixedConnectivity() + { + var graph = new LayoutGraph(); + + // Level 1: the outer container. + var outer = graph.AddNode("outer", 10, 10); + outer.Label = "Outer"; + + // Level 2 (inside outer.Children): a connected pair, an unrelated singleton, and another + // container going one level deeper still. + var mid1 = AddLabelled(outer.Children, "mid1", "Mid1"); + var mid2 = AddLabelled(outer.Children, "mid2", "Mid2"); + Connect(outer.Children, "mid1-mid2", mid1, mid2); + AddLabelled(outer.Children, "midSolo", "MidSolo"); + + var deepContainer = outer.Children.AddNode("deepContainer", 10, 10); + deepContainer.Label = "DeepContainer"; + + // Level 3 (inside deepContainer.Children): again a connected pair plus an unrelated singleton. + var d1 = AddLabelled(deepContainer.Children, "d1", "D1"); + var d2 = AddLabelled(deepContainer.Children, "d2", "D2"); + Connect(deepContainer.Children, "d1-d2", d1, d2); + AddLabelled(deepContainer.Children, "dSolo", "DSolo"); + + // An unrelated root-level singleton forces the genuine multi-group split path at the root too. + AddLabelled(graph, "rootSolo", "RootSolo"); + + return graph; + } + /// /// Twelve small, wide sibling boxes with no edges, suited to the containment algorithm's /// column-count-based content-width candidate: without it, the area-based width estimate alone diff --git a/test/DemaConsulting.Rendering.Gallery/GalleryIndex.cs b/test/DemaConsulting.Rendering.Gallery/GalleryIndex.cs index c6e9b33..a0048dc 100644 --- a/test/DemaConsulting.Rendering.Gallery/GalleryIndex.cs +++ b/test/DemaConsulting.Rendering.Gallery/GalleryIndex.cs @@ -47,8 +47,11 @@ public static string BuildTopIndex() + "each one demonstrates and, where relevant, which bug it guards against. This page just " + "points the way and shows a taste of each."); - AppendParagraph(builder, "| Group | What it's about |"); - AppendParagraph(builder, "| --- | --- |"); + // Written directly (not via AppendParagraph) so the header and separator rows are not + // blank-line-separated from each other or from the data rows: a Markdown table is one + // contiguous block, and a blank line between its rows would break it into separate tables. + builder.Append("| Group | What it's about |").Append('\n'); + builder.Append("| --- | --- |").Append('\n'); foreach (var group in GalleryCatalog.Groups) { builder diff --git a/test/DemaConsulting.Rendering.Gallery/GalleryShowcaseTests.cs b/test/DemaConsulting.Rendering.Gallery/GalleryShowcaseTests.cs index baf9806..99c181f 100644 --- a/test/DemaConsulting.Rendering.Gallery/GalleryShowcaseTests.cs +++ b/test/DemaConsulting.Rendering.Gallery/GalleryShowcaseTests.cs @@ -753,6 +753,30 @@ public void Gallery_AutoNestedRoutesHierarchical_RendersSvg() Themes.Dark); } + /// + /// Renders a three-level-deep nested container to SVG, where the middle and innermost scopes + /// each mix a connected pair with an unrelated singleton and neither re-declares + /// . Proves "auto" is re-evaluated at every scope it cascades + /// to (routing each scope's connected pair through layered and its singleton through + /// containment independently) rather than being resolved once at the root and applied + /// uniformly to every descendant scope. Regression coverage for a bug where a nested scope that + /// inherited "auto" without re-declaring it either threw or, + /// under an earlier incomplete fix, was silently flattened to a single fixed leaf algorithm. + /// + [Fact] + public void Gallery_AutoDeepNestedMixedConnectivity_RendersSvg() + { + // Arrange + var graph = GalleryDiagrams.AutoDeepNestedMixedConnectivity(); + graph.Set(CoreOptions.Algorithm, "auto"); + + // Act / Assert + GalleryWriter.Svg( + GalleryCatalog.AutoDeepNestedMixedConnectivitySvg, + graph, + Themes.Dark); + } + /// /// Renders twelve small, wide boxes packed by the containment algorithm to SVG, proving the /// column-count-based content-width candidate keeps the algorithm from packing them into one diff --git a/test/DemaConsulting.Rendering.Layout.Tests/AutoLayoutAlgorithmTests.cs b/test/DemaConsulting.Rendering.Layout.Tests/AutoLayoutAlgorithmTests.cs index a45c642..14ef244 100644 --- a/test/DemaConsulting.Rendering.Layout.Tests/AutoLayoutAlgorithmTests.cs +++ b/test/DemaConsulting.Rendering.Layout.Tests/AutoLayoutAlgorithmTests.cs @@ -295,10 +295,12 @@ static double SiblingGap(double nodeSpacing) /// Proves that when the graph itself explicitly declares CoreOptions.Algorithm = "auto" /// (as a caller resolving the algorithm from the graph's own options, rather than passing it /// directly, would do) and the multi-group split path routes one group to hierarchical, the - /// hierarchical algorithm's own recursive scope resolution does not attempt to resolve "auto" - /// from its own leaf-only registry and throw. Regression test for a bug where the captured - /// effective options still carried the root graph's own "auto" value down into a nested - /// algorithm's cascade. + /// hierarchical algorithm's own recursive scope resolution does not throw when it re-reads + /// "auto" from its cascaded options: its recursion registry resolves "auto" back to this same + /// instance instead of a leaf-only registry that lacks it. + /// Regression test for a bug where the captured effective options still carried the root graph's + /// own "auto" value down into a nested algorithm's cascade, and that cascade had no way to + /// resolve it. /// [Fact] public void Apply_GraphDeclaresAutoAlgorithmWithNestedContainerGroup_DoesNotThrow() @@ -326,6 +328,103 @@ public void Apply_GraphDeclaresAutoAlgorithmWithNestedContainerGroup_DoesNotThro Assert.Equal(5, CountBoxesRecursively(tree.Nodes)); } + /// + /// Proves that when the graph itself explicitly declares CoreOptions.Algorithm = "auto" + /// and the entire graph is a single component containing a nested container (so routing + /// takes the zero-copy single-group fast path straight to + /// on the original, unmodified graph — not the split-and-pack path), the hierarchical algorithm's + /// own top-scope resolution does not throw when it re-reads + /// "auto" from its cascaded options. Regression test for a bug where the fast path handed the + /// untouched original options straight to , whose first + /// step (graph.OverlayOnto(options)) picks up the root graph's own "auto" override + /// unchanged, and its recursion registry (at the time) had no way to resolve that identifier. + /// + [Fact] + public void Apply_GraphDeclaresAutoAlgorithmAsSoleHierarchicalGroup_DoesNotThrow() + { + // Arrange: the graph itself declares "auto", and its only top-level node is a two-level-deep + // nested container with no other top-level node or edge, so routing produces exactly one group + // (routed to hierarchical) and no singletons — the single-group fast path. + var graph = new LayoutGraph(); + graph.Set(CoreOptions.Algorithm, "auto"); + + var outer = graph.AddNode("outer", 10, 10); + var inner = outer.Children.AddNode("inner", 10, 10); + var leaf1 = inner.Children.AddNode("leaf1", 80, 40); + var leaf2 = inner.Children.AddNode("leaf2", 80, 40); + inner.Children.AddEdge("leaf1-leaf2", leaf1, leaf2); + + // Act + var tree = new AutoLayoutAlgorithm().Apply(graph); + + // Assert: no exception was thrown, and every box (the outer container, the inner container, and + // the two leaves) made it into the tree. + Assert.Equal(4, CountBoxesRecursively(tree.Nodes)); + } + + /// + /// Proves that "auto" is re-evaluated at every scope it cascades to, rather than being resolved + /// once at the root and then locked in as a fixed concrete choice for every descendant scope. + /// A container whose own children mix a connected pair with an unrelated singleton — inheriting + /// "auto" from the root without re-declaring it — must be classified by this algorithm's own + /// connectivity-based routing (packing the singleton separately via containment) exactly like a + /// top-level "auto" graph would, not simply handed to a single fixed leaf algorithm (which would + /// lay every member out uniformly, with no special packing for the singleton). Regression test + /// for a design gap where a container-scope fix merely avoided throwing by forcing a fixed leaf + /// choice onto every descendant scope, rather than honoring "auto"'s documented inheritance rule + /// that an unset option keeps re-evaluating at each level. + /// + [Fact] + public void Apply_AutoInheritedByNestedContainer_ReclassifiesMixedConnectivityAtThatScope() + { + // Arrange: two structurally-identical graphs, differing only in how the container's own children + // scope resolves its algorithm. Both containers hold a connected pair ("a"-"b") plus an unrelated + // singleton ("solo"). "inherited" lets the root's "auto" cascade down unset; "forcedLayered" + // instead explicitly overrides the children scope to plain "layered", so its singleton is laid + // out uniformly alongside the pair rather than packed separately via containment. + static LayoutTree Build(bool forceLayeredOnChildren) + { + var graph = new LayoutGraph(); + graph.Set(CoreOptions.Algorithm, "auto"); + + var outer = graph.AddNode("outer", 10, 10); + outer.Label = "outer"; + if (forceLayeredOnChildren) + { + outer.Children.Set(CoreOptions.Algorithm, LayeredLayoutAlgorithm.AlgorithmId); + } + + var a = outer.Children.AddNode("a", 80, 40); + var b = outer.Children.AddNode("b", 80, 40); + outer.Children.AddEdge("a-b", a, b); + outer.Children.AddNode("solo", 80, 40); + + return new AutoLayoutAlgorithm().Apply(graph); + } + + // Act + var inheritedTree = Build(forceLayeredOnChildren: false); + var forcedLayeredTree = Build(forceLayeredOnChildren: true); + + var inheritedOuter = inheritedTree.Nodes.OfType().Single(box => box.Label == "outer"); + var forcedLayeredOuter = forcedLayeredTree.Nodes.OfType().Single(box => box.Label == "outer"); + + // Assert: every box made it into both trees (the outer container, "a", "b", and "solo"). + Assert.Equal(4, CountBoxesRecursively(inheritedTree.Nodes)); + Assert.Equal(4, CountBoxesRecursively(forcedLayeredTree.Nodes)); + + // Assert: the inherited-"auto" container is sized differently from the forced-plain-"layered" + // container, proving the inherited case actually re-ran this algorithm's own component + // classification (splitting the singleton into its own containment-packed bucket) instead of + // resolving to the same fixed leaf treatment as an explicit "layered" override. + Assert.True( + Math.Abs(inheritedOuter.Width - forcedLayeredOuter.Width) > 0.5 || + Math.Abs(inheritedOuter.Height - forcedLayeredOuter.Height) > 0.5, + $"expected the inherited-\"auto\" container ({inheritedOuter.Width:R}x{inheritedOuter.Height:R}) to be " + + $"sized differently from the forced-\"layered\" container ({forcedLayeredOuter.Width:R}x{forcedLayeredOuter.Height:R}), " + + "proving the nested scope was reclassified by \"auto\" rather than defaulting to a fixed leaf choice"); + } + /// Counts every in a node list, recursing into each box's children. /// The nodes to count within. /// The total number of boxes found.