Skip to content

ENG-1977 Add schema import data layer for Obsidian - #1264

Open
trangdoan982 wants to merge 6 commits into
eng-1976-add-schema-export-command-to-obsidianfrom
eng-1977-add-schema-import-command-to-obsidian
Open

ENG-1977 Add schema import data layer for Obsidian#1264
trangdoan982 wants to merge 6 commits into
eng-1976-add-schema-export-command-to-obsidianfrom
eng-1977-add-schema-import-command-to-obsidian

Conversation

@trangdoan982

@trangdoan982 trangdoan982 commented Jul 29, 2026

Copy link
Copy Markdown
Member

https://www.loom.com/share/7771ec3f58e749599a5bf406debdb101

Summary

Adds the import data layer — file parsing, match planning, and apply logic. The UI modal is in the next PR.

  • specImport.ts
    • buildSchemaImportMatchPlan — pure, no side effects. Maps every id in the schema file to a local id, id match first and name/label fallback second, and records which items already exist locally.
    • applySchemaImportSelection — creates new relation types and relations as provisional, skips existing matches, preserves a node type's template reference when that template is selected for import or already exists locally.
    • parseDgSchemaFile (in specValidation.ts) does full Zod validation against dgSchemaFileSchema, including a version literal guard.
  • schemaMatching.ts — matching primitives shared with the Supabase import path (see below).

Planning is deliberately separated from applying. The Supabase import path interleaves them — mapNodeTypeIdToLocal and mapRelationTypeToLocal both find-or-create inline, so side effects happen during what reads like a lookup. Keeping the plan pure means the preview in #1265 can show exact counts without touching settings.

Shared matching with the Supabase import path

Both import paths have to answer "does this incoming type already exist locally?" — and they answered it differently. The Supabase path compared names and labels case-sensitively; specImport lowercased them. The same vault reached through the two paths would dedupe differently.

schemaMatching.ts now holds the shared primitives:

Helper Replaces duplicated logic in
findLocalNodeTypeMatch mapNodeTypeIdToLocal, buildSchemaImportMatchPlan
findLocalRelationTypeMatch mapRelationTypeToLocal, buildSchemaImportMatchPlan
findExistingTriple findOrCreateTriple, buildSchemaImportMatchPlan
buildSchemaRid mapNodeTypeIdToLocal, mapRelationTypeToLocal, applySchemaImportSelection

buildSchemaRid pins the "schema" RID subtype in one place so the two paths cannot drift. Node instance ("note") and relation instance ("relation") RIDs are a different concern and are untouched.

Only the finder half of findOrCreateTriple was extracted, not the create half — it calls plugin.saveSettings() per triple, while applySchemaImportSelection batches a single save at the end. Sharing the whole function would have turned one write into N.

⚠️ Behavior change to the shipped Supabase import

Name and label matching is now case-insensitive on both paths. Importing a Claim node type into a vault that already holds claim now reuses the local type instead of creating a near-duplicate. This is a change to an already-shipped code path and is the one thing in this PR worth reviewing on its own merits.

importedFromRid provenance

applySchemaImportSelection sets importedFromRid on created node types, relation types, and relations, derived from the file's vaultId (added in #1263) as orn:obsidian.schema:<appId>/<localId>.

This is byte-identical to what the Supabase path produces for the same vault and local id, so schema imported from a file and content imported from that same vault over Supabase resolve to the same RID.

Like the Supabase path, this records the immediate source vault. If vault B exports a type it originally imported from vault C, the RID points at B, not C — consistent with existing behavior, but it means RIDs are one hop of provenance, not a full chain.

Bug fixed along the way

The apply loop guarded discourse relations against triples that existed at plan time, but not against triples created earlier in the same run. Two schema node types collapsing onto one local type therefore produced duplicate triples. The guard now runs against live settings.

This was latent before this PR, but case-insensitive matching widens the window: Claim and claim in one schema file now collapse onto a single local type where previously they did not.

Stack

PR 4 of 5 for FEE-840. Stacks on #1263.

  1. Shared foundation — ENG-1975 (ENG-1975 Define schema file contract and shared foundation for Obsidian export/import #1180)
  2. Schema selection panel UI — ENG-2083 (ENG-2083 Add schema selection panel UI for Obsidian export/import #1262)
  3. Schema export command — ENG-1976 (ENG-1976 Add schema export command to Obsidian #1263)
  4. This PR — schema import data layer (ENG-1977)
  5. Schema import UI (ENG-2084)

Test plan

Verified:

  • pnpm --filter @discourse-graphs/obsidian check-types passes
  • pnpm --filter @discourse-graphs/obsidian lint — 0 errors (75 pre-existing warnings, none in touched files)
  • RID round-trips: ridToSpaceUriAndLocalId(buildSchemaRid(...)) recovers the original spaceUri/localId pair
  • File-import RID is byte-identical to the Supabase-import RID for the same vault and local id
  • findLocalNodeTypeMatch / findLocalRelationTypeMatch — id match beats a differing name; name/label fallback folds case and trims whitespace; genuinely new items return undefined
  • findExistingTriple — matches on endpoints while ignoring the triple's own id; reversed endpoints and a different relation type both correctly miss

Open in Devin Review

@linear-code

linear-code Bot commented Jul 29, 2026

Copy link
Copy Markdown

ENG-1977

@supabase

supabase Bot commented Jul 29, 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 Jul 29, 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 30, 2026 6:37pm

Request Review

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment thread apps/obsidian/src/utils/specImport.ts
@trangdoan982
trangdoan982 force-pushed the eng-1976-add-schema-export-command-to-obsidian branch from 3af9adf to f364d5a Compare July 29, 2026 02:58
@trangdoan982
trangdoan982 force-pushed the eng-1977-add-schema-import-command-to-obsidian branch from 23c69c1 to d417d43 Compare July 29, 2026 02:59
@trangdoan982
trangdoan982 force-pushed the eng-1977-add-schema-import-command-to-obsidian branch from d417d43 to bc0e07d Compare July 29, 2026 19:45
@trangdoan982
trangdoan982 force-pushed the eng-1976-add-schema-export-command-to-obsidian branch from f364d5a to 09bb0e7 Compare July 29, 2026 19:45
@trangdoan982
trangdoan982 force-pushed the eng-1977-add-schema-import-command-to-obsidian branch from bc0e07d to 34254ad Compare July 30, 2026 02:17
@trangdoan982
trangdoan982 force-pushed the eng-1976-add-schema-export-command-to-obsidian branch from 5119839 to cfff9bf Compare July 30, 2026 03:18
@trangdoan982
trangdoan982 force-pushed the eng-1977-add-schema-import-command-to-obsidian branch from 34254ad to 9073b20 Compare July 30, 2026 03:18
@trangdoan982
trangdoan982 force-pushed the eng-1976-add-schema-export-command-to-obsidian branch from cfff9bf to 3820532 Compare July 30, 2026 17:01
@trangdoan982
trangdoan982 force-pushed the eng-1977-add-schema-import-command-to-obsidian branch from 9073b20 to 26d1ad2 Compare July 30, 2026 17:01
@trangdoan982
trangdoan982 force-pushed the eng-1977-add-schema-import-command-to-obsidian branch from 26d1ad2 to 6b684dc Compare July 30, 2026 17:06
@trangdoan982
trangdoan982 force-pushed the eng-1977-add-schema-import-command-to-obsidian branch from b0abbd4 to 2a39a08 Compare July 30, 2026 17:55
@graphite-app

graphite-app Bot commented Jul 30, 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

Comment thread apps/obsidian/src/utils/specImport.ts
trangdoan982 and others added 4 commits July 30, 2026 14:07
…types

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lySchemaImportSelection

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The remote-space import and the schema-file import both had to answer
"does this incoming type already exist locally?", and answered it
differently: the Supabase path compared names and labels case-sensitively
while specImport lowercased them. The same vault reached through the two
paths would dedupe differently.

Extracts schemaMatching.ts with the id-then-name/label fallback, the
triple identity check, and buildSchemaRid — which pins the "schema" RID
subtype so both paths emit byte-identical RIDs. Node instance and relation
instance RIDs keep their own "note"/"relation" subtypes and are untouched.

Matching is now case-insensitive on both paths. This is a behavior change
to the Supabase import: importing a "Claim" type into a vault holding
"claim" now reuses the local type instead of creating a near-duplicate.

specImport sets importedFromRid from the file's vaultId, so schema
imported from a file and content imported from that same vault via
Supabase resolve to the same RID. Like the Supabase path, this records the
immediate source vault rather than preserving an older origin.

Also fixes a duplicate-triple hole the case-insensitive matching widens:
the apply loop guarded against triples that existed at plan time but not
against ones created earlier in the same run, so two schema node types
collapsing onto one local type produced duplicate triples. The check now
runs against live settings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@trangdoan982
trangdoan982 force-pushed the eng-1976-add-schema-export-command-to-obsidian branch from 31a387c to 83e0548 Compare July 30, 2026 18:12
@trangdoan982
trangdoan982 force-pushed the eng-1977-add-schema-import-command-to-obsidian branch from 2a39a08 to 2f98215 Compare July 30, 2026 18:12
A schema file holding both "Event" and "event" created two node types that
matching then treats as one, because the existing-check only compared
against the vault's types as they were before the import. Same hole for
relation types by label.

Fixed in the planner rather than at apply time: the known-set grows as
types are planned, so the second type resolves to the first the same way
it would resolve to a pre-existing local type. Keeping it in the planner
means nodeTypeIdMapping stays correct — skipping the duplicate at apply
time would leave discourse relations pointing at an id that was never
created.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@trangdoan982

Copy link
Copy Markdown
Member Author

Re: size/scope check

Current: +532 −44 across 4 files — within the file-count preference, over the 400-line limit.

Single problem: make an exported schema file importable into a vault, resolving incoming ids against what already exists there.

Why the changes are coupled: specImport.ts (413 of the added lines, a new file) needs to answer "does this incoming type already exist locally?" — a question the Supabase import path already answered, differently. schemaMatching.ts holds the shared answer, and importNodes.ts / importRelations.ts are moved onto it so the two paths cannot drift apart again. Landing the extraction separately from its first consumer would mean knowingly merging duplicated matching logic and then deleting it.

That said, a clean split does exist and I don't want to overstate the coupling:

  1. Extract schemaMatching.ts + move the Supabase path onto it — ~120 lines, 3 files, pure refactor, no new feature. Includes the case-sensitivity behavior change, which is the part of this PR that most deserves isolated review.
  2. specImport.ts consuming the shared helpers — ~430 lines, 1 file, additive.

Splitting this way would actually improve review quality: the behavior change to the shipped Supabase import currently sits underneath 400+ lines of new file, which is exactly where it's easiest to miss. The cost is one more PR in an already 5-deep stack and a re-stack of #1265.

Happy to do the split if a reviewer prefers it — say the word.

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