From 42b7d97044fc0cee3fe8f58216198d41610d3be1 Mon Sep 17 00:00:00 2001 From: sid597 Date: Mon, 13 Jul 2026 00:03:52 +0530 Subject: [PATCH 1/3] [ENG-1857] Validate Roam-origin Obsidian imports --- apps/obsidian/package.json | 2 + .../src/components/ImportNodesModal.tsx | 13 +- apps/obsidian/src/utils/importNodes.ts | 153 +++++++++++------- .../src/utils/sharedNodeImport.test.ts | 90 +++++++++++ apps/obsidian/src/utils/sharedNodeImport.ts | 92 +++++++++++ pnpm-lock.yaml | 3 + 6 files changed, 292 insertions(+), 61 deletions(-) create mode 100644 apps/obsidian/src/utils/sharedNodeImport.test.ts create mode 100644 apps/obsidian/src/utils/sharedNodeImport.ts diff --git a/apps/obsidian/package.json b/apps/obsidian/package.json index 1b715d839..f0915ec1b 100644 --- a/apps/obsidian/package.json +++ b/apps/obsidian/package.json @@ -9,6 +9,7 @@ "build": "tsx scripts/build.ts", "lint": "eslint .", "lint:fix": "eslint . --fix", + "test:unit": "vitest run", "publish": "tsx scripts/publish.ts --version 0.1.0", "check-types": "tsc --noEmit --skipLibCheck" }, @@ -34,6 +35,7 @@ "tslib": "2.5.1", "tsx": "^4.19.2", "typescript": "5.5.4", + "vitest": "catalog:", "uuidv7": "1.1.0", "zod": "^3.24.1" }, diff --git a/apps/obsidian/src/components/ImportNodesModal.tsx b/apps/obsidian/src/components/ImportNodesModal.tsx index 2859eaf12..c8f7f6178 100644 --- a/apps/obsidian/src/components/ImportNodesModal.tsx +++ b/apps/obsidian/src/components/ImportNodesModal.tsx @@ -8,11 +8,12 @@ import { getAvailableGroupIds } from "@repo/database/lib/groups"; import { fetchUserNames, getPublishedNodesForGroups, - getLocalNodeInstanceIds, + getImportedNodeKeys, getSpaceNameFromIds, getSpaceUris, importSelectedNodes, } from "~/utils/importNodes"; +import { getImportedNodeKey } from "~/utils/sharedNodeImport"; import { getLoggedInClient, getSupabaseContext } from "~/utils/supabaseContext"; import { computeImportPreview, @@ -70,11 +71,17 @@ const ImportNodesContent = ({ plugin, onClose }: ImportNodesModalProps) => { currentSpaceId: context.spaceId, }); - const localNodeInstanceIds = getLocalNodeInstanceIds(plugin); + const importedNodeKeys = await getImportedNodeKeys({ plugin, client }); // Filter out nodes that already exist locally const importableNodes = publishedNodes.filter( - (node) => !localNodeInstanceIds.has(node.source_local_id), + (node) => + !importedNodeKeys.has( + getImportedNodeKey({ + spaceId: node.space_id, + sourceLocalId: node.source_local_id, + }), + ), ); const uniqueSpaceIds = [ diff --git a/apps/obsidian/src/utils/importNodes.ts b/apps/obsidian/src/utils/importNodes.ts index d1122eddf..b4142cc50 100644 --- a/apps/obsidian/src/utils/importNodes.ts +++ b/apps/obsidian/src/utils/importNodes.ts @@ -1,5 +1,4 @@ import type { Json } from "@repo/database/dbTypes"; -import matter from "gray-matter"; import { App, Notice, TFile } from "obsidian"; import type { DGSupabaseClient } from "@repo/database/lib/client"; import { listGroupSharedNodes } from "@repo/database/lib/sharedNodes"; @@ -21,6 +20,13 @@ import { } from "./importRelations"; import { createTemplateFile } from "./templates"; import { resolveFolderForSpaceUri } from "./importFolderMetadata"; +import { + buildImportedNodeFrontmatter, + buildSourceNodeTypeIdMap, + getAvailableImportPath, + type SourceNodeConcept, + type SourceNodeSchema, +} from "./sharedNodeImport"; type PublishedNode = { source_local_id: string; @@ -70,22 +76,20 @@ export const getPublishedNodesForGroups = async ({ }); }; -export const getLocalNodeInstanceIds = ( - plugin: DiscourseGraphPlugin, -): Set => { +export const getImportedNodeKeys = async ({ + plugin, + client, +}: { + plugin: DiscourseGraphPlugin; + client: DGSupabaseClient; +}): Promise> => { const queryEngine = new QueryEngine(plugin.app); - const files = queryEngine.getFilesWithNodeInstanceId(); - const nodeInstanceIds = new Set(); - - for (const file of files) { - const cache = plugin.app.metadataCache.getFileCache(file); - const frontmatter = cache?.frontmatter; - if (frontmatter?.nodeInstanceId) { - nodeInstanceIds.add(frontmatter.nodeInstanceId as string); - } - } - - return nodeInstanceIds; + const { nodeKeys } = await getImportedNodesInfo({ + queryEngine, + plugin, + client, + }); + return nodeKeys; }; /** @@ -322,6 +326,47 @@ const fetchNodeContentForImport = async ({ }; }; +const fetchSourceNodeTypeIds = async ({ + client, + spaceId, + nodeInstanceIds, +}: { + client: DGSupabaseClient; + spaceId: number; + nodeInstanceIds: string[]; +}): Promise> => { + const { data: conceptRows, error: conceptError } = 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 (conceptError || !conceptRows) return new Map(); + + const concepts = conceptRows as SourceNodeConcept[]; + const schemaIds = [ + ...new Set( + concepts + .map((concept) => concept.schema_id) + .filter((schemaId): schemaId is number => schemaId !== null), + ), + ]; + if (schemaIds.length === 0) return new Map(); + + const { data: schemaRows, error: schemaError } = await client + .from("my_concepts") + .select("id, source_local_id") + .eq("space_id", spaceId) + .eq("is_schema", true) + .in("id", schemaIds); + if (schemaError || !schemaRows) return new Map(); + + return buildSourceNodeTypeIdMap({ + concepts, + schemas: schemaRows as SourceNodeSchema[], + }); +}; + /** * 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". @@ -937,24 +982,6 @@ const sanitizePathForImport = (path: string): string => { .join("/"); }; -type ParsedFrontmatter = { - nodeTypeId?: string; - nodeInstanceId?: string; - publishedToGroups?: string[]; - authorId?: number; - [key: string]: unknown; -}; - -const parseFrontmatter = ( - content: string, -): { frontmatter: ParsedFrontmatter; body: string } => { - const { data, content: body } = matter(content); - return { - frontmatter: (data ?? {}) as ParsedFrontmatter, - body: body ?? "", - }; -}; - /** * Parse literal_content from a Concept schema into fields for DiscourseNode. * Handles both nested form { label, template, source_data: { format, color, tag } } @@ -1096,6 +1123,8 @@ const processFileContent = async ({ sourceSpaceId, sourceSpaceUri, rawContent, + sourceNodeId, + sourceNodeTypeId, originalFilePath, filePath, importedCreatedAt, @@ -1107,6 +1136,8 @@ const processFileContent = async ({ sourceSpaceId: number; sourceSpaceUri: string; rawContent: string; + sourceNodeId: string; + sourceNodeTypeId: string; originalFilePath?: string; filePath: string; importedCreatedAt?: number; @@ -1131,24 +1162,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, @@ -1161,16 +1174,22 @@ const processFileContent = async ({ file, (fm) => { const record = fm as Record; - if (mappedNodeTypeId !== undefined) { - record.nodeTypeId = mappedNodeTypeId; - } - record.importedFromRid = spaceUriAndLocalIdToRid( + const importedFromRid = spaceUriAndLocalIdToRid( sourceSpaceUri, sourceNodeId, "note", ); - record.lastModified = importedModifiedAt; - if (authorId) record.authorId = authorId; + Object.assign( + record, + buildImportedNodeFrontmatter({ + existingFrontmatter: record, + sourceNodeId, + mappedNodeTypeId, + importedFromRid, + importedModifiedAt, + authorId, + }), + ); }, stat, ); @@ -1238,6 +1257,11 @@ export const importSelectedNodes = async ({ } const spaceName = spaceNames.get(spaceId) ?? `space-${spaceId}`; + const sourceNodeTypeIds = await fetchSourceNodeTypeIds({ + client, + spaceId, + nodeInstanceIds: nodes.map((node) => node.nodeInstanceId), + }); const importFolderPath = await resolveFolderForSpaceUri({ adapter: plugin.app.vault.adapter, spaceUri, @@ -1247,6 +1271,13 @@ export const importSelectedNodes = async ({ // Process each node in this space for (const node of nodes) { try { + const sourceNodeTypeId = sourceNodeTypeIds.get(node.nodeInstanceId); + if (!sourceNodeTypeId) { + failedCount++; + processedCount++; + onProgress?.(processedCount, totalNodes); + continue; + } const importedFromRid = spaceUriAndLocalIdToRid( spaceUri, node.nodeInstanceId, @@ -1299,6 +1330,10 @@ export const importSelectedNodes = async ({ ? sanitizePathForImport(contentFilePath) : `${sanitizedFileName}.md`; finalFilePath = `${importFolderPath}/${pathUnderImport}`; + finalFilePath = await getAvailableImportPath({ + desiredPath: finalFilePath, + pathExists: (path) => plugin.app.vault.adapter.exists(path), + }); // Ensure all parent folders exist (e.g. import/VaultName/Discourse Nodes/SubFolder) const dirParts = finalFilePath.split("/"); @@ -1318,6 +1353,8 @@ export const importSelectedNodes = async ({ sourceSpaceId: spaceId, sourceSpaceUri: spaceUri, rawContent: content, + sourceNodeId: node.nodeInstanceId, + sourceNodeTypeId, originalFilePath: contentFilePath, filePath: finalFilePath, importedCreatedAt: createdAt, diff --git a/apps/obsidian/src/utils/sharedNodeImport.test.ts b/apps/obsidian/src/utils/sharedNodeImport.test.ts new file mode 100644 index 000000000..48f2f2a35 --- /dev/null +++ b/apps/obsidian/src/utils/sharedNodeImport.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from "vitest"; +import { + buildImportedNodeFrontmatter, + buildSourceNodeTypeIdMap, + getAvailableImportPath, + getImportedNodeKey, +} from "./sharedNodeImport"; + +const ROAM_SOURCE_NODE_ID = "tgWb6JozF"; +const ROAM_SOURCE_NODE_TYPE_ID = "rCLM0schema"; +const ROAM_SOURCE_SPACE_ID = 42; + +const roamFullMarkdown = `# Sleep improves memory consolidation + +Multiple studies show that sleep after learning strengthens memory traces. + +- Supported by [[EVD]] - Rasch & Born 2013 +`; + +describe("Roam-origin shared node import", () => { + it("derives node type identity without relying on markdown frontmatter", () => { + const sourceNodeTypeIds = buildSourceNodeTypeIdMap({ + concepts: [ + { + source_local_id: ROAM_SOURCE_NODE_ID, + schema_id: 200, + }, + ], + schemas: [ + { + id: 200, + source_local_id: ROAM_SOURCE_NODE_TYPE_ID, + }, + ], + }); + + expect(roamFullMarkdown).not.toContain("nodeTypeId:"); + expect(sourceNodeTypeIds.get(ROAM_SOURCE_NODE_ID)).toBe( + ROAM_SOURCE_NODE_TYPE_ID, + ); + }); + + it("adds stable source identity while preserving existing metadata", () => { + expect( + buildImportedNodeFrontmatter({ + existingFrontmatter: { aliases: ["Sleep and memory"] }, + sourceNodeId: ROAM_SOURCE_NODE_ID, + mappedNodeTypeId: "local-claim-type", + importedFromRid: "https://roamresearch.com/#/app/MAPLab/tgWb6JozF", + importedModifiedAt: 1_781_275_600_000, + authorId: 17, + }), + ).toEqual({ + aliases: ["Sleep and memory"], + nodeInstanceId: ROAM_SOURCE_NODE_ID, + nodeTypeId: "local-claim-type", + importedFromRid: "https://roamresearch.com/#/app/MAPLab/tgWb6JozF", + lastModified: 1_781_275_600_000, + authorId: 17, + }); + }); + + it("scopes duplicate prevention to the source space and node identity", () => { + expect( + getImportedNodeKey({ + spaceId: ROAM_SOURCE_SPACE_ID, + sourceLocalId: ROAM_SOURCE_NODE_ID, + }), + ).toBe(`${ROAM_SOURCE_SPACE_ID}:${ROAM_SOURCE_NODE_ID}`); + expect( + getImportedNodeKey({ + spaceId: ROAM_SOURCE_SPACE_ID + 1, + sourceLocalId: ROAM_SOURCE_NODE_ID, + }), + ).not.toBe(`${ROAM_SOURCE_SPACE_ID}:${ROAM_SOURCE_NODE_ID}`); + }); + + it("keeps distinct same-title nodes in separate files", async () => { + const existingPaths = new Set([ + "import/Roam/Sleep improves memory consolidation.md", + ]); + + await expect( + getAvailableImportPath({ + desiredPath: "import/Roam/Sleep improves memory consolidation.md", + pathExists: (path) => Promise.resolve(existingPaths.has(path)), + }), + ).resolves.toBe("import/Roam/Sleep improves memory consolidation (1).md"); + }); +}); diff --git a/apps/obsidian/src/utils/sharedNodeImport.ts b/apps/obsidian/src/utils/sharedNodeImport.ts new file mode 100644 index 000000000..b0c0b71f6 --- /dev/null +++ b/apps/obsidian/src/utils/sharedNodeImport.ts @@ -0,0 +1,92 @@ +export type SourceNodeConcept = { + source_local_id: string; + schema_id: number | null; +}; + +export type SourceNodeSchema = { + id: number; + source_local_id: string; +}; + +export const getImportedNodeKey = ({ + spaceId, + sourceLocalId, +}: { + spaceId: number; + sourceLocalId: string; +}): string => `${spaceId}:${sourceLocalId}`; + +export const getAvailableImportPath = async ({ + desiredPath, + pathExists, +}: { + desiredPath: string; + pathExists: (path: string) => Promise; +}): Promise => { + if (!(await pathExists(desiredPath))) return desiredPath; + + const extensionIndex = desiredPath.lastIndexOf("."); + const basePath = + extensionIndex > desiredPath.lastIndexOf("/") + ? desiredPath.slice(0, extensionIndex) + : desiredPath; + const extension = + extensionIndex > desiredPath.lastIndexOf("/") + ? desiredPath.slice(extensionIndex) + : ""; + + let counter = 1; + let availablePath = `${basePath} (${counter})${extension}`; + while (await pathExists(availablePath)) { + counter++; + availablePath = `${basePath} (${counter})${extension}`; + } + return availablePath; +}; + +export const buildSourceNodeTypeIdMap = ({ + concepts, + schemas, +}: { + concepts: SourceNodeConcept[]; + schemas: SourceNodeSchema[]; +}): Map => { + const sourceNodeTypeIdBySchemaId = new Map( + schemas.map((schema) => [schema.id, schema.source_local_id]), + ); + + return new Map( + concepts.flatMap((concept): [string, string][] => { + if (concept.schema_id === null) return []; + const sourceNodeTypeId = sourceNodeTypeIdBySchemaId.get( + concept.schema_id, + ); + return sourceNodeTypeId + ? [[concept.source_local_id, sourceNodeTypeId]] + : []; + }), + ); +}; + +export const buildImportedNodeFrontmatter = ({ + existingFrontmatter, + sourceNodeId, + mappedNodeTypeId, + importedFromRid, + importedModifiedAt, + authorId, +}: { + existingFrontmatter: Record; + sourceNodeId: string; + mappedNodeTypeId: string; + importedFromRid: string; + importedModifiedAt?: number; + authorId?: number; +}): Record => ({ + ...existingFrontmatter, + nodeInstanceId: sourceNodeId, + nodeTypeId: mappedNodeTypeId, + importedFromRid, + lastModified: importedModifiedAt, + ...(authorId === undefined ? {} : { authorId }), +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e80fe1c3a..23e9309f3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -215,6 +215,9 @@ importers: uuidv7: specifier: 1.1.0 version: 1.1.0 + vitest: + specifier: 'catalog:' + version: 4.1.6(@edge-runtime/vm@3.2.0)(@opentelemetry/api@1.9.0)(@types/node@22.20.0)(jsdom@20.0.3)(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2)) zod: specifier: ^3.24.1 version: 3.25.76 From d5ec65bc4a1c36a0bf2785b331a538e0269c6273 Mon Sep 17 00:00:00 2001 From: sid597 Date: Mon, 27 Jul 2026 21:41:27 +0530 Subject: [PATCH 2/3] ENG-1857 Resolve review findings on Roam-origin import Removes the Obsidian unit-test harness this branch introduced (vitest, test:unit, sharedNodeImport.test.ts). The app had no test infra, and the extracted-for-testability helpers had started shaping production code around the tests rather than the feature. sharedNodeImport.ts goes with it, its pieces moving to where they belong. - dedupe and refresh scans find imported notes wherever they now live, not only under import/: a relocated note was re-offered for import and then overwritten in place - node type resolution is handed over from computeImportPreview through precomputedData instead of re-querying the same concept/schema rows at import time; the direct fetch remains for refreshImportedFile - my_concepts lookups filter arity = 0 and use generated row types with explicit null narrowing instead of hand-written types behind casts - failed imports log why: query error, no node type schema, no content - processFileContent returns TFile; its unreachable error arm, the assertion that arm forced, and the unused originalFilePath param are gone, along with the now-always-defined optional params - getImportedNodeKey is the single definition of the spaceId:localId key - getAvailableImportPath covers both the new-file and rename collisions - drops gray-matter, unused since frontmatter parsing was removed --- apps/obsidian/package.json | 3 - .../src/components/ImportNodesModal.tsx | 12 +- apps/obsidian/src/services/QueryEngine.ts | 7 +- apps/obsidian/src/utils/importNodes.ts | 256 +++++++++++------- apps/obsidian/src/utils/importPreview.ts | 52 ++-- apps/obsidian/src/utils/relationsStore.ts | 21 +- .../src/utils/sharedNodeImport.test.ts | 90 ------ apps/obsidian/src/utils/sharedNodeImport.ts | 92 ------- pnpm-lock.yaml | 6 - 9 files changed, 220 insertions(+), 319 deletions(-) delete mode 100644 apps/obsidian/src/utils/sharedNodeImport.test.ts delete mode 100644 apps/obsidian/src/utils/sharedNodeImport.ts diff --git a/apps/obsidian/package.json b/apps/obsidian/package.json index f0915ec1b..93d28ec80 100644 --- a/apps/obsidian/package.json +++ b/apps/obsidian/package.json @@ -9,7 +9,6 @@ "build": "tsx scripts/build.ts", "lint": "eslint .", "lint:fix": "eslint . --fix", - "test:unit": "vitest run", "publish": "tsx scripts/publish.ts --version 0.1.0", "check-types": "tsc --noEmit --skipLibCheck" }, @@ -35,7 +34,6 @@ "tslib": "2.5.1", "tsx": "^4.19.2", "typescript": "5.5.4", - "vitest": "catalog:", "uuidv7": "1.1.0", "zod": "^3.24.1" }, @@ -45,7 +43,6 @@ "@repo/utils": "workspace:*", "@supabase/supabase-js": "catalog:", "date-fns": "^4.1.0", - "gray-matter": "^4.0.3", "mime-types": "^3.0.1", "nanoid": "^4.0.2", "react": "catalog:obsidian", diff --git a/apps/obsidian/src/components/ImportNodesModal.tsx b/apps/obsidian/src/components/ImportNodesModal.tsx index c8f7f6178..11823551a 100644 --- a/apps/obsidian/src/components/ImportNodesModal.tsx +++ b/apps/obsidian/src/components/ImportNodesModal.tsx @@ -8,12 +8,14 @@ import { getAvailableGroupIds } from "@repo/database/lib/groups"; import { fetchUserNames, getPublishedNodesForGroups, - getImportedNodeKeys, getSpaceNameFromIds, getSpaceUris, importSelectedNodes, } from "~/utils/importNodes"; -import { getImportedNodeKey } from "~/utils/sharedNodeImport"; +import { + getImportedNodeKey, + getImportedNodesInfo, +} from "~/utils/relationsStore"; import { getLoggedInClient, getSupabaseContext } from "~/utils/supabaseContext"; import { computeImportPreview, @@ -71,7 +73,10 @@ const ImportNodesContent = ({ plugin, onClose }: ImportNodesModalProps) => { currentSpaceId: context.spaceId, }); - const importedNodeKeys = await getImportedNodeKeys({ plugin, client }); + const { nodeKeys: importedNodeKeys } = await getImportedNodesInfo({ + plugin, + client, + }); // Filter out nodes that already exist locally const importableNodes = publishedNodes.filter( @@ -226,6 +231,7 @@ const ImportNodesContent = ({ plugin, onClose }: ImportNodesModalProps) => { keyToRid: previewData.keyToRid, keyToRelationEndpointId: previewData.keyToRelationEndpointId, relationInstancesBySpace: previewData.relationInstancesBySpace, + sourceNodeTypeIdByKey: previewData.sourceNodeTypeIdByKey, } : undefined, }); diff --git a/apps/obsidian/src/services/QueryEngine.ts b/apps/obsidian/src/services/QueryEngine.ts index 0edee84e6..20f11f4c0 100644 --- a/apps/obsidian/src/services/QueryEngine.ts +++ b/apps/obsidian/src/services/QueryEngine.ts @@ -306,13 +306,15 @@ export class QueryEngine { } /** - * Return all markdown pages under import/ that have importedFromRid and nodeInstanceId. + * Return all markdown pages that have importedFromRid and nodeInstanceId, + * wherever they now live: the frontmatter pair identifies an imported node, + * and users move notes out of import/ after importing them. * Uses DataCore when available; falls back to vault iteration otherwise. */ getImportedNodePages = (): TFile[] => { if (this.dc) { try { - const dcQuery = `@page and path("import") and exists(importedFromRid) and exists(nodeInstanceId)`; + const dcQuery = `@page and exists(importedFromRid) and exists(nodeInstanceId)`; const pages = this.dc.query(dcQuery); const files: TFile[] = []; for (const page of pages) { @@ -498,7 +500,6 @@ export class QueryEngine { const files: TFile[] = []; const allFiles = this.app.vault.getMarkdownFiles(); for (const f of allFiles) { - if (!f.path.startsWith("import/")) continue; const fm = this.app.metadataCache.getFileCache(f)?.frontmatter; if ( (fm as Record | undefined)?.importedFromRid && diff --git a/apps/obsidian/src/utils/importNodes.ts b/apps/obsidian/src/utils/importNodes.ts index b4142cc50..a59d4e431 100644 --- a/apps/obsidian/src/utils/importNodes.ts +++ b/apps/obsidian/src/utils/importNodes.ts @@ -7,6 +7,7 @@ import { getLoggedInClient, getSupabaseContext } from "./supabaseContext"; import type { DiscourseNode, ImportableNode } from "~/types"; import { QueryEngine } from "~/services/QueryEngine"; import { + getImportedNodeKey, getImportedNodesInfo, getLocalNodeKeyToEndpointId, } from "~/utils/relationsStore"; @@ -20,13 +21,6 @@ import { } from "./importRelations"; import { createTemplateFile } from "./templates"; import { resolveFolderForSpaceUri } from "./importFolderMetadata"; -import { - buildImportedNodeFrontmatter, - buildSourceNodeTypeIdMap, - getAvailableImportPath, - type SourceNodeConcept, - type SourceNodeSchema, -} from "./sharedNodeImport"; type PublishedNode = { source_local_id: string; @@ -76,22 +70,6 @@ export const getPublishedNodesForGroups = async ({ }); }; -export const getImportedNodeKeys = async ({ - plugin, - client, -}: { - plugin: DiscourseGraphPlugin; - client: DGSupabaseClient; -}): Promise> => { - const queryEngine = new QueryEngine(plugin.app); - const { nodeKeys } = await getImportedNodesInfo({ - queryEngine, - plugin, - client, - }); - return nodeKeys; -}; - /** * Returns the space name for a given space ID. * Falls back to "space-{id}" if the lookup fails. @@ -326,6 +304,54 @@ const fetchNodeContentForImport = async ({ }; }; +type SourceNodeConcept = Pick< + Tables<"my_concepts">, + "source_local_id" | "schema_id" +>; +type SourceNodeSchema = Pick, "id" | "source_local_id">; + +/** + * Maps each node instance to the source_local_id of its node type schema, keyed by + * getImportedNodeKey. Roam-origin markdown carries no frontmatter, so the node type + * has to be resolved from the source space instead of parsed out of the payload. + */ +export const buildSourceNodeTypeIdMap = ({ + spaceId, + concepts, + schemas, +}: { + spaceId: number; + concepts: SourceNodeConcept[]; + schemas: SourceNodeSchema[]; +}): Map => { + const sourceNodeTypeIdBySchemaId = new Map(); + for (const schema of schemas) { + if (schema.id !== null && schema.source_local_id !== null) { + sourceNodeTypeIdBySchemaId.set(schema.id, schema.source_local_id); + } + } + + const sourceNodeTypeIdByKey = new Map(); + for (const concept of concepts) { + if (concept.source_local_id === null || concept.schema_id === null) + continue; + const sourceNodeTypeId = sourceNodeTypeIdBySchemaId.get(concept.schema_id); + if (!sourceNodeTypeId) continue; + sourceNodeTypeIdByKey.set( + getImportedNodeKey({ + spaceId, + sourceLocalId: concept.source_local_id, + }), + sourceNodeTypeId, + ); + } + return sourceNodeTypeIdByKey; +}; + +/** + * Fallback for callers that have no import preview (refreshImportedFile); + * computeImportPreview resolves the same map while building its preview. + */ const fetchSourceNodeTypeIds = async ({ client, spaceId, @@ -334,37 +360,48 @@ const fetchSourceNodeTypeIds = async ({ client: DGSupabaseClient; spaceId: number; nodeInstanceIds: string[]; -}): Promise> => { - const { data: conceptRows, error: conceptError } = await client +}): Promise<{ + sourceNodeTypeIdByKey: Map; + error?: string; +}> => { + const { data: concepts, error: conceptError } = 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 (conceptError || !conceptRows) return new Map(); + if (conceptError) { + return { sourceNodeTypeIdByKey: new Map(), error: conceptError.message }; + } - const concepts = conceptRows as SourceNodeConcept[]; const schemaIds = [ ...new Set( - concepts - .map((concept) => concept.schema_id) - .filter((schemaId): schemaId is number => schemaId !== null), + concepts.flatMap((concept) => + concept.schema_id === null ? [] : [concept.schema_id], + ), ), ]; - if (schemaIds.length === 0) return new Map(); + if (schemaIds.length === 0) return { sourceNodeTypeIdByKey: new Map() }; - const { data: schemaRows, error: schemaError } = await client + const { data: schemas, error: schemaError } = await client .from("my_concepts") .select("id, source_local_id") .eq("space_id", spaceId) .eq("is_schema", true) + .eq("arity", 0) .in("id", schemaIds); - if (schemaError || !schemaRows) return new Map(); + if (schemaError) { + return { sourceNodeTypeIdByKey: new Map(), error: schemaError.message }; + } - return buildSourceNodeTypeIdMap({ - concepts, - schemas: schemaRows as SourceNodeSchema[], - }); + return { + sourceNodeTypeIdByKey: buildSourceNodeTypeIdMap({ + spaceId, + concepts, + schemas, + }), + }; }; /** @@ -982,6 +1019,35 @@ const sanitizePathForImport = (path: string): string => { .join("/"); }; +/** + * Two source nodes can share a title, so the second one gets a " (n)" suffix + * instead of overwriting the first. + */ +const getAvailableImportPath = async ({ + desiredPath, + pathExists, +}: { + desiredPath: string; + pathExists: (path: string) => Promise; +}): Promise => { + if (!(await pathExists(desiredPath))) return desiredPath; + + const extensionIndex = desiredPath.lastIndexOf("."); + const hasExtension = extensionIndex > desiredPath.lastIndexOf("/"); + const basePath = hasExtension + ? desiredPath.slice(0, extensionIndex) + : desiredPath; + const extension = hasExtension ? desiredPath.slice(extensionIndex) : ""; + + let counter = 1; + let availablePath = `${basePath} (${counter})${extension}`; + while (await pathExists(availablePath)) { + counter++; + availablePath = `${basePath} (${counter})${extension}`; + } + return availablePath; +}; + /** * Parse literal_content from a Concept schema into fields for DiscourseNode. * Handles both nested form { label, template, source_data: { format, color, tag } } @@ -1125,7 +1191,7 @@ const processFileContent = async ({ rawContent, sourceNodeId, sourceNodeTypeId, - originalFilePath, + importedFromRid, filePath, importedCreatedAt, importedModifiedAt, @@ -1138,24 +1204,16 @@ const processFileContent = async ({ rawContent: string; sourceNodeId: string; sourceNodeTypeId: string; - originalFilePath?: string; + importedFromRid: string; filePath: string; - importedCreatedAt?: number; - importedModifiedAt?: number; - authorId?: number; -}): Promise< - { file: TFile; error?: never } | { file?: never; error: string } -> => { + importedCreatedAt: number; + importedModifiedAt: number; + authorId: number; +}): Promise => { // 1. Create or update the file with the fetched content first. // 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 = - importedCreatedAt !== undefined && importedModifiedAt !== undefined - ? { - ctime: importedCreatedAt, - mtime: importedModifiedAt, - } - : undefined; + const stat = { ctime: importedCreatedAt, mtime: importedModifiedAt }; if (!file) { file = await plugin.app.vault.create(filePath, rawContent, stat); } else { @@ -1174,27 +1232,16 @@ const processFileContent = async ({ file, (fm) => { const record = fm as Record; - const importedFromRid = spaceUriAndLocalIdToRid( - sourceSpaceUri, - sourceNodeId, - "note", - ); - Object.assign( - record, - buildImportedNodeFrontmatter({ - existingFrontmatter: record, - sourceNodeId, - mappedNodeTypeId, - importedFromRid, - importedModifiedAt, - authorId, - }), - ); + record.nodeInstanceId = sourceNodeId; + record.nodeTypeId = mappedNodeTypeId; + record.importedFromRid = importedFromRid; + record.lastModified = importedModifiedAt; + record.authorId = authorId; }, stat, ); - return { file }; + return file; }; export const importSelectedNodes = async ({ @@ -1211,6 +1258,7 @@ export const importSelectedNodes = async ({ keyToRid: Map; keyToRelationEndpointId: Map; relationInstancesBySpace: Map; + sourceNodeTypeIdByKey: Map; }; }): Promise<{ success: number; failed: number }> => { const client = await getLoggedInClient(plugin); @@ -1224,6 +1272,7 @@ export const importSelectedNodes = async ({ } const queryEngine = new QueryEngine(plugin.app); + const pathExists = (path: string) => plugin.app.vault.adapter.exists(path); let successCount = 0; let failedCount = 0; @@ -1257,11 +1306,21 @@ export const importSelectedNodes = async ({ } const spaceName = spaceNames.get(spaceId) ?? `space-${spaceId}`; - const sourceNodeTypeIds = await fetchSourceNodeTypeIds({ - client, - spaceId, - nodeInstanceIds: nodes.map((node) => node.nodeInstanceId), - }); + let sourceNodeTypeIdByKey = precomputedData?.sourceNodeTypeIdByKey; + if (!sourceNodeTypeIdByKey) { + const fetched = await fetchSourceNodeTypeIds({ + client, + spaceId, + nodeInstanceIds: nodes.map((node) => node.nodeInstanceId), + }); + if (fetched.error) { + console.error( + `Could not read node types from space ${spaceId}; its ${nodes.length} selected node(s) cannot be imported:`, + fetched.error, + ); + } + sourceNodeTypeIdByKey = fetched.sourceNodeTypeIdByKey; + } const importFolderPath = await resolveFolderForSpaceUri({ adapter: plugin.app.vault.adapter, spaceUri, @@ -1271,8 +1330,16 @@ export const importSelectedNodes = async ({ // Process each node in this space for (const node of nodes) { try { - const sourceNodeTypeId = sourceNodeTypeIds.get(node.nodeInstanceId); + const sourceNodeTypeId = sourceNodeTypeIdByKey.get( + getImportedNodeKey({ + spaceId, + sourceLocalId: node.nodeInstanceId, + }), + ); if (!sourceNodeTypeId) { + console.error( + `Skipping node ${node.nodeInstanceId}: no node type schema for it in space ${spaceId}`, + ); failedCount++; processedCount++; onProgress?.(processedCount, totalNodes); @@ -1296,6 +1363,9 @@ export const importSelectedNodes = async ({ }); if (!nodeContent) { + console.error( + `Skipping node ${node.nodeInstanceId}: source space ${spaceId} has no importable title/body content for it`, + ); failedCount++; processedCount++; onProgress?.(processedCount, totalNodes); @@ -1329,17 +1399,16 @@ export const importSelectedNodes = async ({ contentFilePath && contentFilePath.includes("/") ? sanitizePathForImport(contentFilePath) : `${sanitizedFileName}.md`; - finalFilePath = `${importFolderPath}/${pathUnderImport}`; finalFilePath = await getAvailableImportPath({ - desiredPath: finalFilePath, - pathExists: (path) => plugin.app.vault.adapter.exists(path), + desiredPath: `${importFolderPath}/${pathUnderImport}`, + pathExists, }); // Ensure all parent folders exist (e.g. import/VaultName/Discourse Nodes/SubFolder) const dirParts = finalFilePath.split("/"); for (let i = 1; i < dirParts.length - 1; i++) { const folderPath = dirParts.slice(0, i + 1).join("/"); - if (!(await plugin.app.vault.adapter.exists(folderPath))) { + if (!(await pathExists(folderPath))) { await plugin.app.vault.createFolder(folderPath); } } @@ -1347,7 +1416,7 @@ export const importSelectedNodes = async ({ // Process the file content (maps nodeTypeId, handles frontmatter, stores import timestamps) // This updates existing file or creates new one - const result = await processFileContent({ + const processedFile = await processFileContent({ plugin, client, sourceSpaceId: spaceId, @@ -1355,27 +1424,13 @@ export const importSelectedNodes = async ({ rawContent: content, sourceNodeId: node.nodeInstanceId, sourceNodeTypeId, - originalFilePath: contentFilePath, + importedFromRid, filePath: finalFilePath, importedCreatedAt: createdAt, importedModifiedAt: modifiedAt, authorId, }); - if (result.error) { - console.error( - `Error processing file content for node ${node.nodeInstanceId}:`, - result.error, - ); - failedCount++; - processedCount++; - onProgress?.(processedCount, totalNodes); - continue; - } - - // typescript should not need this assertion? - const processedFile = result.file!; - // Import assets for this node (use originalNodePath so assets go under import/{space}/ relative to note) const assetImportResult = await importAssetsForNode({ plugin, @@ -1407,13 +1462,10 @@ export const importSelectedNodes = async ({ const currentDir = processedFile.path.includes("/") ? processedFile.path.replace(/\/[^/]*$/, "") : importFolderPath; - const newPath = `${currentDir}/${sanitizedFileName}.md`; - let targetPath = newPath; - let counter = 1; - while (await plugin.app.vault.adapter.exists(targetPath)) { - targetPath = `${currentDir}/${sanitizedFileName} (${counter}).md`; - counter++; - } + const targetPath = await getAvailableImportPath({ + desiredPath: `${currentDir}/${sanitizedFileName}.md`, + pathExists, + }); await plugin.app.fileManager.renameFile(processedFile, targetPath); } diff --git a/apps/obsidian/src/utils/importPreview.ts b/apps/obsidian/src/utils/importPreview.ts index 327cda171..649c416b8 100644 --- a/apps/obsidian/src/utils/importPreview.ts +++ b/apps/obsidian/src/utils/importPreview.ts @@ -2,10 +2,11 @@ import type DiscourseGraphPlugin from "~/index"; import type { ImportableNode } from "~/types"; import { getLoggedInClient, getSupabaseContext } from "./supabaseContext"; import { + getImportedNodeKey, getImportedNodesInfo, getLocalNodeKeyToEndpointId, } from "./relationsStore"; -import { getSpaceUris } from "./importNodes"; +import { buildSourceNodeTypeIdMap, getSpaceUris } from "./importNodes"; import { QueryEngine } from "~/services/QueryEngine"; import { fetchRelationInstancesFromSpace, @@ -39,6 +40,8 @@ export type ImportPreviewData = { keyToRelationEndpointId: Map; /** Relation instances per spaceId, for reuse during import */ relationInstancesBySpace: Map; + /** Key -> source node type's source_local_id, so import doesn't re-query the source space */ + sourceNodeTypeIdByKey: Map; }; export const computeImportPreview = async ({ @@ -73,6 +76,8 @@ export const computeImportPreview = async ({ const newNodeTypeSchemas: Array<{ id: string; name: string }> = []; const seenNodeTypeIds = new Set(); + // Key -> source node type id, handed to importSelectedNodes so it doesn't re-query + const sourceNodeTypeIdByKey = new Map(); // Maps source_local_id -> name for all node type schemas we encounter (for triplet resolution) const nodeTypeIdToName = new Map(); @@ -89,20 +94,16 @@ export const computeImportPreview = async ({ .select("source_local_id, schema_id") .eq("space_id", spaceId) .eq("is_schema", false) + .eq("arity", 0) .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), + conceptRows.flatMap((row) => + row.schema_id === null ? [] : [row.schema_id], + ), ), ]; @@ -111,18 +112,25 @@ export const computeImportPreview = async ({ // Resolve schema_ids to node type info const { data: schemaRows } = await client .from("my_concepts") - .select("source_local_id, name") + .select("id, source_local_id, name") .eq("space_id", spaceId) .eq("is_schema", true) + .eq("arity", 0) .in("id", schemaIds); if (!schemaRows) continue; - for (const schema of schemaRows as Array<{ - source_local_id: string; - name: string; - }>) { + for (const [key, sourceNodeTypeId] of buildSourceNodeTypeIdMap({ + spaceId, + concepts: conceptRows, + schemas: schemaRows, + })) { + sourceNodeTypeIdByKey.set(key, sourceNodeTypeId); + } + + for (const schema of schemaRows) { const sourceNodeTypeId = schema.source_local_id; + if (sourceNodeTypeId === null || schema.name === null) continue; // Track name for triplet resolution if (!nodeTypeIdToName.has(sourceNodeTypeId)) { @@ -161,7 +169,10 @@ export const computeImportPreview = async ({ const spaceUri = spaceUris.get(spaceId); if (!spaceUri) continue; for (const node of nodes) { - const key = `${spaceId}:${node.nodeInstanceId}`; + const key = getImportedNodeKey({ + spaceId, + sourceLocalId: node.nodeInstanceId, + }); nodeKeys.add(key); if (!keyToRid.has(key)) { keyToRid.set( @@ -206,8 +217,14 @@ export const computeImportPreview = async ({ ); if (!sourceData || !destData) continue; - const sourceKey = `${sourceData.space_id}:${sourceData.source_local_id}`; - const destKey = `${destData.space_id}:${destData.source_local_id}`; + const sourceKey = getImportedNodeKey({ + spaceId: sourceData.space_id, + sourceLocalId: sourceData.source_local_id, + }); + const destKey = getImportedNodeKey({ + spaceId: destData.space_id, + sourceLocalId: destData.source_local_id, + }); if ( keyToRelationEndpointId.has(sourceKey) && @@ -471,5 +488,6 @@ export const computeImportPreview = async ({ keyToRid, keyToRelationEndpointId, relationInstancesBySpace, + sourceNodeTypeIdByKey, }; }; diff --git a/apps/obsidian/src/utils/relationsStore.ts b/apps/obsidian/src/utils/relationsStore.ts index d27ead83b..6aa49cdb0 100644 --- a/apps/obsidian/src/utils/relationsStore.ts +++ b/apps/obsidian/src/utils/relationsStore.ts @@ -433,9 +433,21 @@ export const buildEndpointToFileMap = ( return map; }; +/** + * The identity of a shared node is its source space plus its id in that space: + * two spaces can each have a node with the same local id. + */ +export const getImportedNodeKey = ({ + spaceId, + sourceLocalId, +}: { + spaceId: number; + sourceLocalId: string; +}): string => `${spaceId}:${sourceLocalId}`; + /** * Build key -> relation endpoint id (RID) for local nodes in this vault. - * Key format: `${localSpaceId}:${nodeInstanceId}`. Value: constructed RID for storage. + * Keys come from getImportedNodeKey. Value: constructed RID for storage. * Uses DataCore when available; falls back to vault iteration otherwise. */ export const getLocalNodeKeyToEndpointId = ( @@ -452,7 +464,10 @@ export const getLocalNodeKeyToEndpointId = ( const fm = cache?.frontmatter as Record | undefined; const nodeInstanceId = fm?.nodeInstanceId as string | undefined; if (nodeInstanceId && fm?.nodeTypeId) { - const key = `${localSpaceId}:${nodeInstanceId}`; + const key = getImportedNodeKey({ + spaceId: localSpaceId, + sourceLocalId: nodeInstanceId, + }); map.set( key, spaceUriAndLocalIdToRid(localSpaceUri, nodeInstanceId, "note"), @@ -496,7 +511,7 @@ export const getImportedNodesInfo = async ({ const spaceUri = ridToSpaceUriAndLocalId(importedFromRid).spaceUri; const spaceId = spaceIdsByUri.get(spaceUri) ?? -1; if (spaceId < 0) continue; - const key = `${spaceId}:${nodeInstanceId}`; + const key = getImportedNodeKey({ spaceId, sourceLocalId: nodeInstanceId }); nodeKeys.add(key); keyToRid.set(key, importedFromRid); } diff --git a/apps/obsidian/src/utils/sharedNodeImport.test.ts b/apps/obsidian/src/utils/sharedNodeImport.test.ts deleted file mode 100644 index 48f2f2a35..000000000 --- a/apps/obsidian/src/utils/sharedNodeImport.test.ts +++ /dev/null @@ -1,90 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { - buildImportedNodeFrontmatter, - buildSourceNodeTypeIdMap, - getAvailableImportPath, - getImportedNodeKey, -} from "./sharedNodeImport"; - -const ROAM_SOURCE_NODE_ID = "tgWb6JozF"; -const ROAM_SOURCE_NODE_TYPE_ID = "rCLM0schema"; -const ROAM_SOURCE_SPACE_ID = 42; - -const roamFullMarkdown = `# Sleep improves memory consolidation - -Multiple studies show that sleep after learning strengthens memory traces. - -- Supported by [[EVD]] - Rasch & Born 2013 -`; - -describe("Roam-origin shared node import", () => { - it("derives node type identity without relying on markdown frontmatter", () => { - const sourceNodeTypeIds = buildSourceNodeTypeIdMap({ - concepts: [ - { - source_local_id: ROAM_SOURCE_NODE_ID, - schema_id: 200, - }, - ], - schemas: [ - { - id: 200, - source_local_id: ROAM_SOURCE_NODE_TYPE_ID, - }, - ], - }); - - expect(roamFullMarkdown).not.toContain("nodeTypeId:"); - expect(sourceNodeTypeIds.get(ROAM_SOURCE_NODE_ID)).toBe( - ROAM_SOURCE_NODE_TYPE_ID, - ); - }); - - it("adds stable source identity while preserving existing metadata", () => { - expect( - buildImportedNodeFrontmatter({ - existingFrontmatter: { aliases: ["Sleep and memory"] }, - sourceNodeId: ROAM_SOURCE_NODE_ID, - mappedNodeTypeId: "local-claim-type", - importedFromRid: "https://roamresearch.com/#/app/MAPLab/tgWb6JozF", - importedModifiedAt: 1_781_275_600_000, - authorId: 17, - }), - ).toEqual({ - aliases: ["Sleep and memory"], - nodeInstanceId: ROAM_SOURCE_NODE_ID, - nodeTypeId: "local-claim-type", - importedFromRid: "https://roamresearch.com/#/app/MAPLab/tgWb6JozF", - lastModified: 1_781_275_600_000, - authorId: 17, - }); - }); - - it("scopes duplicate prevention to the source space and node identity", () => { - expect( - getImportedNodeKey({ - spaceId: ROAM_SOURCE_SPACE_ID, - sourceLocalId: ROAM_SOURCE_NODE_ID, - }), - ).toBe(`${ROAM_SOURCE_SPACE_ID}:${ROAM_SOURCE_NODE_ID}`); - expect( - getImportedNodeKey({ - spaceId: ROAM_SOURCE_SPACE_ID + 1, - sourceLocalId: ROAM_SOURCE_NODE_ID, - }), - ).not.toBe(`${ROAM_SOURCE_SPACE_ID}:${ROAM_SOURCE_NODE_ID}`); - }); - - it("keeps distinct same-title nodes in separate files", async () => { - const existingPaths = new Set([ - "import/Roam/Sleep improves memory consolidation.md", - ]); - - await expect( - getAvailableImportPath({ - desiredPath: "import/Roam/Sleep improves memory consolidation.md", - pathExists: (path) => Promise.resolve(existingPaths.has(path)), - }), - ).resolves.toBe("import/Roam/Sleep improves memory consolidation (1).md"); - }); -}); diff --git a/apps/obsidian/src/utils/sharedNodeImport.ts b/apps/obsidian/src/utils/sharedNodeImport.ts deleted file mode 100644 index b0c0b71f6..000000000 --- a/apps/obsidian/src/utils/sharedNodeImport.ts +++ /dev/null @@ -1,92 +0,0 @@ -export type SourceNodeConcept = { - source_local_id: string; - schema_id: number | null; -}; - -export type SourceNodeSchema = { - id: number; - source_local_id: string; -}; - -export const getImportedNodeKey = ({ - spaceId, - sourceLocalId, -}: { - spaceId: number; - sourceLocalId: string; -}): string => `${spaceId}:${sourceLocalId}`; - -export const getAvailableImportPath = async ({ - desiredPath, - pathExists, -}: { - desiredPath: string; - pathExists: (path: string) => Promise; -}): Promise => { - if (!(await pathExists(desiredPath))) return desiredPath; - - const extensionIndex = desiredPath.lastIndexOf("."); - const basePath = - extensionIndex > desiredPath.lastIndexOf("/") - ? desiredPath.slice(0, extensionIndex) - : desiredPath; - const extension = - extensionIndex > desiredPath.lastIndexOf("/") - ? desiredPath.slice(extensionIndex) - : ""; - - let counter = 1; - let availablePath = `${basePath} (${counter})${extension}`; - while (await pathExists(availablePath)) { - counter++; - availablePath = `${basePath} (${counter})${extension}`; - } - return availablePath; -}; - -export const buildSourceNodeTypeIdMap = ({ - concepts, - schemas, -}: { - concepts: SourceNodeConcept[]; - schemas: SourceNodeSchema[]; -}): Map => { - const sourceNodeTypeIdBySchemaId = new Map( - schemas.map((schema) => [schema.id, schema.source_local_id]), - ); - - return new Map( - concepts.flatMap((concept): [string, string][] => { - if (concept.schema_id === null) return []; - const sourceNodeTypeId = sourceNodeTypeIdBySchemaId.get( - concept.schema_id, - ); - return sourceNodeTypeId - ? [[concept.source_local_id, sourceNodeTypeId]] - : []; - }), - ); -}; - -export const buildImportedNodeFrontmatter = ({ - existingFrontmatter, - sourceNodeId, - mappedNodeTypeId, - importedFromRid, - importedModifiedAt, - authorId, -}: { - existingFrontmatter: Record; - sourceNodeId: string; - mappedNodeTypeId: string; - importedFromRid: string; - importedModifiedAt?: number; - authorId?: number; -}): Record => ({ - ...existingFrontmatter, - nodeInstanceId: sourceNodeId, - nodeTypeId: mappedNodeTypeId, - importedFromRid, - lastModified: importedModifiedAt, - ...(authorId === undefined ? {} : { authorId }), -}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 23e9309f3..60a863f8a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -136,9 +136,6 @@ importers: date-fns: specifier: ^4.1.0 version: 4.1.0 - gray-matter: - specifier: ^4.0.3 - version: 4.0.3 mime-types: specifier: ^3.0.1 version: 3.0.2 @@ -215,9 +212,6 @@ importers: uuidv7: specifier: 1.1.0 version: 1.1.0 - vitest: - specifier: 'catalog:' - version: 4.1.6(@edge-runtime/vm@3.2.0)(@opentelemetry/api@1.9.0)(@types/node@22.20.0)(jsdom@20.0.3)(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2)) zod: specifier: ^3.24.1 version: 3.25.76 From a0da822a5865f830636bd1e47b9959eb4bd6f83b Mon Sep 17 00:00:00 2001 From: sid597 Date: Mon, 27 Jul 2026 21:54:16 +0530 Subject: [PATCH 3/3] ENG-1857 Report why an import failed importSelectedNodes now returns the per node reasons it collected, matching what refreshAllImportedFiles already returns, and the modal logs them next to its summary Notice the way the refresh command does. Before this, a node that could not be imported was counted and dropped in silence. refreshImportedFile passes the collected reason through instead of the fixed "Failed to refresh imported file" string, so refreshAllImportedFiles reports something usable. failed is derived from the collected reasons rather than tracked in its own counter: every path out of a node either succeeds or records exactly one reason, so a separate count was a second source of truth. Reasons carry the instance id because titles are not unique across source spaces, which is the case this branch exists to handle. --- .../src/components/ImportNodesModal.tsx | 1 + apps/obsidian/src/utils/importNodes.ts | 68 +++++++++++-------- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/apps/obsidian/src/components/ImportNodesModal.tsx b/apps/obsidian/src/components/ImportNodesModal.tsx index 11823551a..d91b026e0 100644 --- a/apps/obsidian/src/components/ImportNodesModal.tsx +++ b/apps/obsidian/src/components/ImportNodesModal.tsx @@ -241,6 +241,7 @@ const ImportNodesContent = ({ plugin, onClose }: ImportNodesModalProps) => { `Import completed with some issues:\n${result.success} files imported successfully\n${result.failed} files failed`, 5000, ); + console.error("Import errors:", result.errors); } else { new Notice(`Successfully imported ${result.success} node(s)`, 3000); } diff --git a/apps/obsidian/src/utils/importNodes.ts b/apps/obsidian/src/utils/importNodes.ts index a59d4e431..5d21deb1f 100644 --- a/apps/obsidian/src/utils/importNodes.ts +++ b/apps/obsidian/src/utils/importNodes.ts @@ -1260,7 +1260,11 @@ export const importSelectedNodes = async ({ relationInstancesBySpace: Map; sourceNodeTypeIdByKey: Map; }; -}): Promise<{ success: number; failed: number }> => { +}): Promise<{ + success: number; + failed: number; + errors: Array<{ nodeTitle: string; nodeInstanceId: string; error: string }>; +}> => { const client = await getLoggedInClient(plugin); if (!client) { throw new Error("Cannot get Supabase client"); @@ -1275,9 +1279,25 @@ export const importSelectedNodes = async ({ const pathExists = (path: string) => plugin.app.vault.adapter.exists(path); let successCount = 0; - let failedCount = 0; let processedCount = 0; const totalNodes = selectedNodes.length; + // Collected per node so the caller can report why an import failed, the same + // way refreshAllImportedFiles does. Titles are not unique across source + // spaces, so each entry carries the instance id too. + const errors: Array<{ + nodeTitle: string; + nodeInstanceId: string; + error: string; + }> = []; + const recordFailure = (node: ImportableNode, error: string) => { + errors.push({ + nodeTitle: node.title, + nodeInstanceId: node.nodeInstanceId, + error, + }); + processedCount++; + onProgress?.(processedCount, totalNodes); + }; // Group nodes by space to create folders efficiently const nodesBySpace = new Map(); @@ -1297,10 +1317,8 @@ export const importSelectedNodes = async ({ for (const [spaceId, nodes] of nodesBySpace.entries()) { const spaceUri = spaceUris.get(spaceId); if (!spaceUri) { - for (const _node of nodes) { - failedCount++; - processedCount++; - onProgress?.(processedCount, totalNodes); + for (const node of nodes) { + recordFailure(node, `No space URL found for space ${spaceId}`); } continue; } @@ -1314,10 +1332,13 @@ export const importSelectedNodes = async ({ nodeInstanceIds: nodes.map((node) => node.nodeInstanceId), }); if (fetched.error) { - console.error( - `Could not read node types from space ${spaceId}; its ${nodes.length} selected node(s) cannot be imported:`, - fetched.error, - ); + for (const node of nodes) { + recordFailure( + node, + `Could not read node types from space ${spaceId}: ${fetched.error}`, + ); + } + continue; } sourceNodeTypeIdByKey = fetched.sourceNodeTypeIdByKey; } @@ -1337,12 +1358,7 @@ export const importSelectedNodes = async ({ }), ); if (!sourceNodeTypeId) { - console.error( - `Skipping node ${node.nodeInstanceId}: no node type schema for it in space ${spaceId}`, - ); - failedCount++; - processedCount++; - onProgress?.(processedCount, totalNodes); + recordFailure(node, `No node type schema in space ${spaceId}`); continue; } const importedFromRid = spaceUriAndLocalIdToRid( @@ -1363,12 +1379,10 @@ export const importSelectedNodes = async ({ }); if (!nodeContent) { - console.error( - `Skipping node ${node.nodeInstanceId}: source space ${spaceId} has no importable title/body content for it`, + recordFailure( + node, + `No importable title/body content in space ${spaceId}`, ); - failedCount++; - processedCount++; - onProgress?.(processedCount, totalNodes); continue; } @@ -1473,10 +1487,10 @@ export const importSelectedNodes = async ({ processedCount++; onProgress?.(processedCount, totalNodes); } catch (error) { - console.error(`Error importing node ${node.nodeInstanceId}:`, error); - failedCount++; - processedCount++; - onProgress?.(processedCount, totalNodes); + recordFailure( + node, + error instanceof Error ? error.message : String(error), + ); } } @@ -1514,7 +1528,7 @@ export const importSelectedNodes = async ({ } } - return { success: successCount, failed: failedCount }; + return { success: successCount, failed: errors.length, errors }; }; /** @@ -1588,7 +1602,7 @@ export const refreshImportedFile = async ({ }); return { success: result.success > 0, - error: result.failed > 0 ? "Failed to refresh imported file" : undefined, + error: result.errors[0]?.error, }; };