ENG-1668: Perf: slow load time#984
Merged
sid597 merged 9 commits intoMay 6, 2026
Merged
Conversation
…ConfigTree 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.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
📝 WalkthroughWalkthroughThis PR refactors the Roam configuration initialization flow. It removes discourse node page creation logic from init.ts, updates setting accessor functions to accept an explicit config tree parameter instead of always fetching from the settings page, makes initializeDiscourseNodes return a boolean, and conditions the config tree refresh on whether nodes were created. ChangesSettings and Config Tree Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/roam/src/utils/initializeObserversAndListeners.ts (2)
123-127:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid the second settings pull in this observer.
Line 126 calls
getFeatureFlag(), which does anotherbulkReadSettings()even though this callback already loadedsettingson Line 113. This observer is on the startup hot path, so the extra pull directly works against the perf fix here. Read the flag from the local snapshot instead.Suggested change
- if (getFeatureFlag("Duplicate node alert enabled")) { + if (settings.featureFlags["Duplicate node alert enabled"]) { renderPossibleDuplicates(h1, title, node); }🤖 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/utils/initializeObserversAndListeners.ts` around lines 123 - 127, This observer currently calls getFeatureFlag("Duplicate node alert enabled") which triggers a bulkReadSettings() again; instead read the flag from the local settings snapshot that was loaded earlier in this function (the same settings variable populated around line 113) and use that boolean to gate renderPossibleDuplicates; replace the getFeatureFlag call with a check like settings["Duplicate node alert enabled"] (coerce to boolean as needed) so you avoid the extra settings pull while keeping renderDiscourseContext and renderPossibleDuplicates unchanged.
117-121:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInvalidate the
findDiscourseNodecache when settings change.Passing
snapshothere will not refresh prior matches becauseapps/roam/src/utils/findDiscourseNode.tsreturns the uid-cached value before it ever looks atsnapshot. A page that was cached asfalsewill keep missing discourse UI after node definitions change until the extension reloads.🤖 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/utils/initializeObserversAndListeners.ts` around lines 117 - 121, The cached result from findDiscourseNode is not being invalidated when settings (snapshot) change, so a page previously cached as false will never pick up updated node definitions; fix by adding cache invalidation in findDiscourseNode (e.g., include snapshot in the cache key or expose a clear/reset function) and then call that invalidation from initializeObserversAndListeners before calling findDiscourseNode (the call shown: findDiscourseNode({ uid, title, snapshot: settings })) whenever settings change; alternatively, stop passing snapshot and instead ensure findDiscourseNode's cache logic considers the settings snapshot so results are recomputed on settings updates.apps/roam/src/utils/initializeDiscourseNodes.ts (1)
13-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn
trueonly when a node page was actually created.If the graph already has the default node pages but no user-defined nodes, this branch still returns
truebecausegetPageUidByPageTitle(...)short-circuits the||expression. That makesindex.tsrefresh the config tree on every startup, so the new boolean does not actually represent "writes happened".Suggested change
if (nodes.length === 0) { - await Promise.all( - INITIAL_NODE_VALUES.map( - (n) => - getPageUidByPageTitle(`discourse-graph/nodes/${n.text}`) || - createPage({ - title: `discourse-graph/nodes/${n.text}`, - uid: n.type, - tree: [ - { text: "Format", children: [{ text: n.format || "" }] }, - { text: "Shortcut", children: [{ text: n.shortcut || "" }] }, - { text: "Tag", children: [{ text: n.tag || "" }] }, - { text: "Graph Overview" }, - { - text: "Canvas", - children: [ - { - text: "color", - children: [{ text: n.canvasSettings?.color || "" }], - }, - ], - }, - ], - }), - ), - ); - return true; + const created = await Promise.all( + INITIAL_NODE_VALUES.map(async (n) => { + if (getPageUidByPageTitle(`discourse-graph/nodes/${n.text}`)) { + return false; + } + await createPage({ + title: `discourse-graph/nodes/${n.text}`, + uid: n.type, + tree: [ + { text: "Format", children: [{ text: n.format || "" }] }, + { text: "Shortcut", children: [{ text: n.shortcut || "" }] }, + { text: "Tag", children: [{ text: n.tag || "" }] }, + { text: "Graph Overview" }, + { + text: "Canvas", + children: [ + { + text: "color", + children: [{ text: n.canvasSettings?.color || "" }], + }, + ], + }, + ], + }); + return true; + }), + ); + return created.some(Boolean); }🤖 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/utils/initializeDiscourseNodes.ts` around lines 13 - 39, The current branch returns true even when no pages were created because getPageUidByPageTitle(...) short-circuits the ||; change the logic so each INITIAL_NODE_VALUES item checks existence first and returns a boolean indicating whether createPage(...) was actually called: for each n call await getPageUidByPageTitle(...); if it exists return false, otherwise await createPage(...) and return true; await Promise.all on that boolean array and return true only if any item is true (e.g., use Array.some) so initializeDiscourseNodes returns true only when at least one page was created.apps/roam/src/components/settings/utils/accessors.ts (1)
407-457:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't route legacy Export/Relations reads through
discourseConfigRef.tree.These readers used to fetch directly from the config page, but now they depend on
discourseConfigRef.tree. The hash-change handler inapps/roam/src/utils/initializeObserversAndListeners.tsalready documents that sidebar edits do not refresh that tree, so in legacy graphs Export and Relations can now stay stale until a manual refresh or reload.🤖 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/utils/accessors.ts` around lines 407 - 457, The Export and Relations readers must not rely on the cached discourseConfigRef.tree (which isn't refreshed on sidebar edits); update the code so the Export branch and Relations branch use legacy readers that fetch the config page directly rather than reading discourseConfigRef.tree — specifically ensure getExportSettingsAndUids and getLegacyRelationsSetting (or the helper they call) read the config page/tree fresh from the source (or accept and use the passed-in tree that was loaded from the config page) instead of accessing discourseConfigRef.tree; make analogous changes inside any helper like getSuggestiveModeConfigAndUids if it currently routes through discourseConfigRef.tree so legacy settings stay current without a full reload.
🤖 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.
Outside diff comments:
In `@apps/roam/src/components/settings/utils/accessors.ts`:
- Around line 407-457: The Export and Relations readers must not rely on the
cached discourseConfigRef.tree (which isn't refreshed on sidebar edits); update
the code so the Export branch and Relations branch use legacy readers that fetch
the config page directly rather than reading discourseConfigRef.tree —
specifically ensure getExportSettingsAndUids and getLegacyRelationsSetting (or
the helper they call) read the config page/tree fresh from the source (or accept
and use the passed-in tree that was loaded from the config page) instead of
accessing discourseConfigRef.tree; make analogous changes inside any helper like
getSuggestiveModeConfigAndUids if it currently routes through
discourseConfigRef.tree so legacy settings stay current without a full reload.
In `@apps/roam/src/utils/initializeDiscourseNodes.ts`:
- Around line 13-39: The current branch returns true even when no pages were
created because getPageUidByPageTitle(...) short-circuits the ||; change the
logic so each INITIAL_NODE_VALUES item checks existence first and returns a
boolean indicating whether createPage(...) was actually called: for each n call
await getPageUidByPageTitle(...); if it exists return false, otherwise await
createPage(...) and return true; await Promise.all on that boolean array and
return true only if any item is true (e.g., use Array.some) so
initializeDiscourseNodes returns true only when at least one page was created.
In `@apps/roam/src/utils/initializeObserversAndListeners.ts`:
- Around line 123-127: This observer currently calls getFeatureFlag("Duplicate
node alert enabled") which triggers a bulkReadSettings() again; instead read the
flag from the local settings snapshot that was loaded earlier in this function
(the same settings variable populated around line 113) and use that boolean to
gate renderPossibleDuplicates; replace the getFeatureFlag call with a check like
settings["Duplicate node alert enabled"] (coerce to boolean as needed) so you
avoid the extra settings pull while keeping renderDiscourseContext and
renderPossibleDuplicates unchanged.
- Around line 117-121: The cached result from findDiscourseNode is not being
invalidated when settings (snapshot) change, so a page previously cached as
false will never pick up updated node definitions; fix by adding cache
invalidation in findDiscourseNode (e.g., include snapshot in the cache key or
expose a clear/reset function) and then call that invalidation from
initializeObserversAndListeners before calling findDiscourseNode (the call
shown: findDiscourseNode({ uid, title, snapshot: settings })) whenever settings
change; alternatively, stop passing snapshot and instead ensure
findDiscourseNode's cache logic considers the settings snapshot so results are
recomputed on settings updates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0473d32f-002a-4929-ab6c-0842ede7a04a
📒 Files selected for processing (6)
apps/roam/src/components/settings/utils/accessors.tsapps/roam/src/components/settings/utils/init.tsapps/roam/src/index.tsapps/roam/src/utils/getExportSettings.tsapps/roam/src/utils/initializeDiscourseNodes.tsapps/roam/src/utils/initializeObserversAndListeners.ts
mdroidian
approved these changes
May 6, 2026
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/1bb4e2d1d6254ee2b74f3cbf8c365afb
Results Macbook air 8gb ram 2020
Flag On: 602ms
Flag Off: 192ms
Summary by CodeRabbit