Land orphaned Graphite stack PRs (#764, #765, #769, #793, #794, #805)#826
Land orphaned Graphite stack PRs (#764, #765, #769, #793, #794, #805)#826
Conversation
These PRs were squash-merged into parent branches instead of main due to a Graphite stack merge race condition. Their changes never landed on main despite showing as merged in Graphite. Includes: - #764 ENG-1280: Port query builder/editor for block props - #765 ENG-1291: Port discourse node specification (dual-write) - #769 ENG-1290: Port discourse node config panel - #793 ENG-1440: Port page groups settings in suggestive mode - #794 ENG-1438: Port keyboard shortcut keys/triggers - #805 ENG-1328: Port small blueprint misc settings
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
Because there were some conflicts, be sure to do a compile, load in roam, and run through a quick test of each changed setting component to make sure the existing functionality still works.
This only needs to be a brief (under 5 min) loom video.
Add that to this PR (as a comment) before merging.
specification, config panel) The initial recovery commit claimed to include these but only contained Chain 2 (#793, #794, #805, #804). Chain 1 lives on separate branches that weren't in the merge source. Applied using `gh pr diff` for each PR in order (#764 → #765 → #769). init.ts required manual merge — #765 and #769 both modified it from the same base. Kept #765's safeParse checks inside #769's restructured initializeSettingsBlockProps. - #764 ENG-1280: Port query builder and editor component for block props - #765 ENG-1291: Port discourse node specification - #769 ENG-1290: Port discourse node config panel
|
https://www.loom.com/share/06fab631c187408eaa16ce0f18396f65 can't make in under 5 minutes long list |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
| if (changed) { | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| setBlockProps(globalBlockUid, { Relations: reconciledRelations }, false); |
There was a problem hiding this comment.
🔴 reconcileRelationKeys overwrites entire Global blockprops with only Relations
When reconcileRelationKeys detects placeholder keys in the Global blockprops that need replacing with real Roam block UIDs, it calls setBlockProps(globalBlockUid, { Relations: reconciledRelations }, false). The setBlockProps utility is a full replacement operation (not a merge), as confirmed by how setBlockPropAtPath in apps/roam/src/components/settings/utils/accessors.ts:193-214 reads existing props, mutates the path, then writes the whole object back via setBlockProps.
Root cause and impact
Because setBlockProps replaces the block's entire props payload, calling it with { Relations: reconciledRelations } erases every other Global setting stored on the same block — including Trigger, Canvas page format, Left sidebar, Export, and Suggestive mode.
This runs inside initializeSettingsBlockProps at startup (apps/roam/src/components/settings/utils/init.ts:96-106), specifically when the Global block exists and has placeholder relation keys (i.e., the first launch after a fresh install with default relations). After that call, only the Relations key survives.
Expected: Only the Relations sub-key should be updated, preserving all sibling keys.
Actual: The entire Global blockprops object is replaced with { Relations: reconciledRelations }, losing all other settings.
| setBlockProps(globalBlockUid, { Relations: reconciledRelations }, false); | |
| const existingGlobalProps = (getBlockProps(globalBlockUid) ?? {}) as Record<string, json>; | |
| setBlockProps(globalBlockUid, { ...existingGlobalProps, Relations: reconciledRelations }, false); |
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
No new code was written. This PR recovers 7
already-reviewed-and-approved PRs whose changes never landed on
main due to a Graphite stack merge race condition.
What happened
These PRs were part of a 22-PR Graphite stack. When Graphite
merged the stack bottom-up, the base PR (#696) was
squash-merged to main before the upstack PRs finished merging
into their parent branches. The upstack PRs ended
up squash-merged into now-dead parent branches instead of being
rebased onto main first. Graphite shows them
as "merged", but their changes are orphaned.
The 7 orphaned PRs formed two independent chains:
main
How this PR was created
Chain 2 (4 PRs): Identified the topmost orphaned branch
(
origin/eng-1217-section-components-in-left-sidebarv2) whichcontains all Chain 2 changes.
Ran
git merge --squashof that branch into a fresh branch offmain.
Resolved conflicts in NodeConfig.tsx (due to #784 which did
land on main) and zodSchema.example.ts
(Left sidebar type change from
{}to[]).Chain 1 (3 PRs): Chain 1 was on completely separate
branches not included in Chain 2's merge source
(Graphite had rebased Chain 2 onto main after #784 landed,
losing Chain 1).
Applied each PR's exact diff from GitHub in order using
gh pr diff:gh pr diff 764 | git applygh pr diff 765 | git applygh pr diff 769 | git apply --exclude='*/init.ts'(init.tsapplied manually — ENG-1291: Port discourse node specification #765 and ENG-1290: Port discourse node config panel #769
both modified it from the same base; merged both: ENG-1291: Port discourse node specification #765's
safeParse checks + ENG-1290: Port discourse node config panel #769's
reconcileRelationKeysrestructuring)All code was previously reviewed and approved in these PRs.
Testing
Personal Settings
Home
Queries
Left sidebar
Global Settings
Home
Export
Left sidebar
Grammar
Relations
Nodes
Nodes > [any node]
General
Canvas
Attributes
Index
Format > Specification
Suggestive rules
Admin (Ctrl+Shift+A)
Feature Flags
Suggestive mode