ENG-1716: Bug fix: Template does not render from block props#1016
Merged
sid597 merged 6 commits intoMay 12, 2026
Merged
Conversation
When the new settings store flag is on, materialize the block-props template into an ephemeral Template-Block-props block as the last child of node.type, render that, and delete it on unmount. Flag-off behavior is unchanged.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Contributor
📝 WalkthroughWalkthroughThe DualWriteBlocksPanel now conditionally routes settings edits through a temporary buffer block when the new settings store is enabled. A computed renderUid targets either the buffer (new store) or the original uid (legacy). Serialization, rendering, and pull-watch registration use renderUid. The buffer is created on mount (seeded from defaultValue or a space) and removed on cleanup. ChangesSettings Buffer Routing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx`:
- Around line 76-84: Pending debounced edits are being dropped because the
timeout is cleared when the buffer is torn down; update the component to flush
any pending change immediately before teardown: in the cleanup/unmount path (and
the other effect referenced around lines 120-123) check debounceRef.current and,
if present, clearTimeout(debounceRef.current) and synchronously run the
serialize-and-save logic (the body in handleChange that calls
getFullTreeByParentUid(renderUid), serializeBlockTree(...), and
setDiscourseNodeSetting(nodeType, settingKeys, serialized)); alternatively call
handleChange synchronously (bypassing DEBOUNCE_MS) during cleanup so the last
edit is persisted. Ensure you reference debounceRef, DEBOUNCE_MS, renderUid,
nodeType, settingKeys, getFullTreeByParentUid, serializeBlockTree, and
setDiscourseNodeSetting when making the change.
- Around line 56-74: The effect currently fire-and-forgets createBlock (newUid)
and immediately calls deleteBlock(newUid) in the cleanup, racing with creation;
store the createBlock promise (e.g., const creation = createBlock(...)), use
.then() to setBufferUid if not cancelled, and in the cleanup await
creation.finally(() => { setBufferUid(null); void deleteBlock(newUid); }) so
deletion only runs after creation completes; keep the cancelled flag logic to
prevent state updates but remove the immediate deleteBlock call from cleanup and
invoke it inside creation.finally instead.
- Around line 99-118: The continuation of the ensureChildren promise can run
after renderUid changes and corrupt pullWatchArgsRef; fix by adding an isActive
guard in the surrounding effect: declare let isActive = true at the start, then
before performing side-effects inside ensureChildren.then(...) check if
(!isActive) return; and in the effect cleanup set isActive = false; apply this
to the continuation that clears el.innerHTML, calls
window.roamAlphaAPI.ui.components.renderBlock({ uid: renderUid, el }), and calls
registerPullWatch() so stale renderUid and pull watch registrations are never
executed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 630976e5-8ce6-4589-a7d1-9c9407b97996
📒 Files selected for processing (1)
apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx
When the new settings store flag is on, edits in the buffer Template-Block-props block are mirrored to the legacy Template block via position-walked block.update calls. On length mismatch (only expected during testing/manual divergence) the mirror logs a warning and skips that subtree.
Mirror now creates legacy children when buffer has extras and deletes legacy children when buffer has fewer, in addition to in-place block.update for matching positions. Replaces the prior length-match guard which caused dual-write to silently skip whenever the user added or removed template lines.
The ensureChildren promise resolves asynchronously, so its .then callback could run after the effect's cleanup fired - re-registering a pull watch on a stale renderUid and stomping pullWatchArgsRef. Add a cancelled flag (same pattern as the buffer-lifecycle effect) so the continuation aborts after teardown.
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx (2)
101-119:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAwait buffer creation before deleting it.
The cleanup function calls
deleteBlock(newUid)synchronously, butcreateBlockis fire-and-forget (wrapped withvoid). If the effect unmounts beforecreateBlockresolves,deleteBlockwill attempt to delete a block that hasn't been created yet. Thecancelledguard only prevents state updates; it does not preventdeleteBlockexecution.Store the createBlock promise and use
.finally()to ensure deletion only occurs after creation completes.🐛 Proposed fix
useEffect(() => { if (!useNewStore || !nodeType) return; let cancelled = false; const newUid = window.roamAlphaAPI.util.generateUID(); const dv = defaultValueRef.current; const seed: InputTextNode[] = dv && dv.length > 0 ? dv : [{ text: " " }]; - void createBlock({ + const createPromise = createBlock({ node: { text: TEMPLATE_BUFFER_TEXT, uid: newUid, children: seed }, parentUid: nodeType, order: "last", - }).then(() => { + }); + void createPromise.then(() => { if (!cancelled) setBufferUid(newUid); }); return () => { cancelled = true; setBufferUid(null); - void deleteBlock(newUid); + void createPromise.finally(() => { + void deleteBlock(newUid); + }); }; }, [useNewStore, nodeType, uid]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx` around lines 101 - 119, The effect currently fires createBlock without awaiting it and calls deleteBlock(newUid) in the cleanup immediately, which can delete before creation finishes; capture the promise returned by createBlock inside useEffect (store it in a variable), use its then()/catch()/finally() to setBufferUid(newUid) only if not cancelled and to perform deleteBlock(newUid) in finally() (so deletion waits for creation to settle), and keep the cancelled flag to prevent setBufferUid from running when unmounted; update the cleanup to set cancelled = true and call the stored promise's finally-based deletion instead of calling deleteBlock synchronously.
121-133:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFlush pending edits before the buffer is torn down.
In new-store mode the source of truth is the ephemeral buffer. If the user types and closes the panel within
DEBOUNCE_MS, the cleanup at lines 171-178 cancels the pending serialize, and the buffer-deletion effect removes the buffer, so the last edit never reachessetDiscourseNodeSetting.Flush any pending serialization synchronously during cleanup.
💾 Proposed fix
const handleChange = useCallback(() => { if (!renderUid) return; window.clearTimeout(debounceRef.current); debounceRef.current = window.setTimeout(() => { const tree = getFullTreeByParentUid(renderUid); const serialized = serializeBlockTree(tree.children); setDiscourseNodeSetting(nodeType, settingKeys, serialized); if (useNewStore && renderUid !== uid) { const legacyTree = getFullTreeByParentUid(uid); mirrorBufferToLegacyChildren(tree.children, legacyTree.children, uid); } + debounceRef.current = 0; }, DEBOUNCE_MS); }, [renderUid, uid, useNewStore, nodeType, settingKeys]); @@ return () => { cancelled = true; - window.clearTimeout(debounceRef.current); + if (debounceRef.current && renderUid) { + window.clearTimeout(debounceRef.current); + const tree = getFullTreeByParentUid(renderUid); + const serialized = serializeBlockTree(tree.children); + setDiscourseNodeSetting(nodeType, settingKeys, serialized); + if (useNewStore && renderUid !== uid) { + const legacyTree = getFullTreeByParentUid(uid); + mirrorBufferToLegacyChildren(tree.children, legacyTree.children, uid); + } + debounceRef.current = 0; + } if (pullWatchArgsRef.current) { window.roamAlphaAPI.data.removePullWatch(...pullWatchArgsRef.current); pullWatchArgsRef.current = null; }Also applies to: 171-178
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx` around lines 121 - 133, The cleanup effect must flush any pending debounce work synchronously: if debounceRef.current is set, clear the timeout and immediately perform the same logic used in the handleChange timeout (check renderUid, call getFullTreeByParentUid(renderUid), serializeBlockTree(tree.children), call setDiscourseNodeSetting(nodeType, settingKeys, serialized) and, when useNewStore && renderUid !== uid, call mirrorBufferToLegacyChildren(tree.children, legacyTree.children, uid)). Implement this in the teardown so pending edits are applied before the ephemeral buffer is removed; reuse the same checks/variables (debounceRef, DEBOUNCE_MS, renderUid, uid, useNewStore, nodeType, settingKeys, getFullTreeByParentUid, serializeBlockTree, setDiscourseNodeSetting, mirrorBufferToLegacyChildren) to mirror the handleChange timeout callback synchronously.
🧹 Nitpick comments (2)
apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx (2)
47-51: 💤 Low valueUse named parameters for functions with more than 2 parameters.
mirrorBufferToLegacyChildrenhas 3 parameters but doesn't use object destructuring. As per coding guidelines, functions with more than 2 parameters should use named parameters (object destructuring) for clarity.♻️ Proposed refactor
-const mirrorBufferToLegacyChildren = ( - bufferChildren: TreeNode[], - legacyChildren: TreeNode[], - legacyParentUid: string, -): void => { +const mirrorBufferToLegacyChildren = ({ + bufferChildren, + legacyChildren, + legacyParentUid, +}: { + bufferChildren: TreeNode[]; + legacyChildren: TreeNode[]; + legacyParentUid: string; +}): void => {Then update the call sites at lines 64-68 and 130:
- mirrorBufferToLegacyChildren( - bufferNode.children, - legacyNode.children, - legacyNode.uid, - ); + mirrorBufferToLegacyChildren({ + bufferChildren: bufferNode.children, + legacyChildren: legacyNode.children, + legacyParentUid: legacyNode.uid, + });- mirrorBufferToLegacyChildren(tree.children, legacyTree.children, uid); + mirrorBufferToLegacyChildren({ + bufferChildren: tree.children, + legacyChildren: legacyTree.children, + legacyParentUid: uid, + });As per coding guidelines, "Use named parameters (object destructuring) when a function has more than 2 parameters."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx` around lines 47 - 51, Refactor mirrorBufferToLegacyChildren to accept a single object parameter using named properties { bufferChildren, legacyChildren, legacyParentUid } instead of three positional args; update the function signature and all call sites that invoke mirrorBufferToLegacyChildren to pass an object with those named keys (e.g., mirrorBufferToLegacyChildren({ bufferChildren, legacyChildren, legacyParentUid })), and ensure any TypeScript types/interfaces are adjusted (parameter typing and usages) so the function and callers compile.
24-24: 💤 Low valueAdd explicit return types to functions.
The functions
serializeBlockTree,treeNodeToInputTextNode, andDualWriteBlocksPanelare missing explicit return types. As per coding guidelines, "Use explicit return types for functions in TypeScript."📝 Proposed additions
-const serializeBlockTree = (children: TreeNode[]): RoamNodeType[] => +const serializeBlockTree = (children: TreeNode[]): RoamNodeType[] =>(Already has return type, no change needed)
-const treeNodeToInputTextNode = (node: TreeNode): InputTextNode => ({ +const treeNodeToInputTextNode = (node: TreeNode): InputTextNode => ({(Already has return type, no change needed)
const DualWriteBlocksPanel = ({ nodeType, settingKeys, title, description, uid, defaultValue, -}: DualWriteBlocksPanelProps) => { +}: DualWriteBlocksPanelProps): React.JSX.Element => {As per coding guidelines, "Use explicit return types for functions in TypeScript."
Also applies to: 36-36, 81-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx` at line 24, Add explicit return types to the three functions mentioned: give serializeBlockTree an explicit RoamNodeType[] return type, add an explicit return type to treeNodeToInputTextNode matching the file's input-node type (e.g., RoamInputTextNode or the appropriate alias used in this module), and annotate DualWriteBlocksPanel with a React component return type (e.g., React.FC<Props> or JSX.Element consistent with your other components). Locate the function signatures for serializeBlockTree, treeNodeToInputTextNode, and DualWriteBlocksPanel in this file and update their signatures to include these explicit return types (matching existing local/ exported type names) without changing implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx`:
- Line 119: The useEffect in the EphemeralBlocksPanel component currently
includes an unused uid in its dependency array, causing unnecessary re-runs;
remove uid from the dependency array of the effect (the useEffect that creates
the buffer and depends on useNewStore and nodeType) so it only depends on the
actual used values (useNewStore, nodeType), or if ESLint complains, replace uid
with a proper eslint-disable-next-line comment for that specific hook rule.
- Around line 47-79: mirrorBufferToLegacyChildren currently only updates
block.string when syncing buffer nodes to legacy nodes, so heading level and
open/collapsed state changes are not propagated; update the routine to include
heading and open in the update/create calls: when updating an existing legacy
node (inside the loop comparing bufferNode/legacyNode) include heading:
bufferNode.heading and open: bufferNode.open in the object passed to
window.roamAlphaAPI.data.block.update, and when creating new legacy nodes via
createBlock (using treeNodeToInputTextNode) ensure the created node includes
heading and open fields (or extend treeNodeToInputTextNode to serialize these
from the TreeNode/serializeBlockTree output) so both updates and new blocks
mirror heading and open properties.
---
Duplicate comments:
In `@apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx`:
- Around line 101-119: The effect currently fires createBlock without awaiting
it and calls deleteBlock(newUid) in the cleanup immediately, which can delete
before creation finishes; capture the promise returned by createBlock inside
useEffect (store it in a variable), use its then()/catch()/finally() to
setBufferUid(newUid) only if not cancelled and to perform deleteBlock(newUid) in
finally() (so deletion waits for creation to settle), and keep the cancelled
flag to prevent setBufferUid from running when unmounted; update the cleanup to
set cancelled = true and call the stored promise's finally-based deletion
instead of calling deleteBlock synchronously.
- Around line 121-133: The cleanup effect must flush any pending debounce work
synchronously: if debounceRef.current is set, clear the timeout and immediately
perform the same logic used in the handleChange timeout (check renderUid, call
getFullTreeByParentUid(renderUid), serializeBlockTree(tree.children), call
setDiscourseNodeSetting(nodeType, settingKeys, serialized) and, when useNewStore
&& renderUid !== uid, call mirrorBufferToLegacyChildren(tree.children,
legacyTree.children, uid)). Implement this in the teardown so pending edits are
applied before the ephemeral buffer is removed; reuse the same checks/variables
(debounceRef, DEBOUNCE_MS, renderUid, uid, useNewStore, nodeType, settingKeys,
getFullTreeByParentUid, serializeBlockTree, setDiscourseNodeSetting,
mirrorBufferToLegacyChildren) to mirror the handleChange timeout callback
synchronously.
---
Nitpick comments:
In `@apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx`:
- Around line 47-51: Refactor mirrorBufferToLegacyChildren to accept a single
object parameter using named properties { bufferChildren, legacyChildren,
legacyParentUid } instead of three positional args; update the function
signature and all call sites that invoke mirrorBufferToLegacyChildren to pass an
object with those named keys (e.g., mirrorBufferToLegacyChildren({
bufferChildren, legacyChildren, legacyParentUid })), and ensure any TypeScript
types/interfaces are adjusted (parameter typing and usages) so the function and
callers compile.
- Line 24: Add explicit return types to the three functions mentioned: give
serializeBlockTree an explicit RoamNodeType[] return type, add an explicit
return type to treeNodeToInputTextNode matching the file's input-node type
(e.g., RoamInputTextNode or the appropriate alias used in this module), and
annotate DualWriteBlocksPanel with a React component return type (e.g.,
React.FC<Props> or JSX.Element consistent with your other components). Locate
the function signatures for serializeBlockTree, treeNodeToInputTextNode, and
DualWriteBlocksPanel in this file and update their signatures to include these
explicit return types (matching existing local/ exported type names) without
changing implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 84b5556a-8cd5-4f15-a6c7-a4d381168848
📒 Files selected for processing (1)
apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx
The mirror only pushed block string to legacy, so a heading-level or collapse-state change in the buffer never propagated. Update the same block.update call to include heading and open whenever any of the three differ between buffer and legacy. Also drops uid from the buffer-creation effect's dependency array since the body no longer references it.
sid597
commented
May 11, 2026
| }), | ||
| })); | ||
|
|
||
| const treeNodeToInputTextNode = (node: TreeNode): InputTextNode => ({ |
Collaborator
Author
There was a problem hiding this comment.
This will be removed when we move from legacy, used to sync block prop template to block template
sid597
commented
May 11, 2026
| }), | ||
| }); | ||
|
|
||
| const mirrorBufferToLegacyChildren = ( |
Collaborator
Author
There was a problem hiding this comment.
This will be removed when we move from legacy, used to sync block prop template to block template
useNewStore reads like a React hook (use* convention) but is a boolean. Rename to isNewStore so the name matches the type.
mdroidian
approved these changes
May 12, 2026
mdroidian
left a comment
mdroidian
left a comment
Member
There was a problem hiding this comment.
The only concern I have is that we are adding these "node template only" to a generic "EphemeralBlocksPanel" component. So it is no longer a general "EphemeralBlocksPanel" component, but rather a specific one.
sid597
added a commit
that referenced
this pull request
May 15, 2026
* ENG-1465: Add "Use new settings store" feature flag (#811) * ENG-1454: Enable dual read feature flag * add caller * resolve merge conflicts * review * example fix * review * ENG-1455: Dual-read from old-system settings and from blockprops (#812) * ENG-1455: Dual-read from old-system settings and from blockprops * remove console.logs * address review * prettier * add comment describing why we do it like this * reviewed * rebase, review * address review * explicit throw, now zodschema default reads * replace throw with warn, throw was too strong, with warn we test if legacy and block props are same * ENG-1472: Refactor BlockPropSettingPanels to add accessor-backed default reads (#813) * ENG-1472: Refactor BlockPropSettingPanels to add accessor-backed default reads (with initialValue fallback) * review, fix * Eng-1473 port non boolean personal setting consumers (#823) * ENG-1473: Rebase onto updated eng-1472, resolve conflicts and fix missing import * ENG-1473: Review fixes — remove dead extensionAPI params, fix type casts * ENG-1473: Fix restrict-template-expressions warnings in query utils * rename persistQueryPages → setQueryPages, setQueryPages → setInitialQueryPages * restore legacy type coercion for query-pages * ENG-1467: Port global setting consumer reads (→ getGlobalSetting) (#824) * ENG-1467: Port global setting consumer reads (→ getGlobalSetting) * prettier * rebase onto updated eng-1472, resolve conflicts and fix tsc * ENG-1456: Migrate personal boolean flag consumers (getSetting → getPersonalSetting) (#843) * ENG-1479: Port suggestive mode settings to dual-read (#845) * ENG-1479: Port suggestive mode settings to dual-read Add dual-read routing to getFeatureFlag for flags with legacy counterparts. FEATURE_FLAG_LEGACY_MAP maps "Suggestive mode enabled" and "Enable left sidebar" to their config tree reads, gated by isNewSettingsStoreEnabled(). Flags without legacy entries go straight to block props (no behavior change). Migrate value reads from getFormattedConfigTree() / extensionAPI.settings to accessors: - index.ts: getUidAndBooleanSetting → getFeatureFlag (accessor handles legacy fallback now) - AdminPanel: suggestiveModeEnabled.value → getFeatureFlag in both FeatureFlagsTab and main component. Removed unused useMemo. - SuggestiveModeSettings: includePageRelations state init → getGlobalSetting. Dropped initialValue from both GlobalFlagPanels. - hyde.ts: orphan extensionAPI.settings.get() reads → getGlobalSetting. Fixes bug where sync config toggles had no runtime effect (keys were never written by any code). Structural UIDs remain with getFormattedConfigTree(). pageGroups.groups NOT migrated — type mismatch (legacy PageGroup has UIDs, Zod PageGroup does not). Deferred to ENG-1470. * Align hyde.ts fallback defaults with Zod schema * Fix eslint naming-convention warnings in FEATURE_FLAG_LEGACY_MAP * Add TODO(ENG-1484) for suggestive mode reactivity * ENG-1468: Port node and relations tree based consumers (#825) * Rebase ENG-1468 onto eng-1472 (fresh redo) * Remove unnecessary lazy initializer from complement useState * Break circular dep: inline DISCOURSE_CONFIG_PAGE_TITLE * Address review: case-insensitive attribute lookup, empty-string fallback, move DISCOURSE_CONFIG_PAGE_TITLE to data/constants * ENG-1478: Port left sidebar to dual-readers + reactivity * ENG-1478: DRY merge helpers, real dual-read in settings panels, emitter-based reactivity - Extract mergeGlobalSectionWithAccessor/mergePersonalSectionsWithAccessor to getLeftSidebarSettings.ts (used by LeftSidebarView, GlobalSettings, PersonalSettings) - Settings panels now actually use accessor values (not fire-and-forget) - Replace setLeftSidebarFlagHandler with settingsEmitter pub/sub - useConfig subscribes to emitter instead of old subscribe() - Wire up global/personal/flag handlers in pullWatchers to emit - Remove dead newValue===oldValue guard (hasPropChanged already checks) - Remove dead ?./?? in personal merge (Zod defaults guarantee values) * ENG-1478: refresh config tree before emitter-driven rebuild toggleFoldedState mutates folded.uid in-place then dual-writes to block props, triggering pull watch → emitter → buildConfig(). Without refreshConfigTree(), buildConfig() reads stale UIDs from the cached tree, overwriting the in-place mutation and orphaning Roam blocks. * ENG-1478: extract emitter key constants, fix stale TODO * ENG-1478: fix legacy adapter child.uid → child.text, fix settingKeys on personal panels * ENG-1478: skip reload prompt when new settings store is active * ENG-1478: guard getGlobalSetting/getPersonalSetting against empty keys * ENG-1469: Refactor getDiscourseRelations and getDiscourseNodes to read from block props * Fix: use backedBy 'user' for initial discourse nodes in block props initSingleDiscourseNode was writing backedBy: "default" which caused nodes to be filtered out of the settings panel when the flag is ON. Also fixes up existing graphs that already stored the wrong value. * Hardcode backedBy 'user' in toDiscourseNode to match legacy behavior * ENG-1484: reactive settings for triggers and suggestive mode overlay * ENG-1484: use nullish coalescing for trigger fallbacks to match init semantics * ENG-1484: reactive suggestive mode flag, gate reload prompts on new settings store * ENG-1484: broad selector for suggestive overlay cleanup to match legacy * ENG-1484: fix prettier formatting (root config) * ENG-1484: drop reload prompts — reactivity is unconditional via dual-write * ENG-1499: Replace raw string accessor paths with shared constants from settingKeys.ts Replaces all raw string literals used as block-prop accessor paths in consumer files with typed constants from settingKeys.ts. Adds TEMPLATE_SETTING_KEYS constant and tightens KeyboardShortcutInput blockPropKey prop from string to keyof PersonalSettings. * Fix missed keyImageOption getter to use shared constants * ENG-1519: Add legacy-to-blockprops migration, remove backedBy from schema * Fix shouldWrite skipping after reconciliation, warn on missing personal block * Add migration telemetry for legacy parse failures * Use backend query for node migration * ENG-735: Remove inline settings UI from config pages * Use Object.values instead of unused destructured key * ENG-1503: Replace getFormattedConfigTree consumers with direct helper calls * ENG-1484: extract constants, remove dead defaults per review * Fix useConfig to call buildConfig() for dual-read merge * prettier * Fix import path for DISCOURSE_CONFIG_PAGE_TITLE * fix review * ENG-1470: Fix dual-read gaps found during flag-ON validation (#896) * ENG-1470: Fix dual-read gaps found during flag-ON validation Bootstrap legacy config blocks in initSchema so the plugin works on fresh graphs without createConfigObserver/configPageTabs: - trigger, grammar/relations, export, Suggestive Mode, Left Sidebar - Reuses existing ensureBlocksExist/buildBlockMap helpers + DEFAULT_RELATION_VALUES Fix duplicate block accumulation bugs: - Page Groups: getSubTree auto-create race (ensureLegacyConfigBlocks pre-creates) - Folded: lookup-based delete via getBasicTreeByParentUid instead of stale uid - scratch/enabled: switched getSubTree({ parentUid }) to tree-based reads - Folded in convertToComplexSection: removed erroneous block creation Fix dual-read comparison: - Replace JSON.stringify with deepEqual (handles key order, undefined/empty/false) - Deterministic async ordering: await legacy write → refreshConfigTree → blockProp write (BlockPropSettingPanels, LeftSidebarView toggleFoldedState, AdminPanel suggestive mode) - Use getPersonalSettings() (raw read) in toggleFoldedState to avoid mid-write comparison Fix storedRelations import path (renderNodeConfigPage → data/constants) * Fix dual-read mismatches and ZodError on discourse node parse * Fix dual-read mismatches: alias timing, key-image re-render, deepEqual null * Fix prettier formatting * ENG-1574: Add dual-read console logs to setting setters (#914) * ENG-1574: Add dual-read console logs to setting setters Log legacy and block prop values with match/mismatch status when a setting is changed. Fix broken import in storedRelations. * ENG-1574: Add init-time dual-read log and window.dgDualReadLog() Log all legacy vs block prop settings on init. Remove setter logging. Expose dgDualReadLog() on window for on-demand use. * ENG-1574: Fix eslint naming-convention warnings in init.ts * ENG-1574: Use deepEqual instead of JSON.stringify for comparison JSON.stringify is key-order dependent, causing false mismatches when legacy and block props return keys in different order. * ENG-1574: Remove dead code, use deepEqual for comparison * ENG-1574: Fix review feedback — try-catch, flag exclusion, type guard * Eng 1616 add getconfigtree equivalent for block pros on init (#944) * ENG-1616: Bulk-read settings + thread snapshot (with timing logs) Cut plugin load from ~20925ms to ~1327ms (94%) on a real graph by collapsing per-call settings accessors into a single bulk read at init and threading that snapshot through the init chain + observer callbacks. Key changes: - accessors.ts: bulkReadSettings() runs ONE pull query against the settings page's direct children and returns { featureFlags, globalSettings, personalSettings } parsed via Zod. readPathValue exported. - getDiscourseNodes / getDiscourseRelations / getAllRelations: optional snapshot param threaded through, no breaking changes to existing callers. - initializeDiscourseNodes + refreshConfigTree (+ registerDiscourseDatalog- Translators, getDiscourseRelationLabels): accept and forward snapshot. - index.ts: bulkReadSettings() at the top of init; snapshot threaded into initializeDiscourseNodes, refreshConfigTree, initObservers, installDiscourseFloatingMenu, setInitialQueryPages, and the 3 sync sites inside index.ts itself. - initializeObserversAndListeners.ts: snapshot threaded into the sync-init body; pageTitleObserver + leftSidebarObserver callbacks call bulkReadSettings() per fire (fresh, not stale); nodeTagPopupButtonObserver uses per-sync-batch memoization via queueMicrotask; hashChangeListener and nodeCreationPopoverListener use bulkReadSettings() per fire. - findDiscourseNode: snapshot param added; getDiscourseNodes() default-arg moved inside the cache-miss branch so cache hits don't waste the call. - isQueryPage / isCanvasPage / QueryPagesPanel.getQueryPages: optional snapshot param. - LeftSidebarView.buildConfig / useConfig / mountLeftSidebar: optional initialSnapshot threaded for the first render; emitter-driven updates keep using live reads for post-mount reactivity. - DiscourseFloatingMenu.installDiscourseFloatingMenu: optional snapshot. - posthog.initPostHog: removed redundant internal getPersonalSetting check (caller already guards from the snapshot). - migrateLegacyToBlockProps.hasGraphMigrationMarker: accepts the existing blockMap and does an O(1) lookup instead of a getBlockUidByTextOnPage scan. Includes per-phase timing console.logs across index.ts, refreshConfigTree, init.ts, initSettingsPageBlocks, and initObservers. Committed as a checkpoint so we can reference measurements later; will be removed in the next commit. * ENG-1616: Remove plugin-load timing logs Removes the per-phase console.log instrumentation added in the previous commit. All the [DG Plugin] / [DG Nav] logs and their `mark()` / `markPhase()` helpers are gone. Code behavior unchanged. Dropped in this commit: - index.ts: mark() closure, load start/done logs, and all phase marks. - initializeObserversAndListeners.ts: markPhase() closure, per-observer marks, pageTitleObserver fire log, hashChangeListener [DG Nav] logs. - LeftSidebarView.tsx: openTarget [DG Nav] click/resolve logs. - refreshConfigTree.ts: mark() closure and all phase marks. - init.ts: mark() closures in initSchema and initSettingsPageBlocks. - accessors.ts: bulkReadSettings internal timing log. - index.ts: unused getPluginElapsedTime import. Previous commit (343dc11) kept as a checkpoint for future drill-downs. * ENG-1616: Address review — typed indexing, restore dgDualReadLog, optional snapshot - index.ts: move initPluginTimer() back to its original position (after early-return checks) so timing isn't started for graphs that bail out. - Replace readPathValue + `as T | undefined` casts with direct typed indexing on the Zod-derived snapshot types across: - index.ts (disallowDiagnostics, isStreamlineStylingEnabled) - initializeObserversAndListeners.ts (suggestiveModeOverlay, pagePreview, discourseContextOverlay, globalTrigger, personalTriggerCombo, customTrigger) — also drops dead `?? "\\"` and `?? "@"` fallbacks since Zod defaults already populate them. - isCanvasPage.ts (canvasPageFormat) - setQueryPages.ts + QueryPagesPanel.tsx (nested [Query][Query pages]) - setQueryPages.setInitialQueryPages: snapshot is now optional with a getPersonalSetting fallback, matching the pattern used elsewhere (getQueryPages, isCanvasPage, etc.). - init.ts: restore logDualReadComparison + window.dgDualReadLog so the on-demand console helper is available again. NOT auto-called on init — invoke window.dgDualReadLog() manually to dump the comparison. * ENG-1616: Log total plugin load time Capture performance.now() at the top of runExtension and log the elapsed milliseconds just before the unload handler is wired, so we have a single broad measurement of plugin init cost on each load. * ENG-1616: Tighten init-only leaves to required snapshot, AGENTS.md compliance Make snapshot required at six init-only leaves where caller audit showed every site already passed one: installDiscourseFloatingMenu, initializeDiscourseNodes, setInitialQueryPages, isQueryPage, isCurrentPageCanvas, isSidebarCanvas. No cascade — only at the leaves. Drop dead fallback code that was reachable only via the optional path: - setQueryPages: legacy string|Record coercion ladder (snapshot is Zod-typed string[]) - DiscourseFloatingMenu: getPersonalSetting<boolean> cast site - DiscourseFloatingMenu: unused props parameter (no caller ever overrode default) - initializeObserversAndListeners: !== false dead pattern (Zod boolean default) - initializeObserversAndListeners: as IKeyCombo cast (schema is structurally compatible) AGENTS.md compliance for >2-arg functions: - mountLeftSidebar: object-destructured params, both call sites updated - installDiscourseFloatingMenu: kept at 2 positional via dead-props removal posthog: collapse doInitPostHog wrapper into initPostHog (caller-side gating). accessors: revert speculative readPathValue export (no consumer). LeftSidebarView/DiscourseFloatingMenu: eslint-disable react/no-deprecated on ReactDOM.render rewritten lines, matching existing codebase convention. * ENG-1616: Address review — rename snapshot vars, flag-gate bulkRead, move PostHog guards - Rename settingsSnapshot/callbackSnapshot/snap/navSnapshot → settings - bulkReadSettings now checks "Use new settings store" flag and falls back to legacy reads when off, matching individual getter behavior - Move encryption/offline guards into initPostHog (diagnostics check stays at call site to avoid race with async setSetting in enablePostHog) * Fix legacy bulk settings fallback * ENG-1617: se existing 'getConfigTree equivalent functions' for specific setting groups (#946) * ENG-1616: Bulk-read settings + thread snapshot (with timing logs) Cut plugin load from ~20925ms to ~1327ms (94%) on a real graph by collapsing per-call settings accessors into a single bulk read at init and threading that snapshot through the init chain + observer callbacks. Key changes: - accessors.ts: bulkReadSettings() runs ONE pull query against the settings page's direct children and returns { featureFlags, globalSettings, personalSettings } parsed via Zod. readPathValue exported. - getDiscourseNodes / getDiscourseRelations / getAllRelations: optional snapshot param threaded through, no breaking changes to existing callers. - initializeDiscourseNodes + refreshConfigTree (+ registerDiscourseDatalog- Translators, getDiscourseRelationLabels): accept and forward snapshot. - index.ts: bulkReadSettings() at the top of init; snapshot threaded into initializeDiscourseNodes, refreshConfigTree, initObservers, installDiscourseFloatingMenu, setInitialQueryPages, and the 3 sync sites inside index.ts itself. - initializeObserversAndListeners.ts: snapshot threaded into the sync-init body; pageTitleObserver + leftSidebarObserver callbacks call bulkReadSettings() per fire (fresh, not stale); nodeTagPopupButtonObserver uses per-sync-batch memoization via queueMicrotask; hashChangeListener and nodeCreationPopoverListener use bulkReadSettings() per fire. - findDiscourseNode: snapshot param added; getDiscourseNodes() default-arg moved inside the cache-miss branch so cache hits don't waste the call. - isQueryPage / isCanvasPage / QueryPagesPanel.getQueryPages: optional snapshot param. - LeftSidebarView.buildConfig / useConfig / mountLeftSidebar: optional initialSnapshot threaded for the first render; emitter-driven updates keep using live reads for post-mount reactivity. - DiscourseFloatingMenu.installDiscourseFloatingMenu: optional snapshot. - posthog.initPostHog: removed redundant internal getPersonalSetting check (caller already guards from the snapshot). - migrateLegacyToBlockProps.hasGraphMigrationMarker: accepts the existing blockMap and does an O(1) lookup instead of a getBlockUidByTextOnPage scan. Includes per-phase timing console.logs across index.ts, refreshConfigTree, init.ts, initSettingsPageBlocks, and initObservers. Committed as a checkpoint so we can reference measurements later; will be removed in the next commit. * ENG-1616: Remove plugin-load timing logs Removes the per-phase console.log instrumentation added in the previous commit. All the [DG Plugin] / [DG Nav] logs and their `mark()` / `markPhase()` helpers are gone. Code behavior unchanged. Dropped in this commit: - index.ts: mark() closure, load start/done logs, and all phase marks. - initializeObserversAndListeners.ts: markPhase() closure, per-observer marks, pageTitleObserver fire log, hashChangeListener [DG Nav] logs. - LeftSidebarView.tsx: openTarget [DG Nav] click/resolve logs. - refreshConfigTree.ts: mark() closure and all phase marks. - init.ts: mark() closures in initSchema and initSettingsPageBlocks. - accessors.ts: bulkReadSettings internal timing log. - index.ts: unused getPluginElapsedTime import. Previous commit (343dc11) kept as a checkpoint for future drill-downs. * ENG-1616: Address review — typed indexing, restore dgDualReadLog, optional snapshot - index.ts: move initPluginTimer() back to its original position (after early-return checks) so timing isn't started for graphs that bail out. - Replace readPathValue + `as T | undefined` casts with direct typed indexing on the Zod-derived snapshot types across: - index.ts (disallowDiagnostics, isStreamlineStylingEnabled) - initializeObserversAndListeners.ts (suggestiveModeOverlay, pagePreview, discourseContextOverlay, globalTrigger, personalTriggerCombo, customTrigger) — also drops dead `?? "\\"` and `?? "@"` fallbacks since Zod defaults already populate them. - isCanvasPage.ts (canvasPageFormat) - setQueryPages.ts + QueryPagesPanel.tsx (nested [Query][Query pages]) - setQueryPages.setInitialQueryPages: snapshot is now optional with a getPersonalSetting fallback, matching the pattern used elsewhere (getQueryPages, isCanvasPage, etc.). - init.ts: restore logDualReadComparison + window.dgDualReadLog so the on-demand console helper is available again. NOT auto-called on init — invoke window.dgDualReadLog() manually to dump the comparison. * ENG-1616: Log total plugin load time Capture performance.now() at the top of runExtension and log the elapsed milliseconds just before the unload handler is wired, so we have a single broad measurement of plugin init cost on each load. * ENG-1616: Tighten init-only leaves to required snapshot, AGENTS.md compliance Make snapshot required at six init-only leaves where caller audit showed every site already passed one: installDiscourseFloatingMenu, initializeDiscourseNodes, setInitialQueryPages, isQueryPage, isCurrentPageCanvas, isSidebarCanvas. No cascade — only at the leaves. Drop dead fallback code that was reachable only via the optional path: - setQueryPages: legacy string|Record coercion ladder (snapshot is Zod-typed string[]) - DiscourseFloatingMenu: getPersonalSetting<boolean> cast site - DiscourseFloatingMenu: unused props parameter (no caller ever overrode default) - initializeObserversAndListeners: !== false dead pattern (Zod boolean default) - initializeObserversAndListeners: as IKeyCombo cast (schema is structurally compatible) AGENTS.md compliance for >2-arg functions: - mountLeftSidebar: object-destructured params, both call sites updated - installDiscourseFloatingMenu: kept at 2 positional via dead-props removal posthog: collapse doInitPostHog wrapper into initPostHog (caller-side gating). accessors: revert speculative readPathValue export (no consumer). LeftSidebarView/DiscourseFloatingMenu: eslint-disable react/no-deprecated on ReactDOM.render rewritten lines, matching existing codebase convention. * ENG-1617: Single-pull settings reads + dialog snapshot threading `getBlockPropBasedSettings` now does one Roam `pull` that returns the settings page's children with their string + uid + props in one shot, replacing the `q`-based `getBlockUidByTextOnPage` (~290ms per call) plus a second `pull` for props. `setBlockPropBasedSettings` reuses the same helper for the uid lookup so we have a single pull-and-walk pattern. `SettingsDialog` captures a full settings snapshot once at mount via `useState(() => bulkReadSettings())` and threads `featureFlags` and `personalSettings` down to `HomePersonalSettings`. Each child component (`PersonalFlagPanel`, `NodeMenuTriggerComponent`, `NodeSearchMenuTriggerSetting`, `KeyboardShortcutInput`) accepts an `initialValue` prop and seeds its local state from the snapshot instead of calling `getPersonalSetting` on mount. `PersonalFlagPanel`'s `initialValue` precedence flips so the prop wins when provided; `QuerySettings` callers without a prop still hit the existing fallback. `getDiscourseNodes`, `getDiscourseRelations`, and `getAllRelations` narrow their snapshot parameter to `Pick<SettingsSnapshot, ...>` to declare which fields each function actually reads. Adds a one-line `console.log` in `SettingsDialog` reporting the dialog open time, kept as an ongoing perf monitor. * ENG-1617: Refresh snapshot on Home tab nav + reuse readPathValue CodeRabbit catch: with `renderActiveTabPanelOnly={true}`, the Home tab's panel unmounts/remounts when the user navigates away and back. Each re-mount re-runs `useState(() => initialValue ?? false)` in `BaseFlagPanel`, re-seeding from whatever `initialValue` is currently passed. Because the dialog held the snapshot in a non-updating `useState`, that path served stale values, so toggles made earlier in the same dialog session would visually revert after a tab round-trip. Fix: hold the snapshot in a stateful slot and refresh it via `bulkReadSettings()` from the Tabs `onChange` handler when the user navigates back to Home. The setState batches with `setActiveTabId`, so the new mount sees the fresh snapshot in the same render. Also replace the inline reducer in `getBlockPropBasedSettings` with the existing `readPathValue` util — same traversal but consistent with the rest of the file and adds array-index handling for free. * ENG-1617: Per-tab snapshot threading via bulkReadSettings Replaces the dialog-level snapshot from earlier commits with a per-tab snapshot model that scales to every tab without per-tab plumbing in the dialog itself. In accessors.ts, the three plural getters (getFeatureFlags, getGlobalSettings, getPersonalSettings) now delegate to the existing bulkReadSettings, which does one Roam pull on the settings page and parses all three schemas in a single pass. The slow q-based getBlockPropBasedSettings is deleted (it was only used by the plural getters and the set path); setBlockPropBasedSettings goes back to calling getBlockUidByTextOnPage directly. Writes are infrequent enough that the q cost is acceptable on the set path. Each tab container that renders panels at mount captures one snapshot via useState(() => bulkReadSettings()) and threads the relevant slice as initialValue down to its panels: HomePersonalSettings, QuerySettings, GeneralSettings, ExportSettings. The Personal and Global panels in BlockPropSettingPanels.tsx flip their initialValue precedence to prefer the prop and fall back to the live read only when the prop is missing, so callers that don't pass initialValue (e.g. LeftSidebarGlobalSettings, which already passes its own value) continue to behave the same way. NodeMenuTriggerComponent, NodeSearchMenuTriggerSetting, and KeyboardShortcutInput accept an initialValue prop and seed local state from it instead of calling getPersonalSetting in their useState initializer. Settings.tsx wraps getDiscourseNodes() in useState so it doesn't re-run on every dialog re-render. The dialog-level snapshot, the snapshot-refresh-on-Home-tab-nav workaround, and the Pick<SettingsSnapshot, ...> type narrowings are all gone. * ENG-1617: Lift settings snapshot to SettingsDialog, thread to all tabs Move bulkReadSettings() from per-tab useState into a single call at SettingsDialog mount. Each tab receives its snapshot slice (globalSettings, personalSettings, featureFlags) as props. Remove dual-read mismatch console.warn logic from accessors. Make initialValue caller-provided in BlockPropSettingPanel wrappers. Add TabTiming wrapper for per-tab commit/paint perf measurement. * ENG-1617: Remove timing instrumentation, per-call dual-read, flag-aware bulkReadSettings - Remove TabTiming component and all console.log timing from Settings dialog - Remove per-call dual-read comparison from getGlobalSetting, getPersonalSetting, getDiscourseNodeSetting (keep logDualReadComparison for manual use) - Make bulkReadSettings flag-aware: reads from legacy when flag is OFF, block props when ON - Remove accessor fallbacks from Global/Personal wrapper panels (initialValue now always passed from snapshot) - Remove getGlobalSetting/getPersonalSetting imports from BlockPropSettingPanels * ENG-1617: Eliminate double bulkReadSettings calls in accessor functions getGlobalSetting, getPersonalSetting, getFeatureFlag, getDiscourseNodeSetting now each do a single bulkReadSettings() call instead of calling isNewSettingsStoreEnabled() (which triggered a separate bulkReadSettings) followed by another bulkReadSettings via the getter. bulkReadSettings already handles the flag check and legacy/block-props routing internally. * ENG-1617: Re-read snapshot on tab change to prevent stale initialValues Replace useState with useMemo keyed on activeTabId so bulkReadSettings() runs fresh each time the user switches tabs. Fixes stale snapshot when renderActiveTabPanelOnly unmounts/remounts panels. * ENG-1616: Address review — rename snapshot vars, flag-gate bulkRead, move PostHog guards - Rename settingsSnapshot/callbackSnapshot/snap/navSnapshot → settings - bulkReadSettings now checks "Use new settings store" flag and falls back to legacy reads when off, matching individual getter behavior - Move encryption/offline guards into initPostHog (diagnostics check stays at call site to avoid race with async setSetting in enablePostHog) * ENG-1617: Fix DiscourseNodeSelectPanel fallback to use first option instead of empty string * ENG-1617: Rename snapshot variables to settings for clarity * Fix legacy bulk settings fallback * fix bug similar code * Fix stale startup snapshot: populate config tree before bulkReadSettings Without a prior refreshConfigTree() call, bulkReadSettings() reads an empty discourseConfigRef.tree when the flag is OFF, returning legacy defaults instead of user data. Populate the tree first so the initial snapshot reflects actual settings. * Fix toggleFoldedState race on rapid chevron clicks Move setIsOpen to the top of the function so the optimistic UI update lands synchronously. Add an isTogglingRef guard in PersonalSectionItem and GlobalSection so concurrent clicks while Roam writes are in flight are dropped instead of firing duplicate createBlock/deleteBlock on the same Folded node. * Keep Query pages schema default at discourse-graph/queries/* Previous commit accidentally included an older change that set "Query pages" default to []. Pre-migration, setQueryPages auto-seeds this path on every plugin load, so the effective user-facing default has always been ["discourse-graph/queries/*"]. Restore that value. * Include Reified relation triples in legacy personal settings Add reifiedRelationTriples to PERSONAL_KEYS and map "Reified relation triples" -> "use-reified-relations" in PERSONAL_SCHEMA_PATH_TO_LEGACY_KEY. Without these entries, readAllLegacyPersonalSettings iterates over PERSONAL_KEYS values and omits this field, so the migration to block props fills in the Zod default (false) instead of the user's stored preference. * Close panel races between syncToBlock and refreshConfigTree BaseTextPanel: track the inner setTimeout via debounceRef so unmount cleanup cancels it instead of letting it fire on an unmounted component. BaseNumberPanel and BaseSelectPanel: add the same 100ms ordering gap between the non-awaited syncToBlock write and refreshConfigTree that BaseFlagPanel and BaseTextPanel already have, tracked via refreshTimeoutRef for cleanup. Without the gap, refreshConfigTree can read the tree before the async Roam write settles. * Report per-key mismatches in dgDualReadLog; fix legacy returnNode read - Iterate top-level keys per group (Personal, Global, Feature Flags, each node) and log which specific keys mismatch, with legacy vs block values side-by-side. Previously only group-level match/mismatch was reported. - Skip template, specification, and index in node comparisons — their legacy and block-prop shapes diverge structurally (uid + empty children on templates; condition uids stripped on save) not semantically. Inline comment cites the responsible readers/writers. - Fix getLegacyQuerySettingByParentUid returning a hardcoded "node" for returnNode instead of reading scratch > return > [value] from the tree. * ENG-1664: Replace getBlockUidByTextOnPage scan with pull in settings writer (#975) The Datalog .q scan that resolved the top-level settings block on every write took ~318 ms per call on real graphs. Replaced with a hash-indexed roamAlphaAPI.pull by page title that walks :block/children — same pattern bulkReadSettings already uses. Behavior equivalence: the old query matched any descendant via :block/page; the new pull traverses direct :block/children only. Safe here because setBlockPropBasedSettings is only reached with keys[0] set to one of the three top-level settings keys (feature flags, global, personal user-id), all guaranteed to be direct children of the settings page. Measured on a real graph: BaseFlagPanel click handler 420 ms → 97 ms (−77%). The remaining 95 ms is refreshConfigTree > register translators, the inherent dual-read cost ENG-1616 flagged for post-ENG-1470 cleanup. * ENG-1659: Use typed canvasSettings.color keys on color picker writes (#980) * ENG-1663 Await scratch/conditions/return creation before writing children (#982) Legacy tree init was calling getSubTree with parentUid, which fires createBlock without awaiting. Subsequent createBlock calls under those uids raced Roam's async write and failed with 'Parent entity doesn't exist'. Replace the three fire-and-forget lookups with tree-mode getSubTree + awaited createBlock fallback. * ENG-1671 Default Reified relation triples to true for block props (#1004) * ENG-1668: Perf: slow load time (#984) * ENG-1668: Remove dead initDiscourseNodePages + skip redundant refreshConfigTree Remove initDiscourseNodePages (and helpers hasNonDefaultNodes, initSingleDiscourseNode) from initSchema — migrateDiscourseNodes already handles block prop writes, making this redundant on every load. Make the second refreshConfigTree(settings) conditional — only runs when initializeDiscourseNodes actually creates pages (first install). On existing graphs the tree is identical to what the first call already read. * Add per-step load timing logs to index.ts * Add per-step load timing logs to initObservers * Split listeners + queryBuilder timing logs * Add detailed timing logs to nodeTagPopupButtonObserver callback * Add detailed timing logs to all observer callbacks * Thread config tree through legacy getters to avoid redundant fetches * Remove timing instrumentation from index.ts and initObservers * Add total plugin load time log * Eng 1672: Settings not reading from block props (#983) * ENG-1672: Flip left sidebar merge helpers to iterate snapshot * ENG-1672: Route getExportSettings runtime reads through bulkReadSettings * ENG-1672: Restore defaultValue hydration for Template and Attributes panels PR #784 dropped the defaultValue prop when porting BlocksPanel to DualWriteBlocksPanel, breaking block-props reads when legacy is empty. Restore the hydration path and add flag-gated dev nuke command. * ENG-1672: Fix review findings — children:[] not undefined, merge attributes with tree UIDs * ENG-1672: Route createDiscourseNode template reads through getDiscourseNodeSetting * fix: render left sidebar settings from selected store * ENG-1672: Preserve legacy sidebar config when accessor snapshot is undefined * Revert "ENG-1672: Preserve legacy sidebar config when accessor snapshot is undefined" This reverts commit 3b3f197. * ENG-1713: Relations do not show up in settings panel (#1010) * ENG-1713: Read Relations panel from globalSettings, fix migrate clobbering reconcile DiscourseRelationConfigPanel.refreshRelations now derives the list from globalSettings.Relations (flag-aware via bulkReadSettings) instead of querying the legacy tree directly. This removes the panel's dependency on the cached relations parent uid, which is "" on first open of a fresh graph until the dialog is closed and reopened. initSchema now refreshes discourseConfigRef.tree between initSettingsPageBlocks and migrateGraphLevel when the cache hasn't caught up to the seeded grammar block. Without it, getLegacyGlobalSetting reads the stale cache, returns schema-default placeholder-keyed Relations (_INFO-rel, etc.), and migrateSection overwrites the real-uid keys reconcileRelationKeys just wrote — leaving block props keyed by placeholders that the legacy edit panel cannot resolve. Cold path adds ~6ms once per graph; hot path skips via .some() check. * ENG-1713: Defer back() 50ms after relation save so list reflects write setGlobalSetting fires window.roamAlphaAPI.data.block.update without awaiting the returned promise. The previous code called back() in the same tick, so handleBack's refreshRelations ran a sync pull before Roam's index reflected the write — the list rendered pre-edit data on add / edit / delete from inside RelationEditPanel. setTimeout(back, 50) gives Roam a tick to commit before the read. This is a band-aid: the proper fix is either to await the write chain (plumb async through setBlockProps -> setGlobalSetting) or refresh via a pull watcher / saved-data callback. Tracked separately for cleanup. Also drops the explanatory comment above the conditional refreshConfigTree in initSchema; the why is captured on ENG-1471 instead. * ENG-1717: Default max filename length on invalid stored value (#1008) * Merge origin/main into migration-block-init-staging-branch Single conflict: apps/roam/src/components/LeftSidebarView.tsx — HEAD's dual-read plumbing (initialSnapshot, sectionIndex, handleToggle) unioned with main's drag-and-drop (SortableList, dragHandle, reorderGlobalChildren). SectionChildren removed (dead after main's ChildRow/SortableList path). Known followup: DnD reorder + fold toggle can race on stale section index when toggle fires before reorder write settles. To be tracked separately. * ENG-1716: Bug fix: Template does not render from block props (#1016) * ENG-1716: Render template settings from block props When the new settings store flag is on, materialize the block-props template into an ephemeral Template-Block-props block as the last child of node.type, render that, and delete it on unmount. Flag-off behavior is unchanged. * ENG-1716: Dual-write template edits back to legacy Template block When the new settings store flag is on, edits in the buffer Template-Block-props block are mirrored to the legacy Template block via position-walked block.update calls. On length mismatch (only expected during testing/manual divergence) the mirror logs a warning and skips that subtree. * ENG-1716: Extend buffer->legacy mirror to handle add/delete Mirror now creates legacy children when buffer has extras and deletes legacy children when buffer has fewer, in addition to in-place block.update for matching positions. Replaces the prior length-match guard which caused dual-write to silently skip whenever the user added or removed template lines. * ENG-1716: Guard render effect against stale async continuation The ensureChildren promise resolves asynchronously, so its .then callback could run after the effect's cleanup fired - re-registering a pull watch on a stale renderUid and stomping pullWatchArgsRef. Add a cancelled flag (same pattern as the buffer-lifecycle effect) so the continuation aborts after teardown. * ENG-1716: Mirror heading and open alongside string The mirror only pushed block string to legacy, so a heading-level or collapse-state change in the buffer never propagated. Update the same block.update call to include heading and open whenever any of the three differ between buffer and legacy. Also drops uid from the buffer-creation effect's dependency array since the body no longer references it. * ENG-1716: Rename useNewStore -> isNewStore useNewStore reads like a React hook (use* convention) but is a boolean. Rename to isNewStore so the name matches the type. * ENG-1755: Normalize legacy query-pages snapshot before array operations (#1034) setInitialQueryPages reads the personal query-pages setting from the snapshot and immediately calls .includes / spreads it. With the new settings store flag OFF, bulkReadSettings returns the raw legacy value cast as PersonalSettings, but the legacy extensionAPI shape is string | string[] | Record<string, string>. That throws on objects and spreads strings per-character. Route through the existing getQueryPages helper (already used by isQueryPage, listActiveQueries, resolveQueryBuilderRef), which coerces the legacy shapes to string[] before the append logic runs. * ENG-1757: Read fresh relation settings on edit to prevent duplicate data loss (#1035) * ENG-1757: Read fresh relation settings in editor to prevent duplicate-edit data loss RelationEditPanel was reading the relation from the globalSettings prop, which is a snapshot taken when the Relations tab was opened. After handleDuplicate writes a new relation via setGlobalSetting, the parent's snapshot is not refreshed. Opening the editor on the duplicated row found no entry, initialized source/destination/complement to blanks, and saving overwrote the relation with empty endpoints. Switch the lookup to getGlobalSettings().Relations[uid] so the editor always reads current state. The prop is now unused in both panels and is removed along with the Settings.tsx pass-through. * ENG-1757 Refresh relation state after relation mutations * ENG-1756: Preserve existing feature flag values during legacy migration (#1036) * ENG-1756: Preserve existing feature flag block-prop values during legacy migration readAllLegacyFeatureFlags only sources Enable left sidebar from legacy config. The migration wrote the full FeatureFlags object (with Zod defaults filling the unsourced keys as false) via setBlockProps, which clobbered any pre-existing true values for Duplicate node alert enabled and Suggestive mode overlay enabled. Expose LEGACY_SOURCED_FEATURE_FLAG_KEYS to identify which keys the legacy reader actually sources. The migration now overlays only those keys onto the existing block-prop values, leaving everything else untouched. * ENG-1756: Remove type assertion on LEGACY_SOURCED_FEATURE_FLAG_KEYS Declare the keys as a const tuple and derive the map's key type from it, so the array's type accurately describes its contents without an assertion. * remove nuke command (#1039) * remove unused
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://www.loom.com/share/13a3d3d9428f4890ac7c5b558ce7bd22
Summary by CodeRabbit