Conversation
draw.io expects an edge's parent to be the nearest common ancestor of its source and target cells, but LLM-authored XML (and Mermaid subgraph conversion) leaves every edge on the default layer. Such a file renders correctly, but ELK/--layout lays it out wrongly. skills/drawio/scripts/fix_edge_parents.py is a transcription-close port of mxGraphModel.updateEdgeParent as bundled with draw.io v31.1.8. It modifies only byte ranges of affected attributes and keeps original files as much as possible. Both a bare mxGraphModel and an mxfile are handled. Compressed pages are handled recursively. Supports only UTF-8 and other ASCII- compatible encodings; UTF-16/32 is refused. Only the Python 3.9 standard library is used. SKILL.md is updated to run the script after generating diagrams, before running drawio --layout. claude-code, codex, copilot directories have identical copies of the script. check-skill-sync.yml now covers the script. plugins/tests/ is a stdlib unittest suite for the script: the parenting rule, geometry translation, file formats and encodings, and the CLI. test-scripts.yml runs it on Python 3.9 and 3.13. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Since this pull request is rather big and the script is non-trivial, feel free to close this and reimplement it yourself if that's easier. |
An mxGraph edge belongs to the nearest common ancestor of its terminals, and the editor's model keeps it that way: every edit runs mxGraphModel.updateEdgeParents. XML written by an LLM never does - it parks every edge on the layer, which is what our own XML reference asks for because it renders correctly and keeps the prompt simple. Renders correctly, lays out wrong. ELK reads an edge's coordinates in the frame of the node that contains it, so an edge filed on the layer while both its terminals sit inside a container comes back routed in the layer's frame and the connector jumps out of its container (#64, reported with a repro by taku0). Fixing that inside the layout would mean a layout run rewriting the cell hierarchy as a side effect, which is not something a layout may do with default settings. Normalizing the diagram before any pass touches it is, and it is exactly what the editor would have done to the same file: - mx-model.js moves to shared/ and grows the rest of the mxGraphModel slice updateEdgeParent needs - getRoot, getOrigin, isAncestor, getNearestCommonAncestor, add, mxGeometry.translate, updateEdgeParents - ported from drawio-dev, including draw.io's ignoreRelativeEdgeParent=false. setGeometry now compares by value, so a write that changes nothing (a translated clone, a converged layout re-run) leaves the element alone. - mx-xml.js is the XML <-> model layer lifted out of elk-pass.js, now shared by both passes: transformPages(xml, fn) parses each page, hands the transform a graph, and writes back ONLY the cells it changed. - edge-parents.js is the pass itself, and is where the reasoning lives. It needs no layout engine, is idempotent, and on taku0's test_01.drawio rewrites exactly one attribute. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The plugins write .drawio files themselves and hand them to the desktop CLI's --layout, so the MCP's edge-parent normalization never sees them: for that path the rule has to land in the prompt. It did not. Verified by running the skill headlessly (claude -p --plugin-dir) on taku0's repro from #64 and two more container diagrams, then laying the results out with the desktop CLI: - Today's published skill parks every edge on the layer, reproducing #64. - With the rule only in the XML reference's container section, two of three cases came out right. The third missed it because the agent skimmed the reference (grep -A 40 "container", sed -n 180,240p) and the rule sat at line 245. So it now also sits in the general principles near the top, and in SKILL.md itself — the skill body is always loaded in full, the reference is fetched and often skimmed. The rule is the real one, not the old simplification: the innermost container holding BOTH endpoints, at any nesting depth, which for two cells in different zones of one region is that region, not the layer. After that all three cases came out canonical - `normalizeEdgeParents` reports zero changes on each - and the laid-out diagrams keep every connector inside its container. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks for this — the repro made it easy to verify, and your diagnosis was exactly right: ELK reads an edge's coordinates in the frame of the cell that contains it, so an edge filed on the layer while both terminals sit inside one container gets laid out in the wrong frame. We ended up taking a different route for the fix, for two reasons: we'd rather not ship a second implementation of What's in now:
So we're going to close this PR in favour of those changes — not because the approach was wrong, but because the same algorithm now runs in draw.io's own code on every path. Thank you for the careful write-up and the test files; they're what made this quick to verify and fix. |
2 participants
Summary
ELK auto-layout (
--layout) misplaces the edges of the generated diagrams because thedrawio-mcpplugin sets theparentattributes to "1", the default layer, but ELK auto-layout expects the attributes to be the nearest common ancestors of their source and target cells.This PR adds a Python script to fix it. The script is a port of
mxGraphModel.updateEdgeParentfrom draw.io.Sample diagrams
Those diagrams are generated from the same prompt with the plugin from the
mainbranch and the branch of this PR. The diagram is converted to PNG with auto-layout bydrawio -x -f png -e -b 10 --layout verticalFlow. The styles varies since the prompt doesn't specify it.The
mainbranch:File before auto-layout: test_01.drawio
The branch of this PR:
File before auto-layout: test_02.drawio
See the
parentattribute of the edgee2of each.drawiofile.Modified files
.github/workflows/check-skill-sync.yml: Updated to check the Python scriptfix_edge_parents.pysynced in addition toSKILL.md..github/workflows/test-scripts.yml: (New file) A workflow to run tests (plugins/tests) forfix_edge_parents.py..gitignore: Added__pycache__and*.pyc.CLAUDE.md: Added descriptions regardingfix_edge_parents.pyand its tests.plugins/README.md: Added descriptions regarding thetestsdirectory.plugins/claude-code/DEVELOPING.md: Added an explanation forfix_edge_parents.py.plugins/claude-code/README.md: Added a step to runfix_edge_parents.pyto "How It Works".plugins/claude-code/skills/drawio/SKILL.md: Added an instruction to runfix_edge_parents.pyafter generating diagrams, and an explanation regarding theparentattribute of edges. Also added instructions in "Troubleshooting" to runfix_edge_parents.pywhen the edge layout is broken.plugins/claude-code/skills/drawio/scripts/fix_edge_parents.py: (New file) The script to fix theparentattribute of edges.plugins/codex/: Same asclaude-code.plugins/copilot/: Same asclaude-code.plugins/tests/: (New directory) The tests forfix_edge_parents.py.Why Python?
It has an XML library in the standard library. I chose Python 3.9 to support macOS.
Why byte-level editing?
Steps to reproduce the sample diagram
mkdir tmp cd tmp claude --plugin-dir /path/to/drawio-mcp/plugins/claude-code/Type the following prompt:
drawio -x -f png -e -b 10 --layout verticalFlow -o test.png test.drawioAbout use of coding agent
I used Claude Code but I reviewed the every lines of generated files and revised them thoroughly. I also revised the documentation and comments, but since I'm a non-native English speaker, they might still sound a bit AI-generated.