mirror of
https://github.com/DayuanJiang/next-ai-draw-io.git
synced 2026-09-01 17:10:24 +08:00
fix(diagram-engine): eliminate arrows drawn through boxes, complete the route search
A user's approval-workflow flowchart came out with the return arrow drawn straight through two unrelated steps, and a second report showed arrows leaving a box and bending straight back across it. Measured over 250 generated flowcharts (2722 edges): 347 arrows crossed an unrelated box and 215 waypoints landed inside a shape. Four defects, each measured in isolation: 1. The invisible layer containers draw_graph emits were handed to the router as frames, so every clean return path was rejected for "trespassing" on a border that is not drawn, and the fallback cut through two boxes. Excluding invisible containers: 347 -> 218 crossing arrows, no diagram made worse. 2. The router chose the horizontal-vs-vertical axis BEFORE searching, so when the only clean corridor ran along the other axis it was never looked at. A complete two-bend candidate generator that tries both trunk axes, all four sides at each end, and the port fractions: 218 -> 27. (An independent ablation measured the axis pre-choice alone at a 40% per-edge failure rate.) 3. Nothing stopped a route's first leg from turning back across its own source shape - the obstacle test exempts an edge's own endpoints, and must, since the line has to touch them. A terminal-leg rule refuses such routes outright: 215 -> 4 hooks. 4. A two-bend search cannot express the staircase needed when a box sits directly between two vertically aligned nodes (21 of the last 27 crossings). Added the orthogonal visibility graph + A* from Wybrow, Marriott & Stuckey, "Orthogonal Connector Routing" (GD 2009) - the libavoid algorithm - as the backstop when the candidate search finds nothing. The interesting-points grid is provably sufficient: any valid route shrinks onto it without getting longer or gaining bends. The A* state is (point, incoming direction) with libavoid's bend cost of 10, and the admissible bends-remaining heuristic, so it returns a cheapest route, not merely a route. Implemented from the paper, not ported. After all four: 0 crossing arrows and 0 hooks over the same 250 diagrams, page area unchanged (494k px^2 mean), 260ms for the whole corpus, mean 0.82 bends per edge. The shape ladder still runs first, so routes that were already clean are byte-identical. Also post-nudge validation now checks the whole path (the nudge pass only reverts the single segment it moved, judged in isolation) and restores the search's route if nudging made it dirty.
This commit is contained in:
dayuan.jiang
@@ -302,3 +302,143 @@ describe("drawGraph: the whole pipeline", () => {
|
||||
expect(r.warnings.join(" ")).toContain("b→a")
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* Two defects that both showed up as an arrow drawn over a box, on a flowchart with a
|
||||
* "send it back" loop in it. Neither was visible on the smaller flowcharts above: one
|
||||
* needs a back edge whose return path is lined with other steps, the other needs an edge
|
||||
* that skips over a layer holding more than one node.
|
||||
*/
|
||||
describe("graphToOperations: arrows do not run through boxes", () => {
|
||||
/** Ids of the real steps, excluding the engine's own layout cells. */
|
||||
const steps = (xml: string): string[] =>
|
||||
[...xml.matchAll(/<mxCell id="([^"]+)"[^>]*dai_kind=(?:box|icon);/g)]
|
||||
.map((m) => m[1])
|
||||
.filter((id) => !id.startsWith("__"))
|
||||
|
||||
it("routes a back edge around the steps it returns past", () => {
|
||||
// The reported case: "return for correction" goes back to "submit", and the two
|
||||
// steps between them are directly in the way. The engine wraps each layer in an
|
||||
// INVISIBLE container, and those used to be handed to the router as frames — so
|
||||
// every clean return path was rejected for "trespassing" on a frame that is not
|
||||
// drawn, and the fallback cut straight through both steps.
|
||||
const r = drawGraph(
|
||||
[
|
||||
{ id: "start", label: "Start", shape: "terminator" },
|
||||
{ id: "submit", label: "Submit Request", shape: "data" },
|
||||
n("validate", "Validate Input"),
|
||||
{ id: "valid", label: "Valid?", shape: "decision" },
|
||||
n("fix", "Return for Correction"),
|
||||
n("review", "Manager Review"),
|
||||
{ id: "end", label: "End", shape: "terminator" },
|
||||
],
|
||||
[
|
||||
e("start", "submit"),
|
||||
e("submit", "validate"),
|
||||
e("validate", "valid"),
|
||||
e("valid", "fix", "No"),
|
||||
e("fix", "submit"),
|
||||
e("valid", "review", "Yes"),
|
||||
e("review", "end"),
|
||||
],
|
||||
)
|
||||
expect(r.errors).toEqual([])
|
||||
const xml = r.xml as string
|
||||
const rects = absoluteRects(xml)
|
||||
expect(
|
||||
nodeCollisions(edgePaths(xml, rects), rects, steps(xml)),
|
||||
).toEqual([])
|
||||
})
|
||||
|
||||
it("keeps the placeholders out of the reported layers", () => {
|
||||
// They are an internal device for reserving space; a caller asked about its own nodes.
|
||||
const { layers } = graphToOperations(
|
||||
[n("a"), n("b1"), n("b2"), n("z")],
|
||||
[
|
||||
e("a", "b1"),
|
||||
e("a", "b2"),
|
||||
e("b1", "z"),
|
||||
e("b2", "z"),
|
||||
e("a", "z"),
|
||||
],
|
||||
)
|
||||
expect(layers.flat().filter((id) => id.startsWith("__"))).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* An arrow that leaves its own shape and immediately turns back across it.
|
||||
*
|
||||
* Reported on a request-handling flowchart: a short hook came out of the right side of
|
||||
* "Manager approval" and bent straight back over the box's own edge. The ordinary obstacle
|
||||
* test cannot see this — an edge is exempt from its own two endpoints, and it has to be,
|
||||
* since the line must touch them — so it is checked separately here.
|
||||
*/
|
||||
describe("graphToOperations: arrows leave their own shape cleanly", () => {
|
||||
/** Waypoints that land inside any box, which is what draws the hook. */
|
||||
const waypointsInsideBoxes = (xml: string): string[] => {
|
||||
const rects = absoluteRects(xml)
|
||||
const boxes = [
|
||||
...xml.matchAll(/<mxCell id="([^"]+)"[^>]*dai_kind=(?:box|icon);/g),
|
||||
]
|
||||
.map((m) => m[1])
|
||||
.filter((id) => !id.startsWith("__"))
|
||||
const bad: string[] = []
|
||||
for (const p of edgePaths(xml, rects))
|
||||
for (const w of p.points.slice(1, -1))
|
||||
for (const id of boxes) {
|
||||
const r = rects.get(id)
|
||||
if (!r) continue
|
||||
if (
|
||||
w.x > r.x + 1 &&
|
||||
w.x < r.x + r.w - 1 &&
|
||||
w.y > r.y + 1 &&
|
||||
w.y < r.y + r.h - 1
|
||||
)
|
||||
bad.push(`${p.source}→${p.target} bends inside ${id}`)
|
||||
}
|
||||
return [...new Set(bad)]
|
||||
}
|
||||
|
||||
it("does not bend an arrow back over the box it just left", () => {
|
||||
const r = drawGraph(
|
||||
[
|
||||
{
|
||||
id: "start",
|
||||
label: "Start: Request received",
|
||||
shape: "terminator",
|
||||
},
|
||||
n("intake", "Log request in system"),
|
||||
n("review", "Review details"),
|
||||
{ id: "complete", label: "Info complete?", shape: "decision" },
|
||||
{ id: "askinfo", label: "Request missing info", shape: "data" },
|
||||
{ id: "approve", label: "Approval needed?", shape: "decision" },
|
||||
n("manager", "Manager approval"),
|
||||
n("process", "Process request"),
|
||||
{ id: "report", label: "Generate report", shape: "document" },
|
||||
n("notify", "Notify requester"),
|
||||
{ id: "end", label: "End", shape: "terminator" },
|
||||
{ id: "reject", label: "Reject & close", shape: "round" },
|
||||
],
|
||||
[
|
||||
e("start", "intake"),
|
||||
e("intake", "review"),
|
||||
e("review", "complete"),
|
||||
e("complete", "askinfo", "no"),
|
||||
e("askinfo", "review", "resubmit"),
|
||||
e("complete", "approve", "yes"),
|
||||
e("approve", "manager", "yes"),
|
||||
e("approve", "process", "no"),
|
||||
e("manager", "process", "approved"),
|
||||
e("manager", "reject", "declined"),
|
||||
e("process", "report"),
|
||||
e("report", "notify"),
|
||||
e("reject", "notify"),
|
||||
e("notify", "end"),
|
||||
],
|
||||
{ title: "Sample Request Handling Workflow" },
|
||||
)
|
||||
expect(r.errors).toEqual([])
|
||||
expect(waypointsInsideBoxes(r.xml as string)).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user