-
Notifications
You must be signed in to change notification settings - Fork 6
ENG-2018 Add converter for CrossAppRelation schema to LocalConceptDataInput #1212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: eng-1863-define-cross-app-relation-contract-and-fixture
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,13 +1,16 @@ | ||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||
| LocalRef, | ||||||||||||||||||||||||||||||||||||
| Ref, | ||||||||||||||||||||||||||||||||||||
| LocalOrRemoteRef, | ||||||||||||||||||||||||||||||||||||
| CrossAppEmbedding, | ||||||||||||||||||||||||||||||||||||
| InlineCrossAppContent, | ||||||||||||||||||||||||||||||||||||
| CrossAppBase, | ||||||||||||||||||||||||||||||||||||
| CrossAppNode, | ||||||||||||||||||||||||||||||||||||
| CrossAppRelation, | ||||||||||||||||||||||||||||||||||||
| } from "../crossAppContracts"; | ||||||||||||||||||||||||||||||||||||
| import { LocalContentDataInput, LocalConceptDataInput } from "../inputTypes"; | ||||||||||||||||||||||||||||||||||||
| import { Enums, CompositeTypes } from "../dbTypes"; | ||||||||||||||||||||||||||||||||||||
| import { spaceUriAndLocalIdToRid, ridToSpaceUriAndLocalId } from "./rid"; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| type InlineEmbeddingInput = CompositeTypes<"inline_embedding_input">; | ||||||||||||||||||||||||||||||||||||
| type InlineAbstractBase = Partial<CrossAppBase>; | ||||||||||||||||||||||||||||||||||||
|
|
@@ -39,6 +42,62 @@ const decodeRef = <DbVarName extends string, LocalVarName extends string>( | |||||||||||||||||||||||||||||||||||
| return decodeLocalRef(ref, localVarName); | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const decodeLocalOrRemoteRef = < | ||||||||||||||||||||||||||||||||||||
| LocalVarName extends string, | ||||||||||||||||||||||||||||||||||||
| DbVarName extends string, | ||||||||||||||||||||||||||||||||||||
| >({ | ||||||||||||||||||||||||||||||||||||
| ref, | ||||||||||||||||||||||||||||||||||||
| dbVarName, | ||||||||||||||||||||||||||||||||||||
| localVarName, | ||||||||||||||||||||||||||||||||||||
| currentSpaceUri, | ||||||||||||||||||||||||||||||||||||
| }: { | ||||||||||||||||||||||||||||||||||||
| ref: LocalOrRemoteRef | InlineAbstractBase | undefined; | ||||||||||||||||||||||||||||||||||||
| dbVarName: DbVarName; | ||||||||||||||||||||||||||||||||||||
| localVarName: LocalVarName; | ||||||||||||||||||||||||||||||||||||
| currentSpaceUri: string; | ||||||||||||||||||||||||||||||||||||
| }): | ||||||||||||||||||||||||||||||||||||
| | Record<LocalVarName, string> | ||||||||||||||||||||||||||||||||||||
| | Record<DbVarName, number> | ||||||||||||||||||||||||||||||||||||
| | Record<string, never> => { | ||||||||||||||||||||||||||||||||||||
| if (ref === undefined) return {}; | ||||||||||||||||||||||||||||||||||||
| if ("dbId" in ref) | ||||||||||||||||||||||||||||||||||||
| return { [dbVarName]: ref.dbId } as Record<DbVarName, number>; | ||||||||||||||||||||||||||||||||||||
| if ("rid" in ref && ref.rid !== undefined) { | ||||||||||||||||||||||||||||||||||||
| const { sourceLocalId, spaceUri } = ridToSpaceUriAndLocalId(ref.rid); | ||||||||||||||||||||||||||||||||||||
| if (spaceUri === currentSpaceUri) | ||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||
| [localVarName]: sourceLocalId, | ||||||||||||||||||||||||||||||||||||
| } as Record<LocalVarName, string>; | ||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||
| [localVarName]: ref.rid, | ||||||||||||||||||||||||||||||||||||
| } as Record<LocalVarName, string>; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if ("space" in ref && ref.space !== undefined) { | ||||||||||||||||||||||||||||||||||||
| if ("dbId" in ref.space) { | ||||||||||||||||||||||||||||||||||||
| // not sure how to handle this | ||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||
| [localVarName]: ref.localId, | ||||||||||||||||||||||||||||||||||||
| } as Record<LocalVarName, string>; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if ("url" in ref.space) { | ||||||||||||||||||||||||||||||||||||
| if (ref.space.url === currentSpaceUri) { | ||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||
| [localVarName]: ref.localId, | ||||||||||||||||||||||||||||||||||||
| } as Record<LocalVarName, string>; | ||||||||||||||||||||||||||||||||||||
| } else | ||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||
| [localVarName]: spaceUriAndLocalIdToRid(ref.space.url, ref.localId), | ||||||||||||||||||||||||||||||||||||
| } as Record<LocalVarName, string>; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if ("localId" in ref) { | ||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||
| [localVarName]: ref.localId, | ||||||||||||||||||||||||||||||||||||
| } as Record<LocalVarName, string>; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| return {}; | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const crossAppEmbeddingToDbEmbedding = ( | ||||||||||||||||||||||||||||||||||||
| embedding: CrossAppEmbedding | undefined, | ||||||||||||||||||||||||||||||||||||
| ): InlineEmbeddingInput | undefined => | ||||||||||||||||||||||||||||||||||||
|
|
@@ -109,3 +168,42 @@ export const crossAppNodeToDbConcept = ( | |||||||||||||||||||||||||||||||||||
| last_modified: node.modifiedAt?.toISOString(), | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export const crossAppRelationToDbConcept = ( | ||||||||||||||||||||||||||||||||||||
| node: CrossAppRelation, | ||||||||||||||||||||||||||||||||||||
| currentSpaceUri: string, | ||||||||||||||||||||||||||||||||||||
| ): LocalConceptDataInput => { | ||||||||||||||||||||||||||||||||||||
| 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>; | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+195
to
+199
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The filter logic will never capture This means when source/destination are passed as dbIds (via the Fix: Change the dbVarNames to include the // In decodeLocalOrRemoteRef calls:
dbVarName: "source_id", // instead of "source"
dbVarName: "destination_id", // instead of "destination"Spotted by Graphite |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| 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(), | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+201
to
+208
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( 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 crossAppNodeToDbConceptIn
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
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"atpackages/database/src/lib/crossAppConverters.ts:179,185), so the subsequent filter for keys ending in_idnever 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
decodeLocalOrRemoteRefis called withdbVarName: "source"(line 179) anddbVarName: "destination"(line 185). When the ref is aDbRef, the function returns e.g.{ source: 42 }(packages/database/src/lib/crossAppConverters.ts:64).Then
refIdsat line 195-199 filtersrelDataentries 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 thereference_contentoutput at line 205.The fix is to change the
dbVarNamevalues to"source_id"and"destination_id"so they match the_idsuffix filter, and then strip the_idsuffix when buildingrefIds(similar to howrefLocalIdsstrips_local_id).Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.