Repository navigation
feat: add ZenUML diagram editor - #109
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughZenUML is added as an available external Mermaid plugin. The change adds lazy runtime loading, source-safe semantic editing, starter templates, workspace integration, responsive controls, and browser validation. ChangesZenUML catalog and starters
ZenUML runtime
Source model and semantic capabilities
Workspace editor
Validation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SessionWorkspace
participant ZenUmlRuntime
participant Mermaid
participant DiagramCanvas
participant ZenUmlMutations
SessionWorkspace->>ZenUmlRuntime: prepareMermaidRuntimeForSource(source)
ZenUmlRuntime->>Mermaid: register ZenUML plugin
Mermaid-->>SessionWorkspace: runtime ready or load-failed
SessionWorkspace->>DiagramCanvas: pass ZenUML snapshot and callbacks
DiagramCanvas->>ZenUmlMutations: submit semantic mutation
ZenUmlMutations-->>SessionWorkspace: return canonical source
SessionWorkspace->>Mermaid: parse and render updated source
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (12)
e2e-sequence-validate.ts (2)
218-218: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTighten the participant assertion.
mutated.includes(' Service')matches any line that contains two spaces followed byService, including a message body. Assert the exact declaration line instead.💚 Proposed fix
- if (!mutated.includes(' Service') || !mutated.includes(' Client->API: confirmed') || !mutated.includes(' if(condition) {')) { + if (!/^ {2}Service$/mu.test(mutated) || !mutated.includes(' Client->API: confirmed') || !mutated.includes(' if(condition) {')) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e-sequence-validate.ts` at line 218, Update the participant assertion in the mutated-sequence validation to match the exact Service declaration line rather than using the broad substring check `mutated.includes(' Service')`; preserve the existing Client/API and conditional-content assertions.
217-228: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueRemove the redundant
closeSourceFlyoutcalls.
readCanonicalSourcealready closes the flyout. The later calls are safe no-ops, but they add unnecessary control flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e-sequence-validate.ts` around lines 217 - 228, Remove the redundant closeSourceFlyout calls after readCanonicalSource in the mutation and undo validation flow. Keep readCanonicalSource responsible for closing the flyout, while preserving the existing canvas undo and canonical-source assertions.e2e-workspace-ux-validate.ts (3)
5834-5842: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the document-viewport assertion to the external plugin loop.
The
PRIMARY_STARTER_TEMPLATESloop above callsassertDocumentMatchesViewportat Line 5830 for every row. The newEXTERNAL_STARTER_TEMPLATESloop omits it. A plugin row with a long label can therefore introduce horizontal document overflow on a 320px viewport without failing the gate.💚 Proposed fix
await assertContainedInViewport(page, menu, `${label} ${template.label} plugin chooser containment`); + await assertDocumentMatchesViewport(page, `${label} ${template.label} plugin chooser`); assert(await renderedCanvasCameraTransform(page, `${label} ${template.label} plugin chooser camera`) === menuCamera,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e-workspace-ux-validate.ts` around lines 5834 - 5842, Add an assertDocumentMatchesViewport check inside the EXTERNAL_STARTER_TEMPLATES loop, alongside the existing touch-target, containment, and camera assertions, so every external plugin row validates document bounds on narrow viewports like the PRIMARY_STARTER_TEMPLATES loop.
5913-5917: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the plugin row from the catalog instead of hardcoding
ZenUML.Line 5915 hardcodes
'ZenUML · lazy plugin'. Line 5913 already computes the expected count fromEXTERNAL_STARTER_TEMPLATES. A second available plugin, or a ZenUML label change, breaks this check silently or leaves the new plugin untested on phones.♻️ Proposed refactor
- const availableZenUml = templateMenu.getByTestId('starter-template-create').filter({ hasText: 'ZenUML · lazy plugin' }); - await availableZenUml.scrollIntoViewIfNeeded(); - await assertTouchTarget(page, availableZenUml, `${label} ZenUML plugin template`); + for (const template of EXTERNAL_STARTER_TEMPLATES) { + const pluginRow = templateMenu.getByTestId('starter-template-create').filter({ hasText: `${template.label} · lazy plugin` }); + await pluginRow.scrollIntoViewIfNeeded(); + await assertTouchTarget(page, pluginRow, `${label} ${template.label} plugin template`); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e-workspace-ux-validate.ts` around lines 5913 - 5917, Replace the hardcoded `ZenUML · lazy plugin` filter in the touch-target check with a plugin starter row derived from `EXTERNAL_STARTER_TEMPLATES`. Select an available external template from that catalog, locate its corresponding `starter-template-create` row, and preserve the existing `scrollIntoViewIfNeeded` and `assertTouchTarget` validation for the derived plugin.
1343-1351: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the counted value in the failure message.
Store
await enabledItems.count()in a local variable before the assertion. The current code performs the Playwright count twice on every run.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e-workspace-ux-validate.ts` around lines 1343 - 1351, In the enabled-items assertion near expectedEnabledItems, store the result of await enabledItems.count() in a local variable and reuse it both for the comparison and failure message, avoiding duplicate Playwright count calls.apps/web/src/components/diagram-canvas.tsx (1)
6112-6133: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueScope the form error state per form.
The three add-forms share one
errorstate and oneNumericEditorErrorat Line 6134. A failed participant submission clears any prior message error, and the single banner does not indicate which form produced the message. The per-item forms at Lines 6148-6152 already use their own local error state.Split
errorinto three states, or render the banner inside each form.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/diagram-canvas.tsx` around lines 6112 - 6133, Scope semantic submission errors independently to the participant, message, and control forms instead of sharing one error state and banner. Update each form’s runSemanticForm call to use its corresponding state, and render its NumericEditorError within that form so failures identify the originating form without clearing errors from the others.apps/web/src/lib/diagram-preview.ts (1)
36-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the adapter parameter type instead of repeating the union.
The
adapterparameter repeats almost every member ofDiagramAdapterId. Each new family requires a manual edit here in addition todiagram-capabilities.ts. Deriving the type removes that second edit and prevents the two lists from drifting.♻️ Proposed refactor
-export function canUseSemanticFamilyControls(source: string, preview: DiagramPreview | null, adapter: 'architecture' | 'block' | ... | 'zenuml'): boolean { +type SemanticFamilyAdapterId = Exclude<DiagramAdapterId, 'flowchart' | 'sequence' | 'er' | 'source-only' | 'unavailable-plugin'>; + +export function canUseSemanticFamilyControls(source: string, preview: DiagramPreview | null, adapter: SemanticFamilyAdapterId): boolean {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/diagram-preview.ts` at line 36, Update canUseSemanticFamilyControls to derive the adapter parameter type from the existing DiagramAdapterId type instead of declaring a duplicate union. Import or reference DiagramAdapterId directly, preserving the function’s current accepted adapter values while ensuring future additions require no manual update here.apps/web/src/lib/zenuml-mutations.ts (4)
34-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the duplicate representability predicates.
isZenUmlDiagramSourceandisZenUmlSourceRepresentablehave identical bodies. Two names for one predicate invite divergence later, because "is this ZenUML source" and "can the form represent this source" are different questions. OnlyisZenUmlSourceRepresentableis consumed bydiagram-capabilities.tsanddiagram-canvas.tsxin this cohort.Either delete
isZenUmlDiagramSource, or give it a distinct definition (header detection only) if detection must stay separate from representability.♻️ Proposed change if detection must stay separate
-export function isZenUmlDiagramSource(source: string): boolean { return parseZenUml(source) !== null; } +export function isZenUmlDiagramSource(source: string): boolean { + const header = splitLines(source).find((line) => line.text.trim() && !COMMENT.test(line.text.trim())); + return Boolean(header && HEADER.test(header.text.trim())); +} export function isZenUmlSourceRepresentable(source: string): boolean { return parseZenUml(source) !== null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/zenuml-mutations.ts` around lines 34 - 35, Remove the duplicate isZenUmlDiagramSource predicate, leaving isZenUmlSourceRepresentable as the sole parse-based representability check used by diagram-capabilities.ts and diagram-canvas.tsx. If detection is required elsewhere, redefine isZenUmlDiagramSource with distinct header-detection behavior rather than duplicating parseZenUml(source) !== null.
150-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant parse in
addZenUmlControl.Line 151 calls
requireZenUml(source || 'zenuml')and discards the result. Line 152 parses the same source again. The first call only serves as validation, which the second call already performs. Parsing runs twice on every control insertion.♻️ Proposed fix
export function addZenUmlControl(source: string, control: ZenUmlControl, parent?: ZenUmlBlockIdentity): string { - requireZenUml(source || 'zenuml'); const value = normalizeControl(control); const head = formatControl(value); - const parsed = requireZenUml(source || 'zenuml'); + const parsed = requireZenUml(source || 'zenuml'); + const value = normalizeControl(control); const head = formatControl(value); return requireValidMutation(insertBlockStatement(source || 'zenuml', parsed, `${head} {${preferredEol(source || 'zenuml')}__CLOSE__`, parent)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/zenuml-mutations.ts` around lines 150 - 154, Remove the discarded requireZenUml call at the start of addZenUmlControl; retain the parsed value assignment and subsequent requireValidMutation flow, since the retained call already performs validation.
251-253: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe ZenUML kind members are declared four times.
ZenUmlParticipantKindandZenUmlControlKindare type-literal unions, so no runtime constant exists. Every consumer that needs the members at runtime retypes them. Adding a kind requires four coordinated edits, and any missed edit produces a form option that the normalizer rejects.
apps/web/src/lib/zenuml-mutations.ts#L251-L253: exportZENUML_PARTICIPANT_KINDS,ZENUML_MESSAGE_KINDS, andZENUML_CONTROL_KINDSasas constarrays, derive the three type aliases from them withtypeof X[number], and makenormalizeParticipant,normalizeMessage, andnormalizeControlvalidate against those arrays.apps/web/src/components/diagram-canvas.tsx#L6063-L6064: delete the localZENUML_PARTICIPANT_KINDSandZENUML_CONTROL_KINDSdeclarations and import the exported constants from../lib/zenuml-mutations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/zenuml-mutations.ts` around lines 251 - 253, The ZenUML kind lists are duplicated across the normalizers and UI. In apps/web/src/lib/zenuml-mutations.ts lines 251-253, export as-const participant, message, and control kind arrays, derive the corresponding type aliases from them, and update normalizeParticipant, normalizeMessage, and normalizeControl to validate against those arrays. In apps/web/src/components/diagram-canvas.tsx lines 6063-6064, remove the local participant/control arrays and import the exported constants from ../lib/zenuml-mutations.
21-32: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueStatic analysis ReDoS hints on these regexes are false positives.
The ast-grep
regexp-from-variablewarnings at lines 22-26 and line 268 flagnew RegExpbuilt fromIDENTIFIER.IDENTIFIERis a module-level string literal, not user input, so no untrusted pattern reaches the regex constructor. The OpenGrepcommand-injection.exec-jsfindings are also false positives; those calls areRegExp.prototype.exec, notchild_process.exec.One small improvement remains: line 268 compiles a new
RegExpon everynormalizeIdentifiercall. Hoist it to a module constant.♻️ Proposed fix
+const IDENTIFIER_ONLY = new RegExp(`^${IDENTIFIER}$`, 'u');-function normalizeIdentifier(value: string, label: string): string { const next = value.trim(); if (!new RegExp(`^${IDENTIFIER}$`, 'u').test(next)) throw new Error(`ZenUML ${label} must be a simple identifier.`); return next; } +function normalizeIdentifier(value: string, label: string): string { const next = value.trim(); if (!IDENTIFIER_ONLY.test(next)) throw new Error(`ZenUML ${label} must be a simple identifier.`); return next; }Also applies to: 268-268
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/zenuml-mutations.ts` around lines 21 - 32, Hoist the regular expression currently constructed inside normalizeIdentifier into a module-level constant alongside the other regex definitions. Update normalizeIdentifier to reuse that constant on every call, preserving the existing matching and replacement behavior.Source: Linters/SAST tools
apps/web/src/lib/zenuml-mutations.test.ts (1)
107-138: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd reorder coverage for sibling blocks at equal depth.
The reorder tests move messages and controls that live at the top level only.
moveZenUmlMessageandmoveZenUmlControlselect peers bydepthalone, so two sibling blocks produce peers with equal depth. No test pins the expected behavior for that case.Add a case with two sibling
ifblocks, each containing one message, and assert the intended result. This locks down the behavior flagged inapps/web/src/lib/zenuml-mutations.ts.💚 Proposed test
+ it('keeps reorder inside the owning block', () => { + const source = 'zenuml\n if(a) {\n A->B: one\n }\n if(b) {\n A->B: two\n }'; + const diagram = getZenUmlDiagramSnapshot(source); + expect(moveZenUmlMessage(source, getZenUmlMessageIdentity(diagram.messages[0]!, diagram.messages), 'down')).toBe(source); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/zenuml-mutations.test.ts` around lines 107 - 138, Add reorder coverage in the existing ZenUML mutation tests for two sibling if blocks at the same depth, each containing one message; move one block with moveZenUmlControl and assert the resulting serialized source reflects the intended sibling order while preserving each block’s child message. Use getZenUmlControlIdentity and the existing control creation flow so the test specifically exercises equal-depth sibling selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/diagram-canvas.tsx`:
- Around line 6089-6093: Update the alias select rendering near the participant
and message draft state to provide a visible fallback option when aliases is
empty. Ensure both sender and recipient ZenUmlAliasSelect controls render an
option for the current fallback alias, such as Client, while preserving the
existing alias options when participants are present.
In `@apps/web/src/lib/diagram-capabilities.ts`:
- Around line 320-324: Update the available-plugin branch in the diagram
capability mapping to use externalFamily.id: preserve the ZenUML adapter and
family only for the zenuml ID, and return source-only for other available plugin
families while retaining the shared diagram metadata. Add a regression test
covering a second available plugin.
In `@apps/web/src/lib/zenuml-mutations.ts`:
- Around line 143-148: Update moveZenUmlMessage and the corresponding
control-reorder function to track each record’s owning parent block, then
restrict peer selection to records with the same owner as the current item in
addition to the existing depth and block filters. Use the owning-block
relationship when determining reorder candidates so swaps cannot cross sibling
branches, while preserving existing validation and no-op behavior.
In `@package.json`:
- Around line 11-14: Move the four dependency overrides from the package.json
overrides configuration into the pnpm-workspace.yaml overrides section,
preserving their package selectors and versions. Remove the package.json
override entries, then regenerate pnpm-lock.yaml with the workspace
configuration and validate that the lockfile reflects all four overrides.
---
Nitpick comments:
In `@apps/web/src/components/diagram-canvas.tsx`:
- Around line 6112-6133: Scope semantic submission errors independently to the
participant, message, and control forms instead of sharing one error state and
banner. Update each form’s runSemanticForm call to use its corresponding state,
and render its NumericEditorError within that form so failures identify the
originating form without clearing errors from the others.
In `@apps/web/src/lib/diagram-preview.ts`:
- Line 36: Update canUseSemanticFamilyControls to derive the adapter parameter
type from the existing DiagramAdapterId type instead of declaring a duplicate
union. Import or reference DiagramAdapterId directly, preserving the function’s
current accepted adapter values while ensuring future additions require no
manual update here.
In `@apps/web/src/lib/zenuml-mutations.test.ts`:
- Around line 107-138: Add reorder coverage in the existing ZenUML mutation
tests for two sibling if blocks at the same depth, each containing one message;
move one block with moveZenUmlControl and assert the resulting serialized source
reflects the intended sibling order while preserving each block’s child message.
Use getZenUmlControlIdentity and the existing control creation flow so the test
specifically exercises equal-depth sibling selection.
In `@apps/web/src/lib/zenuml-mutations.ts`:
- Around line 34-35: Remove the duplicate isZenUmlDiagramSource predicate,
leaving isZenUmlSourceRepresentable as the sole parse-based representability
check used by diagram-capabilities.ts and diagram-canvas.tsx. If detection is
required elsewhere, redefine isZenUmlDiagramSource with distinct
header-detection behavior rather than duplicating parseZenUml(source) !== null.
- Around line 150-154: Remove the discarded requireZenUml call at the start of
addZenUmlControl; retain the parsed value assignment and subsequent
requireValidMutation flow, since the retained call already performs validation.
- Around line 251-253: The ZenUML kind lists are duplicated across the
normalizers and UI. In apps/web/src/lib/zenuml-mutations.ts lines 251-253,
export as-const participant, message, and control kind arrays, derive the
corresponding type aliases from them, and update normalizeParticipant,
normalizeMessage, and normalizeControl to validate against those arrays. In
apps/web/src/components/diagram-canvas.tsx lines 6063-6064, remove the local
participant/control arrays and import the exported constants from
../lib/zenuml-mutations.
- Around line 21-32: Hoist the regular expression currently constructed inside
normalizeIdentifier into a module-level constant alongside the other regex
definitions. Update normalizeIdentifier to reuse that constant on every call,
preserving the existing matching and replacement behavior.
In `@e2e-sequence-validate.ts`:
- Line 218: Update the participant assertion in the mutated-sequence validation
to match the exact Service declaration line rather than using the broad
substring check `mutated.includes(' Service')`; preserve the existing
Client/API and conditional-content assertions.
- Around line 217-228: Remove the redundant closeSourceFlyout calls after
readCanonicalSource in the mutation and undo validation flow. Keep
readCanonicalSource responsible for closing the flyout, while preserving the
existing canvas undo and canonical-source assertions.
In `@e2e-workspace-ux-validate.ts`:
- Around line 5834-5842: Add an assertDocumentMatchesViewport check inside the
EXTERNAL_STARTER_TEMPLATES loop, alongside the existing touch-target,
containment, and camera assertions, so every external plugin row validates
document bounds on narrow viewports like the PRIMARY_STARTER_TEMPLATES loop.
- Around line 5913-5917: Replace the hardcoded `ZenUML · lazy plugin` filter in
the touch-target check with a plugin starter row derived from
`EXTERNAL_STARTER_TEMPLATES`. Select an available external template from that
catalog, locate its corresponding `starter-template-create` row, and preserve
the existing `scrollIntoViewIfNeeded` and `assertTouchTarget` validation for the
derived plugin.
- Around line 1343-1351: In the enabled-items assertion near
expectedEnabledItems, store the result of await enabledItems.count() in a local
variable and reuse it both for the comparison and failure message, avoiding
duplicate Playwright count calls.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 05126613-de70-4373-bac3-1d76367ce340
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
apps/web/package.jsonapps/web/src/app/globals.cssapps/web/src/components/diagram-canvas.tsxapps/web/src/components/session-workspace.tsxapps/web/src/components/workspace-template-picker.test.tsapps/web/src/components/workspace-template-picker.tsxapps/web/src/lib/diagram-capabilities.test.tsapps/web/src/lib/diagram-capabilities.tsapps/web/src/lib/diagram-preview.tsapps/web/src/lib/zenuml-mutations.test.tsapps/web/src/lib/zenuml-mutations.tsapps/web/src/lib/zenuml-runtime.test.tsapps/web/src/lib/zenuml-runtime.tsdocs/context-map.mde2e-sequence-validate.tse2e-workspace-ux-validate.tspackage.jsonpackages/shared/src/mermaid-diagram-catalog.tspackages/shared/src/starter-templates.test.tspackages/shared/src/starter-templates.ts
| const aliases = diagram.participants.map((item) => item.alias); | ||
| const firstAlias = aliases[0] ?? 'Client'; | ||
| const secondAlias = aliases[1] ?? firstAlias; | ||
| const [participant, setParticipant] = useState<ZenUmlParticipant>({ alias: 'Service', kind: 'participant', label: 'Service' }); | ||
| const [message, setMessage] = useState<ZenUmlMessage>({ assignment: null, from: firstAlias, kind: 'async', text: 'request', to: secondAlias }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle the zero-participant case in the alias selects.
aliases is empty when the diagram declares no participants and has no messages. firstAlias then resolves to 'Client' and secondAlias to 'Client', but ZenUmlAliasSelect renders no <option> elements. The select displays an empty control while the draft holds 'Client'. A user cannot see or change the sender and recipient before submitting.
Render a fallback option when aliases is empty.
🐛 Proposed fix
-function ZenUmlAliasSelect({ aliases, label, onChange, value }: { aliases: string[]; label: string; onChange: (value: string) => void; value: string }) { return <select aria-label={label} onChange={(event) => onChange(event.target.value)} style={HIERARCHY_CONTROL_STYLE} value={value}>{aliases.map((alias) => <option key={alias}>{alias}</option>)}</select>; }
+function ZenUmlAliasSelect({ aliases, label, onChange, value }: { aliases: string[]; label: string; onChange: (value: string) => void; value: string }) { const options = aliases.length ? aliases : [value]; return <select aria-label={label} onChange={(event) => onChange(event.target.value)} style={HIERARCHY_CONTROL_STYLE} value={value}>{options.map((alias) => <option key={alias}>{alias}</option>)}</select>; }Also applies to: 6120-6122
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/diagram-canvas.tsx` around lines 6089 - 6093, Update
the alias select rendering near the participant and message draft state to
provide a visible fallback option when aliases is empty. Ensure both sender and
recipient ZenUmlAliasSelect controls render an option for the current fallback
alias, such as Client, while preserving the existing alias options when
participants are present.
| if (externalFamily.availability === 'available-plugin') { | ||
| return { | ||
| adapter: 'zenuml', diagramType, editingMode: 'semantic-form', family: 'zenuml', kind: 'generic', label: externalFamily.label, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the external Mermaid plugin family record shape.
set -euo pipefail
fd 'mermaid-diagram-catalog.ts' packages/shared/src --exec rg -n -C6 'EXTERNAL_MERMAID_PLUGIN_FAMILIES|availability|getExternalMermaidDiagramFamily' {}Repository: donovan-yohan/arielcharts
Length of output: 14390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- capability resolver ---'
sed -n '270,350p' apps/web/src/lib/diagram-capabilities.ts
printf '%s\n' '--- related adapter and external-family references ---'
rg -n -C4 'getDiagramSourceModelAdapter|EXTERNAL_MERMAID_PLUGIN_FAMILIES|available-plugin|ZENUML_ADAPTER|source-only' apps/web/src/lib
printf '%s\n' '--- capability tests ---'
sed -n '100,170p' apps/web/src/lib/diagram-capabilities.test.tsRepository: donovan-yohan/arielcharts
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
path = Path("apps/web/src/lib/diagram-capabilities.ts")
text = path.read_text()
branch = re.search(
r"const externalFamily = getExternalMermaidDiagramFamily\(diagramType\);"
r"(.*?)\n\s*}\n\n\s*return \{\n\s*adapter: 'source-only'",
text,
re.S,
)
if not branch:
raise SystemExit("external-family branch not found")
body = branch.group(1)
print("available-plugin branch uses family id:", "externalFamily.id" in body)
print("available-plugin branch returns ZenUML adapter:", "adapter: 'zenuml'" in body)
print("available-plugin branch returns ZenUML family:", "family: 'zenuml'" in body)
def current_dispatch(family):
if family["availability"] == "available-plugin":
return {"adapter": "zenuml", "family": "zenuml"}
return {"adapter": "unavailable-plugin", "family": "zenuml"}
hypothetical = {"id": "other-plugin", "availability": "available-plugin"}
print("hypothetical available other-plugin dispatch:", current_dispatch(hypothetical))
if current_dispatch(hypothetical) != {"adapter": "source-only", "family": "other-plugin"}:
print("FAIL: an available non-ZenUML family is routed to ZenUML")
raise SystemExit(1)
PYRepository: donovan-yohan/arielcharts
Length of output: 459
Key the available-plugin branch by family ID.
externalFamily.id is available, but every available external family currently maps to the ZenUML adapter and family. Return source-only for available families other than zenuml, and add a regression test for a second available plugin.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/lib/diagram-capabilities.ts` around lines 320 - 324, Update the
available-plugin branch in the diagram capability mapping to use
externalFamily.id: preserve the ZenUML adapter and family only for the zenuml
ID, and return source-only for other available plugin families while retaining
the shared diagram metadata. Add a regression test covering a second available
plugin.
| export function moveZenUmlMessage(source: string, identity: ZenUmlMessageIdentity, direction: 'up' | 'down'): string { | ||
| const parsed = requireZenUml(source); const current = resolveMessage(parsed, identity); | ||
| const peers = parsed.messages.filter((item) => item.depth === current.depth && !item.line.text.trimEnd().endsWith('{')); | ||
| const index = peers.indexOf(current); const other = peers[index + (direction === 'up' ? -1 : 1)]; | ||
| return other ? requireValidMutation(swapLineText(source, current.line, other.line)) : source; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect ZenUML reorder peer selection and its test coverage.
set -euo pipefail
# Show the move implementations.
rg -n -C4 'export function moveZenUml' apps/web/src/lib/zenuml-mutations.ts
# Show whether any test covers reordering inside sibling blocks.
rg -n -C3 'moveZenUmlMessage|moveZenUmlControl' apps/web/src/lib/zenuml-mutations.test.tsRepository: donovan-yohan/arielcharts
Length of output: 4675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- ZenUML mutation implementation ---'
sed -n '1,230p' apps/web/src/lib/zenuml-mutations.ts
printf '%s\n' '--- parser and record definitions ---'
rg -n -C5 'interface (MessageRecord|ControlRecord)|type (MessageRecord|ControlRecord)|depth|parent|compoundFragment|parseZen' apps/web/src/lib/zenuml-mutations.ts apps/web/src/lib
printf '%s\n' '--- relevant tests and fixtures ---'
sed -n '1,190p' apps/web/src/lib/zenuml-mutations.test.ts
rg -n -C5 'if\\(a\\)|if\\(b\\)|nested|depth|moveZenUmlMessage|moveZenUmlControl' apps/web/src/lib apps/webRepository: donovan-yohan/arielcharts
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- parser implementation ---'
sed -n '175,240p' apps/web/src/lib/zenuml-mutations.ts
printf '%s\n' '--- reorder test section ---'
sed -n '85,155p' apps/web/src/lib/zenuml-mutations.test.ts
printf '%s\n' '--- swap helpers ---'
rg -n -C8 'function (swapLineText|swapRanges)|const (CONTROL|SYNC|ASYNC|CLOSE_CONTROL)' apps/web/src/lib/zenuml-mutations.tsRepository: donovan-yohan/arielcharts
Length of output: 13354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
source = """zenuml
if(a) {
A->B: one
}
if(b) {
A->B: two
}
"""
control = re.compile(r"^(?:(if|while|for|foreach|forEach|loop|catch)\s*(?:\(([^{}\r\n]*)\))?|(?:else\s+if)\s*\(([^{}\r\n]*)\)|(else|opt|par|try|finally))\s*\{\s*$")
async_message = re.compile(r"^([A-Za-z_$][A-Za-z0-9_$.-]*)\s*->\s*([A-Za-z_$][A-Za-z0-9_$.-]*)\s*:\s*(.+?)\s*$")
close = re.compile(r"^\}\s*$")
stack_depth = 0
messages = []
controls = []
lines = source.splitlines(keepends=True)
for index, raw in enumerate(lines):
text = raw.strip()
if not text or text == "zenuml":
continue
if close.fullmatch(text):
stack_depth -= 1
continue
if async_message.fullmatch(text):
messages.append((index, stack_depth, raw))
continue
if control.fullmatch(text):
controls.append((index, stack_depth, raw))
stack_depth += 1
continue
raise AssertionError(f"unparsed line: {text!r}")
assert [(depth, raw.strip()) for _, depth, raw in messages] == [
(1, "A->B: one"),
(1, "A->B: two"),
]
assert len({depth for _, depth, _ in controls}) == 1
# Model moveZenUmlMessage: same-depth, non-opening message peers.
message_peer_indexes = [i for i, _, _ in messages]
a, b = message_peer_indexes
mutated = list(lines)
mutated[a], mutated[b] = mutated[b], mutated[a]
message_result = "".join(mutated)
assert "if(a) {\n A->B: two\n" in message_result
assert "if(b) {\n A->B: one\n" in message_result
control_source = """zenuml
if(a) {
if(x) {
A->B: one
}
}
if(b) {
if(y) {
A->B: two
}
}
"""
# Model moveZenUmlControl: same-depth controls in separate parent blocks.
control_lines = control_source.splitlines(keepends=True)
control_records = []
depth = 0
for index, raw in enumerate(control_lines):
text = raw.strip()
if not text or text == "zenuml":
continue
if close.fullmatch(text):
depth -= 1
continue
if control.fullmatch(text):
control_records.append((index, depth))
depth += 1
continue
if async_message.fullmatch(text):
continue
raise AssertionError(f"unparsed line: {text!r}")
assert control_records == [(1, 0), (2, 1), (6, 0), (7, 1)]
inner = [record for record in control_records if record[1] == 1]
assert len(inner) == 2
# Compute each same-depth control's full range and model swapRanges.
def matching_close(start):
level = 0
for index in range(start, len(control_lines)):
text = control_lines[index].strip()
if control.fullmatch(text):
level += 1
elif close.fullmatch(text):
level -= 1
if level == 0:
return index
raise AssertionError("missing close")
left_start, right_start = inner[0][0], inner[1][0]
left_end, right_end = matching_close(left_start), matching_close(right_start)
swapped = (
control_lines[:left_start]
+ control_lines[right_start:right_end + 1]
+ control_lines[left_end + 1:right_start]
+ control_lines[left_start:left_end + 1]
+ control_lines[right_end + 1:]
)
control_result = "".join(swapped)
assert "if(a) {\n if(y) {" in control_result
assert "if(b) {\n if(x) {" in control_result
print("same-depth records have different owners; both reorder models move content across branches")
PYRepository: donovan-yohan/arielcharts
Length of output: 254
Restrict ZenUML reorder peers to the same parent block.
depth does not identify a parent. Same-depth messages and controls in sibling branches are treated as peers, so both reorder functions can move content across branches while requireValidMutation accepts the result. Store each record’s owning block and filter peers by that owner.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/lib/zenuml-mutations.ts` around lines 143 - 148, Update
moveZenUmlMessage and the corresponding control-reorder function to track each
record’s owning parent block, then restrict peer selection to records with the
same owner as the current item in addition to the existing depth and block
filters. Use the owning-block relationship when determining reorder candidates
so swaps cannot cross sibling branches, while preserving existing validation and
no-op behavior.
| "@zenuml/core": "3.50.1", | ||
| "@zenuml/core>dompurify": "3.4.13", | ||
| "postcss>nanoid": "3.3.17", | ||
| "tailwindcss>postcss": "8.5.26" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
pnpm view dompurify@3.4.13 version
pnpm view nanoid@3.3.17 version
pnpm view dompurify@3.4.12 version
pnpm view nanoid@3.3.8 versionRepository: donovan-yohan/arielcharts
Length of output: 3498
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- tracked package/config files ---'
git ls-files | rg '(^|/)(package\.json|pnpm-lock\.yaml|pnpm-workspace\.yaml|\.npmrc|\.pnpmfile\.cjs|packageManager|corepack)' || true
printf '%s\n' '--- package.json ---'
cat -n package.json
printf '%s\n' '--- lockfile header and override-related entries ---'
if [ -f pnpm-lock.yaml ]; then
sed -n '1,80p' pnpm-lock.yaml
rg -n -C 3 'dompurify|nanoid|overrides|`@zenuml/core`|tailwindcss|postcss' pnpm-lock.yaml || true
fi
printf '%s\n' '--- package-manager configuration ---'
for f in .npmrc pnpm-workspace.yaml .pnpmfile.cjs; do
if [ -f "$f" ]; then
printf '%s\n' "--- $f ---"
cat -n "$f"
fi
doneRepository: donovan-yohan/arielcharts
Length of output: 14043
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf '%s\n' '--- available package-manager versions ---'
pnpm --version || true
corepack pnpm --version 2>/dev/null || true
printf '%s\n' '--- pnpm config/help references ---'
pnpm config list 2>/dev/null | rg -n -i 'override|workspace|config' || true
pnpm help overrides 2>/dev/null | sed -n '1,100p' || trueRepository: donovan-yohan/arielcharts
Length of output: 855
🌐 Web query:
pnpm 10.15.0 package.json pnpm.overrides supported configuration pnpm-workspace.yaml
💡 Result:
In pnpm version 10.15.0, the overrides configuration is supported and must be defined in the pnpm-workspace.yaml file [1][2]. Key details regarding this configuration: - Location: The overrides field must be placed at the root of the workspace in the pnpm-workspace.yaml file [3][1]. - Compatibility: While older versions of pnpm allowed pnpm settings in package.json, this changed in pnpm v11 [4]. In pnpm 10.x, users are encouraged to define overrides in pnpm-workspace.yaml [1][5]. - Functionality: The overrides field allows you to force specific dependency versions, backport fixes, replace dependencies with forks, or remove unused dependencies [3][2]. - Advanced Usage: You can use catalog references [3], target specific sub-dependencies using the > syntax [3][2], or remove dependencies entirely by setting the value to "-" [3][1]. Note: If you encounter issues with overrides being ignored, ensure they are correctly defined in pnpm-workspace.yaml rather than the legacy pnpm field in package.json, as the behavior for configuration locations was updated in newer major versions of pnpm [6][4].
Citations:
- 1: https://pnpm.io/10.x/settings
- 2: https://github.com/pnpm/pnpm.io/blob/main/versioned_docs/version-10.x/settings.md
- 3: https://pnpm.io/settings/dependency-resolution
- 4: https://pnpm.io/package_json
- 5: https://pnpm.io/10.x/cli/audit
- 6: pnpm 11 silently ignores
pnpm.overridesandpnpm.patchedDependenciesinpackage.json(no deprecation warning, security-critical) pnpm/pnpm#11536
Move the overrides to pnpm-workspace.yaml.
pnpm 10.15.0 ignores pnpm.overrides in package.json. Move all four overrides to the workspace configuration, then regenerate and validate pnpm-lock.yaml.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 11 - 14, Move the four dependency overrides from
the package.json overrides configuration into the pnpm-workspace.yaml overrides
section, preserving their package selectors and versions. Remove the
package.json override entries, then regenerate pnpm-lock.yaml with the workspace
configuration and validate that the lockfile reflects all four overrides.
Summary
External dependency policy
@mermaid-js/mermaid-zenumlis pinned to0.2.3@zenuml/coreis pinned to3.50.13.4.13, PostCSS8.5.26, and NanoID3.3.17/s/[id]initial route build manifest, preserving lazy isolation for built-in diagramsEditor and runtime behavior
} else {/} else if (...) {formsValidation
Exact head:
db77d96387dbef96886a37263056895aa7fc0867Exact tree:
5ded38c2b7a6ddb2d5e8f39885c148a2539ac426Closes #56
Summary by CodeRabbit