Skip to content

feat(view): import an edited draw.io file back as DiagramLayout metadata - #1679

Merged
HuiJun merged 30 commits into
developfrom
feat/drawio-import
Oct 29, 2026
Merged

HuiJun merged 30 commits into
developfrom
feat/drawio-import

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

What and why

The drawio rendering form (#1668) exports a view as an editable diagrams.net / draw.io file whose semantic cells carry the element's qualified name. This adds the way back: read a draw.io file the user edited and write the geometry into the model as DiagramLayout metadata (Layout, Route, Canvas, Style, Note, Picture) through the existing model-edit path, so the positions land in the .sysml text as reviewable metadata Layout about … lines, exactly as a drag in the VS Code panel does.

Reader (internal/ir/view/drawio_import.go): ReadDrawio parses <mxfile> with encoding/xml, accepting both the uncompressed <diagram><mxGraphModel> form the writer emits and the compressed form draw.io desktop saves (<diagram> text = base64 → raw deflate → percent-decoded XML). Multi-page files: the page whose cells' view attribute names the requested view, a typed *DrawioViewError on none or several. Container-relative geometry is resolved to absolute coordinates (a Layout is absolute). Malformed input yields a typed *DrawioFileError, never a panic.

Matcher and diff (internal/ir/view/drawio_import_diff.go): Rendering.ImportDrawio renders the view, writes it with the writer under the same options, and diffs the edited file against that baseline cell by cell. Nodes and ports pair by fqn (and ordinal, when a view draws the same element twice, as a tree does an inherited member); edges by the names of their ends + kind + ordinal — the writer's e<i> ids are an order, not an identity; notes and pictures by position. Only a cell that differs from what the writer would emit now becomes an edit: moved/resized/collapsed node → Layout, re-routed edge → Route (a straightened edge clears it), restyled cell → Style, resized page → Canvas, moved note/picture → Note/Picture. Positions equal within 0.5 px are unchanged, so an untouched export imports zero edits — and the routes the writer computes itself never become annotations unless the user moved them.

Reported, not imported: cells with no fqn the export did not write (shapes drawn in draw.io — nothing creates model elements), semantic cells the file no longer holds (nothing deletes elements), fqns matching no element (renamed or gone), moved edge labels (a Route places a line, not its label; label positions have no annotation yet) and a port moved along its owner's border (ports are drawn there by the renderer).

Edits (internal/check/edit/drawio.go): DrawioOperations turns an import into Operations for the existing SetLayout/SetRoute/SetCanvas/SetStyle and the new SetNote/SetPicture (also on the LSP's applyModelEdit and the MCP's apply_model_edit). A member drawn under a node of another element — a part of the usage's type, an inherited state — is named by the path a Layout resolves through (Loop::sensor::waterOut, not Pump::waterOut), so moving one inheritor moves that one.

Fixed on the way, each with its regression test: a member drawn under an inheriting node was placed and coloured by the first Layout about its declaration rather than the one naming it as that node's (inheritedGeometryOf); a Layout about the declaring body's start grew a start in every body inheriting it, at the same absolute place; a filter verdict, and the resolver's kept enumeration of a filtered expose, reached while the about index was still being built were cached (a view exposing P::*[@Tag] and annotating one of its elements in its own body never exposed it); the edit refused a Layout about a member drawn under an exposed usage as "not exposed".

Surfaces. sysml <model…> -import-diagram <file.drawio> [-view <name>] [-dry-run] prints the planned edits and what is reported; with -dry-run a unified diff per document and no write, otherwise the changed documents are staged beside themselves and swapped in as one set (a failure restores every original; a symlinked model is edited where the link points; an unchanged document is left alone). LSP opensysml/importDiagram {textDocument, path or content, view, inline, dryRun} answers the same WorkspaceEdit the panel's edits produce, pinned to the documents the view was rendered from (stale when they moved). MCP import_diagram wraps it (file under the workspace root or inline content; apply writes, otherwise a dry run) with its catalog row and man page. VS Code opensysml.importDiagram ("Import layout from draw.io…") opens a file picker and applies the WorkspaceEdit, so undo works. %render has no REPL twin that writes a file, so the REPL grows no meta-command; the CLI applies through repl.Session.ApplyModelEdit.

develop merged in (merge commits, no rebase), last at b173793: #1677's edge routing (routed edges carry edgeStyle=none, exit/entry anchors and their bends as waypoints; an untouched routed export still imports zero edits; no golden re-recorded; the draw.io desktop fixture saved again by draw.io desktop 32.6.1 against the routed export), #1671/#1659 at 14e53c0 (clean), and #1676's go.lsp.dev 1.0 migration with #1660 at b173793, which conflicted in internal/frontend/lsp/{modeledit.go,modeledit_cross_document_test.go,server.go}: the import request was ported to the 1.0 handler shape (decodeParams, (any, error), []protocol.DocumentChange) and the MCP round trip accepts the pinned-document entries ApplyEdit now lists. A second merge, at a57a863, brought in #1681–#1687: #1687's inOrder wrapper in internal/frontend/lsp/server.go (the import handler now sits inside the ordered chain, under CancelHandler), and #1683's draw.io writer follow-ups, whose paragraph on fqn in docs/project/view-rendering-forms.md was taken as develop has it with this branch's start/done clause re-applied; no golden moved. #1688 (fix/view-layout-sibling-gap, head 9806ff4, 34 draw.io goldens re-recorded for a 50 px grid gap) dry-merges clean on this head: six goldens are touched by both (action, mixed, state, state-entry, typed-action, typed-state) and merge textually without conflict, since this branch only adds fqn attributes and #1688 only moves geometry; the round-trip test reads the goldens as both export and expected import, so whichever lands second re-runs go test ./internal/ir/view ./internal/check/edit -run Drawio on the merged tree before merging. A third merge, at eb96507 and then ade0359 (clean, no conflicts), brought #1688 and #1689 in. #1688's 50 px grid gap moved the layout golden (the loop now at (50, 170), was (40, 160)), so two tests that had hard-coded the old geometry were re-derived rather than re-numbered: TestDrawioImportMovedNodeIsALayout reads the loop's exported geometry and TestImportDiagramPortsRoundTripSilently the exported bends; the compressed draw.io desktop fixture layout-desktop.drawio was saved again by draw.io desktop 32.6.1 against the new export (tank dragged from (266, 262) to (430, 262)), as it was after #1677. Referee baselines re-recorded with pilot-apijson-diff -update and pilot-diff -update (never by hand), and every moved object is accounted for: in both baselines provenance.inputs[examples].digest moved (bdb2697a… → 710f0a45…) because this branch's self-model edit adds the opensysml.importDiagram line to examples/self-model/surfaces.sysml — a first re-record on develop eb96507 moved that digest and the recording date alone, no count or scorecard row; and provenance.inputs[opensysml-libraries].digest moved (ac6a1e3b… → 29a3bf0f…) because #1689 changed DocumentQueries.sysml and the stdlib snapshot without re-recording, which is why bare develop ade0359 fails the same two provenance tests. #1689's library change is also the only source of row movement, all in the api-json baseline: pilotLibraryElements +198 in total (examples +77, manual-examples +121 — the two roots whose models load the OpenSysML Libraries), properties.presentEqual −68 / presentDifferent +68 in the same two roots, and the scorecard rows InvocationExpression.input (368/96 → 334/130) and InvocationExpression.parameter (382/98 → 348/132). The pilot-differential baseline moved only the two digests and the date. When develop's own re-record for #1689 lands, this branch takes develop's two baseline files and re-runs both -update flags on top.

How it was verified

  • TestDrawioImportUntouchedGoldenIsNoEdit: every one of the 35 .drawio.golden fixtures imported as is yields zero edits and nothing reported.
  • Mutation tests over the layout fixture (a placed view with a Layout, a Route, a Canvas, a Style and notes): moved node → absolute Layout; move under half a pixel → no edit; resized node keeps its height; nested child resolves to its container's offset and a port keeps its relative geometry; re-routed edge → Route with the waypoints; straightened edge → clear; recoloured node → Style with the fill; resized page → Canvas; moved note → Note; moved picture → Picture with its Origin; a renamed fqn, a deleted cell and a hand-drawn shape are reported without an edit.
  • TestDrawioRoundTrip (internal/check/edit): for every golden, the untouched export imports nothing; a move, a container move, a resize, a re-route, a recolour each import as exactly that edit, are applied through Apply, and the re-exported drawio draws the mutated cell as the file had it (cells found by identity, since ids renumber); an edge no Route steers (specialization, typing, containment) is reported rather than imported.
  • TestReadDrawioCompressedDesktopSave: the compressed save draw.io desktop 32.6.1 wrote (testdata/layout-desktop.drawio) reads and imports as the one Layout of the dragged tank, re-parented to the page by the app.
  • Surfaces: TestImportDiagram* in cmd/sysml (dry-run diff and no write, write and idempotent re-import, -import-diagram '' and flag misuse refused, symlinked model edited through the link while an unchanged sibling keeps its inode and mtime), TestImportDiagramAnswersTheLayoutsAsAWorkspaceEdit (LSP), TestImportDiagramRoundTripsARenderedView and TestImportDiagramReadsFilesUnderTheRootAlone (MCP; dry run, apply, the file re-read, a path outside the root refused), the catalog gates (TestRegisteredToolsMatchTheCatalog, TestTheCatalogIsWellFormed, TestEverySysmlFlagIsClassifiedForMCP), TestUnifiedDiffInsertionRanges, VS Code manifest tests (npm test, 420 passing, npm run typecheck).
  • go build ./..., go vet ./..., gofmt -l . empty, go test ./... (all packages), make lint (staticcheck, gosec), make man-check, make docs (strict), python3 scripts/changelog.py check, on the tree with develop b173793 merged.

MCP over stdio, as recorded

sysml-mcp -stdio -root /tmp/mcpx over a workspace holding one plant.sysml (Plant::Loop with parts pump and spare, view loopView { expose Loop; render asInterconnectionDiagram; }); request bodies abridged to what matters.

→ {"method":"initialize","params":{"protocolVersion":"2025-06-18",...},"id":1}
→ {"method":"notifications/initialized"}
→ {"method":"tools/call","params":{"name":"render_view","arguments":{"path":"plant.sysml","view":"Plant::loopView","form":"drawio"}},"id":2}
← artifact: 1846 bytes of uncompressed draw.io; the cell with fqn="Plant::Loop::pump" has `<mxGeometry x="16" y="82" width="54" height="37"/>`
   (edit: the pump's y 82 → 152, as a drag in draw.io would)
→ {"method":"tools/call","params":{"name":"import_diagram","arguments":{"path":"plant.sysml","content":"<?xml version=\"1.0\" …"}},"id":3}
← Plant::loopView: 3 edits planned, 0 refused, edits to 1 documents
   Layout about Plant::Loop
   Layout about Plant::Loop::pump
   Layout about Plant::Loop::spare
→ {"method":"tools/call","params":{"name":"import_diagram","arguments":{"path":"plant.sysml","content":"…","apply":true}},"id":4}
← Plant::loopView: 3 edits planned, 0 refused, edits to 1 documents, written

plant.sysml afterwards:

	view loopView {
		expose Loop;
		render asInterconnectionDiagram;
		metadata DiagramLayout::Layout about Plant::Loop { x = 40; y = 40; width = 189; height = 135; }
		metadata DiagramLayout::Layout about Plant::Loop::pump { x = 56; y = 192; width = 54; height = 37; }
		metadata DiagramLayout::Layout about Plant::Loop::spare { x = 150; y = 122; width = 63; height = 37; }
	}

Two notes from a reviewer's independent run. A Style the import writes reads metadata DiagramLayout::Style about PumpStation::station::pump { … } beside a hand-written Layout about station::tank: the editor spells the target as the operation names it, and both the panel's setStyle (from nodes[].fqn) and the import pass the qualified name the rendering reports, so this is the form develop already writes; both spellings resolve to the same element. The CLI has no -inline: -import-diagram writes view-local annotations only (the panel's default), while the LSP request and MCP tool take inline; the CLI reference says so.

Three edits for one drag: the view had no Layout yet, so the auto-placed container and its other child are pinned where the export drew them, else re-exporting would lay them out afresh around the moved pump. Importing the same file again plans nothing.

Checklist

  • make test and make lint pass locally
  • Tests added or updated for the change
  • Documentation extended where it already covers the surface (see CONTRIBUTING.md)
  • Changelog entry added as changes/unreleased/<slug>.<section>.md, not as an edit to CHANGELOG.md
  • baselines regenerated and make docs-counts run if a gate count moved (no gate count moved)
  • No internal work-item labels (waves, slices, F4, K5) in the body, docs, or changelog

Add the draw.io reader (uncompressed and compressed pages, page selection
by view, container-relative geometry resolved to absolute coordinates) and
the matcher that diffs an edited file against the writer's own export of
the same rendering: nodes and ports pair by qualified name and ordinal,
edges by their ends, kind and ordinal, notes and pictures by position, and
only cells that differ within half a pixel become Layout, Route, Style,
Canvas, Note or Picture edits. Pictures now carry their declaration's
Origin so a moved picture can be written back.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration
devin-ai-integration Bot marked this pull request as ready for review October 29, 2026 04:25
The layout golden as draw.io desktop 32.6.1 saved it compressed after a
drag of the tank, which it re-parented to the page; malformed files yield
typed errors; multi-page files select the page by view; cells without an
element stay apart from the import.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 2 commits October 29, 2026 04:47
… member name

A cell keeps its exported id while it stands for the same element, so a
reordered tree of inherited members or a bundle of parallel edges keeps its
identity; the name, or the ends and kind, pair what the id no longer does.
A start or done no declaration names is written as the member its owner's
body knows (Machines::VehicleStates::start), which a Layout, Style or Note
may now be stated about through the inheriting owner. A recoloured port
and a re-anchored note are reported, a page inflating past 64 MiB refused,
and a page emptied of its cells is of the view its name states.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…io import found

SetNote, SetNoteAt, SetPicture and SetPictureAt join the layout operations,
reaching an annotation stated many times by its own declaration; the LSP
exposes them as setNote and setPicture and the MCP tool describes them.
DrawioOperations turns an import's edits into these operations, naming each
element or declaring it where no name reaches, inline then. The import
reads a node's Layout in the frame the writer drew it in, pinning a
container the model does not place when a child inside it moves.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

…ng built

A view exposing through a filter reads each candidate's annotations while
the about usages are still being indexed, and cached what it read: a Layout
stated about such an element in the view's body was never found. Answers
given during the build are no longer kept. The draw.io import pairs every
cell before diffing any, so pinning a container reads a child's edited cell
whatever id the file gives it, and a pinned container keeps the size the
file shows.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 5 commits October 29, 2026 05:17
Co-Authored-By: jason.han <hanhuijun@gmail.com>
…le the about index is being built

A view exposing through a filter on an annotation evaluates that filter while
the about index is still being built, resolving an unqualified name its own
about usage states through the exposure; the verdict reached then is partial.
The filter cache and the resolver's kept enumerations both skip what is read
while Model.AboutBuilding reports the index incomplete.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…d place inherited members per inheritor

The round-trip test applies every imported edit through the model-edit path,
re-renders and compares the changed cells by identity, which exposed: the
matcher judged whether a Route steers an edge from a clone that carries no
record of what was drawn; a member drawn under a node of another element
(a part of the usage's type, an inherited state) was placed and coloured by
the first Layout about its declaration rather than the one naming it as that
node's, and its import named the declaration rather than the path a Layout
resolves through; a Layout about the declaring body's start grew a start in
every body inheriting it; and the edit refused a Layout about a member drawn
under an exposed usage as not exposed. The unused drawioMatch type is gone.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…port

Edge routing moved the notes of the layout golden, so the desktop save of the
earlier export imported a Note edit besides the tank's Layout; draw.io desktop
32.6.1 saved the routed export again, compressed, with the tank dragged out of
its loop. The merge of develop also left drawioPrepare returning a string for
an error of the router.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

…g edges beside a membership edge unsteered

An element a nested view exposes is drawn under the view's cell but is no
member of the view: a Layout imported for it names the element, not a path
through the view. The import now asks the rendering which members it drew as
an owner's own (Drawn.Inherits) instead of comparing name prefixes.

A usage's typing edge drawn beside its membership edge follows a Route naming
its type, which no edit writes yet; the plain Route steers the membership edge
only. Drawn.Steers now takes the edge kind, so re-routing the typing edge is
reported rather than written as a Route about the usage that would move the
membership edge.

An unplaced container that frames children is pinned once when it moves,
instead of being placed and then pinned again when a child moves.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Heads-up for the matcher: the draw.io writer is about to put fqn on an edge's <object> too (branch fix/drawio-unrouted-edges), for edges that are declarations of their own — connection, binding, association, flow, succession, transition, require/assume/assert/frame/event, import, dependency, derive, refine, expose, satisfy, verify, allocate, include, perform, exhibit — e.g. <object id="e0" label="" fqn="PumpStation::station::fluid" kind="connection" …>. Edges drawn from a member's declaration (composition, reference, typing, specialization, …) still carry no fqn. So the import can prefer fqn when present and fall back to endpoint names + kind + ordinal otherwise. Same branch also rounds waypoints to a thousandth of a pixel and adds exact exit*/entry* anchors (plus a notice) on edges the router found no path for.

devin-ai-integration[bot]

This comment was marked as resolved.

…ws them

Pinning a container the model does not place writes a sized Layout, and the
draw.io writer lays the children of a sized container out by a different gap,
so children left unplaced moved on the next export. The pin now gives every
child the box the file shows, a child container pinned in turn, and a placed
child a size only when the model or the file states one.

A placed container without a size was fitted around its placed children as if
their absolute Layouts were offsets from its corner; the probe now measures
from the corner.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 4 commits October 29, 2026 07:29
sysml <model> -import-diagram <file.drawio> [-view <name>] [-dry-run] reads a
draw.io file the drawio rendering form wrote, renders the view it draws, and
writes what differs back as DiagramLayout metadata through the model editor;
-dry-run prints the planned edits and a unified diff and writes nothing. The
LSP answers the same as opensysml/importDiagram, a WorkspaceEdit like
opensysml/applyModelEdit's, and the MCP serves it as import_diagram, a dry run
until apply. The line diff behind the LSP's text edits moves to the workspace
model package so the CLI prints the same diff.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…s one undoable edit

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…and VS Code

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

The CLI stages every edited model file beside its own and swaps the set in,
restoring each on failure; a whole-model refusal is reported without indexing
the planned edits; -import-diagram '' is refused; the dry-run diff spells an
empty range as the line before it with a zero count. The LSP applies the import
against the documents it rendered and answers stale when they moved. The MCP
resolves the draw.io file under the workspace root as it does documents.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration Bot and others added 3 commits October 29, 2026 08:06
Co-Authored-By: jason.han <hanhuijun@gmail.com>
…side the root

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 2 commits October 29, 2026 08:27
…hanged files alone

The import resolves each edited document to the file it names, stages the new
text beside that file and swaps it in, so a symlinked model keeps its link and
the linked file is edited; a document the import read but left as it was is
neither staged nor reported written.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
The CLI passed declaration sites to the draw.io writer only with -render-link,
so a plain export named a port by its usage path while the import's baseline
named it by its declaration and an untouched export of a view with ports was
reported twice per port. The drawio form now always gets the sites. The reader
resolves a port's fraction-of-owner geometry to absolute coordinates with its
offset, and a port is noticed as dragged only when it moved against its owner.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

…mports

A relative port keeps its fraction and offset, and the import compares ports on
an owner of the baseline's size, so resizing the owner moves no port.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 3 commits October 29, 2026 09:30
A port stays on its owner, so a file that attaches it elsewhere is reported
rather than silently compared on its old owner.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
opensysml/render linked sites only with a link template, so a server export,
and the MCP's render_view and the editor's export over it, named ports by their
usage path while the import's baseline named them by their declaration.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration Bot and others added 4 commits October 29, 2026 15:25
…exported bends

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…amples and libraries digests

The self-model's new opensysml.importDiagram line moved the examples digest;
develop's DocumentQueries change moved the libraries digest and its library
rows. The draw.io desktop fixture is saved again against the 50 px grid gap,
and the two layout tests derive their expectations from the exported geometry.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant