ENG-1225: Discourse node migration simple settings by sid597 · Pull Request #696 · DiscourseGraphs/discourse-graph

sid597 · 2026-01-14T09:00:52Z

https://www.loom.com/share/4d58cd87bf7e40c9b6e1b644d53742d0

ENG-1225: Discourse node migration

Migrate personal settings text inputs

remove unnecessary checks zod takes care

migrate attributes overlay and suggestive rules

Summary by CodeRabbit

Refactor
- Migrated all settings storage from global to personal user-scoped configuration
- Added real-time validation and error message feedback for node configuration settings, including tags and format specifications
- Refactored component interfaces to reduce dependencies and improve maintainability
- Standardized keyboard shortcut configuration across menu trigger features

_{✏️ Tip: You can customize this high-level summary in your review settings.}

linear · 2026-01-14T09:00:55Z

ENG-1225 Discourse node: migrate settings

supabase · 2026-01-14T09:00:58Z

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 ↗︎.

sid597 · 2026-01-14T09:01:27Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

port discourse node config #718
port query settings #717
migrate personal left sidebar setting #705
global left sidebar settings #704
ENG-1280 Discourse node: Query builder and editor component for block props #697
ENG-1225: Discourse node migration simple settings #696 👈 (View in Graphite)
ENG-1273: Migrate small personal settings #691
ENG-1272 Migrate all small global settings components #688 : 1 other dependent PR (#690 )
ENG-1345: Modify existing RoamJS component onChange to also change props #745 : 1 other dependent PR (#684 )
ENG-1190 Replace existing roam-js components with blockProp based ones #689
ENG-1188: prod: pull watchers for prop based settings #682
ENG-1187: Prod: Implement get and set for block prop based settings #669
ENG-1189: Prod: Init block prop based schema #666
ENG-1330: zod schema for discourse relations #719
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

devin-ai-integration

Devin Review found 1 potential issue.

View issue and 6 additional flags in Devin Review.

devin-ai-integration · 2026-01-29T11:37:54Z

apps/roam/src/components/settings/DiscourseNodeSuggestiveRules.tsx

-        uid={node.isFirstChild?.uid || ""}
-        parentUid={parentUid}
-        value={node.isFirstChild?.value || false}
+        settingKeys={["suggestiveRules", "isFirstChild", "value"]}


🔴 isFirstChild setting path creates invalid schema structure missing required uid field

The DiscourseNodeFlagPanel for "First Child" uses settingKeys={["suggestiveRules", "isFirstChild", "value"]} to set only the boolean value.

Click to expand

Problem

The schema in zodSchema.ts:20-26 defines isFirstChild as:

isFirstChild: z.object({ uid: z.string(), value: z.boolean(), }).optional()

When the setter saves to path ["suggestiveRules", "isFirstChild", "value"], it creates:

{ "suggestiveRules": { "isFirstChild": { "value": true } } }

This object is missing the required uid field.

Why validation passes on write but fails on read

The setDiscourseNodeSetting validation at accessors.ts:437-446 validates only the leaf value (true) against the leaf schema (z.boolean()), which passes.

However, when the full node settings are later read via getDiscourseNodeSettings, the entire object is parsed through DiscourseNodeSchema.safeParse() at accessors.ts:399. The isFirstChild: { value: true } object fails validation because uid: z.string() is required.

Impact

User changes to the "First Child" toggle are saved in an invalid format

Subsequent reads of the discourse node settings will fail schema validation

This breaks suggestive mode functionality for affected nodes

Recommendation: Either change the setting path to set the entire isFirstChild object (e.g., settingKeys={["suggestiveRules", "isFirstChild"]} with value { uid: generateUID(), value: checked }), or update the schema to make uid optional or restructure isFirstChild to just be a boolean.

Was this helpful? React with 👍 or 👎 to provide feedback.

graphite-app · 2026-01-31T17:13:08Z

apps/roam/src/components/settings/NodeConfig.tsx

+  const validateTag = (tag: string): string | undefined => {
+    const cleanTag = getCleanTagText(tag);
+    if (!cleanTag) return undefined;
+    const format = getDiscourseNodeSetting<string>(node.type, ["format"])!;
+    const roamTagRegex = /#?\[\[(.*?)\]\]|#(\S+)/g;
+    const formatTags: string[] = [];
+    for (const match of format.matchAll(roamTagRegex)) {
+      const tagName = match[1] || match[2];
+      if (tagName) formatTags.push(tagName.toUpperCase());
+    }
+    if (formatTags.includes(cleanTag)) {
+      return `The tag "${tag}" is referenced in the format. Please use a different tag or format.`;
+    }
+    return undefined;
+  };


Race condition in validation logic. The validateTag function reads format from settings via getDiscourseNodeSetting, but due to the 500ms debounce in BaseTextPanel (BlockPropSettingPanels.tsx:126-128), the format in settings may be stale. If a user types in the format field and immediately switches to edit the tag field, validateTag will validate against the old format value still in settings, not the current input value.

// Fix: Pass the current format value as a parameter const validateTag = (tag: string, currentFormat: string): string | undefined => { const cleanTag = getCleanTagText(tag); if (!cleanTag) return undefined; const roamTagRegex = /#?\[\[(.*?)\]\]|#(\S+)/g; const formatTags: string[] = []; for (const match of currentFormat.matchAll(roamTagRegex)) { const tagName = match[1] || match[2]; if (tagName) formatTags.push(tagName.toUpperCase()); } // ... };

Similarly, validateFormat needs to accept the current tag value as a parameter to avoid reading stale data from settings.

Suggested change

const validateTag = (tag: string): string | undefined => {

const cleanTag = getCleanTagText(tag);

if (!cleanTag) return undefined;

const format = getDiscourseNodeSetting<string>(node.type, ["format"])!;

const roamTagRegex = /#?\[\[(.*?)\]\]|#(\S+)/g;

const formatTags: string[] = [];

for (const match of format.matchAll(roamTagRegex)) {

const tagName = match[1] || match[2];

if (tagName) formatTags.push(tagName.toUpperCase());

}

if (formatTags.includes(cleanTag)) {

return `The tag "${tag}" is referenced in the format. Please use a different tag or format.`;

}

return undefined;

};

const validateTag = (tag: string, currentFormat?: string): string | undefined => {

const cleanTag = getCleanTagText(tag);

if (!cleanTag) return undefined;

const format = currentFormat || getDiscourseNodeSetting<string>(node.type, ["format"])!;

const roamTagRegex = /#?\[\[(.*?)\]\]|#(\S+)/g;

const formatTags: string[] = [];

for (const match of format.matchAll(roamTagRegex)) {

const tagName = match[1] || match[2];

if (tagName) formatTags.push(tagName.toUpperCase());

}

if (formatTags.includes(cleanTag)) {

return `The tag "${tag}" is referenced in the format. Please use a different tag or format.`;

}

return undefined;

};

Spotted by Graphite Agent

Is this helpful? React 👍 or 👎 to let us know.

sid597 · 2026-01-31T17:13:31Z

apps/roam/src/components/canvas/uiOverrides.tsx

      const nodeId = node.type;
      tools[nodeId] = {
        id: nodeId,
+        // TODO: port this when all node settings are done


remove already addressed or noted in the linear ticket

sid597 · 2026-01-31T17:19:23Z

apps/roam/src/components/settings/DiscourseNodeCanvasSettings.tsx

  ControlGroup,
-  Checkbox,
+  Radio,
+  RadioGroup,


changed the format of import why?

apps/roam/src/components/settings/NodeConfig.tsx

apps/roam/src/components/DiscourseNodeMenu.tsx

mdroidian

Could you please record a loom video showing the working changed UI. Show the settings being updating in both the legacy as well as the new prop storage.

Please also format all files with prettier.

Also the CI is failing.

graphite-app · 2026-02-03T19:00:34Z

apps/roam/src/components/settings/components/BlockPropSettingPanels.tsx

+<<<<<<< HEAD
 import React, { useState, useCallback, useRef } from "react";
+=======
+import React, { useState, useRef, useEffect } from "react";
+>>>>>>> 67ecd97e (fix order)


Unresolved merge conflict markers will cause syntax error

The file contains Git merge conflict markers (<<<<<<< HEAD, =======, >>>>>>> 67ecd97e) that will prevent the code from compiling. This needs immediate resolution.

// Remove conflict markers and merge the changes properly: import React, { useState, useRef, useEffect } from "react";

Suggested change

<<<<<<< HEAD

import React, { useState, useCallback, useRef } from "react";

=======

import React, { useState, useRef, useEffect } from "react";

>>>>>>> 67ecd97e (fix order)

import React, { useState, useRef, useEffect } from "react";

Spotted by Graphite Agent

Is this helpful? React 👍 or 👎 to let us know.

devin-ai-integration

Devin Review found 1 new potential issue.

View issue and 14 additional flags in Devin Review.

apps/roam/src/components/settings/components/BlockPropSettingPanels.tsx

devin-ai-integration

Devin Review found 1 new potential issue.

View issue and 20 additional flags in Devin Review.

devin-ai-integration · 2026-02-03T21:52:58Z

apps/roam/src/components/settings/NodeConfig.tsx

+              <DiscourseNodeTextPanel
+                nodeType={node.type}
+                title="Tag"
                description={`Designate a hashtag for marking potential ${node.text}.`}
-                value={tagValue}
-                onChange={handleTagChange}
-                onBlur={handleTagBlur}
-                error={tagError}
+                settingKeys={["tag"]}
+                defaultValue={node.tag}
                placeholder={generateTagPlaceholder(node)}
+                validate={validateTag}
+                onChange={setTagValue}
+                parentUid={node.type}
+                uid={tagUid}
              />


🟡 Tag validation error state becomes stale when format field changes

The tag validation error state is now managed internally within BaseTextPanel and only updates when the tag input changes. However, the validateTag function depends on formatValue to check for conflicts between tag and format.

Click to expand

Root Cause

In the previous implementation, validation ran via useEffect whenever either tagValue or formatValue changed:

useEffect(() => { validate({ tag: tagValue, format: formatValue }); }, [tagValue, formatValue, validate]);

In the new implementation (NodeConfig.tsx:241-243), only formatError is updated:

useEffect(() => { validateFormat({ format: formatValue }); }, [tagValue, formatValue, validateFormat]);

The tag error is now managed inside BaseTextPanel state (BlockPropSettingPanels.tsx:118-119) and is only set via handleChange (BlockPropSettingPanels.tsx:142-144).

Impact

User enters a tag that conflicts with the format → tag error is shown

User modifies the format to remove the conflict

The tag error message persists until the user manually edits the tag field again

This is a regression in UX where stale error messages can confuse users.

Was this helpful? React with 👍 or 👎 to provide feedback.

graphite-app · 2026-02-03T21:57:59Z

apps/roam/src/components/settings/NodeConfig.tsx

+              <DiscourseNodeTextPanel
+                nodeType={node.type}
+                title="Description"
                description={`Describing what the ${node.text} node represents in your graph.`}
-                value={descriptionValue}
-                onChange={handleDescriptionChange}
-                onBlur={handleDescriptionBlur}
+                settingKeys={["description"]}
+                defaultValue={node.description}
+                parentUid={node.type}
+                uid={descriptionUid}


The DiscourseNodeTextPanel for Description is missing the required order prop. Without this prop, hasBlockSync will be false in BaseTextPanel (requires both parentUid and order to be defined), preventing changes from being synced to Roam blocks.

<DiscourseNodeTextPanel nodeType={node.type} title="Description" description={`Describing what the ${node.text} node represents in your graph.`} settingKeys={["description"]} defaultValue={node.description} order={1} // Add this line parentUid={node.type} uid={descriptionUid} />

Suggested change

<DiscourseNodeTextPanel

nodeType={node.type}

title="Description"

description={`Describing what the ${node.text} node represents in your graph.`}

value={descriptionValue}

onChange={handleDescriptionChange}

onBlur={handleDescriptionBlur}

settingKeys={["description"]}

defaultValue={node.description}

parentUid={node.type}

uid={descriptionUid}

<DiscourseNodeTextPanel

nodeType={node.type}

title="Description"

description={`Describing what the ${node.text} node represents in your graph.`}

settingKeys={["description"]}

defaultValue={node.description}

order={1}

parentUid={node.type}

uid={descriptionUid}

Spotted by Graphite Agent

Is this helpful? React 👍 or 👎 to let us know.

devin-ai-integration

Devin Review found 1 new potential issue.

View issue and 24 additional flags in Devin Review.

devin-ai-integration · 2026-02-04T17:59:59Z

apps/roam/src/components/settings/ExportSettings.tsx

+<<<<<<< HEAD
+=======
+          settingKeys={["Export", "Remove Special Characters"]}
+          defaultValue={exportSettings.removeSpecialCharacters.value || false}
+>>>>>>> 84aa5ac2 (restack and fix unnecessary changes)


🔴 Unresolved git merge conflicts in ExportSettings.tsx

The file contains unresolved git merge conflict markers (<<<<<<< HEAD, =======, >>>>>>> 84aa5ac2) at multiple locations throughout the file. This will cause syntax errors and prevent the application from compiling.

Click to expand

Impact

The file has 7 unresolved merge conflicts at lines 18-22, 32-36, 45-49, 59-63, 74-79, 92-96, and 105-109. Each conflict contains both HEAD and the other branch's changes, making the file invalid JavaScript/TypeScript.

Example of the conflict pattern:

<<<<<<< HEAD ======= settingKeys={["Export", "Remove Special Characters"]} defaultValue={exportSettings.removeSpecialCharacters.value || false} >>>>>>> 84aa5ac2 (restack and fix unnecessary changes)

Actual vs Expected

Actual: File contains merge conflict markers that are invalid syntax

Expected: File should have resolved conflicts with valid TypeScript code

Recommendation: Resolve the git merge conflicts by choosing the appropriate code from either HEAD or the other branch, then remove all conflict markers.

Was this helpful? React with 👍 or 👎 to provide feedback.

sid597 mentioned this pull request Jan 14, 2026

ENG-1273: Migrate small personal settings #691

Open

sid597 changed the title ~~ENG-1225: Discourse node migration~~ ENG-1225: Discourse node migration simple settings Jan 14, 2026

sid597 marked this pull request as ready for review January 14, 2026 09:02

This was referenced Jan 14, 2026

ENG-1280 Discourse node: Query builder and editor component for block props #697

Open

global left sidebar settings #704

Draft

migrate personal left sidebar setting #705

Draft

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from 3ac2996 to d549f8d Compare January 17, 2026 18:16

sid597 force-pushed the eng-1273-migrate-all-small-personal-settings branch from a82cfaf to bc0de10 Compare January 17, 2026 18:16

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from d549f8d to 78b7dbb Compare January 18, 2026 05:25

sid597 force-pushed the eng-1273-migrate-all-small-personal-settings branch from bc0de10 to 57abc42 Compare January 18, 2026 05:25

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from 78b7dbb to d66fa03 Compare January 19, 2026 04:51

sid597 force-pushed the eng-1273-migrate-all-small-personal-settings branch from 57abc42 to 7ead54e Compare January 19, 2026 04:51

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from d66fa03 to cb22590 Compare January 19, 2026 05:01

sid597 force-pushed the eng-1273-migrate-all-small-personal-settings branch from 490810f to 8dc5276 Compare January 19, 2026 06:03

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from cb22590 to 78abef0 Compare January 19, 2026 06:03

sid597 force-pushed the eng-1273-migrate-all-small-personal-settings branch from 8dc5276 to 8286d6b Compare January 19, 2026 06:34

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from 78abef0 to 3966245 Compare January 19, 2026 06:34

This was referenced Jan 19, 2026

port query settings #717

Draft

port discourse node config #718

Draft

sid597 force-pushed the eng-1273-migrate-all-small-personal-settings branch from 8286d6b to 78e8311 Compare January 20, 2026 16:06

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from 3966245 to 7015148 Compare January 20, 2026 16:06

sid597 changed the base branch from eng-1273-migrate-all-small-personal-settings to graphite-base/696 January 28, 2026 21:16

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from 7015148 to ecdee21 Compare January 29, 2026 11:34

sid597 force-pushed the graphite-base/696 branch from 78e8311 to 938f6aa Compare January 29, 2026 11:34

sid597 changed the base branch from graphite-base/696 to eng-1273-migrate-all-small-personal-settings January 29, 2026 11:34

devin-ai-integration bot reviewed Jan 29, 2026

View reviewed changes

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from ecdee21 to 96f4575 Compare January 30, 2026 06:10

graphite-app bot reviewed Jan 31, 2026

View reviewed changes

sid597 commented Jan 31, 2026

View reviewed changes

apps/roam/src/components/settings/NodeConfig.tsx Outdated Show resolved Hide resolved

sid597 commented Jan 31, 2026

View reviewed changes

apps/roam/src/components/DiscourseNodeMenu.tsx Outdated Show resolved Hide resolved

sid597 requested a review from mdroidian January 31, 2026 17:33

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from c09b869 to 67ecd97 Compare January 31, 2026 18:25

mdroidian requested changes Feb 2, 2026

View reviewed changes

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from 67ecd97 to 610365f Compare February 3, 2026 18:56

sid597 force-pushed the eng-1273-migrate-all-small-personal-settings branch from 938f6aa to 0dee3d3 Compare February 3, 2026 18:56

graphite-app bot reviewed Feb 3, 2026

View reviewed changes

devin-ai-integration bot reviewed Feb 3, 2026

View reviewed changes

apps/roam/src/components/settings/components/BlockPropSettingPanels.tsx Outdated Show resolved Hide resolved

sid597 force-pushed the eng-1273-migrate-all-small-personal-settings branch from 0dee3d3 to d4ed8a1 Compare February 3, 2026 19:08

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from 610365f to 80bb0fb Compare February 3, 2026 19:08

sid597 changed the base branch from eng-1273-migrate-all-small-personal-settings to graphite-base/696 February 3, 2026 20:35

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from 80bb0fb to dfe4388 Compare February 3, 2026 21:41

sid597 force-pushed the graphite-base/696 branch from d4ed8a1 to 88d4ff5 Compare February 3, 2026 21:41

sid597 changed the base branch from graphite-base/696 to eng-1273-migrate-all-small-personal-settings February 3, 2026 21:41

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from dfe4388 to a449a73 Compare February 3, 2026 21:49

devin-ai-integration bot reviewed Feb 3, 2026

View reviewed changes

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from a449a73 to 84aa5ac Compare February 3, 2026 21:53

graphite-app bot reviewed Feb 3, 2026

View reviewed changes

sid597 added 4 commits February 4, 2026 23:25

ENG-1225: Discourse node migration

e30404b

Migrate personal settings text inputs

18b0bcc

remove unnecessary checks zod takes care

600352f

restack and fix unnecessary changes

8ad6b12

sid597 force-pushed the eng-1273-migrate-all-small-personal-settings branch from 88d4ff5 to f5d0a0a Compare February 4, 2026 17:56

sid597 force-pushed the eng-1225-discourse-node-migrate-settings-prod branch from 84aa5ac to 8ad6b12 Compare February 4, 2026 17:56

devin-ai-integration bot reviewed Feb 4, 2026

View reviewed changes

sid597 changed the base branch from eng-1273-migrate-all-small-personal-settings to graphite-base/696 February 7, 2026 18:29

Conversation

sid597 commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

linear bot commented Jan 14, 2026

Uh oh!

supabase bot commented Jan 14, 2026

Uh oh!

sid597 commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Jan 29, 2026

Choose a reason for hiding this comment

Problem

Why validation passes on write but fails on read

Impact

Uh oh!

graphite-app bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

sid597 Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

sid597 Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mdroidian left a comment

Choose a reason for hiding this comment

Uh oh!

graphite-app bot Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Feb 3, 2026

Choose a reason for hiding this comment

Root Cause

Impact

Uh oh!

graphite-app bot Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Feb 4, 2026

Choose a reason for hiding this comment

Impact

Actual vs Expected

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sid597 commented Jan 14, 2026 •

edited

Loading

sid597 commented Jan 14, 2026 •

edited

Loading