Skip to content

Keep ForceLayout edges on the same body ids when the body set changes [patch] - #520

Merged
matt-edmondson merged 2 commits into
mainfrom
fix/475-forcelayout-edge-ids
Sep 29, 2026
Merged

matt-edmondson merged 2 commits into
mainfrom
fix/475-forcelayout-edge-ids

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #475

What was wrong

ForceLayout.SetNodes rebuilt the body array and idToIndex but kept each edge's stored body indices. It dropped an edge only when an index fell out of range. Once the new body set differed in order or membership, an index pointed at a different body:

  • Removing an unconnected body could drop a valid edge. For example, with [1,2,3] and edge 2->3, SetNodes([2,3]) put index 2 out of range.
  • Removing an endpoint could move its edge onto bodies that were never linked. With [1,2,3] and edge 1->2, SetNodes([2,3]) turned it into an edge between 2 and 3.
  • Reordering rewired every edge.

This applies to the id-keyed ForceLayout API and to the C ABI (Layout_SetNodes / Layout_SetEdges), which goes through it.

Change

  • SetEdges keeps the (SourceBodyId, TargetBodyId) each edge was submitted with, in an array parallel to the core's edge buffer.
  • SetNodes looks those ids up again in the new idToIndex. An edge is dropped (both ends set to -1) only when one of its ids is missing. As a side effect, an edge whose endpoint leaves and later comes back is linked again.
  • An internal GetEdgeIndices(int) lets the tests read what an edge resolves to. The test project already has InternalsVisibleTo.

LayoutCore rebuilds its per-body edge lists every step from SourceIndex/TargetIndex, so updating those indices in place is enough.

Tests

New tests in ForceLayoutTests:

  • SetNodes_RemovingAnUnconnectedBody_KeepsTheEdgeBetweenTheRest
  • SetNodes_RemovingAnEndpoint_DropsItsEdgeRatherThanRetargetingIt
  • SetNodes_Reordering_KeepsEachEdgeOnTheSamePairOfIds
  • SetNodes_RestoringARemovedEndpoint_ReconnectsItsEdge

With the old range check put back in SetNodes, all four fail. With the fix in place, all 82 tests in ForceDirectedLayout.Tests pass on net10.0.

🤖 Generated with Claude Code

https://claude.ai/code/session_018AK9FUHZoZnfAL1aQuEtAR


Generated by Claude Code

SetNodes kept each edge's stored body indices and only dropped those that
fell out of range, so removing or reordering bodies silently dropped valid
edges or moved them onto bodies they never joined. Keep the endpoint ids
each edge was submitted with and re-resolve them through the new id map,
dropping an edge only when one of its ids is gone.

Fixes #475

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AK9FUHZoZnfAL1aQuEtAR
Comment thread tests/ForceDirectedLayout.Tests/ForceLayoutTests.cs Fixed
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AK9FUHZoZnfAL1aQuEtAR
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ForceLayout.SetNodes keeps stale edges by index: removing a body silently drops valid edges or rewires them to the wrong bodies

2 participants