Skip to content

feat(view): import renames, note text and picture sources from draw.io - #1695

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

HuiJun merged 13 commits into
developfrom
feat/drawio-import-rename

Conversation

@devin-ai-integration

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

Copy link
Copy Markdown
Contributor

What and why

The draw.io round trip (#1679) imported what a user moves, resizes, folds, reroutes, recolours and
resizes the page to. This makes three more kinds of edit made in diagrams.net / draw.io Desktop
import back into the model, through the same baseline diff and the same model-edit path:

  • A renamed element. A matched node, port or fqn-bearing edge whose edited label equals the
    writer's own label with exactly the declared name replaced by one valid SysML name (an identifier
    or a quoted '…' name, judged by the lexer) becomes edit.Rename(target, newName), so the
    declaration and every reference across documents move — including the view's own expose,
    about and Layout names. The writer records where it put each name (drawioName spans on the
    object, labeller.headName, portView.pinLabel, edge.Label == edge.Name); nothing re-parses
    the label text. The target is the qualified path the import already uses for that cell's
    Layout (Loop::sensor::waterOut for a member drawn under an inheritor), and resolves to the
    declaration, not the usage; the docs state this. Renames are ordered after the import's
    Layout/Route/Style/Note/Picture edits so those still resolve against the old names,
    and a file that moves and renames the same element applies cleanly (one Rename, one Layout).
  • An edited note text. A position-matched note whose decoded text differs becomes SetNote
    with the new text and the geometry it has (text and move together are still one SetNote).
    html=1 text is decoded as the inverse of what the writer emits (<br>, <div>, &nbsp;,
    entities); multi-line text round-trips \n exactly, and the model-edit writer now escapes
    control characters in string literals so the applied note is valid SysML.
  • A changed picture source. A matched picture whose image= source differs from what the
    writer emits for Picture.Location becomes SetPicture with the new location: a relative
    path, an absolute path or a file: URL, each naming a readable image the renderer accepts
    under the view's directory (stored relative to it); a source outside that directory is
    refused before anything is read. Geometry and alt are kept.

Everything else is reported, one line per cell, never imported (exact messages below). Nothing
creates or deletes elements; the request shapes of LSP opensysml/importDiagram, MCP
import_diagram and VS Code opensysml.importDiagram are unchanged (the result's edit carries a
newName on a Rename), editors/vscode/src/webview is untouched.

The importer's behaviour was verified against a file saved by draw.io Desktop 32.6.1 under Xvfb:
editing an <object> cell rewrites only label (no value fallback; &#xa; for a line break,
html=0 kept), editing a note rewrites the mxCell value. That compressed file is the fixture
internal/ir/view/testdata/layout-desktop-rename.drawio (1.6 KiB), read by
TestReadDrawioCompressedDesktopRenameAndNote.

Reported, not imported

Case Message
label edited beyond the name (keyword, : Type, multiplicity, parameter line, compartment row) <cell>: its label changed beyond the name "<old>"; a keyword, type, multiplicity, parameter or compartment edit is not imported
new name not one valid SysML name <cell>: renamed to "<new>", which <is a keyword / is not an identifier / is an unterminated quoted name / is not a single name>, so the rename is not imported
name synthesized or label carries no name of its own <cell>: its label was edited, but the cell shows no declared name of its own (its name is synthesized or the label carries other text), so nothing is renamed and the label is not imported
two drawn copies disagree <cell>: the view draws it twice and the copies disagree ("<label>" and "<label>"), so no rename is imported
rename conflicts with an existing member the existing CheckRename message, answered as the import's refused edit: <element> cannot be renamed to "<name>": that name already means <element> where <reference> reads it (rename_conflict.go, unchanged)
same text retyped no edit, no report
note emptied note <id> ("<first line>") was emptied; a Note with no text is not a Note, so the note is left as it was
picture given a data: URI picture <id> ("<alt or location>"): its image was changed to embedded image data, which no Picture location names (a Picture names a file), so the picture keeps its file
picture given a URL picture <id> (…): its image was changed to <url>, a URL no drawing tool reads, so the picture keeps its file
picture given a file: URL with a host or no path … changed to <src>, a file URL naming no local file, …
picture given a path or file: URL outside the view's directory (absolute, ../, or any absolute path when the document has no directory) … changed to <src>, a file outside the view's directory, which the import does not read
picture file missing / not an image / not drawable / empty source … changed to "<location>", a file that does not read / "<location>", which is not an image / "<location>", which is not drawn: <err> / no image

What changed in #1679's matcher

  • diffNote reads the note's text from mxCell.value (where draw.io keeps it) through the
    HTML decoder, and refuses empty text; before it compared geometry only.
  • diffPicture compares the image= source as well as geometry; before it compared geometry only.
  • Labels of matched nodes, ports and edges are compared (diffLabel); renames are collected and
    resolved after the matcher finishes (resolveRenames) so duplicate drawn copies can be checked
    against each other by their declaration's FQN and appended last, deepest path first, so a
    member renamed together with its owner is still reached by the path the file names it with.
    The pairing itself (by fqn, ordinal, ends, position) is unchanged.
  • targetName is reused for the rename target, so inherited-member paths stay the usage path.
  • edit.Model.renameable resolves an OpRename stated DeclaredIn another document in that
    document (the import states each rename's origin document, so a view renames what it draws from
    any document of the workspace, the declaration splice written there as reference splices already
    were) and falls back to renameTarget when the path is a usage path (owner member lookup, as
    layoutElement already does); stringLiteral escapes \n, \r, \t.
  • lexer.NameProblem is the one rule for "a valid declared name"; edit/validate.go uses it too.

Specification basis

Tooling only: no language semantics change. The rename reuses the existing model-edit rename
and its conflict rules.

How it was verified

  • internal/ir/view: drawio_import_rename_test.go, drawio_import_note_picture_test.go,
    TestReadDrawioCompressedDesktopRenameAndNote (real Desktop 32.6.1 compressed file); the
    untouched-export invariant over every draw.io golden on both rendering paths still holds.
  • internal/check/edit: rename a part def, a port (connection ends follow), an element referenced
    from another document, to a quoted name, to a conflicting name (refused), keyword/type/
    multiplicity edits (reported), one of two drawn copies (reported), rename + move; note text
    single/multi-line/HTML-encoded, note + move, emptied note; picture relative/absolute/file URL,
    data URI, remote/missing/non-image and outside-the-view's-directory refusals; an owner and its
    member renamed in one import; a view in one document renaming a usage and a definition declared
    in another (both tests fail without their fix). The random-mutation property test
    (TestDrawioRoundTrip) gained rename, note text and note emptied mutations, each
    applied and re-exported.
  • cmd/sysml, internal/frontend/lsp: dry-run diff and result shape.

MCP stdio exchange (real sysml-mcp -root ., two documents, dry run then apply)

→ {"jsonrpc": "2.0", "id": 1, "method": "initialize", "params": {"protocolVersion": "2025-06-18", "capabilities": {}, "clientInfo": {"name": "demo", "version": "0"}}}
← {
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "capabilities": {
      "logging": {},
      "resources": {
        "listChanged": true
      },
      "tools": {
        "listChanged": true
      }
    },
    "instructions": "…",
    "protocolVersion": "2025-06-18",
    "serverInfo": {
      "name": "sysml-mcp",
      "title": "OpenSysML",
      "version": "dev",
      "websiteUrl": "https://github.com/Open-MBEE/OpenSysML"
    }
  }
}
→ {"jsonrpc": "2.0", "method": "notifications/initialized"}
→ {"jsonrpc": "2.0", "id": 2, "method": "tools/call", "params": {"name": "import_diagram", "arguments": {"path": "plant.sysml", "file": "loop-edited.drawio"}}}
← {
  "applied": false,
  "changes": [
    {
      "edits": [
        {"newText": "X", "range": {"start": {"line": 15, "column": 12}, "end": {"line": 15, "column": 12}}},
        {"newText": "X", "range": {"start": {"line": 17, "column": 15}, "end": {"line": 17, "column": 15}}},
        {"newText": "X", "range": {"start": {"line": 22, "column": 35}, "end": {"line": 22, "column": 35}}}
      ],
      "path": "plant.sysml"
    },
    {
      "edits": [
        {"newText": "X", "range": {"start": {"line": 5, "column": 11}, "end": {"line": 5, "column": 11}}}
      ],
      "path": "site.sysml"
    }
  ],
  "edits": [
    {
      "cell": "n0",
      "kind": "Rename",
      "label": "«part»\npumpX",
      "newName": "pumpX",
      "target": "Plant::Loop::pump"
    }
  ],
  "view": "Plant::loopView"
}
→ {"jsonrpc": "2.0", "id": 3, "method": "tools/call", "params": {"name": "import_diagram", "arguments": {"path": "plant.sysml", "file": "loop-edited.drawio", "apply": true}}}
← {
  "applied": true,
  "changes": [
    {
      "edits": [
        {"newText": "X", "range": {"start": {"line": 15, "column": 12}, "end": {"line": 15, "column": 12}}},
        {"newText": "X", "range": {"start": {"line": 17, "column": 15}, "end": {"line": 17, "column": 15}}},
        {"newText": "X", "range": {"start": {"line": 22, "column": 35}, "end": {"line": 22, "column": 35}}}
      ],
      "path": "plant.sysml"
    },
    {
      "edits": [
        {"newText": "X", "range": {"start": {"line": 5, "column": 11}, "end": {"line": 5, "column": 11}}}
      ],
      "path": "site.sysml"
    }
  ],
  "edits": [
    {
      "cell": "n0",
      "kind": "Rename",
      "label": "«part»\npumpX",
      "newName": "pumpX",
      "target": "Plant::Loop::pump"
    }
  ],
  "view": "Plant::loopView"
}

The documents after the apply:

part def Loop {
	part pumpX : Pump;
	part tank : Tank;
	connect pumpX.outlet to tank.inlet;
}
view loopView {
	expose Loop::*;
	metadata Layout about Loop::pumpX { x = 40; y = 60; }
	…
}
// site.sysml
part plant : Loop {
	:>> pumpX { attribute mass = 12.0; }
}

The same import from the CLI, sysml plant.sysml site.sysml -import-diagram loop-edited.drawio -dry-run:

loaded 2 files:
  plant.sysml
  site.sysml
✓ package Plant
✓ package Site
Rename about Plant::Loop::pump
--- a/plant.sysml
+++ b/plant.sysml
@@ -12,14 +12,14 @@
 	}
 
 	part def Loop {
-		part pump : Pump;
+		part pumpX : Pump;
 		part tank : Tank;
-		connect pump.outlet to tank.inlet;
+		connect pumpX.outlet to tank.inlet;
 	}
 
 	view loopView {
 		expose Loop::*;
-		metadata Layout about Loop::pump { x = 40; y = 60; }
+		metadata Layout about Loop::pumpX { x = 40; y = 60; }
 		@Note { text = "the water loop"; x = 10; y = 300; }
 	}
 }
--- a/site.sysml
+++ b/site.sysml
@@ -2,6 +2,6 @@
 	private import Plant::*;
 
 	part plant : Loop {
-		:>> pump { attribute mass = 12.0; }
+		:>> pumpX { attribute mass = 12.0; }
 	}
 }
dry run: 1 edits planned, nothing written

Gate

make build                      ok
go vet ./...                    ok
gofmt -l .                      (empty)
go test ./...                   see below
make lint                       ok
npm test (editors/vscode)       ok
make docs                       ok (mkdocs --strict)
python3 scripts/changelog.py check   ok
make man-check                  ok

go test ./... with Go's default 10 m per-package timeout, run on an 8-core VM with every package
in parallel, timed out in three packages — cmd/sysml (still running
TestExploreTablesTheSpacecraftRaceWithinItsBudget, untouched here), internal/check/edit
(TestDrawioRoundTrip, now 35 goldens × 10 mutations) and tests/codegen (untouched) — and passed
everywhere else (121 ok). The three re-run alone with the CI timeout (make test uses -timeout 45m):

ok  	github.com/Open-MBEE/OpenSysML/cmd/sysml	414.481s
ok  	github.com/Open-MBEE/OpenSysML/internal/check/edit	460.222s
ok  	github.com/Open-MBEE/OpenSysML/tests/codegen	440.858s

For scale, internal/check/edit's TestDrawioRoundTrip on develop measured 635 s against 886 s
here under the same load; the added cost is the rename mutation (≈95 s over 35 goldens), which
applies the rename and re-exports.

After the review-round fixes (head d525aa040: cross-document rename target, member-before-owner
order, picture source confined to the view's directory, MCP tool description): make build,
go vet ./..., gofmt -l . (empty), make lint, make docs, scripts/changelog.py check,
make man-check all pass again; the packages touched re-ran clean —
go test ./internal/check/edit -run Drawio (314 s), ./internal/ir/view -run Drawio (30 s),
./internal/frontend/lsp, ./internal/frontend/mcp/..., ./cmd/sysml -run 'ImportDiagram|Drawio|Usage|Man'.

Review rounds two and three (head d220bf8dc): the new comments were cut to two lines, and the
picture-directory check now resolves symlinks — the directory and the named file through
filepath.EvalSymlinks — for the containment test only, so a link under the view's directory to a
file outside it is refused before anything is read while a link to a file under it imports as the
link the user named (link.png → outside is refused, inlink.png → other.png imports at
inlink.png; both asserted in TestDrawioImportReportsPictureSourcesItCannotName, which fails
without the change). go vet, gofmt -l ., make lint and go test ./internal/ir/view -run 'Picture|Drawio' pass; the full CI tier on d220bf8dc is green (31 checks).

After merging develop (the shared DrawioImporter of #1694; drawio_test.go resolved onto the
importer-per-golden shape, the two added test files likewise): make build, go vet ./...,
gofmt -l . (empty), make lint, make docs, scripts/changelog.py check, make man-check,
npm test and go test -timeout 45m ./... all pass; go test -race -pgo=off -count=1 ./internal/check/edit = ok … internal/check/edit 213.619s (run alongside the lint/docs gate on an
8-core VM; 162.9 s alone), the whole package well inside the CircleCI budget #1694 restored.

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 (compliance rows need nothing: the census is counted at docs build)
  • No internal work-item labels (waves, slices, F4, K5) in the body, docs, or changelog

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/e8b647f232d940628fb882686e2b78c1
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/e8b647f232d940628fb882686e2b78c1?variant=devin
Requested by: @HuiJun

devin-ai-integration Bot and others added 5 commits October 29, 2026 17:58
A matched node, port or edge whose label differs from the writer's own
label by exactly its declared name imports as a Rename through the
model-edit path, ordered after the import's other edits; a usage path
reaching an inherited member renames the declaration it resolves to.
Label edits beyond the name, invalid names, refused renames and drawn
copies that disagree are reported, not imported.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
A note box whose text was retyped imports as a SetNote with its text,
decoded from draw.io's formatted form when the note was formatted, and
together with a move as one SetNote; an emptied note is reported. A
picture whose image was changed to a file the model can locate imports
as a SetPicture naming it relative to the view's directory; embedded
data, a URL and a file that does not read as an image are reported.
String literals the edits write spell line breaks and tabs as escapes.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
The compressed fixture is the layout golden as desktop saved it after the
pump's text and the note's were retyped: only the object's label changes,
the line break is &#xa; and html=0 stays, so the import sees one rename
and one note text and nothing else.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
The -import-diagram usage, the import_diagram catalog row and manual
pages, the MCP reference, the rendering-forms and layout-annotation
notes and the editors guide name the three new kinds of edit — a retyped
name, a retyped note, a re-pointed picture — the rule a rename follows
(the writer's label with only the name changed; the path a Layout would
use, so an inherited member renames its declaration) and each case that
is reported rather than imported.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
The importDiagram result's edit carries newName on a Rename, so an MCP
host or editor sees what the element becomes, and the CLI dry run prints
the rename as "Rename <element> to <name>" above its diff.

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 18:59
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 4 commits October 29, 2026 19:11
A draw.io rename is applied through the view's document, and the editor
only renamed a declaration of that document, so an element the view
draws from another document was reported, not renamed. A rename stated
DeclaredIn the declaring document now reaches it: the declaration splice
rewrites that document as reference splices already did, and the import
states the cell's origin document. The owner path of an inherited member
is resolved in any document, as a Layout's is.

Also tests a member renamed together with its owner in one import.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Renames of an owner and of one of its members in the same import were
applied owner first, after which the member's path in the file no longer
resolved and its rename was refused. The imported renames are now ordered
deepest path first, after the one-per-element and disagreeing-copy rules
have chosen which copy's rename to keep.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
An edited image source naming an absolute path or file URL outside the
view's directory was read to check it is an image, and a Picture keeping
the absolute location was imported. Such a source is now refused before
any read, with a notice, and the accepted sources are stated relative to
the view's directory, as the writer states them.

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

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 19:20
Co-Authored-By: jason.han <hanhuijun@gmail.com>
The directory check compared paths lexically, so a symlink under the
view's directory to a file outside it passed and the file was read.
Both the directory and the named file are resolved through their
symlinks before the check; a file that does not exist keeps its path and
is still reported as one that does not read.

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 19:24
…reaches

The symlink check stored the file a link reaches as the picture's
location, so a link under the view's directory was lost on import. The
lexical path is kept and the resolved path is checked for containment
only; a link under the directory to a file under it imports as the link.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
@HuiJun
HuiJun merged commit 6f817f0 into develop Oct 29, 2026
61 of 63 checks passed
@HuiJun
HuiJun deleted the feat/drawio-import-rename branch October 29, 2026 22:02
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