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
15 changes: 6 additions & 9 deletions apps/obsidian/src/utils/conceptConversion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,13 +231,6 @@ export const relationInstanceToLocalConcept = ({
return null;
}

if (
sourceNode.frontmatter.importedFromRid ||
destinationNode.frontmatter.importedFromRid
)
return null; // punt relation to imported nodes for now.
// otherwise put the importedFromRid in source, dest.

/* eslint-disable @typescript-eslint/naming-convention */
const literal_content: Record<string, Json> = {};
if (importedFromRid) literal_content.importedFromRid = importedFromRid;
Expand All @@ -252,8 +245,12 @@ export const relationInstanceToLocalConcept = ({
last_modified: new Date(lastModified ?? created).toISOString(),
literal_content,
local_reference_content: {
source,
destination,
source:
(sourceNode.frontmatter.importedFromRid as string | undefined) ??
source,
destination:
(destinationNode.frontmatter.importedFromRid as string | undefined) ??
destination,
},
/* eslint-enable @typescript-eslint/naming-convention */
};
Expand Down
2 changes: 1 addition & 1 deletion apps/obsidian/src/utils/publishNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ export const publishNodeRelations = async ({
(fm.publishedToGroups as string[]) || [];
if (!publishedToGroups.includes(myGroup)) return;
}
if (fm.importedFromRid) return; // temporary, should be removed after eng-1475
relevantNodeTypeById[id] = fm.nodeTypeId as string;
});
relations.map((relation) => {
Expand All @@ -199,6 +198,7 @@ export const publishNodeRelations = async ({
resourceIds.add(triple.id);
}
});
if (!resourceIds) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 !resourceIds is always false because a Set object is always truthy

The guard if (!resourceIds) return; at line 204 is intended to skip the Supabase upsert when there are no resource IDs to publish. However, resourceIds is a Set<string> (initialized on line 165), and a Set object is always truthy in JavaScript regardless of its size. This means the check never triggers an early return.

Root Cause and Impact

The condition !resourceIds evaluates the truthiness of the Set object itself, not its size. Since any object (including an empty Set) is truthy in JavaScript, !resourceIds is always false.

The correct check should be if (resourceIds.size === 0) return;.

Impact: When no matching relation triples are found, the function proceeds to call client.from("ResourceAccess").upsert(...) with an empty array ([...resourceIds.values()] yields []). Depending on how Supabase handles an empty upsert, this could be a no-op or could cause an unnecessary network request. More importantly, the function then proceeds to loadRelations, iterate over relations, and potentially call saveRelations — all unnecessary work.

Suggested change
if (!resourceIds) return;
if (resourceIds.size === 0) return;
Open in Devin Review

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

const publishResponse = await client.from("ResourceAccess").upsert(
[...resourceIds.values()].map((sourceLocalId: string) => ({
/* eslint-disable @typescript-eslint/naming-convention */
Expand Down
4 changes: 2 additions & 2 deletions apps/obsidian/src/utils/syncDgNodesToSupabase.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 Missing legacy importedFromSpaceUri check in collectDiscourseNodesFromPaths allows imported files to be synced

Same root cause as in shouldSyncFile: collectDiscourseNodesFromPaths only checks frontmatter.importedFromRid but not the legacy frontmatter.importedFromSpaceUri. The refactored collectDiscourseNodesFromVault at apps/obsidian/src/utils/getDiscourseNodes.ts:32-38 correctly checks both fields, but this function was not updated consistently.

Root Cause and Impact

This function is used by syncDiscourseNodeChanges (called from FileChangeListener). During the migration window or if migration fails, any file change event on a file that still has only the legacy importedFromSpaceUri will pass through both shouldSyncFile and collectDiscourseNodesFromPaths guards, causing the imported node to be synced to Supabase as a locally-created node. This is a second guard that should also check the legacy field for defense-in-depth.

(Refers to lines 717-720)

Open in Devin Review

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

Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ export const syncAllNodesAndRelations = async (
}
console.debug("Supabase client:", supabaseClient);

const allNodes = await collectDiscourseNodesFromVault(plugin);
const allNodes = await collectDiscourseNodesFromVault(plugin, true);

const changedNodeInstances = relationsOnly
? []
Comment on lines +426 to 429
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 Imported nodes are unintentionally synced as Content and Concepts to the user's own space

Changing collectDiscourseNodesFromVault(plugin) to collectDiscourseNodesFromVault(plugin, true) causes allNodes to include imported nodes. When relationsOnly is false, these imported nodes are passed directly to buildChangedNodesFromNodes at line 430. Since imported nodes were never synced as Content to this space, detectNodeChanges (syncDgNodesToSupabase.ts:298-301) treats them as brand-new files (returning ["title", "content"]), so they end up in changedNodeInstances. They then flow into:

  1. upsertNodesToSupabaseAsContentWithEmbeddings (line 444) — creating Content rows and fetching embeddings for nodes that belong to another space
  2. discourseNodeInstanceToLocalConcept via convertDgToSupabaseConcepts (line 452) — creating duplicate Concept rows in the user's space
Root Cause and Impact

The intent of passing includeImported: true was to populate allNodesById in convertDgToSupabaseConcepts so that relationInstanceToLocalConcept can look up imported nodes and use their importedFromRid for local_reference_content. However, the same allNodes array is also fed into buildChangedNodesFromNodes, which has no filter for imported nodes.

Impact: Every sync (when relationsOnly is false) will re-upload all imported nodes as Content with embeddings (wasting API calls and storage), and create duplicate Concept entries in the user's own space for nodes that are supposed to be cross-space references. This could also cause unique constraint violations on (space_id, name) if the imported node's file path collides with an existing concept name.

The fix should separate the two uses: pass only non-imported nodes to buildChangedNodesFromNodes, while passing all nodes (including imported) to convertDgToSupabaseConcepts for the allNodesById lookup.

(Refers to lines 426-434)

Prompt for agents
In apps/obsidian/src/utils/syncDgNodesToSupabase.ts, in the syncAllNodesAndRelations function (lines 406-469), the allNodes variable (line 426) now includes imported nodes via collectDiscourseNodesFromVault(plugin, true). This is needed for convertDgToSupabaseConcepts (line 452-459) but NOT for buildChangedNodesFromNodes (line 430-434). Fix by collecting two sets of nodes:

1. allNodes (with imported) for passing to convertDgToSupabaseConcepts
2. localNodes (without imported) for passing to buildChangedNodesFromNodes

For example, replace lines 426-434 with something like:

    const allNodes = await collectDiscourseNodesFromVault(plugin, true);
    const localNodes = allNodes.filter(n => !n.frontmatter.importedFromRid);

    const changedNodeInstances = relationsOnly
      ? []
      : await buildChangedNodesFromNodes({
          nodes: localNodes,
          supabaseClient,
          context,
        });

This ensures imported nodes are available in allNodesById for relation reference resolution but are not synced as Content/Concepts to the user's own space.
Open in Devin Review

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

Expand Down Expand Up @@ -495,7 +495,7 @@ const convertDgToSupabaseConcepts = async ({
const nodeTypes = plugin.settings.nodeTypes ?? [];
const relationTypes = plugin.settings.relationTypes ?? [];
const discourseRelations = plugin.settings.discourseRelations ?? [];
allNodes = allNodes ?? (await collectDiscourseNodesFromVault(plugin));
allNodes = allNodes ?? (await collectDiscourseNodesFromVault(plugin, true));
const allNodesById = Object.fromEntries(
allNodes.map((n) => [n.nodeInstanceId, n]),
);
Expand Down
15 changes: 15 additions & 0 deletions packages/database/src/dbTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1586,6 +1586,20 @@ export type Database = {
}
Returns: string
}
rid_or_local_id_to_concept_db_id: {
Args: { default_space_id: number; rid: string }
Returns: number
}
rid_to_space_id_and_local_id: {
Args: { rid: string }
Returns: Database["public"]["CompositeTypes"]["accessible_resource"]
SetofOptions: {
from: "*"
to: "accessible_resource"
isOneToOne: true
isSetofReturn: false
}
}
schema_of_concept:
| {
Args: { concept: Database["public"]["Tables"]["Concept"]["Row"] }
Expand Down Expand Up @@ -1967,3 +1981,4 @@ export const Constants = {
},
},
} as const

103 changes: 103 additions & 0 deletions packages/database/supabase/migrations/20260221193625_rid_functions.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
CREATE OR REPLACE FUNCTION public.rid_to_space_id_and_local_id(rid VARCHAR)
RETURNS public.accessible_resource STRICT STABLE
SET search_path = ''
LANGUAGE plpgsql AS $$
DECLARE
uri VARCHAR;
source_local_id VARCHAR;
source_id BIGINT;
BEGIN
source_local_id := split_part(rid, '/', -1);
IF length(source_local_id) = length(rid) THEN
RETURN (null, 'Not a Rid')::public.accessible_resource;
END IF;
uri := substr(rid, 1, length(rid) - length(source_local_id) - 1);
IF rid ~ '^orn:\w+\.\w+:.*$' THEN
uri := concat(split_part(split_part(uri, ':', 2), '.', 1), ':', split_part(uri, ':', 3));
ELSE
IF rid ~ '^orn:\w+:.*$' THEN
uri := substr(uri, 5);
END IF;
END IF;
SELECT id INTO source_id FROM public."Space" where url=uri;
IF source_id IS NULL THEN
RETURN (null, concat('Cannot find ', uri))::public.accessible_resource;
END IF;
RETURN (source_id, source_local_id);
END;
$$;

CREATE OR REPLACE FUNCTION public.rid_or_local_id_to_concept_db_id(rid VARCHAR, default_space_id BIGINT)
RETURNS BIGINT STRICT STABLE
SET search_path = ''
LANGUAGE plpgsql AS $$
DECLARE r public.accessible_resource;
BEGIN
r := (SELECT public.rid_to_space_id_and_local_id(rid));
IF r.space_id IS NULL THEN
RETURN (SELECT id FROM public."Concept" WHERE space_id = default_space_id AND source_local_id = rid);
ELSE
RETURN (SELECT id FROM public."Concept" WHERE space_id = r.space_id AND source_local_id = r.source_local_id);
END IF;
END;
$$;

CREATE OR REPLACE FUNCTION public._local_concept_to_db_concept(data public.concept_local_input)
RETURNS public."Concept" STABLE
SET search_path = ''
LANGUAGE plpgsql
AS $$
DECLARE
concept public."Concept"%ROWTYPE;
reference_content JSONB := jsonb_build_object();
key varchar;
value JSONB;
ref_single_val BIGINT;
ref_array_val BIGINT[];
BEGIN
-- not fan of going through json, but not finding how to populate a record by a different shape record
concept := jsonb_populate_record(NULL::public."Concept", to_jsonb(data));
IF data.author_local_id IS NOT NULL THEN
SELECT id FROM public."PlatformAccount"
WHERE account_local_id = data.author_local_id INTO concept.author_id;
END IF;
IF data.represented_by_id IS NOT NULL THEN
SELECT space_id, source_local_id FROM public."Content"
WHERE id = data.represented_by_id INTO concept.space_id, concept.source_local_id;
END IF;
IF data.space_url IS NOT NULL THEN
SELECT id FROM public."Space"
WHERE url = data.space_url INTO concept.space_id;
END IF;
IF concept.source_local_id = '' THEN
concept.source_local_id := NULL;
END IF;
IF data.represented_by_local_id = '' THEN
data.represented_by_local_id := NULL;
END IF;
IF data.schema_represented_by_local_id IS NOT NULL THEN
SELECT public.rid_or_local_id_to_concept_db_id(
data.schema_represented_by_local_id, concept.space_id) INTO concept.schema_id;
END IF;
concept.source_local_id = COALESCE(concept.source_local_id, data.represented_by_local_id); -- legacy input field
concept.reference_content := coalesce(data.reference_content, '{}'::jsonb);
IF data.local_reference_content IS NOT NULL THEN
FOR key, value IN SELECT * FROM jsonb_each(data.local_reference_content) LOOP
IF jsonb_typeof(value) = 'array' THEN
WITH el AS (SELECT jsonb_array_elements_text(value) as x),
el2 AS (SELECT public.rid_or_local_id_to_concept_db_id(x, concept.space_id) AS id FROM el)
SELECT array_agg(DISTINCT el2.id) INTO STRICT ref_array_val
FROM el2 WHERE el2.id IS NOT NULL;
reference_content := jsonb_set(reference_content, ARRAY[key], to_jsonb(ref_array_val));
ELSIF jsonb_typeof(value) = 'string' THEN
SELECT public.rid_or_local_id_to_concept_db_id(value #>> '{}', concept.space_id) INTO STRICT ref_single_val;
reference_content := jsonb_set(reference_content, ARRAY[key], to_jsonb(ref_single_val));
ELSE
RAISE EXCEPTION 'Invalid value in local_reference_content % %', value, jsonb_typeof(value);
END IF;
END LOOP;
concept.reference_content := concept.reference_content || reference_content;
END IF;
RETURN concept;
END;
$$;
66 changes: 53 additions & 13 deletions packages/database/supabase/schemas/concept.sql
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,49 @@ $$;
COMMENT ON FUNCTION public.author_of_concept(public.my_concepts)
IS 'Computed one-to-one: returns the PlatformAccount which authored a given Concept.';

CREATE OR REPLACE FUNCTION public.rid_to_space_id_and_local_id(rid VARCHAR)
RETURNS public.accessible_resource STRICT STABLE
SET search_path = ''
LANGUAGE plpgsql AS $$
DECLARE
uri VARCHAR;
source_local_id VARCHAR;
source_id BIGINT;
BEGIN
source_local_id := split_part(rid, '/', -1);
IF length(source_local_id) = length(rid) THEN
RETURN (null, 'Not a Rid')::public.accessible_resource;
END IF;
uri := substr(rid, 1, length(rid) - length(source_local_id) - 1);
IF rid ~ '^orn:\w+\.\w+:.*$' THEN
uri := concat(split_part(split_part(uri, ':', 2), '.', 1), ':', split_part(uri, ':', 3));
ELSE
IF rid ~ '^orn:\w+:.*$' THEN
uri := substr(uri, 5);
END IF;
END IF;
SELECT id INTO source_id FROM public."Space" where url=uri;
IF source_id IS NULL THEN
RETURN (null, concat('Cannot find ', uri))::public.accessible_resource;
END IF;
RETURN (source_id, source_local_id);
END;
$$;

CREATE OR REPLACE FUNCTION public.rid_or_local_id_to_concept_db_id(rid VARCHAR, default_space_id BIGINT)
RETURNS BIGINT STRICT STABLE
SET search_path = ''
LANGUAGE plpgsql AS $$
DECLARE r public.accessible_resource;
BEGIN
r := (SELECT public.rid_to_space_id_and_local_id(rid));
IF r.space_id IS NULL THEN
RETURN (SELECT id FROM public."Concept" WHERE space_id = default_space_id AND source_local_id = rid);
ELSE
RETURN (SELECT id FROM public."Concept" WHERE space_id = r.space_id AND source_local_id = r.source_local_id);
END IF;
END;
$$;

CREATE TYPE public.concept_local_input AS (
-- concept columns
Expand Down Expand Up @@ -292,37 +335,34 @@ BEGIN
SELECT id FROM public."Space"
WHERE url = data.space_url INTO concept.space_id;
END IF;
IF data.schema_represented_by_local_id IS NOT NULL THEN
SELECT cpt.id FROM public."Concept" cpt
WHERE cpt.source_local_id = data.schema_represented_by_local_id
AND cpt.space_id = concept.space_id INTO concept.schema_id;
END IF;
IF concept.source_local_id = '' THEN
concept.source_local_id := NULL;
END IF;
IF data.represented_by_local_id = '' THEN
data.represented_by_local_id := NULL;
END IF;
IF data.schema_represented_by_local_id IS NOT NULL THEN
SELECT public.rid_or_local_id_to_concept_db_id(
data.schema_represented_by_local_id, concept.space_id) INTO concept.schema_id;
END IF;
concept.source_local_id = COALESCE(concept.source_local_id, data.represented_by_local_id); -- legacy input field
concept.reference_content := coalesce(data.reference_content, '{}'::jsonb);
IF data.local_reference_content IS NOT NULL THEN
FOR key, value IN SELECT * FROM jsonb_each(data.local_reference_content) LOOP
IF jsonb_typeof(value) = 'array' THEN
WITH el AS (SELECT jsonb_array_elements_text(value) as x),
ela AS (SELECT array_agg(x) AS a FROM el)
SELECT array_agg(DISTINCT cpt.id) INTO STRICT ref_array_val
FROM public."Concept" AS cpt
JOIN ela ON (true) WHERE cpt.source_local_id = ANY(ela.a) AND cpt.space_id=concept.space_id;
el2 AS (SELECT public.rid_or_local_id_to_concept_db_id(x, concept.space_id) AS id FROM el)
SELECT array_agg(DISTINCT el2.id) INTO STRICT ref_array_val
FROM el2 WHERE el2.id IS NOT NULL;
reference_content := jsonb_set(reference_content, ARRAY[key], to_jsonb(ref_array_val));
ELSIF jsonb_typeof(value) = 'string' THEN
SELECT cpt.id INTO STRICT ref_single_val
FROM public."Concept" AS cpt
WHERE cpt.source_local_id = (value #>> '{}') AND cpt.space_id=concept.space_id;
SELECT public.rid_or_local_id_to_concept_db_id(value #>> '{}', concept.space_id) INTO STRICT ref_single_val;
reference_content := jsonb_set(reference_content, ARRAY[key], to_jsonb(ref_single_val));
ELSE
RAISE EXCEPTION 'Invalid value in local_reference_content % %', value, jsonb_typeof(value);
END IF;
END LOOP;
SELECT reference_content INTO concept.reference_content;
concept.reference_content := concept.reference_content || reference_content;
END IF;
RETURN concept;
END;
Expand Down