feat(drawio): infer shape person on re-import for round-trip fidelity#2685
Conversation
inferShape() now returns 'person' when DrawIO cell style contains shape=actor, shape=person, or umlactor. Re-imported actor cells get explicit style { shape person } in emitted .c4.
Made-with: Cursor
inferKind() now includes shape=actor so vertex with shape=actor is emitted as actor (not container). Aligns with upstream likec4#2682 and makes this branch pass ci:test without depending on likec4#2682 merge. Made-with: Cursor
🦋 Changeset detectedLatest commit: 03eb4b2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughExtends Draw.io import inference to treat Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/drawio-infer-shape-person-roundtrip.md:
- Line 7: The changelog text incorrectly asserts the exporter uses `shape=actor`
when the current exporter emits `shape=umlActor`; update the sentence in
.changeset/drawio-infer-shape-person-roundtrip.md so it reflects both possible
export values (e.g., "person → shape=actor or shape=umlActor") and keep the
import wording about inferKind() and inferShape() as-is; reference the symbols
inferKind() and inferShape() in the note so readers can correlate the behavior
with those functions.
In `@packages/generators/src/drawio/parse-drawio.ts`:
- Around line 580-581: The comment above the actor/person shape check is
outdated; update the comment near the conditional that checks
s.includes('shape=actor') || s.includes('shape=person') ||
s.includes('umlactor') to mention that the exporter may emit shape=actor,
shape=person, shape=umlactor or shape=umlActor (current generator emits
shape=umlActor), so document both legacy and current forms to avoid stale
behavior claims.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6079b160-37fc-4541-b14f-210119d17492
⛔ Files ignored due to path filters (1)
packages/generators/src/drawio/__snapshots__/parse-drawio.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
.changeset/drawio-infer-shape-person-roundtrip.mdpackages/generators/src/drawio/parse-drawio.spec.tspackages/generators/src/drawio/parse-drawio.ts
…e=actor or umlActor) CodeRabbit: changeset now states export may emit shape=actor or shape=umlActor; inferShape comment updated to match. Made-with: Cursor
|
Addressed CodeRabbit's review: commit 03eb4b2 updates the changeset wording and the \inferShape\ comment so both reflect that the exporter may use \shape=actor\ or \shape=umlActor. Replied on the two inline comments. |
2 participants
feat(drawio): infer shape person on re-import for round-trip fidelity
Summary
Follow-up to the Draw.io person-shape fix (upstream PR #2682). That PR changes export to use
shape=actorinstead ofshape=umlActorfor LikeC4shape personand updates the parser soinferKindrecognizesshape=actoras actor. This PR adds round-trip fidelity on the import side: re-imported actor cells now get an explicitstyle { shape person }in the emitted .c4 source, so the round-trip is symmetric (export: person → shape=actor; import: shape=actor → actor withshape person).Changes
packages/generators/src/drawio/parse-drawio.tsinferKind()now treatsshape=actoras actor (alongsideumlactorandshape=person), so cells exported with the current Draw.io format are emitted asactor 'title'rather than container. Keeps this PR self-contained and CI green whether or not #2682 is merged first.inferShape()now returns'person'when the DrawIO cell style containsshape=actor,shape=person, orumlactor. Existing logic for cylinder, document, and rectangle is unchanged.packages/generators/src/drawio/parse-drawio.spec.tsNew test: "parse DrawIO to LikeC4 - vertex with shape=actor emits actor with style { shape person } for round-trip fidelity" (minimal diagram with one vertex
style="shape=actor;..."). Asserts that the output containsactor 'User'andshape person, and snapshot updated..changeset/drawio-infer-shape-person-roundtrip.mdPatch changeset for
@likec4/generators.Why
After #2682, re-importing a .drawio that contains an actor (with
shape=actor) already yieldsname = actor 'title'and renders correctly, becauseactorimplies the person shape by default. This PR only makes the emitted source explicit: the .c4 file will includestyle { shape person }for those elements, so the round-trip is fully symmetric and the source faithfully reflects the diagram.Checklist
parse-drawio.spec.ts+ snapshotpnpm test -- packages/generators/src/drawio/parse-drawio.spec.ts(or from repo root with vitest andnode_modules/.binin PATH)@likec4/generators(patch)feat(drawio): ...)Merge order
Safe to merge before or after #2682. Can be proposed to upstream after #2682 is merged, or in parallel at maintainers’.
Summary by CodeRabbit
New Features
Tests