Skip to content

ENG-1857 Import Roam-origin shared nodes through the existing Obsidian importer #1266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sid597 wants to merge 2 commits into
base: main
Choose a base branch
from
+168 −78
Open

ENG-1857 Import Roam-origin shared nodes through the existing Obsidian importer #1266

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22770dc
ENG-1857 Import Roam-origin shared nodes through the existing Obsidia…
sid597 Jul 29, 2026
f44ec99
ENG-1857 Resolve node type before vault writes so a failed schema loo…
sid597 Jul 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 92 additions & 19 deletions apps/obsidian/src/utils/importNodes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,76 @@ const fetchNodeContentForImport = async ({
};
};

type NodeTypeSchemaForInstance = {
nodeTypeId: string;
name: string;
};

export const fetchNodeTypeSchemasForInstances = async ({

@sid597 sid597 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Roam's full variant is body-only markdown (buildFullMarkdown in apps/roam/src/utils/roamToCrossAppConverters.ts emits # Title + body, no YAML), so there is no embedded frontmatter to read identity from — this resolves each instance's node type from the source space's Concept rows instead. It consolidates the two queries computeImportPreview was already running inline (parity table in the description). name is consumed by the preview caller only; the import path uses nodeTypeId. Both queries carry arity = 0 so relation instances / relation-type schemas (arity 2) can't slip in regardless of what ids a caller passes.

client,
spaceId,
nodeInstanceIds,
}: {
client: DGSupabaseClient;
spaceId: number;
nodeInstanceIds: string[];
}): Promise<Map<string, NodeTypeSchemaForInstance>> => {
const result = new Map<string, NodeTypeSchemaForInstance>();

const { data: instanceRows, error: instanceError } = await client
.from("my_concepts")
.select("source_local_id, schema_id")
.eq("space_id", spaceId)
.eq("is_schema", false)
.eq("arity", 0)
.in("source_local_id", nodeInstanceIds);

if (instanceError || !instanceRows) {
console.error("Error fetching node instance concepts:", instanceError);
return result;
}

const schemaIds = [
...new Set(
instanceRows
.map((row) => row.schema_id)
.filter((id): id is number => id !== null),
),
];
if (schemaIds.length === 0) return result;

const { data: schemaRows, error: schemaError } = await client
.from("my_concepts")
.select("id, source_local_id, name")
.eq("space_id", spaceId)
.eq("is_schema", true)
.eq("arity", 0)
.in("id", schemaIds);

if (schemaError || !schemaRows) {
console.error("Error fetching node type schemas:", schemaError);
return result;
}

const schemasById = new Map<number, NodeTypeSchemaForInstance>();
for (const row of schemaRows) {
if (row.id !== null && row.source_local_id !== null && row.name !== null) {
schemasById.set(row.id, {
nodeTypeId: row.source_local_id,
name: row.name,
});
}
}

for (const row of instanceRows) {
if (row.source_local_id === null || row.schema_id === null) continue;
const schema = schemasById.get(row.schema_id);
if (schema) result.set(row.source_local_id, schema);
}

return result;
};

/**
* Fetches created/last_modified from the source space Content (my_contents) for an imported node.
* Used by the discourse context view to show "last modified in original vault".
Expand Down Expand Up @@ -939,9 +1009,6 @@ const sanitizePathForImport = (path: string): string => {

type ParsedFrontmatter = {

@sid597 sid597 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Trimmed to what this parse actually feeds: only nodeTypeId is read from content frontmatter now. The nodeInstanceId/publishedToGroups/authorId reads further down this file (refreshImportedFile) come from the metadata cache — a different object — not this parse.

nodeTypeId?: string;
nodeInstanceId?: string;
publishedToGroups?: string[];
authorId?: number;
[key: string]: unknown;
};

Expand Down Expand Up @@ -1096,22 +1163,24 @@ const processFileContent = async ({
sourceSpaceId,
sourceSpaceUri,
rawContent,
originalFilePath,
filePath,
importedCreatedAt,
importedModifiedAt,
authorId,
nodeInstanceId,
nodeTypeIdFromConcept,
}: {
plugin: DiscourseGraphPlugin;
client: DGSupabaseClient;
sourceSpaceId: number;
sourceSpaceUri: string;
rawContent: string;
originalFilePath?: string;
filePath: string;
importedCreatedAt?: number;
importedModifiedAt?: number;
authorId?: number;
nodeInstanceId: string;
nodeTypeIdFromConcept?: string;
}): Promise<
{ file: TFile; error?: never } | { file?: never; error: string }
> => {
Expand All @@ -1134,20 +1203,16 @@ const processFileContent = async ({
// 2. Parse frontmatter from rawContent (metadataCache is updated async and is
// often empty immediately after create/modify), then map nodeTypeId and update frontmatter.
const { frontmatter } = parseFrontmatter(rawContent);
const sourceNodeTypeId = frontmatter.nodeTypeId;
if (typeof sourceNodeTypeId !== "string") {
const sourceNodeTypeId =
typeof frontmatter.nodeTypeId === "string"
? frontmatter.nodeTypeId
: nodeTypeIdFromConcept;

@sid597 sid597 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ordering: content frontmatter wins when present, so Obsidian-origin imports behave exactly as before; the Concept-derived id is only reached by frontmatter-less (Roam-origin) content. For Obsidian-origin the two values are identical by construction — publish writes schema_represented_by_local_id from the same frontmatter it embeds in the content.

Comment thread
sid597 marked this conversation as resolved.
Outdated
if (sourceNodeTypeId === undefined) {
await plugin.app.vault.delete(file);
Comment thread
sid597 marked this conversation as resolved.
Outdated
return {
error: "importedNode missing sourceNodeTypeId",
};
}
const sourceNodeId = frontmatter.nodeInstanceId;
if (typeof sourceNodeId !== "string") {
await plugin.app.vault.delete(file);
return {
error: "importedNode missing nodeInstanceId",
};
}

const mappedNodeTypeId = await mapNodeTypeIdToLocal({
plugin,
Expand All @@ -1161,12 +1226,11 @@ const processFileContent = async ({
file,
(fm) => {
const record = fm as Record<string, unknown>;
if (mappedNodeTypeId !== undefined) {
record.nodeTypeId = mappedNodeTypeId;
}
record.nodeTypeId = mappedNodeTypeId;
record.nodeInstanceId = nodeInstanceId;

@sid597 sid597 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This line is what makes duplicate prevention work for Roam-origin nodes: findExistingImportedFile (re-import updates in place) and getLocalNodeInstanceIds (already-imported nodes drop out of the modal) both match on nodeInstanceId + importedFromRid frontmatter. Obsidian-origin content already carries this value, so there it's an idempotent rewrite.

record.importedFromRid = spaceUriAndLocalIdToRid(

@sid597 sid597 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Now built from the caller-supplied nodeInstanceId — same value the removed frontmatter read produced. Rid shape is unchanged for Roam spaces: their space URL is https-based and the https branch of spaceUriAndLocalIdToRid ignores the "note" subtype, matching what buildSharedNodes produces for Roam platforms. Identity stays carried once as the RID (per the #1161/#1214 direction).

sourceSpaceUri,
sourceNodeId,
nodeInstanceId,
"note",
);
record.lastModified = importedModifiedAt;
Expand Down Expand Up @@ -1244,6 +1308,12 @@ export const importSelectedNodes = async ({
spaceName,
});

const nodeTypeSchemasByInstance = await fetchNodeTypeSchemasForInstances({

@sid597 sid597 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Batched once per space, outside the per-node loop — mirrors where the preview step does the same resolution.

client,
spaceId,
nodeInstanceIds: nodes.map((n) => n.nodeInstanceId),
});

// Process each node in this space
for (const node of nodes) {
try {
Expand Down Expand Up @@ -1318,11 +1388,14 @@ export const importSelectedNodes = async ({
sourceSpaceId: spaceId,
sourceSpaceUri: spaceUri,
rawContent: content,
originalFilePath: contentFilePath,
filePath: finalFilePath,
importedCreatedAt: createdAt,
importedModifiedAt: modifiedAt,
authorId,
nodeInstanceId: node.nodeInstanceId,
nodeTypeIdFromConcept: nodeTypeSchemasByInstance.get(
node.nodeInstanceId,
)?.nodeTypeId,
});

if (result.error) {
Expand Down
63 changes: 14 additions & 49 deletions apps/obsidian/src/utils/importPreview.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
getImportedNodesInfo,
getLocalNodeKeyToEndpointId,
} from "./relationsStore";
import { getSpaceUris } from "./importNodes";
import { fetchNodeTypeSchemasForInstances, getSpaceUris } from "./importNodes";
import { QueryEngine } from "~/services/QueryEngine";
import {
fetchRelationInstancesFromSpace,
Expand Down Expand Up @@ -82,69 +82,34 @@ export const computeImportPreview = async ({
}

for (const [spaceId, nodes] of nodesBySpace.entries()) {
// Get schema_ids for the selected node instances
const nodeInstanceIds = nodes.map((n) => n.nodeInstanceId);
const { data: conceptRows } = await client
.from("my_concepts")
.select("source_local_id, schema_id")
.eq("space_id", spaceId)
.eq("is_schema", false)
.in("source_local_id", nodeInstanceIds);

if (!conceptRows) continue;

const schemaIds = [
...new Set(
(
conceptRows as Array<{
source_local_id: string;
schema_id: number | null;
}>
)
.map((r) => r.schema_id)
.filter((id): id is number => id != null),
),
];

if (schemaIds.length === 0) continue;

// Resolve schema_ids to node type info
const { data: schemaRows } = await client
.from("my_concepts")
.select("source_local_id, name")
.eq("space_id", spaceId)
.eq("is_schema", true)
.in("id", schemaIds);

if (!schemaRows) continue;

for (const schema of schemaRows as Array<{
source_local_id: string;
name: string;
}>) {
const sourceNodeTypeId = schema.source_local_id;
const nodeTypeSchemasByInstance = await fetchNodeTypeSchemasForInstances({

@sid597 sid597 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Drop-in replacement for the two inline queries that lived here — same filters plus arity = 0, select adds id (keys the instance→schema map), and null-data now logs instead of silently continuing. Dedup behavior is preserved by the existing seenNodeTypeIds / nodeTypeIdToName.has checks: the map yields one entry per instance rather than unique schemas, but outcomes are identical.

client,
spaceId,
nodeInstanceIds: nodes.map((n) => n.nodeInstanceId),
});

for (const { nodeTypeId, name } of nodeTypeSchemasByInstance.values()) {
// Track name for triplet resolution
if (!nodeTypeIdToName.has(sourceNodeTypeId)) {
nodeTypeIdToName.set(sourceNodeTypeId, schema.name);
if (!nodeTypeIdToName.has(nodeTypeId)) {
nodeTypeIdToName.set(nodeTypeId, name);
}

if (seenNodeTypeIds.has(sourceNodeTypeId)) continue;
seenNodeTypeIds.add(sourceNodeTypeId);
if (seenNodeTypeIds.has(nodeTypeId)) continue;
seenNodeTypeIds.add(nodeTypeId);

// Check against local node types (mirrors mapNodeTypeIdToLocal logic without side effects)
const matchById = plugin.settings.nodeTypes.find(
(nt) => nt.id === sourceNodeTypeId,
(nt) => nt.id === nodeTypeId,
);
if (matchById) continue;

const matchByName = plugin.settings.nodeTypes.find(
(nt) => nt.name === schema.name,
(nt) => nt.name === name,
);
if (matchByName) continue;

// This is a new node type that would be created
newNodeTypeSchemas.push({ id: sourceNodeTypeId, name: schema.name });
newNodeTypeSchemas.push({ id: nodeTypeId, name });
}
}

Expand Down
52 changes: 52 additions & 0 deletions packages/database/src/lib/__tests__/sharedNodes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,58 @@ describe("buildSharedNodes", () => {
]);
});

it("builds a Roam-origin shared node with a URL-based rid", () => {

@sid597 sid597 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Roam-origin fixture: for an https space URL the rid is <spaceUrl>/<uid> (no orn: subtype), platform Roam; timestamps go in as production-shape Z-less naive UTC and come out normalized ISO.

const roamSpaces: BuildArgs["spaces"] = [
{
id: 30,
name: "Research graph",
platform: "Roam",
url: "https://roamresearch.com/#/app/research-graph",
},
];
const roamNodes: BuildArgs["nodes"] = [
{ ...nodes[0]!, space_id: 30, source_local_id: "roam-uid-1" },
];
const roamDirect: BuildArgs["directContents"] = [
{
...directContents[0]!,
space_id: 30,
source_local_id: "roam-uid-1",
metadata: null,
text: "CLM - Sleep improves memory consolidation",
},
];
const roamFull: BuildArgs["fullContentSummaries"] = [
{
last_modified: "2026-06-14T15:00:00",
source_local_id: "roam-uid-1",
space_id: 30,
},
];
expect(
build({
nodesOverride: roamNodes,
directOverride: roamDirect,
fullOverride: roamFull,
spacesOverride: roamSpaces,
}),
).toEqual([
{
rid: "https://roamresearch.com/#/app/research-graph/roam-uid-1",
sourceLocalId: "roam-uid-1",
spaceId: 30,
spaceName: "Research graph",
spaceUri: "https://roamresearch.com/#/app/research-graph",
platform: "Roam",
title: "CLM - Sleep improves memory consolidation",
created: "2026-06-14T11:00:00.000Z",
lastModified: "2026-06-14T15:00:00.000Z",
authorId: 42,
directMetadata: null,
},
]);
});

it("discovers a node without full content", () => {
expect(build({ fullOverride: [] })[0]?.lastModified).toBe(
"2026-06-14T13:00:00.000Z",
Expand Down