Skip to content

[ENG-2022] Make Roam publishing sync-independent with complete node upserts - #1280

Open
sid597 wants to merge 5 commits into
mainfrom
eng-2022-make-roam-publishing-sync-independent-with-complete-node
Open

[ENG-2022] Make Roam publishing sync-independent with complete node upserts#1280
sid597 wants to merge 5 commits into
mainfrom
eng-2022-make-roam-publishing-sync-independent-with-complete-node

Conversation

@sid597

@sid597 sid597 commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Makes Roam publishing self-sufficient instead of piggybacking on background sync: publish now upserts the complete node — concept, direct title, and full markdown — through upsert_concepts, rather than only granting access to nodes sync had already pushed. The synced-nodes pre-filter is gone; an RPC failure grants nothing, a per-row failure withholds that node's grant (and any relation touching it) and surfaces as failedSyncedUids, replacing skippedUnsyncedUids.

Node sharing gets its own feature flag (Enable node sharing), decoupled from the suggestive-mode/sync flag: the Publish tab, the page-title Publish button, and DG: Share current node move to it, while discovery/import (DG: Discover shared nodes) stays on the sync flag.

This is the local eng-2022 branch rebased onto current main (clean, no conflicts). #1279 (ENG-1860) is stacked on top.

Scope check

  • Ran $scope-check against ENG-2022 and the final diff.
  • Scope beyond Done When: None.

Open in Devin Review

@linear-code

linear-code Bot commented Aug 7, 2026

Copy link
Copy Markdown

ENG-2022

@vercel

vercel Bot commented Aug 7, 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 Aug 10, 2026 5:41am

Request Review

@graphite-app

graphite-app Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

PR size/scope check

This PR is over our review-size guideline.

  • Recommended: ~200 lines changed
  • Acceptable limit: up to 400 lines when well-scoped/self-contained
  • Preferred file count: fewer than 5 files

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:

  • What single problem this PR solves
  • Why the files/changes are coupled

@supabase

supabase Bot commented Aug 7, 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 ↗︎.

@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 thread apps/roam/src/utils/publishNodesToGroups.ts
Comment thread apps/roam/src/components/Export.tsx Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 232615a8fa

ℹ️ 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".

Comment thread apps/roam/src/utils/publishNodesToGroups.ts Outdated
Comment thread apps/roam/src/utils/publishNodesToGroups.ts
sid597 added 5 commits August 10, 2026 11:10
…erts

Add an 'Enable node sharing' feature flag and gate the Publish tab,
'DG: Share current node' command, and page-title Publish button on it
instead of the suggestive-mode overlay flag. Publishing now converts
selected nodes to full CrossAppNodes (direct title + full markdown) and
upserts schema, concept, and contents before granting group access;
nodes whose upsert fails are excluded from grants.
…ting publish timestamps

upsert_concepts returns -1 for unique violations and -2 for other errors;
only -1 was treated as a failure, so a -2 node kept its ResourceAccess
grant. Node timestamps now follow the sync query's fallback chain
(create -> edit -> page-edit) instead of falling back to Date.now(),
which would have persisted publish time as last_modified whenever
:page/edit-time was absent. failedSyncedUids is renamed to
failedUpsertUids and the publish toast counts only failed selected
nodes.
… without sync

The sync loop now also starts when only node sharing is enabled. In
that mode it scopes node upserts to shared nodes and uploads their
content without generating embeddings; users, shared full-content
refresh, concept conversion, and orphan cleanup run as before. With
the sync flag on, behavior is unchanged.
Shared-nodes-only cycles previously completed the same "embedding" sync task, advancing its watermark without producing embeddings. Enabling the sync flag later would then skip the initial embedding backfill for nodes whose content was already uploaded. Keeping the two modes on separate sync_info rows leaves the embedding watermark untouched until full sync actually runs, and stops the two modes from postponing each other's cycles in mixed-flag spaces.
A failed schema upsert leaves its concept absent, so dependent nodes insert with a null schema_id and shared-node discovery filters them out. Treat those nodes as failed too: no ResourceAccess grant, counted in the failure toast, and relations touching them are withheld.
@sid597
sid597 force-pushed the eng-2022-make-roam-publishing-sync-independent-with-complete-node branch from ff9d52b to 7a4bdd1 Compare August 10, 2026 05:40
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