ENG-1455: Dual-read from old-system settings and from blockprops by sid597 · Pull Request #812 · DiscourseGraphs/discourse-graph

sid597 · 2026-02-22T19:22:58Z

https://www.loom.com/share/1894f6a6fef2493e97ad1bf06fed851f

 ### PR chain
  ```
  main
    └── migration-block-init-staging-branch  (staging base)
         └── **eng-1455** (this PR)
              └── eng-1472 (PR #813)
                   └── siblings (1456, 1473, 1467, 1468, ...)

Summary by CodeRabbit

New Features
- Added "Enable dual read" feature flag to enhance settings compatibility with legacy configurations
- Updated default query metadata visibility settings to improve performance by hiding metadata by default
- Modified default query pages configuration for better content organization

linear · 2026-02-22T19:23:01Z

ENG-1455 Add old-system read routing to accessor getters

supabase · 2026-02-22T19:23:05Z

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-02-22T19:23:22Z

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

sid597 · 2026-02-22T19:24:32Z

apps/roam/src/index.ts

    initializeSupabaseSync();
  }

  const { extensionAPI } = onloadArgs;


For testing add

const debugWindow = window as unknown as { [key: string]: unknown }; debugWindow["__DG_DEBUG__"] = { runDualReadProbe, };

then go to browser console and do

const report = window.__DG_DEBUG__.runDualReadProbe(); report.summary; // this provides a summary report.mismatches; // if there are any mismatches we will see them report.rows; // will print all the values we read from the graph in personal/global/discourse node + blockprops form

sid597 · 2026-02-22T19:24:48Z

apps/roam/src/index.ts

@@ -43,6 +43,7 @@
 } from "./data/userSettings";
 import { initSchema } from "./components/settings/utils/init";
 import { setupPullWatchOnSettingsPage } from "./components/settings/utils/pullWatchers";


for testing add
import { runDualReadProbe } from "./components/settings/utils/accessors";

sid597 · 2026-02-22T19:26:49Z

apps/roam/src/components/settings/utils/accessors.ts

  }

  return nodes;
 };


For testing add the following here

type DualReadProbeCase = { keys: string[] }; type DualReadProbeNodeCase = { nodeType: string; keys: string[]; }; type DualReadProbeItem = { scope: "personal" | "global" | "discourseNode"; keys: string[]; nodeType?: string; legacyValue?: unknown; blockPropsValue?: unknown; legacyError?: string; blockPropsError?: string; match: boolean; }; const normalizeProbeError = (error: unknown): string => { return error instanceof Error ? error.message : String(error); }; const safeSerialize = (value: unknown): string => { try { return JSON.stringify(value); } catch { return String(value); } }; const normalizeForComparison = (value: unknown): unknown => { if (Array.isArray(value)) { return value.map(normalizeForComparison); } if (isRecord(value)) { return Object.keys(value) .sort() .reduce<Record<string, unknown>>((acc, key) => { acc[key] = normalizeForComparison(value[key]); return acc; }, {}); } return value; }; const runProbeRead = ( reader: () => unknown, ): { value?: unknown; error?: string; } => { try { return { value: reader() }; } catch (error) { return { error: normalizeProbeError(error) }; } }; const hasProbeMatch = ({ legacyError, blockPropsError, legacyValue, blockPropsValue, }: { legacyError?: string; blockPropsError?: string; legacyValue?: unknown; blockPropsValue?: unknown; }): boolean => { if (legacyError || blockPropsError) return false; return ( safeSerialize(normalizeForComparison(legacyValue)) === safeSerialize(normalizeForComparison(blockPropsValue)) ); }; const DEFAULT_DISCOURSE_NODE_PROBE_TYPE = "Claim"; const DEFAULT_DISCOURSE_NODE_PROBE_KEYS: string[][] = [ ["description"], ["shortcut"], ["tag"], ["index"], ["format"], ["specification", "enabled"], ["specification", "query"], ["template"], ["attributes"], ["overlay"], ["canvasSettings", "color"], ["canvasSettings", "alias"], ["canvasSettings", "key-image"], ["canvasSettings", "key-image-option"], ["canvasSettings", "query-builder-alias"], ["graphOverview"], ["suggestiveRules", "embeddingRef"], ["suggestiveRules", "isFirstChild"], ]; export const runDualReadProbe = ({ personalCases = [ { keys: ["Discourse context overlay"] }, { keys: ["Suggestive mode overlay"] }, { keys: ["Text selection popup"] }, { keys: ["Disable sidebar open"] }, { keys: ["Page preview"] }, { keys: ["Hide feedback button"] }, { keys: ["Auto canvas relations"] }, { keys: ["Overlay in canvas"] }, { keys: ["Streamline styling"] }, { keys: ["Disable product diagnostics"] }, { keys: ["Discourse tool shortcut"] }, { keys: ["Personal node menu trigger"] }, { keys: ["Node search menu trigger"] }, { keys: ["Query"] }, { keys: ["Query", "Hide query metadata"] }, { keys: ["Query", "Default page size"] }, { keys: ["Query", "Query pages"] }, { keys: ["Query", "Default filters"] }, { keys: ["Left sidebar"] }, ], globalCases = [ { keys: ["Trigger"] }, { keys: ["Canvas page format"] }, { keys: ["Left sidebar"] }, { keys: ["Left sidebar", "Children"] }, { keys: ["Left sidebar", "Settings", "Collapsable"] }, { keys: ["Left sidebar", "Settings", "Folded"] }, { keys: ["Export"] }, { keys: ["Export", "Remove special characters"] }, { keys: ["Export", "Resolve block references"] }, { keys: ["Export", "Resolve block embeds"] }, { keys: ["Export", "Append referenced node"] }, { keys: ["Export", "Link type"] }, { keys: ["Export", "Max filename length"] }, { keys: ["Export", "Frontmatter"] }, { keys: ["Suggestive mode"] }, { keys: ["Suggestive mode", "Include current page relations"] }, { keys: ["Suggestive mode", "Include parent and child blocks"] }, { keys: ["Suggestive mode", "Page groups"] }, { keys: ["Relations"] }, ], discourseNodeCases = DEFAULT_DISCOURSE_NODE_PROBE_KEYS.map((keys) => ({ nodeType: DEFAULT_DISCOURSE_NODE_PROBE_TYPE, keys, })), }: { personalCases?: DualReadProbeCase[]; globalCases?: DualReadProbeCase[]; discourseNodeCases?: DualReadProbeNodeCase[]; } = {}) => { const rows: DualReadProbeItem[] = []; personalCases.forEach(({ keys }) => { const legacy = runProbeRead(() => getLegacyPersonalSetting(keys)); const blockProps = runProbeRead(() => readPathValue(getPersonalSettings(), keys), ); rows.push({ scope: "personal", keys, legacyValue: legacy.value, blockPropsValue: blockProps.value, legacyError: legacy.error, blockPropsError: blockProps.error, match: hasProbeMatch({ legacyError: legacy.error, blockPropsError: blockProps.error, legacyValue: legacy.value, blockPropsValue: blockProps.value, }), }); }); globalCases.forEach(({ keys }) => { const legacy = runProbeRead(() => getLegacyGlobalSetting(keys)); const blockProps = runProbeRead(() => readPathValue(getGlobalSettings(), keys), ); rows.push({ scope: "global", keys, legacyValue: legacy.value, blockPropsValue: blockProps.value, legacyError: legacy.error, blockPropsError: blockProps.error, match: hasProbeMatch({ legacyError: legacy.error, blockPropsError: blockProps.error, legacyValue: legacy.value, blockPropsValue: blockProps.value, }), }); }); discourseNodeCases.forEach(({ nodeType, keys }) => { const legacy = runProbeRead(() => getLegacyDiscourseNodeSetting(nodeType, keys), ); const blockProps = runProbeRead(() => readPathValue(getDiscourseNodeSettings(nodeType), keys), ); rows.push({ scope: "discourseNode", nodeType, keys, legacyValue: legacy.value, blockPropsValue: blockProps.value, legacyError: legacy.error, blockPropsError: blockProps.error, match: hasProbeMatch({ legacyError: legacy.error, blockPropsError: blockProps.error, legacyValue: legacy.value, blockPropsValue: blockProps.value, }), }); }); const mismatches = rows.filter((row) => !row.match); return { dualReadEnabled: isDualReadEnabled(), summary: { total: rows.length, matches: rows.filter((row) => row.match).length, mismatches: mismatches.length, }, rows, mismatches, }; };

sid597 · 2026-02-22T19:37:10Z

@coderabbitai full review

coderabbitai · 2026-02-22T19:37:20Z

✅ Actions performed

Full review triggered.

coderabbitai · 2026-02-22T19:42:37Z

📝 Walkthrough

Walkthrough

Introduces dual-read compatibility for settings by adding extensive legacy reading paths and helper functions. When dual-read is disabled, the system falls back to legacy settings resolution for Personal, Global, and Discourse Node configurations. Updates query-related default values and introduces a new dual-read feature flag.

Changes

Cohort / File(s)	Summary
Dual-read Legacy Compatibility `apps/roam/src/components/settings/utils/accessors.ts`	Adds comprehensive legacy reading fallback paths with helper functions (readPathValue, pathKey) and resolution logic for Personal, Global, Discourse Node, and Relations settings. Updates getGlobalSetting, getPersonalSetting, and getDiscourseNodeSetting to check dual-read status and fall back to legacy reading when disabled.
Schema Defaults & Feature Flags `apps/roam/src/components/settings/utils/zodSchema.ts`, `apps/roam/src/components/settings/utils/zodSchema.example.ts`	Introduces defaultNodeIndexValue constant and updates DiscourseNodeSchema specification.query and index field transforms. Changes Query defaults: "Hide query metadata" from false to true, "Query pages" from empty array to ["discourse-graph/queries/*"]. Adds "Enable dual read" feature flag with false default.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

ENG-853 left sidebar global and personal settings #441: Adds left-sidebar configuration helpers that overlap with the legacy reading logic for left-sidebar personal/global settings introduced in this PR.
ENG-1187: Prod: Implement get and set for block prop based settings #669: Modifies the same settings accessors (getGlobalSetting, getPersonalSetting, getDiscourseNodeSetting) to add dual-read fallback paths.
ENG-1105: zod schema for all the settings #664: Updates zodSchema.ts defaults and introduces the dual-read feature flag that this PR depends on for its legacy reading logic.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: implementing dual-read functionality for both old-system settings and blockprops, which aligns with the primary objective of the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

sid597 · 2026-02-24T19:28:26Z

apps/roam/src/components/settings/utils/zodSchema.ts


 export const QuerySettingsSchema = z.object({
-  "Hide query metadata": z.boolean().default(false),
+  "Hide query metadata": z.boolean().default(true),


same default as legacy settings

sid597 · 2026-02-24T19:28:38Z

apps/roam/src/components/settings/utils/zodSchema.ts

+  "Hide query metadata": z.boolean().default(true),
  "Default page size": z.number().default(10),
-  "Query pages": z.array(z.string()).default([]),
+  "Query pages": z.array(z.string()).default(["discourse-graph/queries/*"]),


same defaults as legacy settings

apps/roam/src/components/settings/utils/accessors.ts

sid597 · 2026-02-24T19:38:07Z

apps/roam/src/components/settings/utils/accessors.ts

+// which returns a flat DiscourseRelation[] with a different structure (one entry per
+// if-block, triples at top level, no nodePositions). When migrating getDiscourseRelations()
+// to read from block props, it will need a conversion from this shape to the flat array.
+const getLegacyRelationsSetting = (): Record<string, unknown> => {


the plan is to eventually merge to main with the blockprop version so not sure about the performance worries, if any. Will do caching if said so

mdroidian

As per our last meeting, I was under the impression that the plan for Wed was to

merge the 5 PR's that were in review (this is complete)
rebase/restack ENG-1454, 1466+
Then create a pulse update for the project

Is this related to #2, rebase/restack? If so, could you explain to in what way it is related?

From this image, it doesn't look like it is part of "rebase/restack ENG-1454, 1466", as the root of that restack should be the base getters PR. Nor is it mentioned in "Land Orphaned ..." PR.

sid597 · 2026-02-25T11:44:46Z

apps/roam/src/components/settings/utils/accessors.ts

+  if (rawValue === undefined) {
+    const legacyValue = getLegacyGlobalSetting(keys);
+    if (legacyValue !== undefined) {
+      throw new Error(getMissingSettingError({ context: "Global", keys }));


explicit throw, we want to know when this happens as it should never happen and not pass through dev testing

…egacy and block props are same

mdroidian · 2026-02-25T18:03:28Z

As per our last meeting, I was under the impression that the plan for Wed was to

merge the 5 PR's that were in review (this is complete)

rebase/restack ENG-1454, 1466+

Then create a pulse update for the project

Is this related to #2, rebase/restack? If so, could you explain to in what way it is related?
From this image, it doesn't look like it is part of "rebase/restack ENG-1454, 1466", as the root of that restack should be the base getters PR. Nor is it mentioned in "[Land Orphaned ...](https://github.com//pull/826)" PR.

Discussed in meeting: no longer using graphite which means this is a non issue.

sid597 marked this pull request as ready for review February 22, 2026 19:23

This was referenced Feb 22, 2026

ENG-1328: Port small blueprint based misc settings into props based #805

Merged

ENG-1465: Add "Use new settings store" feature flag #811

Merged

sid597 commented Feb 22, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

This was referenced Feb 22, 2026

ENG-1472: Refactor BlockPropSettingPanels to add accessor-backed default reads #813

Open

ENG-1456: Migrate personal boolean flag consumers (getSetting → getPersonalSetting) #814

Open

sid597 changed the base branch from eng-1465-add-enable-dual-read-feature-flag-with-reactive-accessor to graphite-base/812 February 23, 2026 10:26

sid597 mentioned this pull request Feb 23, 2026

ENG-1217: Port section component in left sidebar #804

Merged

sid597 force-pushed the eng-1455-add-old-system-read-routing-to-accessor-getters branch from 5a02381 to b67fd5f Compare February 23, 2026 10:40

sid597 force-pushed the graphite-base/812 branch from 3f08f30 to 23d75a0 Compare February 23, 2026 10:40

sid597 changed the base branch from graphite-base/812 to eng-1465-add-enable-dual-read-feature-flag-with-reactive-accessor February 23, 2026 10:40

This comment was marked as resolved.

Sign in to view

sid597 force-pushed the eng-1455-add-old-system-read-routing-to-accessor-getters branch from b67fd5f to adf5fb5 Compare February 23, 2026 13:51

sid597 force-pushed the eng-1465-add-enable-dual-read-feature-flag-with-reactive-accessor branch from 23d75a0 to 9dddded Compare February 23, 2026 13:51

sid597 force-pushed the eng-1455-add-old-system-read-routing-to-accessor-getters branch from adf5fb5 to 75d22ad Compare February 23, 2026 14:13

sid597 force-pushed the eng-1465-add-enable-dual-read-feature-flag-with-reactive-accessor branch from 9dddded to 41691df Compare February 23, 2026 14:13

sid597 force-pushed the eng-1455-add-old-system-read-routing-to-accessor-getters branch from 75d22ad to cc8a3f2 Compare February 23, 2026 14:31

sid597 force-pushed the eng-1465-add-enable-dual-read-feature-flag-with-reactive-accessor branch from 41691df to 176d0d1 Compare February 23, 2026 14:31

sid597 force-pushed the eng-1465-add-enable-dual-read-feature-flag-with-reactive-accessor branch from 0ff501b to 017e4da Compare February 24, 2026 17:15

sid597 force-pushed the eng-1455-add-old-system-read-routing-to-accessor-getters branch from 4475ce3 to 7160d5e Compare February 24, 2026 17:16

sid597 force-pushed the eng-1465-add-enable-dual-read-feature-flag-with-reactive-accessor branch from 017e4da to 990543f Compare February 24, 2026 19:07

sid597 commented Feb 24, 2026

View reviewed changes

apps/roam/src/components/settings/utils/accessors.ts Outdated Show resolved Hide resolved

sid597 commented Feb 24, 2026

View reviewed changes

sid597 force-pushed the eng-1455-add-old-system-read-routing-to-accessor-getters branch from 7160d5e to b86b56f Compare February 24, 2026 19:38

This comment was marked as resolved.

Sign in to view

sid597 requested a review from mdroidian February 24, 2026 20:02

mdroidian requested changes Feb 24, 2026

View reviewed changes

sid597 force-pushed the eng-1465-add-enable-dual-read-feature-flag-with-reactive-accessor branch from 990543f to 1491bbf Compare February 25, 2026 07:34

sid597 force-pushed the eng-1455-add-old-system-read-routing-to-accessor-getters branch 2 times, most recently from feb69cc to aac3698 Compare February 25, 2026 07:44

sid597 changed the base branch from eng-1465-add-enable-dual-read-feature-flag-with-reactive-accessor to migration-block-init-staging-branch February 25, 2026 07:47

sid597 added 8 commits February 25, 2026 15:16

ENG-1455: Dual-read from old-system settings and from blockprops

f6765d1

remove console.logs

29263ac

address review

4aec399

prettier

e36e3fe

add comment describing why we do it like this

6b5d623

reviewed

ed2fe4d

rebase, review

d2c821f

address review

847359f

sid597 force-pushed the eng-1455-add-old-system-read-routing-to-accessor-getters branch from aac3698 to 847359f Compare February 25, 2026 09:47

explicit throw, now zodschema default reads

0d6ee2b

sid597 commented Feb 25, 2026

View reviewed changes

replace throw with warn, throw was too strong, with warn we test if l…

9b10a40

…egacy and block props are same

sid597 requested a review from mdroidian February 25, 2026 18:11

mdroidian approved these changes Feb 27, 2026

View reviewed changes

Conversation

sid597 commented Feb 22, 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 Feb 22, 2026

Uh oh!

supabase bot commented Feb 22, 2026

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sid597 Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

sid597 Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

sid597 commented Feb 22, 2026

Uh oh!

coderabbitai bot commented Feb 22, 2026

Uh oh!

coderabbitai bot commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

sid597 Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

sid597 Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sid597 Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

mdroidian left a comment

Choose a reason for hiding this comment

Uh oh!

sid597 Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

mdroidian commented Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sid597 commented Feb 22, 2026 •

edited

Loading

sid597 commented Feb 22, 2026 •

edited

Loading

sid597 Feb 22, 2026 •

edited

Loading

coderabbitai bot commented Feb 22, 2026 •

edited

Loading