[ENG-1847] Define shared cross-app node content contract#1129
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
PR size/scope checkThis PR is over our review-size guideline.
Please split this into smaller PRs unless there is a clear reason the changes need to land together. If keeping it as one PR, please add a brief justification covering:
|
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
Great work! This was easy to follow, very clear.
One thing Devin noted which might be worth mentioning in the rid.ts to possibly save us in the future:
packages/database/src/lib/rid.ts:R13
Round-trip behavior of URL-based RIDs relies on localId never containing slashes
The spaceUriAndLocalIdToRid function for HTTP-based space URIs (packages/database/src/lib/rid.ts:13) simply concatenates ${spaceUri}/${localId}. The inverse ridToSpaceUriAndLocalId (packages/database/src/lib/rid.ts:33-35) splits on / and pops the last segment as sourceLocalId. This means a localId containing / would break the round-trip. In the example file, Roam UIDs (e.g., tgWb6JozF) don't contain slashes, and Obsidian uses the orn: format instead, so this isn't currently triggered. However, this is a latent fragility in the shared contract that future consumers should be aware of.
* [ENG-1847] Define shared cross-app node content contract
* [ENG-1849] Add node-type schema dependency to cross-app node fixtures
The cross-app node contract (ENG-1847) carries nodeType = { sourceLocalId, label } and the instance Concept references it via schema_represented_by_local_id, but the fixtures persisted no is_schema:true schema Concept for that id. Add the required schemaConcept (Roam Claim, Obsidian Evidence) carrying stable source identity + label only -- no source_data/format/color/tag, per the contract's "without redefining schema shape".
Roam's existing 5-min sync already persists the schema Concept per contract (convertDgToSupabaseConcepts -> discourseNodeSchemaToLocalConcept; all types on initial sync, edited types incrementally via nodeTypeSince), so no Roam sync code change is needed for F4/F9/F13. Stacked on eng-1847 (PR #1129, unmerged).
* [ENG-1849] Persist Roam schema labels
2 participants
Defines the app-neutral contract for Roam ↔ Obsidian node discovery / import / refresh. Definition only.
Linear: https://linear.app/discourse-graphs/issue/ENG-1847
crossAppNodeContract.ts—CrossAppNodetypes: a typed view over the existing Concept/Content model; no new persistence path.lib/rid.ts— RID helper promoted from obsidian into@repo/database; obsidian'srid.tsre-exports it (verbatim move, behavior-preserving).fixtures/crossAppNodes.ts— one Roam + one Obsidian worked example.package.json— exports for the contract + fixtures.Full spec (field mapping, source-side gaps, markdown fidelity limits) lives on Linear ENG-1847. prettier / eslint / tsc green for
@repo/databaseandapps/obsidian.🤖 Generated with Claude Code
https://claude.ai/code/session_01X4YAFA8BL48YJqTCHbJuwT