feat(library): route and lay out straight-edge diagrams

Author: FelixTJDietrich

.changeset/curly-cats-rescue.md

@@ -0,0 +1,9 @@
+---
+"@tumaet/apollon": minor
+---
+
+Keep straight connections clear automatically. They now choose facing sides, spread
+sibling links across shared borders, and route around nodes and nearby connections
+without adding unnecessary bends; clear shots remain straight and hand-placed
+waypoints stay authoritative. Syntax trees also gain a one-click tidy layout that
+arranges valid parent-child forests while leaving malformed portions untouched.

library/lib/apollon-editor.tsx

@@ -405,6 +405,23 @@ export class ApollonEditor {
     requestAnimationFrame(attempt)
   }
 
+  /**
+   * Arrange a syntax-tree diagram into a tidy hierarchical layout, so straight
+   * parent→child links no longer overlap sibling nodes (issue #282). Derives the
+   * hierarchy from `SyntaxTreeLink` edges; malformed graphs (forests, cycles,
+   * multi-parent) are laid out where clean and left untouched elsewhere. It is a
+   * single undo step and a no-op on non-syntax-tree diagrams.
+   */
+  public layoutSyntaxTree(): void {
+    if (
+      this.metadataStore.getState().diagramType !== UMLDiagramType.SyntaxTree
+    ) {
+      return
+    }
+    this.diagramStore.getState().layoutSyntaxTree()
+    this.fitView()
+  }
+
   // ---- Canvas overlay / control API -------------------------------------
   // A library-owned overlay engine: host chrome (header, rails, banners) and the
   // editor's own overlays share one measured, inset-aware layout. Controls

library/lib/chrome/builtins/ZoomControls.tsx

@@ -1,6 +1,7 @@
 import { useReactFlow, useStore } from "@xyflow/react"
 import { useShallow } from "zustand/shallow"
 import {
+  ListTree,
   Maximize,
   Redo2,
   SquareMousePointer,
@@ -13,6 +14,8 @@ import {
   useMetadataStore,
   useOverlayStore,
 } from "@/store/context"
+import { useDiagramModifiable } from "@/hooks/useDiagramModifiable"
+import { UMLDiagramType } from "@/types"
 import { insetAwareFitView } from "@/overlay/fitView"
 import { ariaKeyshortcuts } from "@/keyboard"
 import { Tooltip } from "@/components/ui"
@@ -48,12 +51,17 @@ export function ZoomControls({ history = true }: ZoomControlsProps) {
     }))
   )
 
-  const { multiSelectionMode, setMultiSelectionMode } = useMetadataStore(
-    useShallow((state) => ({
-      multiSelectionMode: state.multiSelectionMode,
-      setMultiSelectionMode: state.setMultiSelectionMode,
-    }))
-  )
+  const { multiSelectionMode, setMultiSelectionMode, isSyntaxTree } =
+    useMetadataStore(
+      useShallow((state) => ({
+        multiSelectionMode: state.multiSelectionMode,
+        setMultiSelectionMode: state.setMultiSelectionMode,
+        isSyntaxTree: state.diagramType === UMLDiagramType.SyntaxTree,
+      }))
+    )
+  const layoutSyntaxTree = useDiagramStore((state) => state.layoutSyntaxTree)
+  const isModifiable = useDiagramModifiable()
+  const showTidyLayout = isSyntaxTree && isModifiable
 
   const { ref: toolbarRef, onKeyDown: onToolbarKeyDown } =
     useRovingToolbar<HTMLDivElement>()
@@ -124,6 +132,18 @@ export function ZoomControls({ history = true }: ZoomControlsProps) {
             <SquareMousePointer width={18} height={18} aria-hidden="true" />
           </button>
         </Tooltip>
+        {showTidyLayout && (
+          <Tooltip title={t.tidyLayoutHint}>
+            <button
+              type="button"
+              className="apollon-chrome-iconbtn"
+              onClick={() => layoutSyntaxTree()}
+              aria-label={t.tidyLayout}
+            >
+              <ListTree width={18} height={18} aria-hidden="true" />
+            </button>
+          </Tooltip>
+        )}
       </div>
 
       {history && undoManagerExist && (

library/lib/components/ConnectionPreviewLine.tsx

@@ -124,6 +124,20 @@ export const ConnectionPreviewLine = ({
       nativeTargetId !== undefined &&
       nativeTargetId !== fromNodeId &&
       nativeTargetPosition !== undefined
+    // A valid native handle is an exact attachment point, not merely a side hint.
+    // Resolve that point into the pending edge too, so the central solver cannot
+    // replace the native ghost endpoint with an automatic facing-side anchor.
+    const nativeDropTarget = hasNativeTarget
+      ? resolveDropTarget({ x: toX, y: toY }, fromNodeId)
+      : null
+    const nativeAnchor =
+      nativeDropTarget && nativeDropTarget.id === nativeTargetId
+        ? getEdgeAnchorFromPoint(
+            nativeDropTarget.type,
+            { x: toX, y: toY },
+            nativeDropTarget.rect
+          )
+        : null
     const target = hasNativeTarget
       ? null
       : resolveDropTarget(pointer, fromNodeId)
@@ -159,7 +173,8 @@ export const ConnectionPreviewLine = ({
         : freeformTarget
           ? getSideHandleIdForPosition(freeformTarget.position)
           : undefined,
-      targetAnchor: hit && dropAnchorIsAimed(hit.type) ? anchor : null,
+      targetAnchor:
+        nativeAnchor ?? (hit && dropAnchorIsAimed(hit.type) ? anchor : null),
       snapPoint: freeformTarget?.showSnapCircle ? freeformTarget.point : null,
       visible: draggedFar || hasNativeTarget || freeformTarget !== null,
     }

library/lib/edges/GenericEdge.tsx

@@ -528,8 +528,10 @@ export const EdgeBendHandle = ({
  * handle on the point itself, not the step edge's elongated segment pill, and it
  * carries a `move` cursor because it travels in two dimensions rather than one.
  *
- * Every authored interior vertex gets one. Segment midpoints carry the same opaque
- * handle as step-edge bendable segments; dragging one materialises a waypoint.
+ * Every interior vertex of the rendered route gets one, whether the user placed it
+ * or the router did: dragging an automatic bend is how you take ownership of a route
+ * the solver chose. Segment midpoints carry the same opaque handle as step-edge
+ * bendable segments; dragging one materialises a waypoint.
  */
 export const EdgeWaypointHandles = ({
   route,
@@ -542,7 +544,7 @@ export const EdgeWaypointHandles = ({
 }: {
   /** Full route `[source, ...interior, target]`. */
   route: IPoint[]
-  /** The editable authored interior vertices. */
+  /** The editable interior vertices — every bend on the rendered route. */
   interior: IPoint[]
   selectedWaypointIndex: number | null
   onWaypointPointerDown: (

library/lib/hooks/useConnect.ts

@@ -219,7 +219,40 @@ export const useConnect = () => {
   const onConnectEnd: OnConnectEnd = useCallback(
     (event, connectionState) => {
       try {
-        if (!connectionState.isValid) {
+        if (connectionState.isValid) {
+          // A valid native handle is an exact attachment point, not merely a side
+          // hint. Persist the preview's resolved anchor on commit; otherwise the
+          // central solver can immediately move the endpoint to an automatic seat.
+          const edgeId = pendingConnectionId.current
+          const dropPosition = getDropPosition(event)
+          const nodeOnTop = resolveDropTarget(
+            dropPosition,
+            connectionState.fromNode?.id
+          )
+          if (edgeId && nodeOnTop) {
+            const anchor = getEdgeAnchorFromPoint(
+              nodeOnTop.type,
+              dropPosition,
+              nodeOnTop.rect
+            )
+            if (anchor) {
+              const endpoint =
+                connectionStartParams.current?.handleType === "target"
+                  ? "source"
+                  : "target"
+              setEdges((eds) =>
+                eds.map((edge) =>
+                  edge.id === edgeId
+                    ? {
+                        ...edge,
+                        data: withEndpointAnchor(edge.data, endpoint, anchor),
+                      }
+                    : edge
+                )
+              )
+            }
+          }
+        } else {
           const dropPosition = getDropPosition(event)
           const nodeOnTop = resolveDropTarget(
             dropPosition,

library/lib/hooks/useStraightPathEdge.ts

@@ -386,41 +386,23 @@ export const useStraightPathEdge = ({
     }),
     [adjustedTargetCoordinates.targetX, adjustedTargetCoordinates.targetY]
   )
-  // The authored route is source → interior waypoints → target. Automatic
-  // straight-edge routing is deliberately a separate concern: this branch
-  // renders only user intent and the endpoint preview already present on main.
+  // The synchronous truth: source → interior waypoints → target. Used as the
+  // fallback before the solver's route lands and whenever the solver route is
+  // stale (its endpoints no longer match, e.g. mid node-move) so the edge never
+  // detaches from its nodes.
   const basePoints = useMemo<IPoint[]>(
     () => [sourceEndpoint, ...interiorPoints, targetEndpoint],
     [sourceEndpoint, interiorPoints, targetEndpoint]
   )
-  const centralPreviewMatchesCommit =
-    dragPreviewPoints !== null &&
-    endpointPreviewCommit !== null &&
-    centralRoute !== undefined &&
-    centralRoute.length >= 2 &&
-    (endpointPreviewCommit.endpoint === "source"
-      ? centralRoute[0].x === endpointPreviewCommit.sourceEndpoint.x &&
-        centralRoute[0].y === endpointPreviewCommit.sourceEndpoint.y
-      : centralRoute[centralRoute.length - 1].x ===
-          endpointPreviewCommit.targetEndpoint.x &&
-        centralRoute[centralRoute.length - 1].y ===
-          endpointPreviewCommit.targetEndpoint.y)
+  // The solver's committed route is the source of truth: its endpoints are the
+  // facing-side attachment sites the port assignment chose (not the drawn handle),
+  // and it carries any automatic obstacle-avoidance bends. Fall back to the analytic
+  // base polyline only before the first solve lands (never painted after that).
   const renderPoints = useMemo<IPoint[]>(
     () =>
-      centralPreviewMatchesCommit
-        ? [
-            centralRoute[0],
-            ...interiorPoints,
-            centralRoute[centralRoute.length - 1],
-          ]
-        : (dragPreviewPoints ?? basePoints),
-    [
-      basePoints,
-      centralPreviewMatchesCommit,
-      centralRoute,
-      dragPreviewPoints,
-      interiorPoints,
-    ]
+      dragPreviewPoints ??
+      (centralRoute && centralRoute.length >= 2 ? centralRoute : basePoints),
+    [basePoints, centralRoute, dragPreviewPoints]
   )
   const renderSourcePosition =
     dragPreviewPositions?.sourcePosition ?? resolvedSourcePosition
@@ -505,8 +487,10 @@ export const useStraightPathEdge = ({
 
   const sourcePoint = renderPoints[0]
   const targetPoint = renderPoints[renderPoints.length - 1]
-  // Every authored bend is editable. Automatic detours are introduced by the
-  // separate auto-layout/routing change and can later reuse the same controls.
+  // Every bend on the RENDERED route is editable, not only the authored ones. An
+  // automatic detour is a perfectly good starting point for a hand-placed route:
+  // dragging one of its bends is how the user takes ownership of it, exactly as
+  // dragging a computed step edge freezes its path into manual points.
   const editableWaypoints = useMemo<IPoint[]>(
     () => (dragHandleRoute ?? renderPoints).slice(1, -1),
     [dragHandleRoute, renderPoints]
@@ -777,9 +761,11 @@ export const useStraightPathEdge = ({
     ]
   )
 
-  // Persist the interior waypoints and pin their visible endpoints when the first
-  // bend is authored. This mirrors the step-edge bend-commit behaviour and keeps
-  // later geometry updates from moving a hand-shaped route at its ends.
+  // Persist the interior waypoints. A bend customises the whole visible route,
+  // including the facing-side attachment sites the port assignment chose, so on the
+  // first bend the endpoints are pinned to `pinSource`/`pinTarget` (when still
+  // automatic) — otherwise a newly-bent edge would snap its endpoints back to the
+  // drawn handle. Mirrors the step-edge bend-commit behaviour.
   const commitWaypoints = useCallback(
     (nextInterior: IPoint[], pinSource: IPoint, pinTarget: IPoint) => {
       setEdges((edges) =>
@@ -825,7 +811,8 @@ export const useStraightPathEdge = ({
 
   // Shared drag routine for both an existing waypoint and a freshly materialised
   // one. `startInterior` is the interior array the drag operates on; `index` is the
-  // waypoint being moved. Only pointer-up commits `data.points`.
+  // waypoint being moved. The live route is published so neighbouring step edges
+  // reflow around the dragged diagonal, and only pointer-up commits `data.points`.
   const beginWaypointDrag = useCallback(
     (
       pointerId: number,
@@ -839,8 +826,10 @@ export const useStraightPathEdge = ({
       dragInteriorRef.current = startInterior
       dragMovedRef.current = false
       dragCollapseRef.current = false
-      // Capture the endpoints at gesture start so the preview and eventual commit
-      // pivot around stable attachment sites.
+      // The endpoints the drag pivots around are the CURRENTLY RENDERED attachment
+      // sites (the solver's facing-side ports), captured at gesture start — not the
+      // drawn handle — so the route and the pinned commit keep the edge attached
+      // exactly where it is on screen.
       const routeSource = sourcePoint
       const routeTarget = targetPoint
       const collapseTolerance =
@@ -851,7 +840,8 @@ export const useStraightPathEdge = ({
       const angleReference = routeAtStart[index + 2] ?? routeAtStart[index]
 
       // Drive the preview through state; the existing layout effect republishes it
-      // to shared edge geometry and clears it when the drag ends.
+      // as an authoritative live override so neighbouring step edges reflow around
+      // the dragged diagonal, and clears it when the drag ends.
       const publish = (
         pathInterior: IPoint[],
         handleInterior: IPoint[] = pathInterior

library/lib/i18n/labels.ts

@@ -21,6 +21,10 @@ export interface ApollonLabels {
   redoHint: string
   multiSelection: string
   multiSelectionHint: string
+  /** Syntax-tree tidy-layout button (accessible name). */
+  tidyLayout: string
+  /** Syntax-tree tidy-layout tooltip. */
+  tidyLayoutHint: string
   /** Accessible name for a straight-edge segment midpoint create handle. */
   addEdgeWaypoint: string
   /** Accessible name for an authored straight-edge waypoint handle. */
@@ -311,6 +315,8 @@ export const DEFAULT_LABELS: ApollonLabels = Object.freeze<ApollonLabels>({
   redoHint: "Redo (Ctrl+Y or Ctrl+Shift+Z)",
   multiSelection: "Select multiple elements",
   multiSelectionHint: "Select multiple: click elements to add or remove",
+  tidyLayout: "Tidy tree layout",
+  tidyLayoutHint: "Arrange the syntax tree so links no longer overlap nodes",
   addEdgeWaypoint: "Drag to add a waypoint",
   moveEdgeWaypoint:
     "Waypoint: drag to move, double-click or press Delete to remove",

library/lib/store/diagramStore.ts

@@ -19,6 +19,7 @@ import {
   STORE_ORIGIN,
 } from "@/sync/ydoc"
 import { recordStoreNodeWrite } from "@/sync/perfCounters"
+import { computeTidyLayout } from "@/utils/tidyTree"
 import { deepEqual } from "@/utils/storeUtils"
 import { Assessment, DraggingNode, InteractiveElements } from "@/typings"
 import {
@@ -153,6 +154,9 @@ export type DiagramStore = {
    */
   endTransientNodeBroadcast: () => void
   setNodes: (payload: Node[] | ((nodes: Node[]) => Node[])) => void
+  /** Reposition syntax-tree nodes into a tidy hierarchical layout (issue #282).
+   * A single undo step; a no-op on non-syntax-tree or already-tidy diagrams. */
+  layoutSyntaxTree: () => void
   setEdges: (payload: Edge[] | ((edges: Edge[]) => Edge[])) => void
   setNodesAndEdges: (nodes: Node[], edges: Edge[]) => void
   addEdge: (edge: Edge) => void
@@ -503,6 +507,13 @@ export const createDiagramStore = (
             )
           },
 
+          layoutSyntaxTree: () => {
+            // Tidy the syntax tree by repositioning nodes (issue #282). Routed
+            // through `setNodes`, so it is a single Yjs transaction / one undo step
+            // and the deep-equal guard makes an already-tidy layout a no-op.
+            get().setNodes(computeTidyLayout(get().nodes, get().edges))
+          },
+
           setEdges: (payload) => {
             const edges =
               typeof payload === "function" ? payload(get().edges) : payload