Skip to content

ENG-1188: prod: pull watchers for prop based settings#682

Merged
sid597 merged 4 commits intomainfrom
eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod
Feb 6, 2026
Merged

ENG-1188: prod: pull watchers for prop based settings#682
sid597 merged 4 commits intomainfrom
eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod

Conversation

@sid597
Copy link
Collaborator

@sid597 sid597 commented Jan 11, 2026

https://www.loom.com/share/609bba5ec2b54b2aad94d4c09a4921ce

Summary by CodeRabbit

Release Notes

  • Refactor
    • Introduced reactive watchers to detect and respond to changes in feature flags, global settings, personal settings, and discourse node pages.
    • Streamlined system initialization and removed debug diagnostic output for cleaner runtime behavior.
    • System now dynamically handles settings modifications with customizable handlers for different setting types.

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


Open with Devin

@linear
Copy link

linear bot commented Jan 11, 2026

@supabase
Copy link

supabase bot commented Jan 11, 2026

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 sid597 changed the title pull watchers for prop settings ENG-1188: prod: pull watchers for prop based settings Jan 11, 2026
@sid597 sid597 marked this pull request as ready for review January 11, 2026 06:32
@sid597 sid597 force-pushed the eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod branch from f11b47f to 10445e6 Compare January 11, 2026 19:54
@sid597 sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch from 3d7e4c7 to fb87a45 Compare January 11, 2026 19:54
@sid597 sid597 force-pushed the eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod branch from 10445e6 to bd6a3cf Compare January 12, 2026 15:26
@sid597 sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch from 3fbeb9f to 6cf85ea Compare January 17, 2026 18:16
@sid597 sid597 force-pushed the eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod branch from bd6a3cf to 1cdc812 Compare January 17, 2026 18:16
@sid597 sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch from 6cf85ea to c5ec51b Compare January 18, 2026 05:25
@sid597 sid597 force-pushed the eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod branch 2 times, most recently from b206a69 to 9f56c90 Compare January 19, 2026 04:51
@sid597 sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch 2 times, most recently from d511ae0 to 8c48aa9 Compare January 19, 2026 05:01
@sid597 sid597 force-pushed the eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod branch from 9f56c90 to a7867cb Compare January 19, 2026 05:01
@sid597 sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch from 8c48aa9 to e6b3b29 Compare January 19, 2026 06:03
@sid597 sid597 force-pushed the eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod branch from a7867cb to 464eb1a Compare January 19, 2026 06:03
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

📝 Walkthrough

Walkthrough

The pull request removes debug stubs and implements a new pull-watch system to observe and react to changes in Roam block properties for settings-related blocks and discourse nodes. The system provides handler registries for feature flags, global settings, personal settings, and discourse node changes, with setup and cleanup utilities.

Changes

Cohort / File(s) Summary
Stub Code Removal
apps/roam/src/components/settings/utils/accessors.ts, apps/roam/src/components/settings/utils/init.ts
Removed stubGetLeftSidebarPersonalSections getter function and its debug console logging. Removed printAllSettings diagnostic function and related imports; simplified initialization to only invoke stubSetLeftSidebarPersonalSections after 2-second delay.
Pull-Watch Infrastructure
apps/roam/src/components/settings/utils/pullWatchers.ts
New module introducing a pull-watch system with handler registries for feature flags, global settings, and personal settings. Provides setupPullWatchOnSettingsPage() and setupPullWatchDiscourseNodes() setup functions with cleanup callbacks, plus utility functions for property normalization and change detection using zod validation.
Main Integration
apps/roam/src/index.ts
Integrated pull-watch system into initialization flow by destructuring blockUids and nodePageUids from initSchema, setting up watchers with their cleanup callbacks, and invoking cleanup functions during unload.

Sequence Diagram

sequenceDiagram
    participant Init as Initialization Flow
    participant PullWatch as Pull-Watch System
    participant RoamAPI as Roam Block API
    participant Handlers as Handler Registries
    participant Cleanup as Cleanup

    Init->>PullWatch: setupPullWatchOnSettingsPage(blockUids)
    activate PullWatch
    PullWatch->>RoamAPI: Register watchers on settings blocks
    PullWatch->>PullWatch: Return cleanup callback
    deactivate PullWatch
    
    Init->>PullWatch: setupPullWatchDiscourseNodes(nodePageUids)
    activate PullWatch
    PullWatch->>RoamAPI: Register watchers on discourse nodes
    PullWatch->>PullWatch: Return cleanup callback
    deactivate PullWatch

    RoamAPI->>PullWatch: Block property change detected
    activate PullWatch
    PullWatch->>PullWatch: Normalize props & validate with zod
    PullWatch->>Handlers: Invoke registered handler<br/>(newValue, oldValue, allSettings)
    deactivate PullWatch

    Init->>Cleanup: On unload: invoke cleanup callbacks
    activate Cleanup
    Cleanup->>RoamAPI: Unregister all watchers
    deactivate Cleanup
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: introducing pull watchers for property-based settings, which is the core focus of the changeset across multiple files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 sid597 requested a review from mdroidian January 23, 2026 19:05
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional flags.

Open in Devin Review

Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff!

Mostly a lot of DRY code can be removed, and we can use roamjs-component definitions.

@sid597 sid597 changed the base branch from eng-1187-implement-get-and-set-setttings-as-block-properties-prod to graphite-base/682 January 28, 2026 09:53
@sid597 sid597 force-pushed the graphite-base/682 branch from 67cb84e to b40fcdc Compare January 28, 2026 18:08
@sid597 sid597 force-pushed the eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod branch from d29fb40 to 0dab0a5 Compare January 28, 2026 18:08
@sid597 sid597 changed the base branch from graphite-base/682 to eng-1187-implement-get-and-set-setttings-as-block-properties-prod January 28, 2026 18:08
@sid597 sid597 requested review from mdroidian and removed request for mdroidian January 28, 2026 18:09
Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, a few cleanup comments.

It isn't clear what path we are taking with all of these branches to get to main, so make sure not to merge code that makes changes to users graphs (eg: discourse node init) to main.

@sid597 sid597 changed the base branch from eng-1187-implement-get-and-set-setttings-as-block-properties-prod to graphite-base/682 February 2, 2026 09:21
@sid597 sid597 force-pushed the graphite-base/682 branch from b40fcdc to e11e431 Compare February 2, 2026 09:22
@sid597 sid597 force-pushed the eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod branch from 0dab0a5 to 34e7587 Compare February 2, 2026 09:22
@sid597 sid597 changed the base branch from graphite-base/682 to eng-1187-implement-get-and-set-setttings-as-block-properties-prod February 2, 2026 09:22
@sid597 sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch from e11e431 to 0cf76d1 Compare February 4, 2026 17:56
@sid597 sid597 force-pushed the eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod branch from 34e7587 to 74fabe6 Compare February 4, 2026 17:56
@sid597 sid597 changed the base branch from eng-1187-implement-get-and-set-setttings-as-block-properties-prod to graphite-base/682 February 5, 2026 15:11
@sid597 sid597 force-pushed the eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature-prod branch from 74fabe6 to 36fe060 Compare February 6, 2026 12:23
@sid597 sid597 force-pushed the graphite-base/682 branch from 0cf76d1 to a35a194 Compare February 6, 2026 12:23
@sid597 sid597 changed the base branch from graphite-base/682 to main February 6, 2026 12:23
@sid597 sid597 merged commit 67f9604 into main Feb 6, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants