From 307ab0e194ff750ed6fc0e4fae50c45a2623f685 Mon Sep 17 00:00:00 2001 From: jedarden Date: Fri, 3 Jul 2026 19:00:37 -0400 Subject: [PATCH] feat(simulator): seed spread geometry for default virtual nodes in registry bridge (bf-xrej) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Virtual nodes created without an explicit position carry the nodes-table DB defaults (pos_x=0, pos_y=0, pos_z=1), so SyncToRegistry/SyncOneNode wrote co-located origin positions into the fleet registry — and downstream into the fusion engine — collapsing Fresnel excess paths (core symptom in bf-18yn). Resolve positions through a new effectivePositions helper: nodes still at the default origin are reassigned distinct, spread-out geometry from DefaultNodePositions (sized to the full node count, across the store's space), while explicitly-placed nodes keep their position. Slot assignment is keyed by sorted node ID so it is deterministic regardless of map iteration order, and a single-node sync writes the same position a full sync would. The effective positions flow through SyncToRegistry, SyncOneNode, and ToRegistryRecords, so the registry and fusion-engine seeding observe non-degenerate geometry. Co-Authored-By: Claude --- .../internal/simulator/registry_bridge.go | 155 ++++++-- .../simulator/registry_bridge_test.go | 337 ++++++++++++++++++ 2 files changed, 468 insertions(+), 24 deletions(-) create mode 100644 mothership/internal/simulator/registry_bridge_test.go diff --git a/mothership/internal/simulator/registry_bridge.go b/mothership/internal/simulator/registry_bridge.go index b60775b..31c9f85 100644 --- a/mothership/internal/simulator/registry_bridge.go +++ b/mothership/internal/simulator/registry_bridge.go @@ -3,8 +3,22 @@ package simulator import ( "fmt" + "sort" ) +// DefaultNodeOrigin is the degenerate position virtual nodes carry when they +// are created without an explicit placement — it matches the fleet registry's +// nodes-table schema defaults (pos_x=0, pos_y=0, pos_z=1). Nodes still sitting +// at this origin are treated as "unset": the bridge reassigns them spread-out +// geometry at sync time so the registry and downstream fusion engine never see +// co-located / all-at-origin virtual nodes (core symptom in bf-18yn / bf-4q5w). +var DefaultNodeOrigin = Point{X: 0, Y: 0, Z: 1} + +// isDefaultOrigin reports whether p is the unset DB-default origin position. +func isDefaultOrigin(p Point) bool { + return p.X == DefaultNodeOrigin.X && p.Y == DefaultNodeOrigin.Y && p.Z == DefaultNodeOrigin.Z +} + // FleetRegistryBridge integrates virtual nodes with the fleet registry. // This allows virtual nodes to participate in coverage planning and role assignment. type FleetRegistryBridge struct { @@ -12,6 +26,78 @@ type FleetRegistryBridge struct { registryKey string // Prefix for MAC addresses in registry } +// space returns the geometry the bridge spreads default nodes across. It +// prefers the store's configured space and falls back to DefaultSpace() so the +// bridge is safe even when the store has no space attached. +func (b *FleetRegistryBridge) space() *Space { + if b.store != nil { + if s := b.store.GetSpace(); s != nil { + return s + } + } + return DefaultSpace() +} + +// effectivePositions returns the registry position to sync for each node ID. +// Nodes still at the default DB origin (DefaultNodeOrigin) are reassigned +// distinct, spread-out geometry from DefaultNodePositions — sized to the full +// node count so even when every node is unset the geometry spans the room — +// while explicitly-placed (non-origin) nodes keep their position. Default-origin +// nodes are assigned successive spread slots in sorted-ID order, so the mapping +// is deterministic regardless of map iteration order and a single-node sync +// produces the same position as a full sync. The result therefore has no two +// co-located points and is never entirely at the origin. +func (b *FleetRegistryBridge) effectivePositions(nodes []*VirtualNodeState) map[string]Point { + effective := make(map[string]Point, len(nodes)) + var defaults []*VirtualNodeState + for _, n := range nodes { + if isDefaultOrigin(n.Position) { + defaults = append(defaults, n) + } else { + effective[n.ID] = n.Position + } + } + if len(defaults) == 0 { + return effective + } + + // Deterministic slot assignment independent of map iteration order. + sort.Slice(defaults, func(i, j int) bool { return defaults[i].ID < defaults[j].ID }) + + space := b.space() + spread := DefaultNodePositions(space, len(nodes)) + + // Track positions already in use so default-origin nodes never collide + // with each other or with an explicitly-placed node. + taken := make(map[Point]bool, len(effective)+len(defaults)) + for _, p := range effective { + taken[p] = true + } + + si := 0 + for _, n := range defaults { + for si < len(spread) && taken[spread[si]] { + si++ + } + var pos Point + if si < len(spread) { + pos = spread[si] + si++ + } else { + // Spread set exhausted: more default nodes than slots. The set is + // sized to len(nodes) so this is effectively unreachable; fall back + // to the room center, which is still distinct from the origin for + // any non-degenerate room. + minX, minY, minZ, maxX, maxY, maxZ := space.Bounds() + pos = Point{X: (minX + maxX) / 2, Y: (minY + maxY) / 2, Z: (minZ + maxZ) / 2} + } + effective[n.ID] = pos + taken[pos] = true + } + + return effective +} + // NewFleetRegistryBridge creates a new bridge between virtual nodes and fleet registry func NewFleetRegistryBridge(store *VirtualNodeStore) *FleetRegistryBridge { return &FleetRegistryBridge{ @@ -42,16 +128,24 @@ type NodeRecord struct { Enabled bool } -// SyncToRegistry synchronizes all virtual nodes to the fleet registry +// SyncToRegistry synchronizes all virtual nodes to the fleet registry. +// +// Positions are resolved through effectivePositions: any node still at the +// default DB origin (DefaultNodeOrigin) is reassigned distinct, spread-out +// geometry across the store's space so the registry — and the fusion engine +// fed from it via the existing wiring — never observes co-located or +// all-at-origin virtual nodes. func (b *FleetRegistryBridge) SyncToRegistry(registry RegistryNodeAdapter) error { if registry == nil { return fmt.Errorf("registry is nil") } nodes := b.store.ListNodes() + positions := b.effectivePositions(nodes) for _, node := range nodes { mac := b.virtualMAC(node.ID) + pos := positions[node.ID] // Check if node exists in registry existing, err := registry.GetNode(mac) @@ -60,21 +154,21 @@ func (b *FleetRegistryBridge) SyncToRegistry(registry RegistryNodeAdapter) error if err := registry.AddVirtualNode( mac, node.Name, - node.Position.X, - node.Position.Y, - node.Position.Z, + pos.X, + pos.Y, + pos.Z, ); err != nil { return fmt.Errorf("add virtual node %s: %w", node.ID, err) } } else { // Node exists, update position/role if changed - if existing.PosX != node.Position.X || - existing.PosY != node.Position.Y || - existing.PosZ != node.Position.Z { + if existing.PosX != pos.X || + existing.PosY != pos.Y || + existing.PosZ != pos.Z { if err := registry.SetNodePosition(mac, - node.Position.X, - node.Position.Y, - node.Position.Z, + pos.X, + pos.Y, + pos.Z, ); err != nil { return fmt.Errorf("update position for %s: %w", node.ID, err) } @@ -94,7 +188,12 @@ func (b *FleetRegistryBridge) SyncToRegistry(registry RegistryNodeAdapter) error return nil } -// SyncOneNode syncs a single virtual node to the registry +// SyncOneNode syncs a single virtual node to the registry. +// +// The effective position is resolved over the full node set (not just this +// node) so a single-node sync produces the same spread geometry a full sync +// would write: default-origin nodes receive deterministic spread slots keyed +// by their rank among the unset nodes. func (b *FleetRegistryBridge) SyncOneNode(registry RegistryNodeAdapter, nodeID string) error { if registry == nil { return fmt.Errorf("registry is nil") @@ -105,6 +204,10 @@ func (b *FleetRegistryBridge) SyncOneNode(registry RegistryNodeAdapter, nodeID s return fmt.Errorf("get node %s: %w", nodeID, err) } + // Resolve over the full set so the slot assignment matches SyncToRegistry. + positions := b.effectivePositions(b.store.ListNodes()) + pos := positions[nodeID] + mac := b.virtualMAC(nodeID) existing, err := registry.GetNode(mac) @@ -113,20 +216,20 @@ func (b *FleetRegistryBridge) SyncOneNode(registry RegistryNodeAdapter, nodeID s return registry.AddVirtualNode( mac, node.Name, - node.Position.X, - node.Position.Y, - node.Position.Z, + pos.X, + pos.Y, + pos.Z, ) } // Update existing node - if existing.PosX != node.Position.X || - existing.PosY != node.Position.Y || - existing.PosZ != node.Position.Z { + if existing.PosX != pos.X || + existing.PosY != pos.Y || + existing.PosZ != pos.Z { if err := registry.SetNodePosition(mac, - node.Position.X, - node.Position.Y, - node.Position.Z, + pos.X, + pos.Y, + pos.Z, ); err != nil { return fmt.Errorf("update position: %w", err) } @@ -180,19 +283,23 @@ func (b *FleetRegistryBridge) VirtualNodeID(mac string) (string, bool) { return "", true // TODO: implement reverse mapping } -// ToRegistryRecords converts virtual nodes to fleet registry records +// ToRegistryRecords converts virtual nodes to fleet registry records. Positions +// are resolved through effectivePositions so the records reflect exactly what +// SyncToRegistry would write (spread geometry for default-origin nodes). func (b *FleetRegistryBridge) ToRegistryRecords() []NodeRecord { nodes := b.store.ListNodes() + positions := b.effectivePositions(nodes) records := make([]NodeRecord, 0, len(nodes)) for _, node := range nodes { + pos := positions[node.ID] records = append(records, NodeRecord{ MAC: b.virtualMAC(node.ID), Name: node.Name, Role: string(node.Role), - PosX: node.Position.X, - PosY: node.Position.Y, - PosZ: node.Position.Z, + PosX: pos.X, + PosY: pos.Y, + PosZ: pos.Z, Virtual: true, Enabled: node.Enabled, }) diff --git a/mothership/internal/simulator/registry_bridge_test.go b/mothership/internal/simulator/registry_bridge_test.go new file mode 100644 index 0000000..2b1c611 --- /dev/null +++ b/mothership/internal/simulator/registry_bridge_test.go @@ -0,0 +1,337 @@ +package simulator + +import ( + "fmt" + "testing" +) + +// fakeRegistry is an in-memory RegistryNodeAdapter that records every mutation +// so bridge tests can assert what was written to the registry (and in what order). +type fakeRegistry struct { + nodes map[string]NodeRecord + addCalls []NodeRecord + setPosCalls []NodeRecord + setRoleCalls []macRole +} + +type macRole struct { + MAC, Role string +} + +func newFakeRegistry() *fakeRegistry { + return &fakeRegistry{nodes: make(map[string]NodeRecord)} +} + +func (f *fakeRegistry) AddVirtualNode(mac, name string, x, y, z float64) error { + rec := NodeRecord{MAC: mac, Name: name, PosX: x, PosY: y, PosZ: z, Virtual: true, Enabled: true} + f.nodes[mac] = rec + f.addCalls = append(f.addCalls, rec) + return nil +} + +func (f *fakeRegistry) SetNodePosition(mac string, x, y, z float64) error { + rec := f.nodes[mac] + rec.PosX, rec.PosY, rec.PosZ = x, y, z + f.nodes[mac] = rec + f.setPosCalls = append(f.setPosCalls, NodeRecord{MAC: mac, PosX: x, PosY: y, PosZ: z}) + return nil +} + +func (f *fakeRegistry) SetNodeRole(mac, role string) error { + rec := f.nodes[mac] + rec.Role = role + f.nodes[mac] = rec + f.setRoleCalls = append(f.setRoleCalls, macRole{MAC: mac, Role: role}) + return nil +} + +func (f *fakeRegistry) DeleteNode(mac string) error { + delete(f.nodes, mac) + return nil +} + +func (f *fakeRegistry) GetNode(mac string) (*NodeRecord, error) { + rec, ok := f.nodes[mac] + if !ok { + return nil, fmt.Errorf("not found: %s", mac) + } + return &rec, nil +} + +func (f *fakeRegistry) GetAllNodes() ([]NodeRecord, error) { + out := make([]NodeRecord, 0, len(f.nodes)) + for _, r := range f.nodes { + out = append(out, r) + } + return out, nil +} + +// storeWithNodes builds a fresh store (DefaultSpace) holding one virtual node +// per entry. Each entry gives (id, position). Positions at the default origin +// are created as-is (the origin is in-bounds for DefaultSpace). +func storeWithNodes(t *testing.T, nodes ...struct{ ID string; Pos Point }) *VirtualNodeStore { + t.Helper() + store, _ := tempStore(t) + for _, n := range nodes { + if _, err := store.CreateVirtualNode(n.ID, n.ID, n.Pos); err != nil { + t.Fatalf("create %s: %v", n.ID, err) + } + } + return store +} + +// originStore builds a store with count nodes all at the default DB origin. +func originStore(t *testing.T, count int) *VirtualNodeStore { + t.Helper() + entries := make([]struct{ ID string; Pos Point }, 0, count) + for i := 0; i < count; i++ { + entries = append(entries, struct{ ID string; Pos Point }{ + ID: fmt.Sprintf("node-%d", i+1), + Pos: DefaultNodeOrigin, + }) + } + return storeWithNodes(t, entries...) +} + +// assertDistinctNonOrigin fails the test if any record is still at the default +// origin or if any two records share the same position. +func assertDistinctNonOrigin(t *testing.T, recs []NodeRecord) { + t.Helper() + seen := make(map[Point]bool, len(recs)) + for _, r := range recs { + p := Point{X: r.PosX, Y: r.PosY, Z: r.PosZ} + if isDefaultOrigin(p) { + t.Errorf("node %s still at default origin %v", r.MAC, p) + } + if seen[p] { + t.Errorf("co-located registry nodes at %v (mac %s)", p, r.MAC) + } + seen[p] = true + } +} + +func TestIsDefaultOrigin(t *testing.T) { + cases := []struct { + name string + p Point + want bool + }{ + {"db default origin", DefaultNodeOrigin, true}, + {"explicit zero", Point{X: 0, Y: 0, Z: 0}, false}, + {"origin wrong Z", Point{X: 0, Y: 0, Z: 1.5}, false}, + {"origin shifted X", Point{X: 1, Y: 0, Z: 1}, false}, + {"placed node", Point{X: 3, Y: 2, Z: 1.5}, false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := isDefaultOrigin(c.p); got != c.want { + t.Errorf("isDefaultOrigin(%v) = %v, want %v", c.p, got, c.want) + } + }) + } +} + +// TestSyncToRegistry_OriginNodesGetSpreadGeometry is the core acceptance check: +// a fleet of nodes created at the default origin must be written to the +// registry at distinct, non-degenerate positions (spread across the room). +func TestSyncToRegistry_OriginNodesGetSpreadGeometry(t *testing.T) { + for _, count := range []int{1, 2, 3, 4, 6, 9} { + t.Run(fmt.Sprintf("count=%d", count), func(t *testing.T) { + store := originStore(t, count) + bridge := NewFleetRegistryBridge(store) + reg := newFakeRegistry() + + if err := bridge.SyncToRegistry(reg); err != nil { + t.Fatalf("SyncToRegistry: %v", err) + } + + if len(reg.nodes) != count { + t.Fatalf("expected %d registry nodes, got %d", count, len(reg.nodes)) + } + if len(reg.addCalls) != count { + t.Fatalf("expected %d AddVirtualNode calls, got %d", count, len(reg.addCalls)) + } + + recs := make([]NodeRecord, 0, len(reg.nodes)) + for _, r := range reg.nodes { + recs = append(recs, r) + } + assertDistinctNonOrigin(t, recs) + }) + } +} + +// TestSyncToRegistry_ExplicitPositionsPreserved verifies nodes placed at a real +// (non-origin) position are written through unchanged — the bridge only +// substitutes geometry for unset nodes. +func TestSyncToRegistry_ExplicitPositionsPreserved(t *testing.T) { + a := Point{X: 1, Y: 1, Z: 1.5} + b := Point{X: 4, Y: 4, Z: 2} + store := storeWithNodes(t, + struct{ ID string; Pos Point }{"a", a}, + struct{ ID string; Pos Point }{"b", b}, + ) + bridge := NewFleetRegistryBridge(store) + reg := newFakeRegistry() + + if err := bridge.SyncToRegistry(reg); err != nil { + t.Fatalf("SyncToRegistry: %v", err) + } + + for id, want := range map[string]Point{"a": a, "b": b} { + rec, ok := reg.nodes[bridge.virtualMAC(id)] + if !ok { + t.Fatalf("node %s missing from registry", id) + } + got := Point{X: rec.PosX, Y: rec.PosY, Z: rec.PosZ} + if got != want { + t.Errorf("node %s: expected explicit %v, got %v", id, want, got) + } + } +} + +// TestSyncToRegistry_MixedNodes keeps explicit placements while spreading the +// unset ones, and asserts the final set is distinct and non-origin. +func TestSyncToRegistry_MixedNodes(t *testing.T) { + store := storeWithNodes(t, + struct{ ID string; Pos Point }{"explicit", Point{X: 5, Y: 4, Z: 2}}, + struct{ ID string; Pos Point }{"o1", DefaultNodeOrigin}, + struct{ ID string; Pos Point }{"o2", DefaultNodeOrigin}, + struct{ ID string; Pos Point }{"o3", DefaultNodeOrigin}, + ) + bridge := NewFleetRegistryBridge(store) + reg := newFakeRegistry() + + if err := bridge.SyncToRegistry(reg); err != nil { + t.Fatalf("SyncToRegistry: %v", err) + } + + // Explicit node keeps its position. + rec := reg.nodes[bridge.virtualMAC("explicit")] + if (Point{X: rec.PosX, Y: rec.PosY, Z: rec.PosZ}) != (Point{X: 5, Y: 4, Z: 2}) { + t.Errorf("explicit node moved: got %v", rec) + } + + recs := make([]NodeRecord, 0, len(reg.nodes)) + for _, r := range reg.nodes { + recs = append(recs, r) + } + assertDistinctNonOrigin(t, recs) +} + +// TestSyncToRegistry_Idempotent asserts a second sync is a no-op: because the +// spread slot assignment is deterministic, the registry already holds the +// effective positions and no further writes occur. +func TestSyncToRegistry_Idempotent(t *testing.T) { + store := originStore(t, 4) + bridge := NewFleetRegistryBridge(store) + reg := newFakeRegistry() + + if err := bridge.SyncToRegistry(reg); err != nil { + t.Fatalf("first SyncToRegistry: %v", err) + } + firstAdds, firstSets := len(reg.addCalls), len(reg.setPosCalls) + + if err := bridge.SyncToRegistry(reg); err != nil { + t.Fatalf("second SyncToRegistry: %v", err) + } + + if len(reg.addCalls) != firstAdds { + t.Errorf("second sync added nodes again: %d != %d", len(reg.addCalls), firstAdds) + } + if len(reg.setPosCalls) != firstSets { + t.Errorf("second sync rewrote positions: %d != %d", len(reg.setPosCalls), firstSets) + } +} + +// TestSyncOneNode_MatchesFullSync asserts a single-node sync writes the same +// effective position that a full sync would — including spread geometry for a +// default-origin node. +func TestSyncOneNode_MatchesFullSync(t *testing.T) { + store := originStore(t, 3) + + fullReg := newFakeRegistry() + bridge := NewFleetRegistryBridge(store) + if err := bridge.SyncToRegistry(fullReg); err != nil { + t.Fatalf("SyncToRegistry: %v", err) + } + + target := "node-2" + mac := bridge.virtualMAC(target) + wantRec, ok := fullReg.nodes[mac] + if !ok { + t.Fatalf("node %s missing from full sync", target) + } + + oneReg := newFakeRegistry() + if err := bridge.SyncOneNode(oneReg, target); err != nil { + t.Fatalf("SyncOneNode: %v", err) + } + gotRec, ok := oneReg.nodes[mac] + if !ok { + t.Fatalf("node %s not written by SyncOneNode", target) + } + + want := Point{X: wantRec.PosX, Y: wantRec.PosY, Z: wantRec.PosZ} + got := Point{X: gotRec.PosX, Y: gotRec.PosY, Z: gotRec.PosZ} + if got != want { + t.Errorf("SyncOneNode position %v != SyncToRegistry position %v", got, want) + } + if isDefaultOrigin(got) { + t.Errorf("SyncOneNode left default-origin node at the origin: %v", got) + } +} + +// TestSyncOneNode_ExplicitPreserved asserts SyncOneNode passes an explicit +// (non-origin) position through unchanged. +func TestSyncOneNode_ExplicitPreserved(t *testing.T) { + store := storeWithNodes(t, struct{ ID string; Pos Point }{"a", Point{X: 2, Y: 3, Z: 1.2}}) + bridge := NewFleetRegistryBridge(store) + reg := newFakeRegistry() + + if err := bridge.SyncOneNode(reg, "a"); err != nil { + t.Fatalf("SyncOneNode: %v", err) + } + rec := reg.nodes[bridge.virtualMAC("a")] + got := Point{X: rec.PosX, Y: rec.PosY, Z: rec.PosZ} + if got != (Point{X: 2, Y: 3, Z: 1.2}) { + t.Errorf("expected explicit position preserved, got %v", got) + } +} + +// TestSyncToRegistry_NilRegistry guards the nil-adapter guard. +func TestSyncToRegistry_NilRegistry(t *testing.T) { + store := originStore(t, 2) + bridge := NewFleetRegistryBridge(store) + if err := bridge.SyncToRegistry(nil); err == nil { + t.Fatal("expected error syncing to nil registry") + } + if err := bridge.SyncOneNode(nil, "node-1"); err == nil { + t.Fatal("expected error syncing one node to nil registry") + } +} + +// TestToRegistryRecords_UsesEffectivePositions ensures the record view matches +// what SyncToRegistry writes (spread geometry for origin nodes). +func TestToRegistryRecords_UsesEffectivePositions(t *testing.T) { + store := originStore(t, 3) + bridge := NewFleetRegistryBridge(store) + + records := bridge.ToRegistryRecords() + if len(records) != 3 { + t.Fatalf("expected 3 records, got %d", len(records)) + } + assertDistinctNonOrigin(t, records) + + // Cross-check against an actual sync. + reg := newFakeRegistry() + if err := bridge.SyncToRegistry(reg); err != nil { + t.Fatalf("SyncToRegistry: %v", err) + } + for _, r := range records { + synced := reg.nodes[r.MAC] + if synced.PosX != r.PosX || synced.PosY != r.PosY || synced.PosZ != r.PosZ { + t.Errorf("record %s %v != synced %v", r.MAC, r, synced) + } + } +}