Add a benchmark harness for iterating on layout behaviour - #378
Merged
Conversation
Judging a layout change by running a graph once and looking at the number does not work. The simulation is chaotic: which local minimum one starting arrangement falls into says nothing about the change that was made, and a single-start assertion flips between passing and failing across parameter values that are all reasonable. Repulsion_IsWhatSpreadsAGraphOut did exactly that while the clear-space repulsion change was being tuned - it passed at repulsion 1,200,000 and 600,000, failed at 800,000, and passed again at 400,000. Every measured claim in this library's comments was established with a harness someone hand-rolled and threw away. tests/ForceDirectedLayout.Tests/Bench/ makes it a first-class thing: - GraphCorpus: four graphs that break a layout differently. Counter is a real twenty-node document with sizes from a 60-wide literal to a 118x180 function; Chain is the shape that most wants to be a horizontal row; FanIn is eight sources arriving at eight pins on one target; MixedSizes alternates 400-wide slabs with 50-wide literals. Sizes and pin rows are load-bearing, since that is what the forces actually measure between. - LayoutMetrics: settled area, mean edge angle, links drawn across a body they are no end of, tightest and mean clear gap, worst overlap, crossed link pairs, settled. Read as a row, because a collapse into a crushed ribbon flatters both the area and the angle. - LayoutBench.Run/Sweep/Compare/Table: settles a configuration over several starts, walked from nodes piled on top of each other to nodes flung a thousand units apart, and renders rows as a table. Deterministic, so the difference between two rows is the setting and nothing else. - LayoutSvg: writes a settled graph to SVG - links first as the cubic the renderer draws, nodes over them, overlaps outlined in red - so a link hidden in the picture is a link hidden in the editor. No window, no GPU, no ImGui. Three existing tests move onto it. ASmallGraph_IsReadableWithinTenSeconds and Repulsion_IsWhatSpreadsAGraphOut were single-start and are now measured over six and eight; Untwisting_ReducesCrossingsWithoutLeavingBodiesOverlapping was already multi-start by hand and gets simpler. Their private helpers - the Counter graph, Shape, LinksOverBodies, WorstOverlap - are deleted in favour of the corpus and the metrics, leaving ForceLayoutTests 156 lines shorter with more starts behind every claim. The corpus gate promptly found a real defect, recorded rather than fixed since it is a change to gravity and not to this harness: centre gravity coils a long chain. A plain twelve-node chain settles at about 53 degrees mean edge angle with two starts in six reading left to right, and it is not settling time - 4000, 12000 and 30000 frames all land on 52.6. Sweeping GravityStrength over the same starts gives 0.6 degrees at 0, 1.9 at 10, 52.9 at the default 50, and 58.4 at 200, and raising DirectionalBias makes it worse, because ordering pairs says nothing about the shape of the whole. Chain's and MixedSizes' thresholds in the gate are loose for that reason, and say so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018b2bn5CKSzpjRV4nxv3c3y
40 + (sources * 21) is integer arithmetic widened to the double the BenchNode height parameter takes, so a large enough source count would overflow in 32-bit before ever reaching the double. Doing the multiplication in double removes the overflow entirely rather than moving it to a wider integer. No behavioural change: the corpus builds FanIn(8), and 40 + 8 * 21 and 40.0 + 8 * 21.0 are both 208. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018b2bn5CKSzpjRV4nxv3c3y
SonarQube flagged twelve MSTEST0037 code smells across the new bench tests and the three ported ones: Assert.IsTrue(a < b) where MSTest has IsLessThan, IsGreaterThan and IsGreaterThanOrEqualTo. The specific assert reports both operands on failure rather than just "expected true", which is worth having in tests whose whole output is measured numbers. Every custom failure message is kept - the overload takes one, and those messages carry the measurements that make a failure diagnosable. The argument order is (bound, value), which is easy to get backwards and would silently invert an assertion, so each of the three kinds was checked by moving its bound to a value that must fail and confirming it did: IsGreaterThan(10000, tightestGap) -> failed, "it had 70.1" IsLessThan(1.0, meanEdgeAngle) -> failed, "should stay under 1; it was 25.8" IsGreaterThanOrEqualTo(99, starts) -> failed, "6 of 6 starts were" Four Assert.IsTrue calls in the same files are left alone: two are compound range checks rather than a single comparison, and two are StartsWith/EndsWith returning a bool. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018b2bn5CKSzpjRV4nxv3c3y
|
This was referenced Sep 9, 2026
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.



Why
Judging a layout change by running a graph once and looking at the number does not work. The simulation is chaotic — which local minimum one starting arrangement falls into says nothing about the change that was made, and a single-start assertion flips between passing and failing across parameter values that are all perfectly reasonable.
Repulsion_IsWhatSpreadsAGraphOutdid exactly that while #375 was being tuned: it passed at repulsion 1,200,000 and 600,000, failed at 800,000, and passed again at 400,000. That is chaos, not a threshold. And every measured claim in this library's comments ("measured over forty starting arrangements…") was established with a harness someone hand-rolled and threw away.What
tests/ForceDirectedLayout.Tests/Bench/:GraphCorpus— four graphs that break a layout differently.Counteris the real twenty-node document with sizes from a 60-wide literal to a 118x180 function;Chainis the shape that most wants to be a horizontal row;FanInis eight sources arriving at eight pins on one target, which is where crossings come from;MixedSizesalternates 400-wide slabs with 50-wide literals. Node sizes and pin rows are load-bearing — repulsion measures clear space between boxes and every angle force measures between pins, so a graph of equal-sized points exercises none of it.LayoutMetrics— settled area, mean edge angle, links drawn across a body they are no end of, tightest and mean clear gap, worst overlap, crossed link pairs, settled. Meant to be read as a row: a collapse into a crushed ribbon flatters both the area and the angle while being the worst outcome available.LayoutBench.Run/.Sweep/.Compare/.Table— settles a configuration over several starts, walked from nodes piled on top of each other to nodes flung a thousand units apart, and renders the rows as a fixed-width table. Deterministic, so the difference between two rows is the setting and nothing else.LayoutSvg— writes a settled graph to SVG: links first, as the cubic the renderer actually draws, nodes over them, overlapping bodies outlined in red. A link hidden in the picture is a link hidden in the editor. No window, no GPU, no ImGui context.Iterating is now: add a scratch
[TestMethod]that prints a sweep, run the suite, read the column that should have moved.Existing tests moved onto it
ASmallGraph_IsReadableWithinTenSecondsandRepulsion_IsWhatSpreadsAGraphOutwere single-start and are now measured over six and eight.Untwisting_ReducesCrossingsWithoutLeavingBodiesOverlappingwas already multi-start by hand and gets simpler. Their private helpers — the Counter graph,Shape,LinksOverBodies,WorstOverlap— are deleted in favour of the corpus and the metrics, leavingForceLayoutTests156 lines shorter with more starts behind every claim.It immediately found a real defect
Corpus_SettlesIntoAReadableShape_UnderTheDefaultsis the new quality gate. It came up red on first run, and the cause is genuine: centre gravity coils a long chain.A plain twelve-node chain — the shape that most obviously wants to be a horizontal row — settles at ~53° mean edge angle with only two starts in six reading left to right. It is not settling time; 4000, 12000 and 30000 frames all land on 52.6. Sweeping
GravityStrengthover the same six starts:Raising
DirectionalBiasmakes it worse (62.7° at bias 2), because ordering pairs left-to-right says nothing about the shape of the whole.MixedSizesis a chain too and coils the same way.I have not fixed this — it is a change to gravity, not to this harness, and it wants its own PR and its own judgement call about whether compactness or left-to-right readability wins. The gate's per-graph thresholds are current behaviour with headroom, and the two loose ones carry the evidence above in a comment so nobody mistakes them for targets. Happy to follow up on the gravity side if you want it.
Notes for whoever uses this next
Two things that bite, both documented in CLAUDE.md:
--show-stdout All --show-test-results allto see a sweep printed by a passing test.IDE0005,IDE2001andIDE0055are errors here, so a quick{ s.X = v; return s; }lambda will not build. Use awithexpression.CLAUDE.md also gains a
ForceDirectedLayoutlibrary entry, which was missing entirely.Verification
ForceDirectedLayout.Tests61/61 (was 43 — the harness adds 12 of its own tests and the ports consolidate the rest),ImGui.NodeEditor.Tests71/71,NodeGraph.Tests106/106. Full solution builds clean in Release with no warnings. The whole layout suite still runs in about 2 seconds.🤖 Generated with Claude Code
https://claude.ai/code/session_018b2bn5CKSzpjRV4nxv3c3y
Generated by Claude Code