Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 38 additions & 0 deletions apps/roam/src/utils/__tests__/conceptConversion.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { SupabaseContext } from "~/utils/supabaseContext";

vi.mock("~/utils/getBlockProps", () => ({ default: () => ({}) }));
vi.mock("~/utils/getDiscourseNodes", () => ({ default: () => [] }));
vi.mock("~/utils/getDiscourseRelations", () => ({ default: () => [] }));
vi.mock("~/utils/createReifiedBlock", () => ({
DISCOURSE_GRAPH_PROP_NAME: "discourse-graph",
}));
vi.mock("roamjs-components/queries/getPageTitleByPageUid", () => ({
default: () => "",
}));

import { discourseNodeBlockToLocalConcept } from "~/utils/conceptConversion";

const context = { spaceId: 42 } as SupabaseContext;

describe("discourseNodeBlockToLocalConcept", () => {
beforeEach(() => {
(globalThis as { window: unknown }).window = {
roamAlphaAPI: {
q: () => [["author-1", "page-1", 1000, 2000]],
},
};
});

it("writes the core title into literal_content", () => {
const concept = discourseNodeBlockToLocalConcept(context, {
nodeUid: "node-1",
schemaUid: "schema-1",
text: "CLM - my claim",
coreTitle: "my claim",
});
expect(concept.literal_content).toEqual({ core_title: "my claim" });
expect(concept.name).toBe("CLM - my claim");
expect(concept.source_local_id).toBe("node-1");
});
});
68 changes: 68 additions & 0 deletions apps/roam/src/utils/__tests__/extractContentFromTitle.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { describe, expect, it } from "vitest";
import extractContentFromTitle from "~/utils/extractContentFromTitle";

describe("extractContentFromTitle", () => {
it("extracts the content from a title matching the format", () => {
expect(
extractContentFromTitle("[[CLM]] - my claim", {
format: "[[CLM]] - {content}",
}),
).toBe("my claim");
});

it("returns the title when the type has no format", () => {
expect(extractContentFromTitle("my claim", { format: "" })).toBe(
"my claim",
);
});

it("returns the title when it does not match the format", () => {
expect(
extractContentFromTitle("random page", {
format: "[[CLM]] - {content}",
}),
).toBe("random page");
});

it("extracts the content from a format with a {Source} placeholder", () => {
expect(
extractContentFromTitle("[[EVD]] - finding - @smith2020", {
format: "[[EVD]] - {content} - {Source}",
}),
).toBe("finding");
});

it('keeps a trailing content containing " - " whole', () => {
expect(
extractContentFromTitle("[[CLM]] - a - b", {
format: "[[CLM]] - {content}",
}),
).toBe("a - b");
});

it('extracts the shortest match when the content contains " - " before another placeholder (accepted for v0)', () => {
expect(
extractContentFromTitle("[[EVD]] - a - b - @smith2020", {
format: "[[EVD]] - {content} - {Source}",
}),
).toBe("a");
});

it("round trips a title built from the core title", () => {
const coreTitle = "sleep improves memory";
const simpleFormat = "[[CLM]] - {content}";
expect(
extractContentFromTitle(simpleFormat.replace("{content}", coreTitle), {
format: simpleFormat,
}),
).toBe(coreTitle);

const sourceFormat = "[[EVD]] - {content} - {Source}";
const title = sourceFormat
.replace("{content}", coreTitle)
.replace("{Source}", "@smith2020");
expect(extractContentFromTitle(title, { format: sourceFormat })).toBe(
coreTitle,
);
});
});
12 changes: 11 additions & 1 deletion apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,15 @@ const claimSchema: DiscourseNode = {
const makeCrossAppNode = ({
uid,
title,
coreTitle = title,
}: {
uid: string;
title: string;
coreTitle?: string;
}): CrossAppNode => ({
localId: uid,
nodeType: SCHEMA_UID,
coreTitle,
authorId: "user-1",
createdAt: new Date("2026-01-02T00:00:00.000Z"),
modifiedAt: new Date("2026-01-03T00:00:00.000Z"),
Expand Down Expand Up @@ -160,7 +163,13 @@ describe("publishNodesToGroups", () => {
client,
spaceId: SPACE_ID,
groupIds: [GROUP_ID],
nodes: [makeCrossAppNode({ uid: "node-1", title: "CLM - new claim" })],
nodes: [
makeCrossAppNode({
uid: "node-1",
title: "CLM - new claim",
coreTitle: "new claim",
}),
],
});

expect(rpcCalls).toHaveLength(1);
Expand All @@ -177,6 +186,7 @@ describe("publishNodesToGroups", () => {
source_local_id: "node-1",
name: "CLM - new claim",
schema_represented_by_local_id: SCHEMA_UID,
literal_content: { core_title: "new claim" },
});
expect(data[1].contents_inline).toEqual([
expect.objectContaining({
Expand Down
75 changes: 73 additions & 2 deletions apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { describe, expect, it, vi } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { Json } from "@repo/database/dbTypes";
import type { DiscourseNode } from "~/utils/getDiscourseNodes";

const mocks = vi.hoisted(() => ({
getDiscourseNodes: vi.fn(),
}));

vi.mock("roamjs-components/queries/getFullTreeByParentUid", () => ({
default: () => ({ children: [] }),
Expand All @@ -8,8 +13,28 @@ vi.mock("roamjs-components/queries/getPageViewType", () => ({
default: () => "bullet",
}));
vi.mock("~/utils/pageToMarkdown", () => ({ toMarkdown: () => "" }));
vi.mock("~/utils/getDiscourseNodes", () => ({
default: mocks.getDiscourseNodes,
}));

import {
fullContentNodeToCrossApp,
nodeUidsWithTypeToCrossApp,
} from "~/utils/roamToCrossAppConverters";

import { nodeUidsWithTypeToCrossApp } from "~/utils/roamToCrossAppConverters";
const claimSchema: DiscourseNode = {
type: "schema-1",
text: "Claim",
shortcut: "C",
specification: [],
backedBy: "user",
canvasSettings: {},
format: "CLM - {content}",
};

beforeEach(() => {
mocks.getDiscourseNodes.mockReturnValue([claimSchema]);
});

const USER_ROW = { ":db/id": 5, ":user/uid": "user-1" };

Expand Down Expand Up @@ -60,3 +85,49 @@ describe("nodeUidsWithTypeToCrossApp timestamps", () => {
expect(node.modifiedAt).toEqual(new Date(1000));
});
});

describe("nodeUidsWithTypeToCrossApp coreTitle", () => {
it("extracts the content from a title matching the node type's format", async () => {
const node = await convertRow(baseRow);
expect(node.coreTitle).toBe("claim");
});

it("keeps the whole title when the node type is unknown", async () => {
mocks.getDiscourseNodes.mockReturnValue([]);
const node = await convertRow(baseRow);
expect(node.coreTitle).toBe("CLM - claim");
});
});

describe("fullContentNodeToCrossApp coreTitle", () => {
const baseNode = {
author_local_id: "user-1",
source_local_id: "node-1",
created: 1000,
last_modified: 2000,
node_type_id: "schema-1",
text: "CLM - claim",
};

it("extracts the content from the title", () => {
const node = fullContentNodeToCrossApp(baseNode);
expect(node.coreTitle).toBe("claim");
});

it("extracts from the page title when node_title is present", () => {
const node = fullContentNodeToCrossApp({
...baseNode,
text: "some block text",
node_title: "CLM - claim",
});
expect(node.coreTitle).toBe("claim");
});

it("keeps the whole title when it does not match the format", () => {
const node = fullContentNodeToCrossApp({
...baseNode,
text: "unrelated title",
});
expect(node.coreTitle).toBe("unrelated title");
});
});
5 changes: 5 additions & 0 deletions apps/roam/src/utils/conceptConversion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,12 @@ export const discourseNodeBlockToLocalConcept = (
nodeUid,
schemaUid,
text,
coreTitle,
}: {
nodeUid: string;
schemaUid: string;
text: string;
coreTitle: string;
},
): LocalConceptDataInput => {
return {
Expand All @@ -116,6 +118,9 @@ export const discourseNodeBlockToLocalConcept = (
source_local_id: nodeUid,
schema_represented_by_local_id: schemaUid,
is_schema: false,
literal_content: {
core_title: coreTitle,
},
/* eslint-enable @typescript-eslint/naming-convention */
...getNodeExtraData(nodeUid),
};
Expand Down
10 changes: 10 additions & 0 deletions apps/roam/src/utils/roamToCrossAppConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ import { toMarkdown } from "./pageToMarkdown";
import getFullTreeByParentUid from "roamjs-components/queries/getFullTreeByParentUid";
import getPageViewType from "roamjs-components/queries/getPageViewType";
import { contentTypes } from "@repo/content-model";
import getDiscourseNodes from "./getDiscourseNodes";
import extractContentFromTitle from "./extractContentFromTitle";

const getCoreTitle = (title: string, nodeTypeUid: string): string => {

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.

Callers only carry {uid, type}, so the format lookup lives in the converter. Calling getDiscourseNodes() per node is cheap: the accessor is version-cached.

const format =
getDiscourseNodes().find((node) => node.type === nodeTypeUid)?.format ?? "";
return extractContentFromTitle(title, { format });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve empty content placeholders

When a valid formatted title has an empty or whitespace-only {content} capture (for example, [[EVD]] - - @smith2020), this call stores the decorated title instead of an empty core_title. extractContentFromTitle currently evaluates the trimmed capture with || title, so an empty matched value falls through to the original title; the same behavior affects periodic sync. Preserve a successfully matched empty string rather than treating it as no match.

Useful? React with 👍 / 👎.

};
Comment on lines +21 to +25

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Node type settings are re-read for every node during sync and publish, slowing large syncs

The node type configuration is re-read from Roam for each node being converted (getDiscourseNodes() at apps/roam/src/utils/roamToCrossAppConverters.ts:23) instead of once per batch, so syncing or publishing many nodes repeats the same expensive work thousands of times.
Impact: Large or initial syncs take noticeably longer and can block the UI, and the work is entirely wasted in the full-content path where the extracted title is not used.

Per-node configuration parsing in getCoreTitle

getCoreTitle calls getDiscourseNodes() on every invocation. It is invoked once per node from fullContentNodeToCrossApp (apps/roam/src/utils/roamToCrossAppConverters.ts:84) and inside the nodeRows.map of nodeUidsWithTypeToCrossApp (apps/roam/src/utils/roamToCrossAppConverters.ts:134).

getDiscourseNodes is only cached on the new-settings-store path (apps/roam/src/components/settings/utils/accessors.ts:1153-1157); on the legacy discourseConfigRef path it re-parses every node type's config tree (specification conditions, canvas settings, template) on each call (apps/roam/src/utils/getDiscourseNodes.ts:113-160).

fullContentNodeToCrossApp runs for every node in a sync via convertRoamNodeToFullContent (apps/roam/src/utils/convertRoamNodeToFullContent.ts:20-23, called at apps/roam/src/utils/syncDgNodesToSupabase.ts:805), and that path only uses the full content — the computed coreTitle is discarded.

Compare the approach used in convertDgToSupabaseConcepts, which builds a format lookup map once (apps/roam/src/utils/syncDgNodesToSupabase.ts:671-673).

Prompt for agents
getCoreTitle in apps/roam/src/utils/roamToCrossAppConverters.ts calls getDiscourseNodes() once per node. On the legacy settings-store path getDiscourseNodes re-parses every node type's configuration tree on each call (no cache), and the function is called per node from fullContentNodeToCrossApp and from the nodeRows.map inside nodeUidsWithTypeToCrossApp, so a sync of thousands of nodes repeats the parsing thousands of times. Additionally, the full-content path (convertRoamNodeToFullContent -> crossAppNodeToDbContent(node, 'full')) never reads coreTitle, so that work is wasted there. Consider building a format-by-node-type map once per batch (as convertDgToSupabaseConcepts does) and passing it in, or memoizing the format lookup, and avoid computing coreTitle where it is not consumed.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


const FULL_MARKDOWN_OPTS = {
refs: true,
Expand Down Expand Up @@ -73,6 +81,7 @@ export const fullContentNodeToCrossApp = (
createdAt: new Date(node.created || Date.now()),
modifiedAt: new Date(node.last_modified || Date.now()),
nodeType: node.node_type_id,
coreTitle: getCoreTitle(title, node.node_type_id),

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.

Nothing reads this coreTitle yet: this producer feeds upsert_content, not upsert_concepts. It is set so any CrossAppNode that reaches concept conversion carries it (the ticket lists all three producers).

content: {
direct: {
localId: node.source_local_id,
Expand Down Expand Up @@ -122,6 +131,7 @@ export const nodeUidsWithTypeToCrossApp = async (
authorId: userUid,
createdAt: new Date(createdTime),
modifiedAt: new Date(Math.max(editTime, pageEditTime)),
coreTitle: getCoreTitle(title, typesByUid[uid]),
content: {
direct: {
localId: uid,
Expand Down
7 changes: 7 additions & 0 deletions apps/roam/src/utils/syncDgNodesToSupabase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
nodeTypeSince,
} from "./getAllDiscourseNodesSince";
import getDiscourseNodeFormatExpression from "./getDiscourseNodeFormatExpression";
import extractContentFromTitle from "./extractContentFromTitle";
import { cleanupOrphanedNodes } from "./cleanupOrphanedNodes";
import {
getLoggedInClient,
Expand Down Expand Up @@ -667,11 +668,17 @@ export const convertDgToSupabaseConcepts = async ({
return discourseNodeSchemaToLocalConcept(context, node);
});

const formatByNodeTypeUid = new Map(
allNodeTypes.map((nodeType) => [nodeType.type, nodeType.format]),
);
const nodeBlockToLocalConcepts = nodesSince.map((node) => {
const localConcept = discourseNodeBlockToLocalConcept(context, {
nodeUid: node.source_local_id,
schemaUid: node.type,
text: node.node_title ? `${node.node_title} ${node.text}` : node.text,
coreTitle: extractContentFromTitle(node.node_title ?? node.text, {

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.

Inlined instead of reusing the publish-side helper because allNodeTypes is already in scope; the helper would refetch via getDiscourseNodes(). node_title ?? node.text matters for block-backed types: node_title is the format-matched page title, text is the block string.

format: formatByNodeTypeUid.get(node.type) ?? "",
}),
});
return localConcept;
});
Expand Down
4 changes: 4 additions & 0 deletions packages/database/src/crossAppContracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ type InlineCrossAppTypedContent = InlineCrossAppContent & {
// A node instance
export type CrossAppNode = CrossAppBase & {
nodeType: LocalId;
// The title stripped of the node type's title format ("[[CLM]] - {content}"
// -> the {content} part). Equals the title when the type has no format or
// the title does not match it.
coreTitle: string;

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.

Required on purpose. upsert_concepts replaces literal_content wholesale and defaults it to {}, so a producer that omits the key erases core_title on the next re-upsert. Required makes "every producer writes it" a compile error.

content: {
direct: InlineCrossAppContent;
full?: InlineCrossAppTypedContent;
Expand Down
2 changes: 2 additions & 0 deletions packages/database/src/crossAppNodeContract.example.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Multiple studies show that sleep after learning strengthens memory traces.
export const roamOriginNodeExample: CrossAppNode = {
localId: ROAM_SOURCE_NODE_ID,
nodeType: "rCLM0schema",
coreTitle: "Sleep improves memory consolidation",
content: {
direct: {
value: "Sleep improves memory consolidation",
Expand Down Expand Up @@ -47,6 +48,7 @@ Participants with more REM sleep showed better next-day recall.
export const obsidianOriginNodeExample: CrossAppNode = {
localId: OBSIDIAN_SOURCE_NODE_ID,
nodeType: OBSIDIAN_SOURCE_NODE_TYPE_ID,
coreTitle: "REM sleep and recall",
content: {
direct: {
value: "EVD - REM sleep and recall",
Expand Down
3 changes: 3 additions & 0 deletions packages/database/src/lib/crossAppConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ export const crossAppNodeToDbConcept = (
name: node.content.direct.value,
author_local_id: node.authorId,
schema_represented_by_local_id: node.nodeType,
literal_content: {
core_title: node.coreTitle,

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.

No migration needed: literal_content jsonb already exists (MAP confirmed on the ticket). name and the direct content keep the decorated title; core_title is added next to it.

},
contents_inline: filterUndefinedArray([
crossAppNodeToDbContent(node, "direct"),
crossAppNodeToDbContent(node, "full"),
Expand Down