Skip to content

Conversation

@maparent
Copy link
Collaborator

@maparent maparent commented Jan 8, 2026

The source of the problem is that I failed if many relations had the same source, target and name. But relations can differ through their triple and have the same Id. I do not send an internal error in that innocuous case anymore.
Also, I do not fail the process even in that case but take the first relation.
https://www.loom.com/share/1c829585567c49fa873d51697847ffbb

Summary by CodeRabbit

  • Bug Fixes
    • Refined relation matching error handling to reduce false error reporting when multiple similar relations exist, with improved fallback behavior for edge cases.

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

@linear
Copy link

linear bot commented Jan 8, 2026

@supabase
Copy link

supabase bot commented Jan 8, 2026

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

@maparent
Copy link
Collaborator Author

maparent commented Jan 8, 2026

@CodeRabbit review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 8, 2026

📝 Walkthrough

Walkthrough

The identifyRelationMatch function in CreateRelationDialog.tsx has been refined to handle multiple candidate relations more intelligently. Instead of immediately erroring on any length other than 1, the function now normalizes direction-specific IDs (forward: r.id, reverse: -r.id) and only reports errors when truly distinct relations are found, while still falling through to use the first candidate as a fallback.

Changes

Cohort / File(s) Summary
identifyRelationMatch error handling refinement
apps/roam/src/components/CreateRelationDialog.tsx
Refined multi-candidate relation handling: instead of erroring on any non-single candidate count, now normalizes direction-specific IDs into a set and only errors when multiple distinct identities persist. Falls through to first candidate in innocuous cases where the same relation appears with different triples.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ 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 accurately reflects the main change: allowing handling of the same relation ID with different triples in the identifyRelationMatch function.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1be01db and a6dfc29.

📒 Files selected for processing (1)
  • apps/roam/src/components/CreateRelationDialog.tsx
🧰 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/CreateRelationDialog.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/CreateRelationDialog.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/CreateRelationDialog.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/CreateRelationDialog.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/CreateRelationDialog.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/CreateRelationDialog.tsx
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality

Applied to files:

  • apps/roam/src/components/CreateRelationDialog.tsx
🪛 GitHub Actions: CI
apps/roam/src/components/CreateRelationDialog.tsx

[error] 144-144: TypeScript error TS2339: Property 'join' does not exist on type 'IterableIterator'.

@maparent maparent requested a review from mdroidian January 8, 2026 21:28
@maparent maparent force-pushed the eng-1262-failed-to-add-reified-relation-from-discourse-context branch from 5b68dfb to 394f847 Compare January 8, 2026 23:58
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