fix(topology): stop breadth translating into width (v0.23.22-beta)

Closes #115.

The map gave every leaf its own column, so a subtree's width was simply its leaf
count. That is reasonable for a deep branching network and useless for a real
one: a core switch with fifty-eight access switches under it produced a single
row fifty-eight slots wide — over ten thousand pixels, which is exactly how it
was reported.

Real networks are shallow and wide, so breadth had to stop translating directly
into horizontal distance. Large groups of sibling leaves are now laid out as a
block rather than a row, trading width for height and turning linear growth into
roughly square-root. The reported shape goes from 58 slots to 10, six rows deep.

Children that have subtrees of their own keep the classic tidy-tree treatment,
because there the hierarchy is the information and flattening it would destroy
meaning. Only leaf groups — where the sibling order carries nothing — are packed.

Block proportions are deliberately wider than tall: screens are landscape, and a
column of forty reads worse than a band four rows deep.

The layout moved to utils/topologyLayout.ts in slot and row units, with pixels
left to the renderer, and 12 tests. That split is what makes the fix checkable at
all — the old arithmetic was only observable by looking at a rendered graph of a
network none of us has. The tests pin the reported case and, as importantly, the
shapes that must not regress: a three-node row stays a row, a branching tree
keeps its levels, a deep chain stays one slot wide, and no two nodes land on the
same coordinate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

frontend/package.json

@@ -1,6 +1,6 @@
 {
   "name": "mikrotik-manager-frontend",
-  "version": "0.23.21-beta",
+  "version": "0.23.22-beta",
   "private": true,
   "type": "module",
   "scripts": {

frontend/src/pages/TopologyPage.tsx

@@ -26,6 +26,7 @@ import { topologyApi } from '../services/api';
 import type { TopologyDevice, TopologyLink, ExternalTopologyNode } from '../types';
 import clsx from 'clsx';
 import { useThemeStore } from '../store/themeStore';
+import { layoutTree } from '../utils/topologyLayout';
 
 // ─── Helpers ─────────────────────────────────────────────────────────────────
 
@@ -335,40 +336,19 @@ function buildGraph(
       });
     }
 
-    const slotWidth = new Map<string, number>();
-    function computeWidth(id: string): number {
-      const kids = children.get(id) || [];
-      if (!kids.length) { slotWidth.set(id, 1); return 1; }
-      let total = 0;
-      for (const k of kids) total += computeWidth(k);
-      slotWidth.set(id, Math.max(1, total));
-      return slotWidth.get(id)!;
+    // Layout runs in slots and rows; pixels are this file's business. Wide groups
+    // of sibling leaves become blocks rather than one enormous row — see
+    // utils/topologyLayout for why breadth must stop meaning width (#115).
+    const laid = layoutTree(rootId, children, depth);
+    for (const [id, p] of laid.positions) {
+      positions.set(id, {
+        x: componentOffsetX + (p.x + 0.5) * NODE_SLOT - NODE_W / 2,
+        y: p.y * (NODE_H + V_GAP),
+      });
     }
-    computeWidth(rootId);
 
-    function assign(id: string, leftSlot: number): void {
-      const kids = children.get(id) || [];
-      if (!kids.length) {
-        positions.set(id, {
-          x: componentOffsetX + (leftSlot + 0.5) * NODE_SLOT - NODE_W / 2,
-          y: (depth.get(id) || 0) * (NODE_H + V_GAP),
-        });
-        return;
-      }
-      let cursor = leftSlot;
-      for (const k of kids) { assign(k, cursor); cursor += slotWidth.get(k)!; }
-      const fk = positions.get(kids[0])!, lk = positions.get(kids[kids.length - 1])!;
-      positions.set(id, { x: (fk.x + lk.x) / 2, y: (depth.get(id) || 0) * (NODE_H + V_GAP) });
-    }
-    assign(rootId, 0);
-
-    let compMaxDepth = 0;
-    for (const id of comp) {
-      const d = depth.get(id);
-      if (d !== undefined) compMaxDepth = Math.max(compMaxDepth, d);
-    }
-    globalMaxDepth = Math.max(globalMaxDepth, compMaxDepth);
-    componentOffsetX += (slotWidth.get(rootId) || 1) * NODE_SLOT + COMPONENT_GAP;
+    globalMaxDepth = Math.max(globalMaxDepth, laid.maxRow);
+    componentOffsetX += laid.widthSlots * NODE_SLOT + COMPONENT_GAP;
   }
 
   // ── Orphan row at the bottom ─────────────────────────────────────────────────

frontend/src/utils/topologyLayout.test.ts

@@ -0,0 +1,127 @@
+import { describe, it, expect } from 'vitest';
+import { layoutTree, blockColumns, DEFAULT_LAYOUT } from './topologyLayout';
+
+/** Build children/depth maps from a plain description, as the page does by BFS. */
+function tree(spec: Record<string, string[]>, root: string) {
+  const children = new Map<string, string[]>();
+  const depth = new Map<string, number>([[root, 0]]);
+  const walk = [root];
+  while (walk.length) {
+    const id = walk.shift()!;
+    const kids = spec[id] || [];
+    children.set(id, kids);
+    for (const k of kids) {
+      if (!depth.has(k)) { depth.set(k, depth.get(id)! + 1); walk.push(k); }
+    }
+  }
+  return { children, depth };
+}
+
+const flatFleet = (n: number) => {
+  const kids = Array.from({ length: n }, (_, i) => `sw${i}`);
+  return tree({ core: kids }, 'core');
+};
+
+describe('blockColumns', () => {
+  it('leaves small groups as a single row', () => {
+    expect(blockColumns(1)).toBe(1);
+    expect(blockColumns(6)).toBe(6);
+  });
+
+  it('wraps larger groups, favouring width over height', () => {
+    // Screens are landscape; a tall narrow column reads worse than a wide band.
+    expect(blockColumns(58)).toBe(10);
+    expect(blockColumns(20)).toBe(6);
+  });
+
+  it('never exceeds the ceiling', () => {
+    expect(blockColumns(10_000)).toBe(DEFAULT_LAYOUT.maxBlockCols);
+  });
+});
+
+describe('layoutTree — the #115 case', () => {
+  it('bounds the width of a shallow, wide network', () => {
+    // One core, 58 access switches. The old layout gave every leaf its own
+    // column: 58 slots, over ten thousand pixels, one unreadable row.
+    const { children, depth } = flatFleet(58);
+    const r = layoutTree('core', children, depth);
+    expect(r.widthSlots).toBe(10);
+    expect(r.widthSlots).toBeLessThan(58);
+    expect(r.maxRow).toBe(6); // traded width for a handful of rows
+  });
+
+  it('places every node exactly once, with no two on the same spot', () => {
+    const { children, depth } = flatFleet(58);
+    const r = layoutTree('core', children, depth);
+    expect(r.positions.size).toBe(59);
+    const seen = new Set([...r.positions.values()].map((p) => `${p.x}:${p.y}`));
+    expect(seen.size).toBe(59);
+  });
+
+  it('keeps children below their parent', () => {
+    const { children, depth } = flatFleet(58);
+    const r = layoutTree('core', children, depth);
+    const coreY = r.positions.get('core')!.y;
+    for (const [id, p] of r.positions) {
+      if (id !== 'core') expect(p.y).toBeGreaterThan(coreY);
+    }
+  });
+
+  it('centres the parent over its children', () => {
+    const { children, depth } = flatFleet(58);
+    const r = layoutTree('core', children, depth);
+    const xs = [...r.positions.entries()].filter(([id]) => id !== 'core').map(([, p]) => p.x);
+    const core = r.positions.get('core')!.x;
+    expect(core).toBeGreaterThanOrEqual(Math.min(...xs));
+    expect(core).toBeLessThanOrEqual(Math.max(...xs));
+  });
+});
+
+describe('layoutTree — shapes that must not regress', () => {
+  it('leaves a small tree as a plain row', () => {
+    const { children, depth } = tree({ core: ['a', 'b', 'c'] }, 'core');
+    const r = layoutTree('core', children, depth);
+    expect(r.widthSlots).toBe(3);
+    expect(r.maxRow).toBe(1);
+    expect(r.positions.get('a')!.y).toBe(1);
+    expect(r.positions.get('c')!.x).toBe(2);
+  });
+
+  it('preserves hierarchy where there genuinely is one', () => {
+    const { children, depth } = tree({
+      core: ['d1', 'd2'],
+      d1: ['a1', 'a2'],
+      d2: ['b1', 'b2'],
+    }, 'core');
+    const r = layoutTree('core', children, depth);
+    expect(r.positions.get('d1')!.y).toBe(1);
+    expect(r.positions.get('a1')!.y).toBe(2);
+    expect(r.positions.get('d1')!.x).toBeLessThan(r.positions.get('d2')!.x);
+    expect(r.maxRow).toBe(2);
+  });
+
+  it('handles a mix of branches and a wide leaf block', () => {
+    const leaves = Array.from({ length: 20 }, (_, i) => `l${i}`);
+    const { children, depth } = tree({ core: ['dist', ...leaves], dist: ['x', 'y'] }, 'core');
+    const r = layoutTree('core', children, depth);
+    expect(r.positions.get('dist')!.y).toBe(1);
+    expect(r.positions.get('x')!.y).toBe(2);
+    expect(r.widthSlots).toBe(2 + blockColumns(20));
+    expect(r.positions.get('l0')!.x).toBeGreaterThan(r.positions.get('dist')!.x);
+  });
+
+  it('handles a single isolated node', () => {
+    const { children, depth } = tree({ only: [] }, 'only');
+    const r = layoutTree('only', children, depth);
+    expect(r.widthSlots).toBe(1);
+    expect(r.maxRow).toBe(0);
+    expect(r.positions.get('only')).toEqual({ x: 0, y: 0 });
+  });
+
+  it('handles a deep chain without widening', () => {
+    const { children, depth } = tree({ a: ['b'], b: ['c'], c: ['d'], d: [] }, 'a');
+    const r = layoutTree('a', children, depth);
+    expect(r.widthSlots).toBe(1);
+    expect(r.maxRow).toBe(3);
+  });
+});

frontend/src/utils/topologyLayout.ts

@@ -0,0 +1,130 @@
+/**
+ * Tree layout for the topology map, in abstract slot/row units.
+ *
+ * The previous layout gave every leaf its own column, so a subtree's width was
+ * simply its leaf count. That is fine for a deep, branching network and useless
+ * for a real one: a core switch with fifty-eight access switches under it
+ * produced a single row fifty-eight slots wide — well over ten thousand pixels,
+ * which is what made the map unusable at scale (#115).
+ *
+ * Real networks are shallow and wide, so breadth has to stop translating
+ * directly into horizontal distance. Large groups of sibling leaves are laid out
+ * as a block instead of a row, which trades width for height and turns a linear
+ * growth curve into roughly a square-root one. Children with subtrees of their
+ * own keep the classic tidy-tree treatment, because there the hierarchy is the
+ * information and flattening it would lose meaning.
+ *
+ * Coordinates are slots and rows rather than pixels. The renderer owns the
+ * conversion, and this stays testable without a DOM.
+ */
+
+export interface LayoutOptions {
+  /** Sibling leaves beyond this are laid out as a block rather than a row. */
+  wrapThreshold: number;
+  /** Hard ceiling on block width, so one huge group cannot dominate the canvas. */
+  maxBlockCols: number;
+}
+
+export const DEFAULT_LAYOUT: LayoutOptions = { wrapThreshold: 6, maxBlockCols: 12 };
+
+export interface Placement {
+  /** Horizontal position in slots. Fractional for a parent centred over its children. */
+  x: number;
+  /** Vertical position in rows. */
+  y: number;
+}
+
+export interface LayoutResult {
+  positions: Map<string, Placement>;
+  /** Total width in slots, which is what the caller offsets the next component by. */
+  widthSlots: number;
+  /** Deepest row used, so callers can place anything that follows below it. */
+  maxRow: number;
+}
+
+/**
+ * Columns for a block of `n` sibling leaves.
+ *
+ * Slightly wider than tall — screens are landscape and a column of forty reads
+ * worse than a wide band of four rows.
+ */
+export function blockColumns(n: number, opts: LayoutOptions = DEFAULT_LAYOUT): number {
+  if (n <= opts.wrapThreshold) return Math.max(1, n);
+  return Math.max(1, Math.min(opts.maxBlockCols, Math.ceil(Math.sqrt(n * 1.6))));
+}
+
+/**
+ * Lay out one connected component rooted at `rootId`.
+ *
+ * `children` must describe a tree — the caller derives it by breadth-first walk,
+ * which is what makes cycles safe: a node is reached once and keeps the first
+ * parent that finds it.
+ */
+export function layoutTree(
+  rootId: string,
+  children: Map<string, string[]>,
+  depth: Map<string, number>,
+  opts: LayoutOptions = DEFAULT_LAYOUT,
+): LayoutResult {
+  const positions = new Map<string, Placement>();
+  const isLeaf = (id: string) => (children.get(id) || []).length === 0;
+
+  /** Width in slots, and how many rows the node's own leaf block occupies. */
+  const width = new Map<string, number>();
+  const blockRows = new Map<string, number>();
+
+  const measure = (id: string): number => {
+    const kids = children.get(id) || [];
+    if (!kids.length) { width.set(id, 1); blockRows.set(id, 0); return 1; }
+
+    const leaves = kids.filter(isLeaf);
+    const branches = kids.filter((k) => !isLeaf(k));
+
+    const branchWidth = branches.reduce((sum, k) => sum + measure(k), 0);
+    const cols = leaves.length ? blockColumns(leaves.length, opts) : 0;
+    const rows = leaves.length ? Math.ceil(leaves.length / cols) : 0;
+    blockRows.set(id, rows);
+
+    const total = Math.max(1, branchWidth + cols);
+    width.set(id, total);
+    return total;
+  };
+  measure(rootId);
+
+  let maxRow = 0;
+
+  const place = (id: string, left: number): void => {
+    const row = depth.get(id) ?? 0;
+    maxRow = Math.max(maxRow, row);
+
+    const kids = children.get(id) || [];
+    if (!kids.length) {
+      positions.set(id, { x: left, y: row });
+      return;
+    }
+
+    const leaves = kids.filter(isLeaf);
+    const branches = kids.filter((k) => !isLeaf(k));
+
+    // Branch subtrees first, left to right, so the block sits beside them.
+    let cursor = left;
+    for (const b of branches) {
+      place(b, cursor);
+      cursor += width.get(b)!;
+    }
+
+    // Then the block of leaf children, filling left to right, top to bottom.
+    const cols = leaves.length ? blockColumns(leaves.length, opts) : 0;
+    leaves.forEach((leaf, i) => {
+      const r = row + 1 + Math.floor(i / cols);
+      positions.set(leaf, { x: cursor + (i % cols), y: r });
+      maxRow = Math.max(maxRow, r);
+    });
+
+    // Centre the parent over everything beneath it.
+    positions.set(id, { x: left + (width.get(id)! - 1) / 2, y: row });
+  };
+  place(rootId, 0);
+
+  return { positions, widthSlots: width.get(rootId) || 1, maxRow };
+}

frontend/src/version.ts

@@ -1 +1 @@
-export const APP_VERSION = 'v0.23.21 Beta';
+export const APP_VERSION = 'v0.23.22 Beta';