ENG-2158 Publish node type format with schemas - #1316
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
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:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
sid597
left a comment
sid597
left a comment
There was a problem hiding this comment.
Notes on the non-obvious decisions, one per thread.
| label: string; | ||
| template?: string; | ||
| templateTitle?: string; | ||
| format?: string; |
There was a problem hiding this comment.
Flat literal_content.format, per the decision on the ticket (MG, team chat 2026-08-19). Obsidian's schema parser already reads flat format as a fallback, so the Obsidian producer stays unchanged. A separate key also avoids template, which the sync producer already uses for the block template.
| const literalInfo = filterUndefined({ | ||
| template: node.templateTitle, | ||
| template_content: node.template, | ||
| format: node.format, |
There was a problem hiding this comment.
The publish path writes only format here, no label or template. That asymmetry with the sync producer is pre-existing: publishNodesToGroups only upserts schemas that are not yet in my_concepts, and the next periodic sync rewrites the row with the full key set. Unifying the two shapes is out of scope for this ticket.
| ): LocalConceptDataInput => { | ||
| const titleParts = node.text.split("/"); | ||
| const label = titleParts[titleParts.length - 1] ?? node.text; | ||
| const literalContent: { [key: string]: Json } = { |
There was a problem hiding this comment.
literal_content is now built once, then the template branch adds its key. The old shape replaced the whole object in that branch, so every new key had to be written in two places. No behavior change for label and template.
| label: s.text, | ||
| authorId: userUid, | ||
| createdAt: new Date(relData[":create/time"] || Date.now()), | ||
| format: s.format, |
There was a problem hiding this comment.
DiscourseNode.format is marked deprecated in favor of specification, but that deprecation is about node identification. format is still the title pattern, and the title pattern is what ENG-2156 and ENG-2157 read from the schema row.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ea0a1b019
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| label: s.text, | ||
| authorId: userUid, | ||
| createdAt: new Date(relData[":create/time"] || Date.now()), | ||
| format: s.format, |
There was a problem hiding this comment.
When a node type was synchronized before this deployment, publishNodesToGroups finds its UID in my_concepts and only maps missingNodeSchemas into the upsert request, so this newly added field never reaches the existing schema row. The periodic path also selects only node types changed since the last sync (or associated with a refreshed shared node), meaning an unchanged schema can remain formatless indefinitely even after its nodes are published; recipients then import it using Obsidian's generated fallback format rather than the Roam format. Publishing should update the referenced schema while preserving its other literal_content keys, not skip it solely because it already exists.
Useful? React with 👍 / 👎.
1 participant
Roam now publishes a node type's format with its schema row, so an importer can seed a new local type from it (ENG-2156, ENG-2157).
CrossAppNodeSchemagainsformat?: string, mapped toliteral_content.formatincrossAppNodeSchemaToDbConcept.nodeSchemaToCrossApp(publish) anddiscourseNodeSchemaToLocalConcept(periodic sync).upsert_conceptsassignsliteral_contentwholesale, so a producer that omits the key erases it.formatas a fallback.Tests cover both producers, key preservation with a template, and the publish path end to end.
Closes ENG-2158