[ENG-641] Allow reverse relation creation

[trangdoan982]

Summary by CodeRabbit

• Bug Fixes

  • Relation binding now properly validates directional constraints
  • Invalid node connections prevented with improved error feedback

• Improvements

  • Enhanced error messages display valid connection options
  • Binding direction accurately reflected in text and arrows

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[linear]

ENG-641 add/combine complement labels as relation arrows

[supabase]

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 ↗︎.

[trangdoan982]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes enhance discourse relation binding validation to support bidirectional connections. A new directional validation system is introduced via checkConnectionType() and isValidNodeConnection() methods. Target node compatibility is expanded to consider both relation endpoints, and binding text is updated to reflect directionality. Error handling for invalid connections is also improved.

Changes

Cohort / File(s)	Summary
Discourse Relation Validation System apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx, apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx	Enhanced target node lookup to consider both relation source and destination types; introduced bidirectional validation with checkConnectionType() and isValidNodeConnection() public methods; updated binding text handling to reflect direction (label vs. complement); adjusted validation logic to prevent invalid bindings with improved error messaging.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Bidirectional validation logic: Review the checkConnectionType() method carefully to ensure isDirect and isReverse flags are computed correctly for all relation direction combinations.
• Connection validation consistency: Verify that isValidNodeConnection() properly reflects all constraints and integrates correctly with the binding flow.
• Binding text directionality: Examine the logic that selects between relation.label and relation.complement to ensure it correctly represents the connection direction.
• Error handling paths: Trace through the validation failure scenarios and confirm error toasts are displayed appropriately and bindings are rolled back correctly.
• Type compatibility edge cases: Test scenarios where relations have identical source and destination types, or where bidirectional connections are attempted.

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title '[ENG-641] Allow reverse relation creation' is a specific and clear summary of the main change, accurately reflecting the PR's primary objective of enabling reverse relation bindings.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx (1)

814-819: Consider returning early after deleting the shape.

After deleteShapes([shape.id]) is called on line 817, the shape no longer exists. Returning update on line 818 is redundant and could potentially cause issues if the editor tries to apply the update to a deleted shape.

🔎 Suggested improvement:

               dispatchToastEvent({
                 id: `tldraw-invalid-connection-${shape.id}`,
                 title: "Invalid Connection",
                 description: errorMessage,
                 severity: "error",
               });

               removeArrowBinding(this.editor, shape, handleId);
-              update.props![handleId] = { x: handle.x, y: handle.y };
               this.editor.deleteShapes([shape.id]);
-              return update;
+              return;
             }

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7b4cb58 and 6d7905d.

📒 Files selected for processing (2)

• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx (1 hunks)
• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx (6 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/main.mdc)

**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefer type over interface in TypeScript
Use explicit return types for functions
Avoid any types when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability

Files:

• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx
• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx

apps/roam/**/*.{js,ts,tsx,jsx,json}

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

Prefer existing dependencies from package.json when working on the Roam Research extension

Files:

• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx
• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx

apps/roam/**/*.{ts,tsx,jsx,js,css,scss}

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension

Files:

• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx
• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx

apps/roam/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality

Files:

• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx
• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx

apps/roam/**

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

Implement the Discourse Graph protocol in the Roam Research extension

Files:

• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx
• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx

🧠 Learnings (2)

📚 Learning: 2025-06-17T23:42:29.279Z

Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:95-104
Timestamp: 2025-06-17T23:42:29.279Z
Learning: In the DiscourseGraphs/discourse-graph codebase, DiscourseRelation type properties are either string or Triple[], and the STANDARD_ROLES filter in conceptConversion.ts excludes Triple[] properties, so only string values remain after filtering.

Applied to files:

• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx
• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx

📚 Learning: 2025-07-13T16:47:14.352Z

Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-07-13T16:47:14.352Z
Learning: In the discourse-graph codebase, types.gen.ts contains automatically generated database function type definitions that may have reordered signatures between versions. This reordering is expected behavior from the code generation process and should not be flagged as an issue requiring fixes.

Applied to files:

• apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx

🧬 Code graph analysis (2)

apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx (1)

apps/roam/src/components/canvas/Tldraw.tsx (1)

• discourseContext (112-117)

apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx (3)

apps/roam/src/components/canvas/Tldraw.tsx (1)

• discourseContext (112-117)

apps/roam/src/components/canvas/ToastListener.tsx (1)

• dispatchToastEvent (4-8)

apps/roam/src/components/canvas/DiscourseRelationShape/helpers.tsx (1)

• removeArrowBinding (1629-1639)

🔇 Additional comments (6)

apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx (1)

365-380: LGTM! Bidirectional starting node validation looks correct.

The logic properly extends valid starting nodes to include both source and destination types, enabling reverse relation creation. The flatMap with filter(Boolean) safely handles potentially undefined node texts, and deduplication via Set ensures clean user messaging.

apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx (5)

1552-1584: LGTM! Clean helper methods for connection validation.

The checkConnectionType and isValidNodeConnection methods provide a clear and reusable API for directional validation. The logic correctly identifies both direct and reverse connections.

---

899-928: LGTM! Dynamic text update based on direction.

The logic correctly updates the arrow label to use relation.complement for reverse connections and relation.label for direct connections. The check on line 922 prevents unnecessary updates when text hasn't changed.

---

569-572: Good practice refreshing the shape reference after mutation.

Correctly re-fetches the arrow shape after updateShapes to ensure subsequent operations use the updated state.

---

550-567: LGTM! Comprehensive error messaging for invalid connections.

The logic correctly aggregates valid target types from both forward and reverse relation directions, providing clear feedback to users about which node types are acceptable.

---

592-594: Verify if symmetric relation handling is intentional and documented.

When a relation type is symmetric (source and destination are the same node type), both isDirect and isReverse will be true, causing isOriginal to be false. The code treats this case as a reverse relation by defaulting to relation.label instead of relation.complement. This behavior should be explicitly documented or tested to clarify whether it's the intended semantics for symmetric discourse relations.