Eng 1616 add getconfigtree equivalent for block pros on init#944
Open
sid597 wants to merge 4 commits intoeng-1470-test-dual-read-with-flag-on-and-fix-gapsv2from
Conversation
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.
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.
|
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}
Comment on lines
122
to
127
| export const installDiscourseFloatingMenu = ( | ||
| onLoadArgs: OnloadArgs, | ||
| snapshot?: SettingsSnapshot, | ||
| props: DiscourseFloatingMenuProps = { | ||
| position: "bottom-right", | ||
| theme: "bp3-light", |
Contributor
There was a problem hiding this comment.
🔴 AGENTS.md violation: installDiscourseFloatingMenu has >2 positional parameters without object destructuring
The installDiscourseFloatingMenu function was changed from 2 to 3 positional parameters (onLoadArgs, snapshot?, props), violating the AGENTS.md rule: "Use named parameters (object destructuring) when a function has more than 2 parameters." This rule is explicitly stated in the repository's style guide at AGENTS.md.
(Refers to lines 122-130)
Prompt for agents
The function installDiscourseFloatingMenu in apps/roam/src/components/DiscourseFloatingMenu.tsx now has 3 parameters (onLoadArgs, snapshot, props) which violates the AGENTS.md rule requiring object destructuring for functions with more than 2 parameters. Refactor the signature to use a single object parameter with destructuring, e.g. ({ onLoadArgs, snapshot, props }: { onLoadArgs: OnloadArgs; snapshot?: SettingsSnapshot; props?: DiscourseFloatingMenuProps }). Update the single call site in apps/roam/src/index.ts:158 to pass an object instead of positional arguments.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
622
to
626
| export const mountLeftSidebar = async ( | ||
| wrapper: HTMLElement, | ||
| onloadArgs: OnloadArgs, | ||
| initialSnapshot?: SettingsSnapshot, | ||
| ): Promise<void> => { |
Contributor
There was a problem hiding this comment.
🔴 AGENTS.md violation: mountLeftSidebar has >2 positional parameters without object destructuring
The mountLeftSidebar function was changed from 2 to 3 positional parameters (wrapper, onloadArgs, initialSnapshot?), violating the AGENTS.md rule: "Use named parameters (object destructuring) when a function has more than 2 parameters." This rule is explicitly stated in the repository's style guide at AGENTS.md. Call sites at apps/roam/src/index.ts:184 and apps/roam/src/utils/initializeObserversAndListeners.ts:348 would need to be updated accordingly.
Prompt for agents
The function mountLeftSidebar in apps/roam/src/components/LeftSidebarView.tsx now has 3 parameters (wrapper, onloadArgs, initialSnapshot) which violates the AGENTS.md rule requiring object destructuring for functions with more than 2 parameters. Refactor the signature to use a single object parameter with destructuring, e.g. ({ wrapper, onloadArgs, initialSnapshot }: { wrapper: HTMLElement; onloadArgs: OnloadArgs; initialSnapshot?: SettingsSnapshot }). Update all call sites: apps/roam/src/index.ts:184, apps/roam/src/utils/initializeObserversAndListeners.ts:348.
Was this helpful? React with 👍 or 👎 to provide feedback.
…ional 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.
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.
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
1 participant
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.
Uh oh!
There was an error while loading. Please reload this page.