Skip to content

ERD infinite canvas#67

Merged
peterjthomson merged 13 commits intomasterfrom
claude/plan-erd-infinite-canvas-38Tye
Jan 20, 2026
Merged

ERD infinite canvas#67
peterjthomson merged 13 commits intomasterfrom
claude/plan-erd-infinite-canvas-38Tye

Conversation

@peterjthomson
Copy link
Owner

@peterjthomson peterjthomson commented Jan 18, 2026

Note

Adds a full ERD visualization feature with multi-renderer support and backend parsing.

  • New ERD panel: erd-canvas option wired into CanvasRenderer; toggle between tldraw (infinite canvas), React Flow (graph), and JSON inspector
  • Schema parsing service: Parses Laravel migrations/models, Rails schema.rb/models, and Mermaid ERD; shared types in lib/services/erd/*
  • IPC + schemas: New ERD IPC handlers (get-erd-schema, detect-erd-framework, parse-mermaid-erd), Zod schemas, preload exposure, and main registration
  • Layout & rendering: Dagre-based layout shared across renderers; custom tldraw entity shape and relationship arrows; React Flow custom node; zoom-to-fit utilities
  • UI/UX & styles: Comprehensive ERD styles; header with framework badge, counts, renderer toggle, refresh; column drag behavior changed to handle-only to avoid interfering with canvas
  • Security: CSP updated to allow tldraw CDN assets
  • Types/docs: Extended VizPanelType and electron types; added ERD docs under docs/features/*

Written by Cursor Bugbot for commit 79d2ea4. This will update automatically on new commits. Configure here.

Comprehensive plan for integrating tldraw SDK to visualize
Laravel and Rails database schemas as interactive ERD diagrams
on an infinite canvas in the viz column.
Implement Entity Relationship Diagram visualization using tldraw SDK:

- Add tldraw (4.2.3) and dagre for infinite canvas and auto-layout
- Create ERD parser service supporting:
  - Laravel migrations and Eloquent models
  - Rails schema.rb and ActiveRecord models
  - Mermaid ERD syntax import
- Create custom EntityShapeUtil for tldraw shapes
- Add ERDCanvasPanel component with:
  - Framework detection (Laravel/Rails badge)
  - Auto-layout using Dagre algorithm
  - Pan/zoom infinite canvas via tldraw
- Register 'erd-canvas' as new VizPanelType
- Add IPC handlers: get-erd-schema, detect-erd-framework, parse-mermaid-erd
- Add comprehensive CSS styles for ERD entities
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@peterjthomson
Copy link
Owner Author

@claude review
Note: You must review the diff fresh and ignore previous PR comments as they are now superseded and have already been resolved.

@claude
Copy link

claude bot commented Jan 20, 2026

Claude finished @peterjthomson's task in 2m 2s —— View job


Code Review

I've completed a fresh review of the ERD infinite canvas implementation. Overall, this is a well-architected feature with clean separation of concerns, but I've identified several issues ranging from critical bugs to code quality improvements.


Critical Issues

1. Race condition preventing schema from rendering after refresh

Location: app/components/panels/viz/erd/ERDCanvasPanel.tsx:64-74

The loadSchema function increments loadVersionRef to handle rapid repo changes, but TldrawRenderer doesn't reset isEditorReady when unmounting. When refresh is clicked:

  1. setLoadingState('loading') triggers TldrawRenderer to unmount
  2. But isEditorReady stays true and editorRef.current points to disposed editor
  3. New schema loads → useEffect at line 49 tries to render with stale editor
  4. After remount, setIsEditorReady(true) doesn't trigger re-render (already true)
  5. Result: blank canvas after refresh

Fix: In TldrawRenderer.tsx:65, reset editor ready state:

useEffect(() => {
  return () => {
    if (editorRef.current) {
      clearERDShapes(editorRef.current)
      setIsEditorReady(false)  // Add this line
    }
  }
}, [])

Fix this →

2. Rails schema parsing fails on common patterns

Location: lib/services/erd/erd-parser-service.ts:371

The regex /create_table\s+"(\w+)"(?:\s*,[^\n]*)?\s+do\s+\|t\|([\s\S]*?)^\s*end/gm uses [^\n]* which fails to handle multiline options blocks. Real Rails schemas often look like:

create_table "users",
  id: :uuid,
  default: -> { "gen_random_uuid()" },
  force: :cascade do |t|

The [^\n]* stops at the first newline, missing the do |t| part.

Fix: Change to non-greedy match that includes newlines:

const tableRegex = /create_table\s+"(\w+)"[\s\S]*?do\s+\|t\|([\s\S]*?)^\s*end/gm

Fix this →


High-Severity Issues

3. Incorrect relationship deduplication discards valid multi-FKs

Location: lib/services/erd/erd-parser-service.ts:754-772

The deduplication key at line 762 is:

const discriminator = rel.from.attribute || rel.label || ''
const key = `${rel.from.entity}-${rel.to.entity}-${discriminator}`

But when parsing Rails/Laravel, many relationships don't have the from.attribute populated (it's added in schema parsing but not always in model parsing). This causes valid multi-FKs like orders.customer_id → users and orders.salesperson_id → users to be deduplicated incorrectly.

Impact: ERDs with self-referential relationships or multiple FKs to the same table will show incomplete data.

Fix: Ensure attribute is always included for FK relationships:

// For FK relationships from schema parsing
const discriminator = rel.from.attribute || rel.to.attribute || rel.label || ''

Fix this →

4. React strict mode issue with useMemo side effects

Location: app/components/panels/viz/erd/renderers/ReactFlowRenderer.tsx:103-108

Using useMemo for side effects (setting state):

const { nodes: initialNodes, edges: initialEdges } = useMemo(
  () => (schema ? schemaToFlow(schema) : { nodes: [], edges: [] }),
  []  // Empty deps with schema in the function
)

Then immediately followed by a useEffect that also sets nodes/edges. This is confusing and violates React patterns. The useMemo with empty deps only runs once, but then useEffect at line 114 handles updates anyway.

Fix: Remove the useMemo and just use initial values:

const [nodes, setNodes, onNodesChange] = useNodesState([])
const [edges, setEdges, onEdgesChange] = useEdgesState([])

useEffect(() => {
  if (schema) {
    const { nodes: newNodes, edges: newEdges } = schemaToFlow(schema)
    setNodes(newNodes)
    setEdges(newEdges)
  }
}, [schema, setNodes, setEdges])

Fix this →


Medium-Severity Issues

5. Zoom implementation doesn't respect maxZoom parameter

Location: app/components/panels/viz/erd/erdUtils.ts:254-285

The function signature and docs say maxZoom is a "max zoom limit" (ceiling), but line 283 passes it as targetZoom to tldraw's zoomToBounds. Looking at the implementation:

const targetZoom = Math.min(fitZoom, maxZoom)

editor.zoomToBounds(bounds, {
  animation: { duration: 300 },
  inset,
  targetZoom,  // This sets exact zoom, not max
})

The zoomToBounds API's targetZoom option sets an exact zoom level, not a maximum. So small diagrams zoom to exactly 0.8 instead of fitting naturally, and large diagrams get cut off.

Fix: Calculate the fit zoom first, then clamp it:

const fitZoom = Math.min(viewportWidth / bounds.width, viewportHeight / bounds.height)
const actualZoom = Math.min(fitZoom, maxZoom)

editor.zoomToBounds(bounds, {
  animation: { duration: 300 },
  inset,
  // Don't use targetZoom, let it fit naturally then clamp
})
editor.setCamera({ ...editor.getCamera(), z: actualZoom })

Fix this →

6. Missing dependency in useCallback causes stale data

Location: app/components/canvas/CanvasRenderer.tsx:384-546

The renderVizSlot callback at line 534 has dependencies:

[
  data.commits,
  data.repoPath,  // ✓ included
  selection.selectedCommit,
  // ... others
]

But the ERD case at line 521 uses data.repoPath, yet the dependency array at line 534 only includes data.commits and data.repoPath as separate properties. When switching repos, if data.commits doesn't change, the callback won't update and ERDCanvasPanel gets stale repoPath.

Fix: Include full data object:

[
  data,  // Full data object
  selection.selectedCommit,
  handlers,
  // ...
]

Fix this →

7. Mermaid cardinality regex incomplete

Location: lib/services/erd/erd-parser-service.ts:619

The left-side cardinality pattern (\|[|o]|[{}]o?) matches ||, |o, {}, {, }o, } but misses valid patterns like o| (zero-or-one) and }| (one-or-more). The right side correctly has [|o][|{] which matches both orders.

Fix: Make it symmetric:

const cardinalityPattern = '(?:\\|[|o]|o\\||[{}][|o]?|o[{}])'

Fix this →


Low-Severity Issues

8. Debug console.log statements left in production

Location: Multiple files

Found in schema parsing and rendering code - these should be removed or converted to proper logging:

  • Validation warnings are good, but should use a logger
  • Performance/debug logs should be removed

Fix: Search for console.log and remove or replace with proper error logging.

Fix this →

9. Potential memory leak from uncanceled RAF

Location: app/components/panels/viz/erd/erdUtils.ts:233-246

The renderERDSchema function returns a cleanup function to cancel requestAnimationFrame, and TldrawRenderer.tsx:52 correctly uses it. However, the RAF callback at line 233 captures the editor reference. If the component unmounts between renderERDSchema call and RAF execution, the cleanup cancels the RAF but the editor is already captured in closure.

Impact: Low - RAF callbacks are lightweight and editor disposal is handled by tldraw.

Recommendation: Add a try-catch in the RAF callback (already done at line 234-238, good!).


Code Quality & Best Practices

10. Excellent architecture patterns

Positives:

  • ✅ Clean separation: renderer-agnostic layout in erd-layout.ts
  • ✅ Shared types and utilities across renderers
  • ✅ Race condition handling with loadVersionRef
  • ✅ Proper cleanup functions in useEffects
  • ✅ Type safety with Zod schemas for IPC
  • ✅ Good error handling and loading states

11. Schema parsing robustness

Positives:

  • ✅ Handles multiple frameworks (Laravel, Rails, Mermaid)
  • ✅ Graceful degradation with fallbacks
  • ✅ Deduplication logic (though needs fixes above)
  • ✅ Validation of numeric values before rendering

Improvements needed:

  • Add tests for edge cases in schema.rb parsing
  • Handle more Laravel migration patterns (e.g., morphs, uuidMorphs)
  • Add validation for entity/relationship IDs to prevent injection

12. Performance considerations

Good:

  • Layout calculation is memoized and shared
  • Dagre layout is efficient for graph visualization
  • Proper use of React hooks for state management

Potential improvements:

  • Large schemas (100+ tables) might be slow - consider virtualization
  • Initial relationship filter is smart (line 17) but could be configurable

13. Documentation

Excellent:

  • Comprehensive docs in docs/features/
  • Clear inline comments explaining complex logic
  • Type definitions are self-documenting
  • Good use of JSDoc for public APIs

Security Review

No significant security issues found. The implementation properly:

  • ✅ Validates file paths before reading
  • ✅ Uses parameterized IPC handlers
  • ✅ Doesn't execute user-provided code
  • ✅ Validates numeric inputs before rendering
  • ✅ CSP updated correctly for tldraw CDN

Testing Recommendations

  1. Add tests for Rails schema regex edge cases
  2. Test multi-FK relationships (orders → users twice)
  3. Test rapid repo switching (race conditions)
  4. Test refresh button functionality
  5. Test empty/invalid schemas
  6. Test very large schemas (performance)

Summary

This is a well-designed feature with good architecture and separation of concerns. The main issues are:

  • Critical: Refresh button breaks due to stale editor state
  • Critical: Rails schema regex too strict
  • High: Multi-FK deduplication bug
  • High: React pattern violation in ReactFlowRenderer
  • Medium: Various parsing and zoom issues

The implementation shows good practices (cleanup functions, race condition handling, type safety), but needs the fixes above to be production-ready.


  • Gather context and analyze PR changes
  • Review architecture and design patterns
  • Check for bugs and security issues
  • Review code quality and best practices
  • Post comprehensive review feedback

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

if (allowedEntityIds.has(rel.from.entity) && allowedEntityIds.has(rel.to.entity)) {
relationshipCounts.set(rel.from.entity, (relationshipCounts.get(rel.from.entity) ?? 0) + 1)
relationshipCounts.set(rel.to.entity, (relationshipCounts.get(rel.to.entity) ?? 0) + 1)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-referencing relationships double-counted in filter

Low Severity

The filterSchemaByRelationshipCount function's documentation states it filters entities with at least N relationships "to OTHER filtered entities", but self-referencing relationships (where rel.from.entity === rel.to.entity) are counted twice. Lines 61-62 both increment the same entity's counter when the relationship is self-referential, giving +2 for a single relationship. Per the documented intent, self-referencing relationships should either be excluded entirely or count as one, not two.

Fix in Cursor Fix in Web

to: { entity: toEntity, cardinality: toCard },
label,
type: 'identifying',
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mermaid parser creates duplicate relationship IDs

Medium Severity

The Mermaid ERD parser generates relationship IDs using only ${fromEntity}_${toEntity}, without including the label or cardinality. When multiple relationships exist between the same two entities (e.g., "USER places ORDER" and "USER manages ORDER"), they receive identical IDs. The deduplication function correctly keeps both relationships based on labels, but their id fields collide. This causes tldraw to overwrite earlier arrow shapes and React Flow to have key collisions, resulting in silently dropped relationships.

Additional Locations (1)

Fix in Cursor Fix in Web

@peterjthomson peterjthomson merged commit 052678d into master Jan 20, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants