diff --git a/apps/obsidian/src/utils/conceptConversion.ts b/apps/obsidian/src/utils/conceptConversion.ts index 339311234..931ff36ab 100644 --- a/apps/obsidian/src/utils/conceptConversion.ts +++ b/apps/obsidian/src/utils/conceptConversion.ts @@ -10,6 +10,7 @@ import type { DiscourseNodeInVault } from "./getDiscourseNodes"; import type { LocalConceptDataInput } from "@repo/database/inputTypes"; import type { ObsidianDiscourseNodeData } from "./syncDgNodesToSupabase"; import type { Json } from "@repo/database/dbTypes"; +import { extractContentFromTitle } from "./extractContentFromTitle"; /** * Get extra data (author, timestamps) from file metadata @@ -160,12 +161,17 @@ export const discourseRelationTripleSchemaToLocalConcept = ({ export const discourseNodeInstanceToLocalConcept = ( context: SupabaseContext, nodeData: ObsidianDiscourseNodeData, + nodeTypesById: Record, ): LocalConceptDataInput => { const extraData = getNodeExtraData(nodeData.file, context.userId); const { nodeInstanceId, nodeTypeId, importedFromRid, ...otherData } = nodeData.frontmatter; const literal_content: Record = { label: nodeData.file.basename, + core_title: extractContentFromTitle( + nodeTypesById[nodeData.nodeTypeId]?.format ?? "", + nodeData.file.basename, + ), source_data: otherData as unknown as Json, }; if (importedFromRid && typeof importedFromRid === "string") diff --git a/apps/obsidian/src/utils/syncDgNodesToSupabase.ts b/apps/obsidian/src/utils/syncDgNodesToSupabase.ts index 9eb73e40a..25207a24d 100644 --- a/apps/obsidian/src/utils/syncDgNodesToSupabase.ts +++ b/apps/obsidian/src/utils/syncDgNodesToSupabase.ts @@ -30,6 +30,10 @@ import { isAcceptedSchema } from "./typeUtils"; import { getTemplatePluginInfo } from "./templates"; import { difference } from "@repo/utils/setOperations"; import { getAllPages } from "@repo/database/lib/pagination"; +import { + CORE_TITLE_PROBE_SELECT, + partitionByCoreTitle, +} from "@repo/database/lib/coreTitleBackfill"; const DEFAULT_TIME = "1970-01-01"; export type ChangeType = "title" | "content"; @@ -231,6 +235,17 @@ type BuildChangedNodesOptions = { fullSync?: boolean; }; +type CoreTitleBackfillCounts = { + backfilled: number; + skipped: number; + orphaned: number; +}; + +type BuildChangedNodesResult = { + changedNodes: ObsidianDiscourseNodeData[]; + coreTitleBackfill: CoreTitleBackfillCounts | null; +}; + const mergeChangeTypes = ( base: ChangeType[], additional: ChangeType[], @@ -320,15 +335,33 @@ const detectNodeChanges = ( return changeTypes; }; +const noticeCoreTitleBackfill = ({ + backfilled, + skipped, + orphaned, +}: CoreTitleBackfillCounts): void => { + if (backfilled === 0 && orphaned === 0) return; + const messages = [ + `Backfilled core title for ${backfilled} node${backfilled === 1 ? "" : "s"}.`, + `${skipped} already had one.`, + ]; + if (orphaned > 0) { + messages.push( + `${orphaned} no longer match a discourse node in this vault.`, + ); + } + new Notice(messages.join(" "), 5000); +}; + const buildChangedNodesFromNodes = async ({ nodes, supabaseClient, context, changeTypesByPath, fullSync = false, -}: BuildChangedNodesOptions): Promise => { +}: BuildChangedNodesOptions): Promise => { if (nodes.length === 0) { - return []; + return { changedNodes: [], coreTitleBackfill: null }; } const nodeInstanceIds = nodes.map((node) => node.nodeInstanceId); @@ -344,11 +377,14 @@ const buildChangedNodesFromNodes = async ({ ); const changedNodes: ObsidianDiscourseNodeData[] = []; let missingConcepts: Set | undefined; + let coreTitleProbe: + | { missingCoreTitleIds: Set; skipped: number; orphaned: number } + | undefined; if (fullSync) { const existingConceptIds = await getAllPages( supabaseClient .from("my_concepts") - .select("source_local_id") + .select(CORE_TITLE_PROBE_SELECT) .eq("space_id", context.spaceId) .eq("arity", 0) .eq("is_schema", false) @@ -369,6 +405,13 @@ const buildChangedNodesFromNodes = async ({ .filter((id) => id !== null), ); missingConcepts = difference(nodeIds, dbConceptIds); + const { missingCoreTitleIds, withCoreTitleCount } = + partitionByCoreTitle(existingConceptIds); + coreTitleProbe = { + missingCoreTitleIds, + skipped: withCoreTitleCount, + orphaned: difference(missingCoreTitleIds, nodeIds).size, + }; } } @@ -389,7 +432,8 @@ const buildChangedNodesFromNodes = async ({ if ( finalChangeTypes.length === 0 && - !missingConcepts?.has(node.nodeInstanceId) + !missingConcepts?.has(node.nodeInstanceId) && + !coreTitleProbe?.missingCoreTitleIds.has(node.nodeInstanceId) ) { continue; } @@ -405,7 +449,18 @@ const buildChangedNodesFromNodes = async ({ }); } - return changedNodes; + return { + changedNodes, + coreTitleBackfill: coreTitleProbe + ? { + backfilled: changedNodes.filter((node) => + coreTitleProbe.missingCoreTitleIds.has(node.nodeInstanceId), + ).length, + skipped: coreTitleProbe.skipped, + orphaned: coreTitleProbe.orphaned, + } + : null, + }; }; export const syncAllNodesAndRelations = async ( @@ -426,14 +481,15 @@ export const syncAllNodesAndRelations = async ( const allNodes = await collectDiscourseNodesFromVault(plugin, true); - const changedNodeInstances = relationsOnly - ? [] - : await buildChangedNodesFromNodes({ - nodes: allNodes, - supabaseClient, - context, - fullSync: true, - }); + const { changedNodes: changedNodeInstances, coreTitleBackfill } = + relationsOnly + ? { changedNodes: [], coreTitleBackfill: null } + : await buildChangedNodesFromNodes({ + nodes: allNodes, + supabaseClient, + context, + fullSync: true, + }); const accountLocalId = plugin.settings.accountLocalId; if (!accountLocalId) { @@ -458,8 +514,15 @@ export const syncAllNodesAndRelations = async ( fullSync: true, }); + if (coreTitleBackfill !== null) { + noticeCoreTitleBackfill(coreTitleBackfill); + } + // When synced nodes are already published, ensure non-text assets are in storage. - await syncPublishedNodesAssets(plugin, changedNodeInstances); + await syncPublishedNodesAssets( + plugin, + changedNodeInstances.filter((node) => node.changeTypes.length > 0), + ); } catch (error) { console.error("syncAllNodesAndRelations: Process failed:", error); throw error; @@ -594,7 +657,7 @@ const convertDgToSupabaseConcepts = async ({ .filter((n) => !!n); const nodeInstanceToLocalConcepts = nodesSince.map((node) => { - return discourseNodeInstanceToLocalConcept(context, node); + return discourseNodeInstanceToLocalConcept(context, node, nodeTypesById); }); const relationInstancesData = await loadRelations(plugin); @@ -935,7 +998,7 @@ export const syncDiscourseNodeChanges = async ( return; } - const changedNodes = await buildChangedNodesFromNodes({ + const { changedNodes } = await buildChangedNodesFromNodes({ nodes: dgNodesInVault, supabaseClient, context, diff --git a/apps/roam/src/utils/__tests__/coreTitleBackfill.test.ts b/apps/roam/src/utils/__tests__/coreTitleBackfill.test.ts new file mode 100644 index 000000000..59eeddb45 --- /dev/null +++ b/apps/roam/src/utils/__tests__/coreTitleBackfill.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, it } from "vitest"; +import { + buildCoreTitleBackfill, + mergeNodesBySourceLocalId, +} from "../coreTitleBackfill"; +import type { RoamDiscourseNodeData } from "../getAllDiscourseNodesSince"; + +const node = (sourceLocalId: string): RoamDiscourseNodeData => ({ + author_local_id: "author", + author_name: "Author", + source_local_id: sourceLocalId, + created: "1", + last_modified: "2", + text: `CLM - ${sourceLocalId}`, + type: "claim-type", +}); + +describe("buildCoreTitleBackfill", () => { + it("forces in local nodes whose row has no core_title", () => { + const backfill = buildCoreTitleBackfill({ + conceptRows: [ + { source_local_id: "a", core_title: null }, + { source_local_id: "b", core_title: "already set" }, + ], + localNodes: [node("a"), node("b")], + }); + + expect(backfill.nodesToBackfill.map((n) => n.source_local_id)).toEqual([ + "a", + ]); + expect(backfill.withCoreTitleCount).toBe(1); + expect(backfill.orphanedCount).toBe(0); + }); + + it("reports rows that are no longer in the graph as orphaned", () => { + const backfill = buildCoreTitleBackfill({ + conceptRows: [ + { source_local_id: "a", core_title: null }, + { source_local_id: "gone", core_title: null }, + ], + localNodes: [node("a")], + }); + + expect(backfill.nodesToBackfill.map((n) => n.source_local_id)).toEqual([ + "a", + ]); + expect(backfill.orphanedCount).toBe(1); + }); + + it("skips rows without a source_local_id", () => { + const backfill = buildCoreTitleBackfill({ + conceptRows: [{ source_local_id: null, core_title: null }], + localNodes: [node("a")], + }); + + expect(backfill.nodesToBackfill).toEqual([]); + expect(backfill.withCoreTitleCount).toBe(0); + expect(backfill.orphanedCount).toBe(0); + }); + + it("is a no-op once every row has a core_title", () => { + const backfill = buildCoreTitleBackfill({ + conceptRows: [ + { source_local_id: "a", core_title: "a" }, + { source_local_id: "b", core_title: "b" }, + ], + localNodes: [node("a"), node("b")], + }); + + expect(backfill.nodesToBackfill).toEqual([]); + expect(backfill.withCoreTitleCount).toBe(2); + expect(backfill.orphanedCount).toBe(0); + }); +}); + +describe("mergeNodesBySourceLocalId", () => { + it("appends nodes that are not already in the batch", () => { + const merged = mergeNodesBySourceLocalId( + [node("a")], + [node("b"), node("c")], + ); + + expect(merged.map((n) => n.source_local_id)).toEqual(["a", "b", "c"]); + }); + + it("keeps the original node when both batches hold the same id", () => { + const original = node("a"); + const merged = mergeNodesBySourceLocalId([original], [node("a")]); + + expect(merged).toEqual([original]); + }); +}); diff --git a/apps/roam/src/utils/conceptConversion.ts b/apps/roam/src/utils/conceptConversion.ts index 35c8f76a5..47d02803a 100644 --- a/apps/roam/src/utils/conceptConversion.ts +++ b/apps/roam/src/utils/conceptConversion.ts @@ -13,7 +13,7 @@ import getPageTitleByPageUid from "roamjs-components/queries/getPageTitleByPageU const getNodeExtraData = ( node_uid: string, ): { - author_uid: string; + author_local_id: string; created: string; last_modified: string; page_uid: string; @@ -50,7 +50,7 @@ const getNodeExtraData = ( const created = new Date(created_t).toISOString(); const last_modified = new Date(last_modified_t).toISOString(); return { - author_uid, + author_local_id: author_uid, created, last_modified, page_uid, @@ -202,7 +202,7 @@ export const discourseRelationDataToLocalConcept = ( const created = new Date( Math.max(...nodeData.map((nd) => new Date(nd.created).getTime())), ).toISOString(); - const author_local_id: string = nodeData[0].author_uid; // take any one; again until I get the relation object + const author_local_id: string = nodeData[0].author_local_id; // take any one; again until I get the relation object return { space_id: context.spaceId, source_local_id: relationUid, diff --git a/apps/roam/src/utils/coreTitleBackfill.ts b/apps/roam/src/utils/coreTitleBackfill.ts new file mode 100644 index 000000000..8794aee43 --- /dev/null +++ b/apps/roam/src/utils/coreTitleBackfill.ts @@ -0,0 +1,44 @@ +import { difference } from "@repo/utils/setOperations"; +import { + partitionByCoreTitle, + type CoreTitleProbeRow, +} from "@repo/database/lib/coreTitleBackfill"; +import { type RoamDiscourseNodeData } from "./getAllDiscourseNodesSince"; + +export type CoreTitleBackfill = { + nodesToBackfill: RoamDiscourseNodeData[]; + withCoreTitleCount: number; + orphanedCount: number; +}; + +export const buildCoreTitleBackfill = ({ + conceptRows, + localNodes, +}: { + conceptRows: CoreTitleProbeRow[]; + localNodes: RoamDiscourseNodeData[]; +}): CoreTitleBackfill => { + const { missingCoreTitleIds, withCoreTitleCount } = + partitionByCoreTitle(conceptRows); + const localIds = new Set(localNodes.map((node) => node.source_local_id)); + return { + nodesToBackfill: localNodes.filter((node) => + missingCoreTitleIds.has(node.source_local_id), + ), + withCoreTitleCount, + orphanedCount: difference(missingCoreTitleIds, localIds).size, + }; +}; + +export const mergeNodesBySourceLocalId = ( + nodes: RoamDiscourseNodeData[], + additionalNodes: RoamDiscourseNodeData[], +): RoamDiscourseNodeData[] => { + const nodesById = new Map(nodes.map((node) => [node.source_local_id, node])); + for (const node of additionalNodes) { + if (!nodesById.has(node.source_local_id)) { + nodesById.set(node.source_local_id, node); + } + } + return [...nodesById.values()]; +}; diff --git a/apps/roam/src/utils/syncDgNodesToSupabase.ts b/apps/roam/src/utils/syncDgNodesToSupabase.ts index d0c90168d..e7c8483be 100644 --- a/apps/roam/src/utils/syncDgNodesToSupabase.ts +++ b/apps/roam/src/utils/syncDgNodesToSupabase.ts @@ -26,6 +26,12 @@ import { } from "./convertRoamNodeToFullContent"; import type { DGSupabaseClient } from "@repo/database/lib/client"; import { intersection } from "@repo/utils/setOperations"; +import { CORE_TITLE_PROBE_SELECT } from "@repo/database/lib/coreTitleBackfill"; +import { + buildCoreTitleBackfill, + mergeNodesBySourceLocalId, + type CoreTitleBackfill, +} from "./coreTitleBackfill"; import type { Json, Enums } from "@repo/database/dbTypes"; import { render as renderToast } from "roamjs-components/components/Toast"; import internalError from "~/utils/internalError"; @@ -870,6 +876,44 @@ export const setSyncActivity = (active: boolean) => { } }; +const reportCoreTitleBackfill = ({ + backfilled, + deferred, + skipped, + orphaned, +}: { + backfilled: number; + deferred: number; + skipped: number; + orphaned: number; +}): void => { + posthog.capture("Sync core_title backfill", { + backfilled, + deferred, + skipped, + orphaned, + }); + if (backfilled === 0 && deferred === 0 && orphaned === 0) return; + const messages = [ + `Backfilled core title for ${backfilled} node${backfilled === 1 ? "" : "s"}.`, + `${skipped} already had one.`, + ]; + if (deferred > 0) { + messages.push(`${deferred} waiting for sync to be enabled.`); + } + if (orphaned > 0) { + messages.push( + `${orphaned} no longer match a discourse node in this graph.`, + ); + } + renderToast({ + id: "core-title-backfill", + intent: orphaned > 0 ? "warning" : "success", + content: messages.join(" "), + timeout: 5000, + }); +}; + const getAllMissingOrNewDiscourseNodes = async ({ supabaseClient, spaceId, @@ -880,9 +924,12 @@ const getAllMissingOrNewDiscourseNodes = async ({ spaceId: number; since: number | undefined; nodeTypes: DiscourseNode[]; -}): Promise => { +}): Promise<{ + nodes: RoamDiscourseNodeData[]; + coreTitleBackfill: CoreTitleBackfill | null; +}> => { const allNodes = await getAllDiscourseNodesSince(undefined, nodeTypes); - if (since === undefined) return allNodes; + if (since === undefined) return { nodes: allNodes, coreTitleBackfill: null }; const newNodes = await getAllDiscourseNodesSince(since, nodeTypes); const existingContentIdsReq = await getAllPages( supabaseClient @@ -896,7 +943,7 @@ const getAllMissingOrNewDiscourseNodes = async ({ const existingConceptIdsReq = await getAllPages( supabaseClient .from("my_concepts") - .select("source_local_id") + .select(CORE_TITLE_PROBE_SELECT) .eq("space_id", spaceId) .eq("arity", 0) .eq("is_schema", false) @@ -911,10 +958,16 @@ const getAllMissingOrNewDiscourseNodes = async ({ ), ...newNodes.map((n) => n.source_local_id), ]); - return [ - ...newNodes, - ...allNodes.filter((n) => !existingIds.has(n.source_local_id)), - ]; + return { + nodes: [ + ...newNodes, + ...allNodes.filter((n) => !existingIds.has(n.source_local_id)), + ], + coreTitleBackfill: buildCoreTitleBackfill({ + conceptRows: existingConceptIdsReq, + localNodes: allNodes, + }), + }; }; const getSharedNodeInstanceSourceLocalIds = async ({ @@ -1209,21 +1262,28 @@ export const createOrUpdateDiscourseEmbedding = async ( (n) => n.backedBy === "user", ); - const changedNodeInstances = await measureSyncPhase({ - phase: isInitialSync - ? "getAllMissingOrNewDiscourseNodes" - : "getAllDiscourseNodesSince", - phases, - operation: () => - isInitialSync - ? getAllMissingOrNewDiscourseNodes({ - supabaseClient: activeSupabaseClient, - spaceId: activeContext.spaceId, - since: sinceTime, - nodeTypes: allDgNodeTypes, - }) - : getAllDiscourseNodesSince(sinceTime, allDgNodeTypes), - }); + const { nodes: changedNodeInstances, coreTitleBackfill } = + await measureSyncPhase({ + phase: isInitialSync + ? "getAllMissingOrNewDiscourseNodes" + : "getAllDiscourseNodesSince", + phases, + operation: async () => + isInitialSync + ? getAllMissingOrNewDiscourseNodes({ + supabaseClient: activeSupabaseClient, + spaceId: activeContext.spaceId, + since: sinceTime, + nodeTypes: allDgNodeTypes, + }) + : { + nodes: await getAllDiscourseNodesSince( + sinceTime, + allDgNodeTypes, + ), + coreTitleBackfill: null, + }, + }); const sharedSourceLocalIds = await measureSyncPhase({ phase: "getSharedNodeInstanceSourceLocalIds", phases, @@ -1238,6 +1298,16 @@ export const createOrUpdateDiscourseEmbedding = async ( sharedSourceLocalIds.has(node.source_local_id), ) : changedNodeInstances; + const nodesToBackfillCoreTitle = ( + coreTitleBackfill?.nodesToBackfill ?? [] + ).filter( + (node) => + !sharedNodesOnlySync || sharedSourceLocalIds.has(node.source_local_id), + ); + const conceptNodesToSync = mergeNodesBySourceLocalId( + nodeInstancesToSync, + nodesToBackfillCoreTitle, + ); const sharedSourceLocalIdsToBackfill = await measureSyncPhase({ phase: "getSharedSourceLocalIdsMissingFullContent", phases, @@ -1316,7 +1386,7 @@ export const createOrUpdateDiscourseEmbedding = async ( phases, operation: () => convertDgToSupabaseConcepts({ - nodesSince: nodeInstancesToSync, + nodesSince: conceptNodesToSync, since: sinceTime, allNodeTypes: allDgNodeTypes, sharedNodeTypeIds, @@ -1324,6 +1394,16 @@ export const createOrUpdateDiscourseEmbedding = async ( context: activeContext, }), }); + if (coreTitleBackfill !== null) { + reportCoreTitleBackfill({ + backfilled: nodesToBackfillCoreTitle.length, + deferred: + coreTitleBackfill.nodesToBackfill.length - + nodesToBackfillCoreTitle.length, + skipped: coreTitleBackfill.withCoreTitleCount, + orphaned: coreTitleBackfill.orphanedCount, + }); + } await measureSyncPhase({ phase: "cleanupOrphanedNodes", phases, diff --git a/packages/database/src/lib/__tests__/coreTitleBackfill.test.ts b/packages/database/src/lib/__tests__/coreTitleBackfill.test.ts new file mode 100644 index 000000000..6dfa6e33b --- /dev/null +++ b/packages/database/src/lib/__tests__/coreTitleBackfill.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vitest"; +import { partitionByCoreTitle } from "../coreTitleBackfill"; + +describe("partitionByCoreTitle", () => { + it("treats a null core_title as missing", () => { + const { missingCoreTitleIds, withCoreTitleCount } = partitionByCoreTitle([ + { source_local_id: "a", core_title: null }, + { source_local_id: "b", core_title: "already set" }, + ]); + + expect([...missingCoreTitleIds]).toEqual(["a"]); + expect(withCoreTitleCount).toBe(1); + }); + + it("counts an empty core_title as present", () => { + const { missingCoreTitleIds, withCoreTitleCount } = partitionByCoreTitle([ + { source_local_id: "a", core_title: "" }, + ]); + + expect([...missingCoreTitleIds]).toEqual([]); + expect(withCoreTitleCount).toBe(1); + }); + + it("ignores rows without a source_local_id", () => { + const { missingCoreTitleIds, withCoreTitleCount } = partitionByCoreTitle([ + { source_local_id: null, core_title: null }, + { source_local_id: null, core_title: "set" }, + ]); + + expect([...missingCoreTitleIds]).toEqual([]); + expect(withCoreTitleCount).toBe(0); + }); + + it("returns nothing to backfill for an empty probe", () => { + const { missingCoreTitleIds, withCoreTitleCount } = partitionByCoreTitle( + [], + ); + + expect([...missingCoreTitleIds]).toEqual([]); + expect(withCoreTitleCount).toBe(0); + }); +}); diff --git a/packages/database/src/lib/coreTitleBackfill.ts b/packages/database/src/lib/coreTitleBackfill.ts new file mode 100644 index 000000000..c563983dc --- /dev/null +++ b/packages/database/src/lib/coreTitleBackfill.ts @@ -0,0 +1,23 @@ +export const CORE_TITLE_PROBE_SELECT = + "source_local_id, core_title:literal_content->>core_title"; + +export type CoreTitleProbeRow = { + source_local_id: string | null; + core_title: string | null; +}; + +export const partitionByCoreTitle = ( + rows: CoreTitleProbeRow[], +): { missingCoreTitleIds: Set; withCoreTitleCount: number } => { + const missingCoreTitleIds = new Set(); + let withCoreTitleCount = 0; + for (const row of rows) { + if (row.source_local_id === null) continue; + if (row.core_title === null) { + missingCoreTitleIds.add(row.source_local_id); + } else { + withCoreTitleCount += 1; + } + } + return { missingCoreTitleIds, withCoreTitleCount }; +};