Skip to content

eng-1863 Relation cross-app contract #1197

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.

Sign up for GitHub

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

Draft
maparent wants to merge 1 commit into
base: main
Choose a base branch
from
+29 −0
Draft

eng-1863 Relation cross-app contract #1197

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 29 additions & 0 deletions packages/database/src/crossAppContracts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,28 @@ type DbRef = {
// Generalized reference
export type Ref = LocalRef | DbRef;

type SpaceRef = DbRef | { url: string; sourceApp: Enums<"Platform"> };

// A potentially cross-space reference
export type LocalOrRemoteRef =
| DbRef
| {
localId: string;
// Treat as LocalRef if space is absent
space?: SpaceRef;
// make the options mutually exclusive
dbId?: never;
rid?: never;
}
| {
// A string that contains combined space and localId
rid: string;
// make the options mutually exclusive
dbId?: never;
localId?: never;
space?: never;
};
Comment on lines +21 to +37

@mdroidian mdroidian Jul 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are all of these union variants required? If so, could we add examples showing where each is already used or needed by upcoming work? Otherwise, I’d prefer to keep the contract limited to the variants we currently need and add others when a concrete use case arises.

Including speculative variants increases the contract’s conceptual surface area and requires additional validation, tests, and converter paths, which may create unnecessary confusion and complexity.

@maparent maparent Jul 13, 2026

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.

We do use the Rid in

discourse-graph/apps/obsidian/src/utils/conceptConversion.ts

Line 224 in 7e058c1

(sourceNode.frontmatter.importedFromRid as string | undefined) ??

And I expect this pattern to hold: We store a Rid and send it back.
We do not currently have a version that requires space directly at the upsert_concept call points, because the burden of computing the Rid from space+localId is currently on the platform (8 calls to spaceUriAndLocalIdToRid in the Obsidian code base, many upstream of upsert_concept.)
Having the space+localId variant in the contract will help centralize the Rid computation in the database package.

@mdroidian mdroidian Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, that confirms the { rid } variant is required.

I agree that centralizing the spaceUri + localId → RID normalization could be useful. What I’m trying to separate is centralizing the RID implementation—which already lives in @repo/database/lib/rid—from expanding the shared relation contract to accept another representation of the same identity.

The existing Obsidian call sites use RIDs for several platform-level purposes, including imported provenance, duplicate detection, previews, and endpoint lookup. Because of that, moving normalization into the upsert converter would not replace all eight call sites.

This rationale also seems to support, at most, a { localId, spaceUri } input. I don’t see the same justification yet for the top-level { dbId }, { localId, space: { dbId } }, or the sourceApp property. The current converter does not use sourceApp, and its space.dbId branch is explicitly unresolved.

I’d prefer to keep the shared endpoint contract as LocalRef | RidRef for now:

type RelationEndpointRef =
  | { localId: string }
  | { rid: string };

This gives the shared boundary one canonical representation for remote identity: an RID. A platform that naturally has spaceUri + localId can normalize it before constructing the contract.

If an upcoming Roam or Obsidian producer genuinely benefits from emitting { localId, spaceUri }, we can add that form alongside the producer and a test demonstrating its normalization to the expected RID.


// Common attributes for most types
export type CrossAppBase = LocalRef & {
createdAt: Date;
Expand Down Expand Up @@ -85,3 +107,10 @@ export type CrossAppNode = CrossAppBase & {
full: InlineCrossAppTypedContent;
};
};

// A relation instance
export type CrossAppRelation = CrossAppBase & {
relationType: Ref;
source: LocalOrRemoteRef;
destination: LocalOrRemoteRef;
};