feat(drawio): infer shape person on re-import for round-trip fidelity#26
feat(drawio): infer shape person on re-import for round-trip fidelity#26
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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughUpdates DrawIO import inference so Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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 |
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
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4aa31ac1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!style) return undefined | ||
| const s = style.toLowerCase() | ||
| // Actor/person shape (export uses shape=actor; legacy may have shape=person or umlactor) | ||
| if (s.includes('shape=actor') || s.includes('shape=person') || s.includes('umlactor')) return 'person' |
There was a problem hiding this comment.
This new inferShape branch makes shape=actor produce shape person, but inferKind still only treats umlactor and shape=person as actors, so Draw.io cells exported as shape=actor are still emitted as container declarations. In diagrams that rely on LikeC4’s current export format (shape=actor), the round-trip remains incorrect (container + person style instead of actor), which also makes the added actor snapshot expectation unreachable.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/generators/src/drawio/parse-drawio.ts (1)
556-574:⚠️ Potential issue | 🔴 Critical
inferKind()not updated to recognizeshape=actor— this is the root cause of the CI test failure.The
inferShape()function was updated to return'person'forshape=actor, butinferKind()was not similarly updated. Line 564 only checks forumlactorandshape=person, but notshape=actor. This causes cells withstyle="shape=actor"to be emitted ascontainer 'User'instead ofactor 'User'.🐛 Proposed fix to also recognize shape=actor in inferKind
function inferKind( style: string | undefined, parentCell?: DrawioCell, ): 'actor' | 'system' | 'container' | 'component' { const s = style?.toLowerCase() ?? '' switch (true) { case !style: return parentCell?.style?.toLowerCase().includes('container=1') ? 'component' : 'container' - case s.includes('umlactor') || s.includes('shape=person'): + case s.includes('umlactor') || s.includes('shape=person') || s.includes('shape=actor'): return 'actor' case s.includes('swimlane'): case s.includes('container=1'): return 'system' case !!parentCell?.style?.toLowerCase().includes('container=1'): return 'component' default: return 'container' } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/generators/src/drawio/parse-drawio.ts` around lines 556 - 574, The inferKind function currently checks for 'umlactor' and 'shape=person' but misses 'shape=actor', causing actor-shaped cells to be treated as containers; update inferKind (the function named inferKind) so the actor check includes s.includes('shape=actor') alongside s.includes('umlactor') and s.includes('shape=person') (and ensure the lowercase-normalized s variable is used), leaving the rest of the switch logic unchanged so cells with style="shape=actor" resolve to 'actor'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/generators/src/drawio/parse-drawio.ts`:
- Around line 556-574: The inferKind function currently checks for 'umlactor'
and 'shape=person' but misses 'shape=actor', causing actor-shaped cells to be
treated as containers; update inferKind (the function named inferKind) so the
actor check includes s.includes('shape=actor') alongside s.includes('umlactor')
and s.includes('shape=person') (and ensure the lowercase-normalized s variable
is used), leaving the rest of the switch logic unchanged so cells with
style="shape=actor" resolve to 'actor'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6712fa68-0385-428f-ba26-3125cd5627de
⛔ 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
|
Thanks for the reviews (@chatgpt-codex-connector, @coderabbitai). This is already addressed in the next commit on this branch: 71ab678 — \ ix(drawio): recognize shape=actor in inferKind for CI/self-contained PR. \inferKind()\ now includes \shape=actor\ in the actor condition (parse-drawio.ts ~line 564: \case s.includes('umlactor') || s.includes('shape=person') || s.includes('shape=actor'):). So cells with \shape=actor\ are emitted as \�ctor 'title'\ with \style { shape person }, and the round-trip is correct. The actor snapshot test passes in CI. No further code change needed. |
|
To use Codex here, create an environment for this repo. |
|
Tip For best results, initiate chat on the files or code changes.
(´• ω •`) ♡ |
…e=actor or umlActor) CodeRabbit: changeset now states export may emit shape=actor or shape=umlActor; inferShape comment updated to match. Made-with: Cursor
1 participant
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.tsinferShape()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 likec4#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): ...)Upstream
Intended as a small follow-up to likec4#2682. Can be proposed to upstream after likec4#2682 is merged, or in parallel at maintainers’ preference.
Summary by CodeRabbit
Bug Fixes
Tests
Changelog