Skip to content

diagram: adjustFlows crashes on degenerate self-loop flow (both ends attached to the same stock) #720

Description

@bpowers

Problem

adjustFlows in src/diagram/drawing/Flow.tsx (called from UpdateCloudAndFlow, also called directly on stock moves) crashes when handed a degenerate self-loop flow: a flow whose first and last points are both attached to the same stock (attachedToUid === stock.uid for both endpoints).

The function walks the flow's endpoints and only records otherEnd when it finds an endpoint that is not attached to stock:

// src/diagram/drawing/Flow.tsx:497-554
let otherEnd: IPoint | undefined;
const points = flow.points.map((point, i) => {
  ...
  if (point.attachedToUid !== stock.uid) {
    otherEnd = point;
    return point;
  }
  ...
});

otherEnd = defined(otherEnd);   // line 554 -- throws if both ends matched stock.uid

If both endpoints are attached to the same stock, otherEnd is never assigned, and defined(otherEnd) at line 554 throws. Because adjustFlows runs on the canvas render/interaction path (stock drag, cloud+flow adjustment via UpdateCloudAndFlow), the throw crashes the canvas. The diagram package's ErrorBoundary only catches render-phase throws, so depending on where it fires this can take down the editor view.

Why it matters

  • Robustness / correctness: A single malformed flow renders the whole model un-openable in the editor rather than degrading gracefully.
  • This is latent and pre-existing -- it was discovered during a refactor of the flow attachment logic in src/diagram, not introduced by it.

How it can be reached

Such a degenerate flow cannot be produced through normal editor creation/reattachment interactions, but it can arrive via:

  • Imported models (XMILE / Vensim) where a flow's source and sink resolve to the same stock.
  • Programmatic patches (MCP tools, API edits) that construct a flow with both endpoints attached to one stock.

Component(s) affected

  • src/diagram -- src/diagram/drawing/Flow.tsx (adjustFlows, UpdateCloudAndFlow)
  • Potentially the import path (@simlin/core / engine) if validation/normalization is the chosen fix location.

Possible approaches

  1. Tolerate the degenerate case in adjustFlows: if otherEnd is never found, return the flow unchanged (early-out) instead of calling defined(otherEnd). Cheapest, keeps the canvas resilient to any malformed input.
  2. Validate / normalize on import: reject or repair flows whose endpoints attach to the same stock when loading XMILE/Vensim or applying programmatic patches, so the bad shape never reaches the renderer.

Approach 1 (defensive at the render boundary) and approach 2 (validate at the data boundary) are complementary; defense-in-depth would do both.

Context

Identified during a refactor of src/diagram flow attachment logic. Verified the crash site exists at src/diagram/drawing/Flow.tsx:554. Distinct from existing flow/layout issues: #51 (multi-part/bent flows), #110 (flows invisible on creation when dragged up/left), #445 and #446 (layout-engine face/side classification in the Rust layout/mod.rs).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions