[ENG-1855] Address shared-node discovery review feedback

Author: sid597

apps/roam/src/components/DiscoverSharedNodesDialog.tsx

@@ -17,6 +17,7 @@ import {
   discoverSharedNodes,
   type DiscoveredSharedNode,
 } from "~/utils/discoverSharedNodes";
+import internalError from "~/utils/internalError";
 import { getLoggedInClient, getSupabaseContext } from "~/utils/supabaseContext";
 
 const formatModifiedAt = (modifiedAt: string): string =>
@@ -80,7 +81,7 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
   const [nodes, setNodes] = useState<DiscoveredSharedNode[]>([]);
   const [loading, setLoading] = useState(true);
   const [error, setError] = useState("");
-  const [search, setSearch] = useState("");
+  const [searchTerm, setSearchTerm] = useState("");
 
   const loadNodes = useCallback(async (): Promise<void> => {
     setLoading(true);
@@ -97,7 +98,12 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
         }),
       );
     } catch (loadError) {
-      console.error("Failed to discover shared nodes:", loadError);
+      internalError({
+        error: loadError,
+        type: "Shared node discovery failed",
+        context: { operation: "load-shared-nodes" },
+        sendEmail: false,
+      });
       setError(
         loadError instanceof Error
           ? loadError.message
@@ -113,7 +119,7 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
   }, [loadNodes]);
 
   const visibleNodes = useMemo(() => {
-    const normalizedSearch = search.trim().toLocaleLowerCase();
+    const normalizedSearch = searchTerm.trim().toLocaleLowerCase();
     if (!normalizedSearch) return nodes;
     return nodes.filter((node) =>
       [
@@ -124,7 +130,7 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
         node.sourceNodeId,
       ].some((value) => value?.toLocaleLowerCase().includes(normalizedSearch)),
     );
-  }, [nodes, search]);
+  }, [nodes, searchTerm]);
 
   return (
     <Dialog
@@ -147,10 +153,10 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
             className="min-w-0 flex-1"
             leftIcon="search"
             onChange={(event: React.ChangeEvent<HTMLInputElement>) =>
-              setSearch(event.target.value)
+              setSearchTerm(event.target.value)
             }
             placeholder="Search shared nodes"
-            value={search}
+            value={searchTerm}
           />
           <Tooltip content="Reload shared nodes">
             <Button
@@ -178,7 +184,9 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
           <div className="flex min-h-52 items-center justify-center">
             <NonIdealState
               icon="search"
-              title={search ? "No matching shared nodes" : "No shared nodes"}
+              title={
+                searchTerm ? "No matching shared nodes" : "No shared nodes"
+              }
             />
           </div>
         ) : (

apps/roam/src/utils/__tests__/discoverSharedNodes.test.ts

@@ -18,50 +18,51 @@ const spaces: BuildArgs["spaces"] = [
 const concepts: BuildArgs["concepts"] = [
   {
     is_schema: false,
-    last_modified: "2026-06-14T12:00:00.000Z",
+    last_modified: "2026-06-14T12:00:00",
     schema_id: 200,
     source_local_id: "node-1",
     space_id: 20,
   },
 ];
-const contents: BuildArgs["contents"] = [
+const directContents: BuildArgs["directContents"] = [
   {
-    content_type: "text/plain",
-    last_modified: "2026-06-14T13:00:00.000Z",
+    last_modified: "2026-06-14T13:00:00",
     source_local_id: "node-1",
     space_id: 20,
     text: "EVD - REM sleep and recall",
-    variant: "direct",
   },
+];
+const fullContentSummaries: BuildArgs["fullContentSummaries"] = [
   {
     content_type: "text/markdown",
-    last_modified: "2026-06-14T15:00:00.000Z",
+    last_modified: "2026-06-14T15:00:00",
     source_local_id: "node-1",
     space_id: 20,
-    text: "# EVD - REM sleep and recall",
-    variant: "full",
   },
 ];
 const sourceNodeRid = "orn:obsidian.note:vault-a/node-1";
 
 const build = ({
   conceptsOverride = concepts,
-  contentsOverride = contents,
+  directContentsOverride = directContents,
+  fullContentSummariesOverride = fullContentSummaries,
   currentSpaceId = 10,
   importedSourceRids = new Set<string>(),
   resourcesOverride = resources,
   spacesOverride = spaces,
 }: {
   conceptsOverride?: typeof concepts;
-  contentsOverride?: typeof contents;
+  directContentsOverride?: typeof directContents;
+  fullContentSummariesOverride?: typeof fullContentSummaries;
   currentSpaceId?: number;
   importedSourceRids?: ReadonlySet<string>;
   resourcesOverride?: typeof resources;
   spacesOverride?: typeof spaces;
 } = {}) =>
   buildDiscoveredSharedNodes({
     concepts: conceptsOverride,
-    contents: contentsOverride,
+    directContents: directContentsOverride,
+    fullContentSummaries: fullContentSummariesOverride,
     currentSpaceId,
     importedSourceRids,
     resources: resourcesOverride,
@@ -100,30 +101,54 @@ describe("buildDiscoveredSharedNodes", () => {
     {
       name: "schema concept",
       conceptsOverride: [{ ...concepts[0], is_schema: true }],
-      contentsOverride: contents,
+      directContentsOverride: directContents,
+      fullContentSummariesOverride: fullContentSummaries,
     },
     {
       name: "missing node type",
       conceptsOverride: [{ ...concepts[0], schema_id: null }],
-      contentsOverride: contents,
+      directContentsOverride: directContents,
+      fullContentSummariesOverride: fullContentSummaries,
     },
     {
       name: "missing direct content",
       conceptsOverride: concepts,
-      contentsOverride: [contents[1]],
-    },
-    {
-      name: "missing full content",
-      conceptsOverride: concepts,
-      contentsOverride: [contents[0]],
+      directContentsOverride: [],
+      fullContentSummariesOverride: fullContentSummaries,
     },
     {
       name: "untyped full content",
       conceptsOverride: concepts,
-      contentsOverride: [contents[0], { ...contents[1], content_type: null }],
+      directContentsOverride: directContents,
+      fullContentSummariesOverride: [
+        { ...fullContentSummaries[0], content_type: null },
+      ],
+    },
+  ])(
+    "filters a node with $name",
+    ({
+      conceptsOverride,
+      directContentsOverride,
+      fullContentSummariesOverride,
+    }) => {
+      expect(
+        build({
+          conceptsOverride,
+          directContentsOverride,
+          fullContentSummariesOverride,
+        }),
+      ).toEqual([]);
     },
-  ])("filters a node with $name", ({ conceptsOverride, contentsOverride }) => {
-    expect(build({ conceptsOverride, contentsOverride })).toEqual([]);
+  );
+
+  it("discovers a node without optional full content", () => {
+    expect(build({ fullContentSummariesOverride: [] })).toMatchObject([
+      {
+        modifiedAt: "2026-06-14T13:00:00.000Z",
+        sourceNodeRid,
+        title: "EVD - REM sleep and recall",
+      },
+    ]);
   });
 
   it("matches imports by RID rather than source-local ID alone", () => {
@@ -134,25 +159,49 @@ describe("buildDiscoveredSharedNodes", () => {
     ).toBe(false);
   });
 
+  it.each([
+    "orn:obsidian.note:vault-a/node-1",
+    "https://example.com/shared/node-1",
+  ])("preserves a RID-shaped source-local ID: %s", (rid) => {
+    expect(
+      build({
+        conceptsOverride: [{ ...concepts[0], source_local_id: rid }],
+        directContentsOverride: [
+          { ...directContents[0], source_local_id: rid },
+        ],
+        fullContentSummariesOverride: [
+          { ...fullContentSummaries[0], source_local_id: rid },
+        ],
+        resourcesOverride: [{ space_id: 20, source_local_id: rid }],
+      })[0]?.sourceNodeRid,
+    ).toBe(rid);
+  });
+
   it("sorts newest nodes first", () => {
     const olderConcept = {
       ...concepts[0],
-      last_modified: "2026-06-10T12:00:00.000Z",
+      last_modified: "2026-06-10T12:00:00",
+      source_local_id: "node-2",
+    };
+    const olderDirectContent = {
+      ...directContents[0],
+      last_modified: "2026-06-10T12:00:00",
       source_local_id: "node-2",
+      text: "Older shared node",
     };
-    const olderContents = contents.map((content) => ({
-      ...content,
-      last_modified: "2026-06-10T12:00:00.000Z",
+    const olderFullContentSummary = {
+      ...fullContentSummaries[0],
+      last_modified: "2026-06-10T12:00:00",
       source_local_id: "node-2",
-      text:
-        content.variant === "direct"
-          ? "Older shared node"
-          : "# Older shared node",
-    }));
+    };
     expect(
       build({
         conceptsOverride: [olderConcept, concepts[0]],
-        contentsOverride: [...olderContents, ...contents],
+        directContentsOverride: [olderDirectContent, ...directContents],
+        fullContentSummariesOverride: [
+          olderFullContentSummary,
+          ...fullContentSummaries,
+        ],
         resourcesOverride: [
           ...resources,
           { space_id: 20, source_local_id: "node-2" },