ENG-2131 Review and accept imported relation types and triples in Roam by sid597 · Pull Request #1383 · DiscourseGraphs/discourse-graph

sid597 · 2026-09-02T18:21:26Z

Reviewer brief

Result: Settings → Grammar → Relations shows imported relation schemas in a separate read-only "Imported relations" table with their source and Provisional/Accepted status, plus Accept and Delete actions (delete is blocked while reified instances use the schema). Provisional schemas are excluded from relation creation (create relation dialog, suggestions, canvas tools) and from publishing.
Review focus: acceptance is stored in the schema block's props (discourse-graph.status: "accepted") next to ENG-1867's importedFrom, not in the settings record. Provisional is derived as "has importedFrom and not accepted", which also covers schemas imported before this PR. In gatherCorrespondingRelations, filtering the schema list is sufficient to also drop instances of provisional schemas (the existing schema-existence filter removes them).
Note on ticket wording: Roam has no separate relation-type entity. A relation schema row is both the "type" and the "triple", so review/accept applies to that single entity, and no accept cascade is needed (Obsidian cascades triple → type because it stores them separately).
Canvas semantics: provisional schemas stay in discourseContext.relations and in shape-util registration so existing arrows render and stay re-bindable (isValidNodeConnection passes includeProvisional); only creation paths filter them. Accepting updates mounted canvases live via provisionalRelationIds; the toolbar gains a new tool on remount when the accepted label previously had no accepted relation.
Duplicate imports (ticket item, no new code): importSharedRelations.matchImportedRelationSchemas already dedupes by imported rid, then by localId, then by label + source + destination. An accepted schema keeps importedFrom, so a re-import maps onto it and does not reset its status. Deleting an imported schema and re-importing recreates it as provisional; there is no rejected state in v0.
Intentionally unfiltered: query and display surfaces (Export.tsx, getExportTypes, registerDiscourseDatalogTranslators, getDiscourseContextResults, discourse context overlay, relation bindings) still see provisional schemas. The ticket scopes exclusion to creation and publishing.

Verification

Live Roam demo passes: imported actions stay centered before and after acceptance, and delete confirmation fits inside the dialog.
pnpm install --frozen-lockfile and pnpm ci:validate pass from the repo root.
New unit tests for the acceptance util (relationSchemaAcceptance.test.ts, 5 passing).
eslint, prettier, and tsc --noEmit clean on touched files; remaining eslint warnings are pre-existing on untouched lines.
A full pre-PR review ran in a fresh-context subagent; its findings are addressed in 9c05b19 (existing provisional arrows stay re-bindable, live canvas update on accept, error handling + PostHog on accept/delete, invisible instead of opacity-0 confirm buttons, sortable headers on the imported table, getCreatableRelations rename, shared constants).

Loom video

eng-2131-aligned-20260907.mp4

Scope check

Ran $scope-check against ENG-2131 and the final diff.
Scope beyond Done When: None.

Local delegated full review

Ran a comprehensive review of the entire final diff in a subagent with a fresh context.

linear-code · 2026-09-02T18:21:29Z

ENG-2131

vercel · 2026-09-02T18:21:32Z

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

1 Skipped Deployment

Project	Deployment	Actions	Updated
discourse-graph	Skipped		Sep 6, 2026 8:51pm UTC

supabase · 2026-09-02T18:21:33Z

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

chatgpt-codex-connector · 2026-09-02T18:21:37Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-02T18:27:22.711397Z	`d545c52`	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

graphite-app · 2026-09-02T18:21:40Z

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

sid597

Inline notes on the non-obvious decisions in this diff.

sid597 · 2026-09-02T19:20:31Z

+
+type ImportStatus = "provisional" | "accepted";
+
+export const RELATION_SCHEMA_STATUS_PROP_KEY = "status";


The prop key is status to match Obsidian's field (ImportStatus in apps/obsidian/src/types.ts), and provisional is derived the same way as Obsidian's isProvisionalSchema: has importedFrom and status is not accepted. ENG-1611 is the precedent. Schemas imported before this PR carry no status and therefore read as provisional, so no migration is needed.

sid597 · 2026-09-02T19:20:31Z

+        });
+      });
+  };
+  const handleDeleteImported = (rel: Relation) => {


Imported instances arrive together with the schema (ENG-1867 creates them as tentative) and they count as in use here, so a schema that came with instances cannot be deleted until its instances are removed. That is deliberate: deleting the schema would orphan the tentative instances that ENG-1869's instance review still needs. ENG-1869 unblocks deletion by letting users remove those instances.

sid597 · 2026-09-02T19:20:31Z

      </HTMLTable>
+      {importedRelations.length > 0 && (
+        <>
+          <h4 className="mb-1 mt-6">Imported relations</h4>


Imported relations get their own table rather than a badge in the local table because the ticket asks for imported schemas shown separately from local ones. The section is gated on data presence rather than a feature flag: these rows already exist in the graph, and hiding them while they stay excluded from creation would leave no way to accept or delete them. On main today they render as editable local rows, which is the bug this replaces.

sid597 · 2026-09-02T19:20:31Z

 };
+type ImportedRelation = Relation & { importMeta: RelationSchemaImportMeta };
+
+const formatImportedSource = (sourceNodeRid: string): string => {


Obsidian formats source spaces with formatImportSource plus a spaceNames map (apps/obsidian/src/utils/typeUtils.ts). This version stays app-local and shows the raw space URI for non-Roam sources. A shared formatter next to the rid helpers is a follow-up candidate once Roam resolves space names.

sid597 · 2026-09-02T19:20:31Z

@@ -778,7 +791,9 @@ const TldrawCanvasShared = ({
    return Object.keys(allRelationsById);
  }, [allRelationsById]);
  const allRelationNames = useMemo(() => {


A label drops out of the canvas toolbar only when every relation carrying it is provisional. A label with one local and one imported relation keeps its tool, and the endpoint-matching step picks from the accepted list only.

sid597 · 2026-09-02T19:20:50Z

Re the size guideline: 251 of the changed lines are non-test. The hunks are coupled to one problem: the accept UI is only safe to ship together with the creation and publishing exclusions, otherwise imported schemas are either usable with no way to accept them (exclusions alone) or unusable with no way to enable them (UI alone). Splitting would leave one of those intermediate states on main.

devin-ai-integration

Devin Review found 3 potential issues.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

ENG-2131 Review and accept imported relation types and triples in Roam

d545c52

This comment was marked as resolved.

Sign in to view

ENG-2131 Address pre-PR review findings

9c05b19

vercel Bot temporarily deployed to Preview September 2, 2026 19:19 Inactive

sid597 commented Sep 2, 2026

View reviewed changes

Fix imported relation matching and refresh the grammar cache

0b82df3

vercel Bot temporarily deployed to Preview September 6, 2026 19:11 Inactive

Read current relation choices when opening the creation dialog

dfc584f

vercel Bot temporarily deployed to Preview September 6, 2026 19:13 Inactive

ENG-2131 Align imported relation actions with row content

b58f957

vercel Bot temporarily deployed to Preview September 6, 2026 19:52 Inactive

ENG-2131 Keep delete confirmation within the actions column

1a5f2ce

vercel Bot temporarily deployed to Preview September 6, 2026 20:04 Inactive

sid597 requested review from maparent and mdroidian September 6, 2026 20:13

devin-ai-integration Bot reviewed Sep 6, 2026

View reviewed changes

Comment thread apps/roam/src/components/CreateRelationDialog.tsx

Comment thread apps/roam/src/components/settings/DiscourseRelationConfigPanel.tsx Outdated

Comment thread apps/roam/src/components/settings/DiscourseRelationConfigPanel.tsx Outdated

ENG-2131 Refresh mounted relation creation after schema changes

a574c20

vercel Bot temporarily deployed to Preview September 6, 2026 20:51 Inactive


		type ImportStatus = "provisional" \| "accepted";

		export const RELATION_SCHEMA_STATUS_PROP_KEY = "status";

Uh oh!

Conversation

sid597 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer brief

Verification

Loom video

Scope check

Local delegated full review

Uh oh!

linear-code Bot commented Sep 2, 2026

Uh oh!

vercel Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

supabase Bot commented Sep 2, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

graphite-app Bot commented Sep 2, 2026

PR size/scope check

Uh oh!

This comment was marked as resolved.

Uh oh!

sid597 left a comment

Choose a reason for hiding this comment

Uh oh!

sid597 Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

sid597 Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

sid597 Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

sid597 Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

sid597 Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

sid597 commented Sep 2, 2026

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

sid597 commented Sep 2, 2026 •

edited

Loading

vercel Bot commented Sep 2, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 2, 2026 •

edited

Loading