From fa3f6f3d461fe2729064b4afa0965c795380d98a Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:31:11 +0000 Subject: [PATCH 1/2] fix(view): prepare the draw.io import baseline once per rendering ImportDrawio laid the rendering out and routed every edge anew on each call to rebuild the same baseline. DrawioImporter prepares it once and Import diffs any file of the rendering against it; ImportDrawio delegates to it. TestDrawioRoundTrip reuses one importer per golden, so its trial imports no longer re-route the rendering, and runs its goldens in parallel. Co-Authored-By: jason.han --- internal/check/edit/drawio_test.go | 25 ++++++++++---- internal/ir/view/drawio_import_diff.go | 44 ++++++++++++++++++++---- internal/ir/view/drawio_import_test.go | 46 ++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 14 deletions(-) diff --git a/internal/check/edit/drawio_test.go b/internal/check/edit/drawio_test.go index 63ff8fef02..c1b1ddb9a2 100644 --- a/internal/check/edit/drawio_test.go +++ b/internal/check/edit/drawio_test.go @@ -282,13 +282,22 @@ func drawioMutate(t *testing.T, document, id string, edit func(cell string) stri return strings.Replace(document, whole, edited, 1) } -func drawioImport(t *testing.T, rendering *view.Rendering, options view.Options, document string) (*view.DrawioFile, *view.DrawioImport) { +func drawioImporter(t *testing.T, rendering *view.Rendering, options view.Options) *view.DrawioImporter { t.Helper() - file, err := view.ReadDrawio([]byte(document), rendering.View) + importer, err := rendering.DrawioImporter(options) + if err != nil { + t.Fatal(err) + } + return importer +} + +func drawioImport(t *testing.T, importer *view.DrawioImporter, viewName, document string) (*view.DrawioFile, *view.DrawioImport) { + t.Helper() + file, err := view.ReadDrawio([]byte(document), viewName) if err != nil { t.Fatal(err) } - imp, err := rendering.ImportDrawio(file, options) + imp, err := importer.Import(file) if err != nil { t.Fatal(err) } @@ -306,18 +315,20 @@ func TestDrawioRoundTrip(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { + t.Parallel() content, err := os.ReadFile(filepath.Join(viewFixtures, c.file)) if err != nil { t.Fatal(err) } m, rendering, options := drawioModel(t, string(content), c.view) document, exported := drawioExport(t, rendering, options) - if _, imp := drawioImport(t, rendering, options, document); len(imp.Edits)+len(imp.Unmatched)+len(imp.Deleted)+len(imp.Unknown)+len(imp.Notices) != 0 { + importer := drawioImporter(t, rendering, options) + if _, imp := drawioImport(t, importer, rendering.View, document); len(imp.Edits)+len(imp.Unmatched)+len(imp.Deleted)+len(imp.Unknown)+len(imp.Notices) != 0 { t.Fatalf("untouched export: %+v", imp) } try := func(mut drawioMutation) func(c *view.DrawioCell) *view.DrawioImport { return func(c *view.DrawioCell) *view.DrawioImport { - _, imp := drawioImport(t, rendering, options, drawioMutate(t, document, c.ID, mut.edit)) + _, imp := drawioImport(t, importer, rendering.View, drawioMutate(t, document, c.ID, mut.edit)) return imp } } @@ -328,7 +339,7 @@ func TestDrawioRoundTrip(t *testing.T) { } t.Run(mut.name, func(t *testing.T) { edited := drawioMutate(t, document, cell.ID, mut.edit) - editedFile, imp := drawioImport(t, rendering, options, edited) + editedFile, imp := drawioImport(t, importer, rendering.View, edited) if mut.notice != "" { if len(imp.Edits) != 0 || len(imp.Notices) != 1 || !strings.Contains(imp.Notices[0], mut.notice) { t.Fatalf("edits %+v notices %q, want none and one saying %q", imp.Edits, imp.Notices, mut.notice) @@ -378,7 +389,7 @@ func TestDrawioRoundTrip(t *testing.T) { } } // The edited cells are where the file has them; unplaced neighbours may have flowed around them. - if _, imp := drawioImport(t, redrawn, reoptions, edited); len(imp.Edits) != 0 { + if _, imp := drawioImport(t, drawioImporter(t, redrawn, reoptions), rendering.View, edited); len(imp.Edits) != 0 { for _, e := range imp.Edits { if e.Cell == cell.ID { t.Errorf("the edited file still imports %+v against the edited model", e) diff --git a/internal/ir/view/drawio_import_diff.go b/internal/ir/view/drawio_import_diff.go index 5a725c6c2c..26901e136f 100644 --- a/internal/ir/view/drawio_import_diff.go +++ b/internal/ir/view/drawio_import_diff.go @@ -112,12 +112,32 @@ type DrawioImport struct { // cells that differ from it are imported, so the writer's own geometry — grids, // fitted sizes, computed routes — never becomes an annotation. func (r *Rendering) ImportDrawio(file *DrawioFile, options Options) (*DrawioImport, error) { - norm, w, err := r.drawioPrepare(options) + importer, err := r.DrawioImporter(options) if err != nil { return nil, err } - if file.View != "" && file.View != norm.View { - return nil, &DrawioViewError{View: norm.View, Pages: []string{file.View}} + return importer.Import(file) +} + +// DrawioImporter imports edited draw.io files of one rendering under one set +// of options against a baseline prepared once: laying the rendering out and +// routing its edges is the bulk of an import, and the baseline is the same for +// every file of the rendering. +type DrawioImporter struct { + norm *Rendering + drawn *Drawn + w *drawioWriter + base *DrawioFile + nodes map[string]*Node + ports map[string]Port +} + +// DrawioImporter prepares the baseline that Import diffs edited files of this +// rendering against: the file the writer would emit now under options. +func (r *Rendering) DrawioImporter(options Options) (*DrawioImporter, error) { + norm, w, err := r.drawioPrepare(options) + if err != nil { + return nil, err } pages, err := drawioPages([]byte(w.document(norm, options))) if err != nil { @@ -127,14 +147,24 @@ func (r *Rendering) ImportDrawio(file *DrawioFile, options Options) (*DrawioImpo if err != nil { return nil, err } - out := &DrawioImport{View: norm.View} - m := &drawioMatcher{norm: norm, drawn: r.drawn, w: w, base: base, file: file, out: out, nodes: map[string]*Node{}, ports: map[string]Port{}, framed: map[string]bool{}, paired: map[string]*DrawioCell{}} + importer := &DrawioImporter{norm: norm, drawn: r.drawn, w: w, base: base, nodes: map[string]*Node{}, ports: map[string]Port{}} for _, node := range drawioNodes(norm.Roots) { - m.nodes[node.ID] = node + importer.nodes[node.ID] = node for _, port := range node.Ports { - m.ports[node.ID+":"+port.ID] = port + importer.ports[node.ID+":"+port.ID] = port } } + return importer, nil +} + +// Import reads an edited draw.io file of the rendering back as the edits by +// which it differs from the baseline, as ImportDrawio does. +func (i *DrawioImporter) Import(file *DrawioFile) (*DrawioImport, error) { + if file.View != "" && file.View != i.norm.View { + return nil, &DrawioViewError{View: i.norm.View, Pages: []string{file.View}} + } + out := &DrawioImport{View: i.norm.View} + m := &drawioMatcher{norm: i.norm, drawn: i.drawn, w: i.w, base: i.base, file: file, out: out, nodes: i.nodes, ports: i.ports, framed: map[string]bool{}, paired: map[string]*DrawioCell{}} m.match() m.canvas() return out, nil diff --git a/internal/ir/view/drawio_import_test.go b/internal/ir/view/drawio_import_test.go index 692b9358f4..64f49043a6 100644 --- a/internal/ir/view/drawio_import_test.go +++ b/internal/ir/view/drawio_import_test.go @@ -2,10 +2,12 @@ package view import ( "bytes" + "errors" "image" "image/png" "os" "path/filepath" + "reflect" "regexp" "strconv" "strings" @@ -338,3 +340,47 @@ func TestDrawioImportReportsWhatItCannotImport(t *testing.T) { t.Errorf("edits = %+v, want none", imp.Edits) } } + +// TestDrawioImporterSharesOneBaseline is the reuse contract: an importer +// prepared once answers every file of its rendering as ImportDrawio does, the +// untouched export and an edited one alike, and refuses another view's page. +func TestDrawioImporterSharesOneBaseline(t *testing.T) { + golden, _ := os.ReadFile(filepath.Join("testdata", layoutCase.name+".drawio.golden")) + moved := mutateCell(t, string(golden), "n2", func(cell string) string { + return setGeometryAttr(t, setGeometryAttr(t, cell, "x", "533"), "y", "71.5") + }) + rendering, options := renderWithSites(t, layoutCase.file, layoutCase.view) + importer, err := rendering.DrawioImporter(options) + if err != nil { + t.Fatal(err) + } + for i, document := range []string{string(golden), moved, string(golden)} { + file, err := ReadDrawio([]byte(document), layoutCase.view) + if err != nil { + t.Fatal(err) + } + shared, err := importer.Import(file) + if err != nil { + t.Fatal(err) + } + fresh, err := rendering.ImportDrawio(file, options) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(shared, fresh) { + t.Errorf("shared baseline imported %+v, a fresh one %+v", shared, fresh) + } + if i != 1 { + if len(shared.Edits) != 0 { + t.Errorf("untouched import %d: edits = %+v, want none", i, shared.Edits) + } + } else if edit := editOf(t, shared, DrawioEditLayout, "n2"); edit.FQN != "Plant::Loop::tank" { + t.Errorf("moved cell imported as %+v", edit) + } + } + _, err = importer.Import(&DrawioFile{View: "Other"}) + var viewErr *DrawioViewError + if !errors.As(err, &viewErr) || viewErr.View != layoutCase.view || !reflect.DeepEqual(viewErr.Pages, []string{"Other"}) { + t.Errorf("another view's page imported with %v, want a DrawioViewError naming %s", err, layoutCase.view) + } +} From 306263a606ecdadc54a5e82e46a7382e6ddf0c1d Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:04:42 +0000 Subject: [PATCH 2/2] docs(view): keep the draw.io importer comments to two lines Co-Authored-By: jason.han --- internal/ir/view/drawio_import_diff.go | 6 ++---- internal/ir/view/drawio_import_test.go | 5 ++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/internal/ir/view/drawio_import_diff.go b/internal/ir/view/drawio_import_diff.go index 26901e136f..2bcd30f640 100644 --- a/internal/ir/view/drawio_import_diff.go +++ b/internal/ir/view/drawio_import_diff.go @@ -119,10 +119,8 @@ func (r *Rendering) ImportDrawio(file *DrawioFile, options Options) (*DrawioImpo return importer.Import(file) } -// DrawioImporter imports edited draw.io files of one rendering under one set -// of options against a baseline prepared once: laying the rendering out and -// routing its edges is the bulk of an import, and the baseline is the same for -// every file of the rendering. +// DrawioImporter imports edited draw.io files of one rendering against a baseline +// prepared once; laying out and routing the rendering is the bulk of an import. type DrawioImporter struct { norm *Rendering drawn *Drawn diff --git a/internal/ir/view/drawio_import_test.go b/internal/ir/view/drawio_import_test.go index 64f49043a6..66508dbbd4 100644 --- a/internal/ir/view/drawio_import_test.go +++ b/internal/ir/view/drawio_import_test.go @@ -341,9 +341,8 @@ func TestDrawioImportReportsWhatItCannotImport(t *testing.T) { } } -// TestDrawioImporterSharesOneBaseline is the reuse contract: an importer -// prepared once answers every file of its rendering as ImportDrawio does, the -// untouched export and an edited one alike, and refuses another view's page. +// TestDrawioImporterSharesOneBaseline pins that an importer prepared once answers +// untouched and edited files as ImportDrawio does and refuses another view's page. func TestDrawioImporterSharesOneBaseline(t *testing.T) { golden, _ := os.ReadFile(filepath.Join("testdata", layoutCase.name+".drawio.golden")) moved := mutateCell(t, string(golden), "n2", func(cell string) string {