Skip to content

ENG-2018 Add converter for CrossAppRelation schema to LocalConceptDataInput#1212

Open
maparent wants to merge 1 commit into
eng-1863-define-cross-app-relation-contract-and-fixturefrom
eng-2018-add-converter-for-crossapprelation-schema-to
Open

ENG-2018 Add converter for CrossAppRelation schema to LocalConceptDataInput#1212
maparent wants to merge 1 commit into
eng-1863-define-cross-app-relation-contract-and-fixturefrom
eng-2018-add-converter-for-crossapprelation-schema-to

Conversation

@maparent

@maparent maparent commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

@linear-code

linear-code Bot commented Jul 10, 2026

Copy link
Copy Markdown

ENG-2018

@vercel

vercel Bot commented Jul 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
discourse-graph Ready Ready Preview, Comment Jul 10, 2026 1:50pm

Request Review

@supabase

supabase Bot commented Jul 10, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@maparent maparent force-pushed the eng-2018-add-converter-for-crossapprelation-schema-to branch from 8a6239e to b7016cc Compare July 10, 2026 13:01
@maparent maparent force-pushed the eng-1863-define-cross-app-relation-contract-and-fixture branch from 8ea3d29 to ab64936 Compare July 10, 2026 13:01
@maparent maparent force-pushed the eng-2018-add-converter-for-crossapprelation-schema-to branch from b7016cc to 0cfe8ce Compare July 10, 2026 13:40
@maparent maparent force-pushed the eng-1863-define-cross-app-relation-contract-and-fixture branch from ab64936 to f14ab44 Compare July 10, 2026 13:40
@maparent maparent force-pushed the eng-2018-add-converter-for-crossapprelation-schema-to branch from 0cfe8ce to bf2dd0c Compare July 10, 2026 13:48
@maparent maparent marked this pull request as ready for review July 10, 2026 13:52
@maparent maparent requested a review from mdroidian July 10, 2026 13:52

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 2 potential issues.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment on lines +176 to +199
const relData = {
...decodeLocalOrRemoteRef({
ref: node.source,
dbVarName: "source",
localVarName: "source_local_id",
currentSpaceUri,
}),
...decodeLocalOrRemoteRef({
ref: node.destination,
dbVarName: "destination",
localVarName: "destination_local_id",
currentSpaceUri,
}),
};
const refLocalIds = Object.fromEntries(
Object.entries(relData)
.filter(([k]) => k.endsWith("_local_id"))
.map(([k, v]) => [k.substring(0, k.length - 9), v]),
) as Record<string, string>;
const refIds = Object.fromEntries(
Object.entries(relData).filter(
([k]) => k.endsWith("_id") && !k.endsWith("_local_id"),
),
) as Record<string, number>;

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.

🔴 Database IDs for relation source and destination are silently dropped

The relation's source and destination database IDs are stored under keys that don't match the filter (dbVarName: "source" and "destination" at packages/database/src/lib/crossAppConverters.ts:179,185), so the subsequent filter for keys ending in _id never captures them.

Impact: When a relation's source or destination is referenced by database ID, the reference is silently lost and the resulting concept has empty reference_content.

Key mismatch between dbVarName and refIds filter logic

decodeLocalOrRemoteRef is called with dbVarName: "source" (line 179) and dbVarName: "destination" (line 185). When the ref is a DbRef, the function returns e.g. { source: 42 } (packages/database/src/lib/crossAppConverters.ts:64).

Then refIds at line 195-199 filters relData entries for keys ending in "_id" but not "_local_id". Since "source" and "destination" don't end in "_id", they are filtered out. The numeric database IDs are never included in the reference_content output at line 205.

The fix is to change the dbVarName values to "source_id" and "destination_id" so they match the _id suffix filter, and then strip the _id suffix when building refIds (similar to how refLocalIds strips _local_id).

Prompt for agents
In crossAppRelationToDbConcept (packages/database/src/lib/crossAppConverters.ts:172-209), the dbVarName values passed to decodeLocalOrRemoteRef are "source" and "destination", but the refIds filter at lines 195-199 looks for keys ending in "_id" (excluding "_local_id"). Since "source" and "destination" don't end in "_id", database IDs are silently dropped.

Two approaches to fix:
1. Change dbVarName to "source_id" and "destination_id", then in the refIds construction, strip the "_id" suffix from keys (like refLocalIds strips "_local_id") so the final reference_content JSON has keys "source" and "destination" with numeric values.
2. Alternatively, change the refIds filter to check for entries that are numbers rather than relying on key naming conventions.

Approach 1 is more consistent with the existing pattern. The refIds block would need a .map step like: .map(([k, v]) => [k.substring(0, k.length - 3), v]) to strip the "_id" suffix.
Open in Devin Review

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

Comment on lines +201 to +208
return filterUndefined<LocalConceptDataInput>({
...decodeLocalRef(node, "source_local_id"),
...decodeRef(node.author, "author_id", "author_local_id"),
local_reference_content: refLocalIds,
reference_content: refIds,
created: node.createdAt?.toISOString(),
last_modified: node.modifiedAt?.toISOString(),
});

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.

🔴 Relation type is not included when converting a relation to a database concept

The relation's type is not mapped to any database field (crossAppRelationToDbConcept at packages/database/src/lib/crossAppConverters.ts:201-208), unlike the parallel node converter which maps node type to the schema fields.

Impact: Relations stored in the database will have no schema/type association, making it impossible to distinguish different kinds of relations.

Missing relationType mapping compared to crossAppNodeToDbConcept

In crossAppNodeToDbConcept (packages/database/src/lib/crossAppConverters.ts:155-170), node.nodeType is mapped via decodeRef(node.nodeType, "schema_id", "schema_represented_by_local_id") at line 162. The CrossAppRelation type has an analogous relationType: Ref field (packages/database/src/crossAppContracts.ts:113), but crossAppRelationToDbConcept never maps it. The concept_local_input type has schema_id and schema_represented_by_local_id fields that should receive this value.

Suggested change
return filterUndefined<LocalConceptDataInput>({
...decodeLocalRef(node, "source_local_id"),
...decodeRef(node.author, "author_id", "author_local_id"),
local_reference_content: refLocalIds,
reference_content: refIds,
created: node.createdAt?.toISOString(),
last_modified: node.modifiedAt?.toISOString(),
});
return filterUndefined<LocalConceptDataInput>({
...decodeLocalRef(node, "source_local_id"),
...decodeRef(node.author, "author_id", "author_local_id"),
...decodeRef(node.relationType, "schema_id", "schema_represented_by_local_id"),
local_reference_content: refLocalIds,
reference_content: refIds,
created: node.createdAt?.toISOString(),
last_modified: node.modifiedAt?.toISOString(),
});
Open in Devin Review

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

Comment on lines +195 to +199
const refIds = Object.fromEntries(
Object.entries(relData).filter(
([k]) => k.endsWith("_id") && !k.endsWith("_local_id"),
),
) as Record<string, number>;

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.

The filter logic will never capture source or destination dbIds. The decodeLocalOrRemoteRef function returns dbVarNames as "source" and "destination" (lines 179, 185), which don't end with "_id". This filter checks for keys ending with "_id" but not "_local_id", so it will never match "source" or "destination".

This means when source/destination are passed as dbIds (via the "dbId" property path on lines 63-64), they will be lost and not included in reference_content.

Fix: Change the dbVarNames to include the _id suffix:

// In decodeLocalOrRemoteRef calls:
dbVarName: "source_id",  // instead of "source"
dbVarName: "destination_id",  // instead of "destination"

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant