Skip to content

ENG-1955 Full migration to discourse relation shape#1167

Open
mdroidian wants to merge 3 commits into
mainfrom
eng-1955-full-migration-to-discourse-relation-only
Open

ENG-1955 Full migration to discourse relation shape#1167
mdroidian wants to merge 3 commits into
mainfrom
eng-1955-full-migration-to-discourse-relation-only

Conversation

@mdroidian

@mdroidian mdroidian commented Jun 28, 2026

Copy link
Copy Markdown
Member

Canvas relation shapes previously used the relation block UID as the tldraw shape type. That made relation records graph-specific and brittle: if the relation type was deleted, missing from another graph, or copied across platforms, tldraw could fail to recognize the shape and break the canvas.

This migration moves all canvas relations to a single discourse-relation shape type and stores the actual relation subtype in props.relationTypeId. With one stable shape type, canvases can load gracefully even when the subtype is unknown, deleted, or from a different graph.

This also makes cross-graph and cross-platform tldraw sharing easier because the canvas record format no longer depends on graph-local relation UIDs as shape types.

Manual Test Checklist

Baseline Setup

  • Test in a graph with at least two discourse node types and at least two relation types.
  • Include relation types with complements, for example Supports / Supported By.
  • Include two distinct relation types that can connect the same source and target node types.
  • Test with both auto canvas relations enabled and disabled.
  • Test both local canvas mode and Cloudflare sync mode.

Existing Canvas Migration

  • Open an existing pre-migration canvas with relation arrows whose shape type is a relation id.
  • Confirm the canvas loads without an upgrade/error screen.
  • Confirm old relation arrows render with correct label, color, endpoints, bend, and arrowhead.
  • Confirm old relation bindings still attach to the correct source and target nodes.
  • Confirm migrated relation shapes now behave as discourse-relation with props.relationTypeId.
  • Open a canvas containing an old relation id that no longer exists in settings; confirm it does not break the canvas.
  • Confirm normal tldraw arrows remain normal arrows and are not converted.
  • Confirm Add Source / referenced-node relation tools are not accidentally migrated into canonical discourse relations.

Cloudflare Sync

  • Open an existing Cloudflare-synced room created before this migration.
  • Confirm no invalidRecord error appears.
  • Confirm legacy relation arrows in that room load before/after migration.
  • In two browser windows, create a new relation in one window and confirm it appears correctly in the other.
  • Drag relation endpoints in one window and confirm the other window updates.
  • Delete a relation or connected node and confirm deletion syncs cleanly.
  • Migrate a local canvas to cloud sync and confirm the cloud canvas preserves the local canvas exactly.
  • Confirm local-to-cloud migration does not auto-create duplicate relation arrows.

New Relation Creation

  • Create a relation using the relation toolbar/tool.
  • Confirm the created shape has type discourse-relation, not the relation id.
  • Confirm props.relationTypeId is set to the selected relation id.
  • Confirm label, complement label, and color are correct.
  • Create a relation using the drag-handle overlay/dropdown.
  • Convert a regular tldraw arrow to a discourse relation via the context menu.
  • Confirm arrow conversion preserves geometry, bend, bindings, parent/layer position, and selection.
  • Confirm invalid source/target node combinations warn and do not persist invalid relations.

Relation Editing

  • Drag a relation endpoint from one valid node to another valid node.
  • Confirm the binding updates and the relation remains attached.
  • Reverse a relation direction and confirm the displayed text switches to the complement when appropriate.
  • Move connected nodes and confirm arrows follow them.
  • Delete a connected node and confirm attached relation arrows are removed.
  • Duplicate/copy/paste relation arrows and confirm bindings and relationTypeId remain valid.
  • Confirm relation editing still persists to Roam when stored relations are enabled.

Auto Canvas Relations

  • Add a node with auto canvas relations enabled and confirm expected relations are auto-created.
  • Confirm auto-created arrows use type discourse-relation.
  • Confirm exact duplicate relations are not created on repeated runs.
  • Confirm two different relation types between the same two nodes can both be created.
  • Confirm complement/reverse direction auto-relations are not incorrectly de-duped.
  • Confirm existing legacy relation arrows still count when preventing exact duplicates.

Roam Persistence

  • Create a new relation and confirm the corresponding Roam relation/reified block is created.
  • Create a reverse/complement relation and confirm persistence uses the correct source, target, and relation type.
  • Trigger a persistence rejection case and confirm the temporary arrow is removed.
  • Reload Roam and confirm persisted relation arrows reappear correctly.
  • Rename a referenced Roam page/block and confirm canvas node title sync still works.
  • Delete a referenced Roam page/block and confirm the canvas node and attached relations are cleaned up.

Export / Sharing

  • Export a canvas containing new discourse-relation arrows.
  • Confirm exported canvas/image/SVG includes relation labels and arrows.
  • Import or open exported canvas data in another graph.
  • Confirm canvases still load if the target graph does not have the same relation ids.
  • Confirm unknown relation ids do not crash rendering or export.
  • Confirm referenced-node/add-source arrows still export correctly.

Cross-Graph / Missing Schema Cases

  • Copy/paste or import a canvas relation into a graph where the relation type exists.
  • Copy/paste or import a canvas relation into a graph where the relation type is missing.
  • Confirm missing relation types render gracefully instead of breaking the canvas.
  • Confirm known relation types still show correct labels/complements after import.

Analytics / Instrumentation

  • Create a relation and confirm analytics/logging records the actual relation id/type, not just discourse-relation.
  • Confirm relation creation events still distinguish toolbar, drag-handle, and conversion flows if applicable.

Regression Smoke Test

  • Create, edit, delete, undo, and redo a normal node.
  • Create, edit, delete, undo, and redo a normal tldraw arrow.
  • Create, edit, delete, undo, and redo a discourse relation.
  • Reload the canvas after each major flow and confirm state survives reload.
  • Check browser console for canvas store, migration, schema, or invalidRecord errors.

@linear-code

linear-code Bot commented Jun 28, 2026

Copy link
Copy Markdown

ENG-1955

@graphite-app

graphite-app Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

PR size/scope check

This PR is over our review-size guideline.

  • Recommended: ~200 lines changed
  • Acceptable limit: up to 400 lines when well-scoped/self-contained
  • Preferred file count: fewer than 5 files

Please split this into smaller PRs unless there is a clear reason the changes need to land together.

If keeping it as one PR, please add a brief justification covering:

  • What single problem this PR solves
  • Why the files/changes are coupled

@supabase

supabase Bot commented Jun 28, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@vercel

vercel Bot commented Jun 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
discourse-graph Skipped Skipped Jul 6, 2026 6:06am

Request Review

@mdroidian

Copy link
Copy Markdown
Member Author

@codex review

Comment thread apps/roam/src/components/canvas/DiscourseRelationShape/helpers.tsx

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da09f94deb

ℹ️ 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".

? createLegacyDiscourseNodeShapeUtils(allNodes)
: []),
...createAllRelationShapeUtils(allRelationIds),
...createAllRelationShapeUtils(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep legacy relation types in Cloudflare schemas

For Cloudflare sync rooms that already contain pre-migration relation arrows, the worker keeps serving records whose shape.type is the old relation id because its schema is built from customShapeTypes and it does not run this app's createMigrations. This helper now registers only discourse-relation while the Cloudflare path only preserves legacy node utils, so those old relation records are applied to the client store without a matching shape util and the canvas fails to hydrate. Keep the old relation shape utils/types for the Cloudflare adapter until rooms are migrated.

Useful? React with 👍 / 👎.

Comment thread apps/roam/src/components/canvas/DiscourseNodeUtil.tsx
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.

1 participant