Conversation
|
Is this related to a specific issue? |
Sorry no, was part of new capability to migrate to CALM. Thank You |
There was a problem hiding this comment.
Pull request overview
Adds SVG-to-CALM import support to the VS Code extension, supporting Draw.io metadata and generic SVG diagrams.
Changes:
- Adds import commands and canvas toolbar integration.
- Parses SVG nodes, relationships, containment, geometry, and styles.
- Generates CALM 1.2 JSON with parser and builder tests.
Reviewed changes
Copilot reviewed 19 out of 24 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
package-lock.json |
Locks SVG/XML dependencies. |
calm-plugins/vscode/package.json |
Registers the command and dependencies. |
calm-plugins/vscode/src/svg-parser.d.ts |
Declares SVG parser types. |
calm-plugins/vscode/src/extension/extension.ts |
Registers the import command. |
calm-plugins/vscode/src/extension/types/messages.ts |
Adds the import message type. |
calm-plugins/vscode/src/extension/webview/canvas-panel.ts |
Handles canvas import requests. |
calm-plugins/vscode/src/webview/App.tsx |
Adds the Import SVG button. |
calm-plugins/vscode/src/webview/stores/sync-bridge.ts |
Sends import requests. |
calm-plugins/vscode/src/extension/services/svg-import/index.ts |
Exports import APIs. |
calm-plugins/vscode/src/extension/services/svg-import/types.ts |
Defines import graph types. |
calm-plugins/vscode/src/extension/services/svg-import/svg-import-service.ts |
Orchestrates file selection and output. |
calm-plugins/vscode/src/extension/services/svg-import/format-detector.ts |
Detects SVG formats. |
calm-plugins/vscode/src/extension/services/svg-import/format-detector.test.ts |
Tests format detection. |
calm-plugins/vscode/src/extension/services/svg-import/drawio-parser.ts |
Parses Draw.io metadata. |
calm-plugins/vscode/src/extension/services/svg-import/drawio-parser.test.ts |
Tests Draw.io parsing. |
calm-plugins/vscode/src/extension/services/svg-import/generic-svg-parser.ts |
Parses generic SVG geometry. |
calm-plugins/vscode/src/extension/services/svg-import/generic-svg-parser.test.ts |
Tests generic parsing. |
calm-plugins/vscode/src/extension/services/svg-import/shape-mapper.ts |
Maps shapes to CALM types. |
calm-plugins/vscode/src/extension/services/svg-import/shape-mapper.test.ts |
Tests shape mapping. |
calm-plugins/vscode/src/extension/services/svg-import/calm-builder.ts |
Builds CALM JSON. |
calm-plugins/vscode/src/extension/services/svg-import/calm-builder.test.ts |
Tests CALM generation. |
calm-plugins/vscode/src/extension/services/svg-import/__fixtures__/generic-simple.svg |
Provides a generic SVG fixture. |
calm-plugins/vscode/src/extension/services/svg-import/__fixtures__/drawio-simple.svg |
Provides a Draw.io fixture. |
calm-plugins/vscode/src/extension/services/svg-import/__fixtures__/drawio-nested.svg |
Provides a nested Draw.io fixture. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Can we add one? I think this is a significant enough feature proposal that we should have a record of what was intended. |
My Apologies @rocketstack-matt has been very busy. Here is the issue #3025 . Please do the needful. Thank You. |
|
@rocketstack-matt resurfacing this PR and looking for some movement on this. Thank You. |
|
@byrash there are lots of failing builds, I've been holding off until they're resolved. |
|
Looking into it. Thanks! |
0bfbc86 to
249b2f1
Compare
|
Hello @markscott-ms & @markscott-ms have to rebase to main. Build is all green now. Thanks! |
rocketstack-matt
left a comment
There was a problem hiding this comment.
Solid iteration-1 feature — schema-valid CALM output, 240 tests/lint/build all green locally. A few gaps below are worth addressing before merge, particularly the missing transform composition (can silently mis-position or duplicate nodes) and the untested importSvgToNewFile path. Also verified most of Copilot's automated findings against the current code and replied where they don't hold up (see thread replies).
|
Thanks @rocketstack-matt , All mentioned issues are addressed and will be available soon. Thank You. |
|
As discussed this is ready for review |
Fixes raised by rocketstack-matt and markscott-ms: - Bound inflateRaw output to 20MB to prevent zip-bomb OOM - Reorder label keyword patterns so specific categories (database, network) match before broad actor pattern - Detect generic-SVG containers by child references, not just style props - Return nearest text label, not first within radius - Tighten drawio format detection to require proper XML tag or encoding - Fall back to generic parser when drawio parse yields zero nodes - Anchor content= regex to root <svg> tag - Lenient percent-decode fallback for malformed escapes - Tag import updates with source: 'import' to bypass echo-suppression - Compose scale() and matrix() transforms in generic SVG parser
Fixes raised by rocketstack-matt and markscott-ms: - Bound inflateRaw output to 20MB to prevent zip-bomb OOM - Reorder label keyword patterns so specific categories (database, network) match before broad actor pattern - Detect generic-SVG containers by child references, not just style props - Return nearest text label, not first within radius - Tighten drawio format detection to require proper XML tag or encoding - Fall back to generic parser when drawio parse yields zero nodes - Anchor content= regex to root <svg> tag - Lenient percent-decode fallback for malformed escapes - Tag import updates with source: 'import' to bypass echo-suppression - Compose scale() and matrix() transforms in generic SVG parser
|
Thanks for your time on this matter @rocketstack-matt & @markscott-ms , we have now addressed the issues and change should be reflected soon on PR. Thanks! |
|
Hello @rocketstack-matt & @markscott-ms , Can we please get this PR moving if all good ? Thank You. |
There was a problem hiding this comment.
Hi @byrash please resolve conversations that you have addressed so it's clear they're ready for re-review. I'm marking them all now based on re-review.
Two things to fix before merge: the modelUpdated source type is now wrong (fails tsc) and package-lock.json isn't updated for the new dependencies. Two parsing gaps below are also worth fixing (AWS-style draw.io containers, connectors inside transformed groups). Please add Closes #3025 to the description and tick the Testing/Checklist boxes that apply.
| const json = await this.importService.importSvgIntoDocument(this.currentDocument); | ||
| if (json) { | ||
| this.log.appendLine(`[CanvasPanel] Import successful, updating webview`); | ||
| this.postMessage({ type: 'modelUpdated', json, source: 'import' }); |
There was a problem hiding this comment.
'import' isn't in the modelUpdated source union, so this fails tsc:
canvas-panel.ts(517,60): error TS2322: Type '"import"' is not assignable to type '"file" | "ai" | "text-editor"'.
It only slipped through because neither esbuild nor CI typechecks. Add 'import' to the union in src/extension/types/messages.ts:23 and to ModelUpdateCallback in src/webview/stores/sync-bridge.ts:6.
| }, | ||
| "dependencies": { | ||
| "@finos/calm-models": "file:../../calm-models", | ||
| "svg-parser": "^2.0.4", |
There was a problem hiding this comment.
package-lock.json isn't updated for these: the calm-plugins/vscode workspace entry in the lockfile lists no svg-parser / xml2js / @types/xml2js, and xml2js is still flagged dev: true. npm ci passes only because hoisted copies already exist (xml2js@0.5.0 via @vscode/vsce, svg-parser via @svgr/plugin-jsx), so the first change to either of those parents would silently break the extension.
Run npm install --package-lock-only at the repo root and commit the resulting lockfile diff as-is.
| const style = info.cellAttrs.style ?? ''; | ||
| if (style.includes('group')) { |
There was a problem hiding this comment.
style.includes('group') also matches AWS/Azure container shapes (shape=mxgraph.aws4.group, grIcon=mxgraph.aws4.group_aws_cloud, …), which are real vertices. They're kept as nodes, but every child's parentId is cleared at :70, so containment inside an AWS Cloud / VPC box is lost. Use the same test parseVertex already uses for real groups:
| const style = info.cellAttrs.style ?? ''; | |
| if (style.includes('group')) { | |
| if (parseStyleString(info.cellAttrs.style ?? '')['group'] === '1') { |
| return { id, label, shapeHint, geometry: shapeGeo, styleProps: {} }; | ||
| } | ||
|
|
||
| function extractEdgesFromElement(element: ElementNode, nodes: SvgNode[], edges: SvgEdge[]): void { |
There was a problem hiding this comment.
Node geometry is now composed through ancestor transforms, but <line> / <polyline> endpoints aren't, so the second half of Copilot's comment at :128 still holds: a connector inside <g transform="translate(300,200)"> is matched with its raw local coords against nodes that now sit at (300+x, 200+y), and silently yields no edge.
Thread the accumulated transform through here the same way extractNodesFromElement does:
const elTransform = composeTransforms(accTransform, parseTransform(String(props.transform ?? '')));
// line: findNearestNode(applyPoint({ x: x1, y: y1 }, elTransform), nodes)
// polyline: applyPoint(start, elTransform) / applyPoint(end, elTransform)
// g: extractEdgesFromElement(child, nodes, edges, elTransform)With that change the existing svg-import tests still pass, and a probe with two grouped rects plus a line inside a translated <g> goes from 0 edges to 1.
Description
Iteration 1 of importing SVG into VS Code Plugin that converts to CALM JSON file to boot strap migration form other Diagram as code tools like Drawio ( more to be added )
Type of Change
Affected Components
cli/)calm/)calm-ai/)calm-hub/)calm-hub-ui/)calm-server/)calm-widgets/)docs/)shared/)calm-plugins/vscode/)Commit Message Format ✅
Testing
Checklist