Repository navigation
Add multi-format exports and PowerPoint fidelity modes - #6
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesForward conversion platform
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
LJAYi
changed the title
![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
🧹 Nitpick comments (4)
packages/core/src/layout/apply.ts (1)
358-359: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid spreading
pointsintoMath.max.
expandCanvascollects every coordinate from groups, nodes, edges, and labels intopoints. For large diagrams,Math.max(...points.map(...))can hit the engine argument limit and throw. Areducekeeps the same result without that risk.🤖 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 `@packages/core/src/layout/apply.ts` around lines 358 - 359, Update the width and height calculations in expandCanvas to aggregate coordinate values with reduce rather than spreading mapped points into Math.max. Preserve the existing maximum-coordinate results for both x and y while avoiding large argument lists.apps/web/src/layout/editor.ts (1)
995-999: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueHoist the key array out of the
.some()predicate.
[...this.nodeHandles.keys()][index]reconstructs the full key array on every iteration, making this O(n²) with per-iteration allocations.notify()runs on every mutation/selection, so this is worth hoisting once.♻️ Suggested change
- (this.sidecar.groups ?? []).some(({ id }) => id.startsWith("layout-group-")) || - this.zOrder.some((key, index) => key !== [...this.nodeHandles.keys()][index]), + (this.sidecar.groups ?? []).some(({ id }) => id.startsWith("layout-group-")) || + (() => { + const order = [...this.nodeHandles.keys()]; + return this.zOrder.some((key, index) => key !== order[index]); + })(),🤖 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/layout/editor.ts` around lines 995 - 999, Update the hasOverrides calculation in notify() to materialize this.nodeHandles.keys() once before the zOrder.some() predicate, then compare each key against that reused array. Preserve the existing ordering comparison and other override checks unchanged.packages/core/src/routing/orthogonal.ts (1)
117-143: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffOpen list is fully re-sorted on every pop — replace with a binary heap.
open.sort(compareSearchNodes)followed byopen.shift()on each iteration makes the search ~O(V²·log V). WithmaxSearchBudgetdefaulting to 20_000 and this running synchronously on the main thread (viarouteSelectedEdgein the web editor), a dense diagram can visibly stall the UI. A binary min-heap keyed oncompareSearchNodesgives O(log n) push/pop and preserves the deterministic ordering.🤖 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 `@packages/core/src/routing/orthogonal.ts` around lines 117 - 143, The search loop’s open list currently re-sorts and shifts on every iteration, causing inefficient O(V²·log V) behavior. Replace the open-list operations in the routing search with a binary min-heap ordered by compareSearchNodes, using heap push and pop while preserving the existing deterministic comparator ordering and search behavior.packages/core/src/exporters/svg.ts (1)
336-341: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove imports to the top of the file.
Type imports for
DiagramExporter,DiagramExportOptions,ConversionDiagnostic, andDiagramIRare declared at the very bottom of the file, after all the code that uses them (e.g. Lines 19, 25, 129). ES module import hoisting makes this compile, but it's inconsistent with the rest of the codebase's module layout and can trip lint rules that enforce imports-at-top (e.g.import/first).♻️ Proposed fix
+import type { + DiagramExporter, + DiagramExportOptions, +} from "../export-contract.js"; +import type { ConversionDiagnostic, DiagramIR } from "../types.js"; import type { ExportBounds, ExportDiagram, ... } from "./model.js"; import { assertUniqueIds, number, stableId, styleColor, styleFontFamily, xml } from "./xml.js"; import { exporterFailure, exporterResult, unsupportedMermaidDiagramResult, } from "./result.js"; -import type { - DiagramExporter, - DiagramExportOptions, -} from "../export-contract.js"; -import type { ConversionDiagnostic, DiagramIR } from "../types.js";🤖 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 `@packages/core/src/exporters/svg.ts` around lines 336 - 341, Move the type imports for DiagramExporter, DiagramExportOptions, ConversionDiagnostic, and DiagramIR from the bottom of the SVG exporter module to the top-level import section, before executable code and other declarations. Keep the existing import paths and type-only semantics unchanged.
🤖 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 `@packages/cli/src/run.ts`:
- Around line 149-156: Update pathsReferToSameFile to compare both resolved
paths’ stat().dev and stat().ino values, detecting hard-link aliases as well as
symlinks before writing. Preserve the false result when the output path does not
exist, and add a regression test covering an output hard-linked to the input.
In `@packages/core/test/core.test.ts`:
- Line 493: Update the XML assertion in the relevant test to check for the
compact self-closing DrawingML form "<a:close/>" instead of "<a:close />". Apply
the same correction to the corresponding assertion in the edge-capability test.
---
Nitpick comments:
In `@apps/web/src/layout/editor.ts`:
- Around line 995-999: Update the hasOverrides calculation in notify() to
materialize this.nodeHandles.keys() once before the zOrder.some() predicate,
then compare each key against that reused array. Preserve the existing ordering
comparison and other override checks unchanged.
In `@packages/core/src/exporters/svg.ts`:
- Around line 336-341: Move the type imports for DiagramExporter,
DiagramExportOptions, ConversionDiagnostic, and DiagramIR from the bottom of the
SVG exporter module to the top-level import section, before executable code and
other declarations. Keep the existing import paths and type-only semantics
unchanged.
In `@packages/core/src/layout/apply.ts`:
- Around line 358-359: Update the width and height calculations in expandCanvas
to aggregate coordinate values with reduce rather than spreading mapped points
into Math.max. Preserve the existing maximum-coordinate results for both x and y
while avoiding large argument lists.
In `@packages/core/src/routing/orthogonal.ts`:
- Around line 117-143: The search loop’s open list currently re-sorts and shifts
on every iteration, causing inefficient O(V²·log V) behavior. Replace the
open-list operations in the routing search with a binary min-heap ordered by
compareSearchNodes, using heap push and pop while preserving the existing
deterministic comparator ordering and search behavior.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ef4beebd-b7b7-4ce9-9119-fd05d4fb4748
⛔ Files ignored due to path filters (4)
packages/core/test/fixtures/faithful-edges.svgis excluded by!**/*.svgpackages/core/test/fixtures/mermaid-11-renderer-prefix.svgis excluded by!**/*.svgpackages/core/test/fixtures/mermaid-11-synthetic-semantics.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (83)
.github/workflows/ci.ymlREADME.mdapps/web/package.jsonapps/web/src/layout/arrange.tsapps/web/src/layout/editor.tsapps/web/src/layout/history.tsapps/web/src/layout/index.tsapps/web/src/main.tsapps/web/src/styles.cssapps/web/src/svg-viewer.tsapps/web/tests/e2e/layout-editor.spec.tsapps/web/tests/e2e/workbench.spec.tsapps/web/tests/layout/arrange.test.tsapps/web/tests/layout/history.test.tsdocs/adr/0001-forward-only-unified-ir.mddocs/architecture.mddocs/compatibility.mddocs/edge-capability.mddocs/openapi.yamldocs/roadmap.mdpackage.jsonpackages/cli/README.mdpackages/cli/src/arguments.test.tspackages/cli/src/arguments.tspackages/cli/src/index.tspackages/cli/src/openapi-consistency.test.tspackages/cli/src/run.test.tspackages/cli/src/run.tspackages/core/README.mdpackages/core/src/diagram-ir/classify.tspackages/core/src/diagram-ir/index.tspackages/core/src/diagram-ir/path.tspackages/core/src/diagram-ir/types.tspackages/core/src/export-contract.tspackages/core/src/exporters/capabilities.tspackages/core/src/exporters/drawio.tspackages/core/src/exporters/index.tspackages/core/src/exporters/json-canvas.tspackages/core/src/exporters/model.tspackages/core/src/exporters/result.tspackages/core/src/exporters/svg.tspackages/core/src/exporters/xml.tspackages/core/src/index.tspackages/core/src/layout/apply.tspackages/core/src/layout/identity.tspackages/core/src/layout/index.tspackages/core/src/layout/reconcile.tspackages/core/src/layout/sidecar.tspackages/core/src/layout/types.tspackages/core/src/parse-svg.tspackages/core/src/pptx-connector-patch.tspackages/core/src/pptx.tspackages/core/src/routing/collisions.tspackages/core/src/routing/index.tspackages/core/src/routing/orthogonal.tspackages/core/src/routing/types.tspackages/core/src/source-mapping/extract.tspackages/core/src/source-mapping/index.tspackages/core/src/source-mapping/merge.tspackages/core/src/source-mapping/types.tspackages/core/src/types.tspackages/core/test/collisions.test.tspackages/core/test/core.test.tspackages/core/test/diagram-ir.test.tspackages/core/test/exporters/contract.test.tspackages/core/test/exporters/corpus.test.tspackages/core/test/exporters/exporters.test.tspackages/core/test/exporters/json-canvas.test.tspackages/core/test/exporters/quality.test.tspackages/core/test/exporters/semantic-fidelity.test.tspackages/core/test/fixtures/forward-export-corpus.tspackages/core/test/fixtures/pptx-edge-capability.tspackages/core/test/layout.test.tspackages/core/test/pptx-connector-patch.test.tspackages/core/test/pptx-edge-capability.test.tspackages/core/test/pptx-groups-z-order.test.tspackages/core/test/pptx-package-integrity.test.tspackages/core/test/pptx-preflight.test.tspackages/core/test/public-api.test.tspackages/core/test/routing.test.tspackages/core/test/source-mapping.test.tsscripts/generate-compatibility.mjsscripts/verify-pptx-compatibility.mjs
LJAYi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/main.ts`:
- Line 478: Apply the current layout sidecar to parsed.data before every IR
consumer so layout edits reach diagnostics and exported output. Update
apps/web/src/main.ts lines 478-478 before PPTX/exporter preflight and lines
611-611 before diagramToPptxBlob or forward exporters; both sites should use the
same sidecar-application flow rather than passing raw parsed.data.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 403827f5-48bf-44f3-aa57-8148f833fb7f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
README.mdapps/web/package.jsonapps/web/src/main.tsapps/web/src/styles.cssdocs/adr/0001-forward-only-unified-ir.mddocs/architecture.mddocs/roadmap.mdpackages/core/README.mdpackages/core/src/index.tspackages/core/test/public-api.test.ts
💤 Files with no reviewable changes (4)
- apps/web/src/styles.css
- packages/core/test/public-api.test.ts
- packages/core/README.md
- packages/core/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/architecture.md
- README.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/core/src/stroke-style.ts`:
- Around line 4-30: Update nonZeroDashArray to validate every dash entry before
returning the custom array: any negative or non-finite value must produce
undefined so callers treat the pattern as solid. Ensure effectiveDashKind and
svgDashArray therefore never classify or serialize invalid dash arrays, while
preserving valid custom arrays and existing empty/all-zero behavior.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ed9f953-9b3d-4b2b-a032-72f20a0bc06d
📒 Files selected for processing (12)
apps/web/src/main.tsapps/web/src/styles.cssapps/web/src/svg-viewer.tsapps/web/tests/e2e/workbench.spec.tspackages/core/src/exporters/drawio.tspackages/core/src/exporters/svg.tspackages/core/src/exporters/xml.tspackages/core/src/parse-svg.tspackages/core/src/pptx.tspackages/core/src/stroke-style.tspackages/core/test/exporters/exporters.test.tspackages/core/test/exporters/stroke-consistency.test.ts
💤 Files with no reviewable changes (1)
- apps/web/src/styles.css
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/core/test/exporters/exporters.test.ts
- packages/core/src/exporters/xml.ts
- apps/web/tests/e2e/workbench.spec.ts
- packages/core/src/exporters/svg.ts
- packages/core/src/pptx.ts
- packages/core/src/exporters/drawio.ts
- apps/web/src/main.ts
- packages/core/src/parse-svg.ts
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
LJAYi
marked this pull request as ready for review
July 22, 2026 13:24
LJAYi
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Scope
This PR establishes the forward-export foundation for mmd2pptx. The browser-based visual layout workbench is tracked separately in Draft PR #7. Reverse conversion remains deferred.
Export fidelity
This work also aligns live preview and forward exports across Mermaid curve defaults, vector zoom, dash styles, draw.io, PowerPoint, and Illustrator-compatible SVG output.
Testing
Summary by CodeRabbit
--formatplus PPTX--modeand background color validation.