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

Merged
sid597 merged 2 commits into from
Aug 3, 2026
+168 −78
Merged

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

Show file tree
Hide file tree
Changes from all commits
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
131 changes: 102 additions & 29 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.

@maparent maparent Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the first part of this function is re-fetching data that we already fetched, and could be removed altogether. It's used in importSelectedNodes which receives ImportableNode from ImportNodeModal; those are composed from PublishedNodes obtained from getPublishedNodesForGroups; and those are made from SharedNodes. SharedNode has the schema_id, it should be possible to add that field back to PublishedNodes and ImportableNode, and you would have a list of schema ids to fetch. Fetching the schemas themselves (2nd part) could be done outside of the loop at 1293.
Now... I realize that this is code you inherited and refactored. That refactoring may be out of scope. (I cannot believe I missed that database loop the first time around.) We can discuss whether this should be done before or after this PR.

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,26 +1163,42 @@ 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 }
> => {
// 1. Create or update the file with the fetched content first.
// 1. Parse frontmatter from rawContent (metadataCache is updated async and is
// often empty immediately after create/modify) and resolve the node type
// before any vault write, so a failed lookup leaves existing files untouched.
const { frontmatter } = parseFrontmatter(rawContent);
const sourceNodeTypeId =
typeof frontmatter.nodeTypeId === "string"
? frontmatter.nodeTypeId
: nodeTypeIdFromConcept;
if (sourceNodeTypeId === undefined) {
return {
error: "importedNode missing sourceNodeTypeId",
};
}

// 2. Create or update the file with the fetched content.
// On create, set file metadata (ctime/mtime) to original vault dates via vault adapter.
let file: TFile | null = plugin.app.vault.getFileByPath(filePath);
const stat =
Expand All @@ -1131,24 +1214,6 @@ const processFileContent = async ({
await plugin.app.vault.process(file, () => rawContent, stat);
}

// 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") {
await plugin.app.vault.delete(file);
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,
client,
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.

@maparent maparent Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Database access inside a loop: to be avoided. But the whole function needs to go.

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