ENG-1112: add suggestive mode flag and settings to admin panel #589
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
sid597
changed the title
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis PR consolidates Suggestive Mode UI configuration into AdminPanel by adding it as a new tab with memoized config retrieval, while removing the Suggestive Mode UI panels from GeneralSettings, HomePersonalSettings, and Settings components. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRsPre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/roam/src/components/settings/AdminPanel.tsx (1)
333-337: Config-driven Suggestive Mode FlagPanel wiring looks correct; consider small ergonomics tweaksDeriving
settingsviarefreshConfigTree()+getFormattedConfigTree()and feedingsettings.suggestiveModeEnabledintoFlagPanelis a good fit for using the Roam config page as the single source of truth. This should keep the admin UI in sync with the underlying config bullets.Two small, optional follow-ups:
- If
refreshConfigTreeever becomes asynchronous or more expensive, consider moving this intouseEffectplus auseState-backedsettingsvalue instead of doing a side effect insideuseMemo.- You could use
value={settings.suggestiveModeEnabled.value ?? false}to be explicit about treatingundefinedas the only “fallback-to-false” case.Please double-check
refreshConfigTree’s implementation to ensure it is safe to call synchronously on render and that it doesn’t rely on side effects that must be awaited before reading fromconfigTreeRef.Also applies to: 340-347
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/roam/src/components/settings/AdminPanel.tsx(4 hunks)apps/roam/src/components/settings/GeneralSettings.tsx(0 hunks)apps/roam/src/components/settings/HomePersonalSettings.tsx(2 hunks)apps/roam/src/components/settings/Settings.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- apps/roam/src/components/settings/GeneralSettings.tsx
- apps/roam/src/components/settings/Settings.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/roam/src/components/settings/AdminPanel.tsxapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**/*.{js,ts,tsx,jsx,json}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Prefer existing dependencies from package.json when working on the Roam Research extension
Files:
apps/roam/src/components/settings/AdminPanel.tsxapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**/*.{ts,tsx,jsx,js,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Files:
apps/roam/src/components/settings/AdminPanel.tsxapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Files:
apps/roam/src/components/settings/AdminPanel.tsxapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Implement the Discourse Graph protocol in the Roam Research extension
Files:
apps/roam/src/components/settings/AdminPanel.tsxapps/roam/src/components/settings/HomePersonalSettings.tsx
🧠 Learnings (4)
📚 Learning: 2025-08-27T02:23:45.696Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 377
File: packages/ui/tsconfig.json:5-7
Timestamp: 2025-08-27T02:23:45.696Z
Learning: When reviewing TypeScript configuration changes, check if settings like "declaration" are inherited from base configs before suggesting to add them explicitly. Many monorepo packages extend base configurations that already include necessary compiler options.
Applied to files:
apps/roam/src/components/settings/AdminPanel.tsx
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,jsx,js,css,scss} : Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Applied to files:
apps/roam/src/components/settings/AdminPanel.tsxapps/roam/src/components/settings/HomePersonalSettings.tsx
📚 Learning: 2025-06-19T19:43:43.380Z
Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 226
File: apps/roam/src/components/settings/HomePersonalSettings.tsx:123-149
Timestamp: 2025-06-19T19:43:43.380Z
Learning: The "Fetch Embeddings for nodes" button in HomePersonalSettings.tsx is for testing purposes only, so it doesn't require production-level error handling or user feedback improvements.
Applied to files:
apps/roam/src/components/settings/HomePersonalSettings.tsx
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Applied to files:
apps/roam/src/components/settings/HomePersonalSettings.tsx
🧬 Code graph analysis (1)
apps/roam/src/components/settings/AdminPanel.tsx (1)
apps/roam/src/utils/discourseConfigRef.ts (1)
getFormattedConfigTree(51-86)
🔇 Additional comments (3)
apps/roam/src/components/settings/HomePersonalSettings.tsx (1)
83-110: Commented-out Suggestive Mode Overlay control matches the new admin-only flowWrapping this Checkbox in a JSX block comment cleanly hides the per-user Suggestive Mode overlay toggle while preserving the implementation and adding a clear TODO for reintroduction later. No functional or structural issues here.
apps/roam/src/components/settings/AdminPanel.tsx (2)
1-2: New Suggestive Mode imports are consistent and fully utilizedAll five imports—
useMemo,FlagPanel,SuggestiveModeSettings,getFormattedConfigTree, andrefreshConfigTree—are present and actively used in the file. FlagPanel usage is consistent with patterns across the codebase (multiple components use it with the same title/description/value prop structure), and SuggestiveModeSettings is properly imported and rendered as a component at line 424.
420-425: New "Suggestive Mode" admin tab is wired correctly and properly isolatedThe Tab component in AdminPanel.tsx correctly follows the existing Tabs pattern. Verification confirms that
SuggestiveModeSettingsis now exclusively imported and used in AdminPanel.tsx (lines 33, 424), with no competing references in Settings.tsx or other settings entry points. The refactoring successfully makes Suggestive Mode configuration admin-only. The overflow-y-auto class appropriately handles potentially tall content.
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
A few changes.
Also: the "suggestive mode" in the nodes tab should only show up if the suggestive mode flag is on.
|
|
||
| return ( | ||
| <div className="flex flex-col gap-4 p-4"> | ||
| <FlagPanel |
There was a problem hiding this comment.
There needs to be an explicit toast when this is turned on that notifies them that their data (nodes) will be sent to our servers and open ai servers
add confirmation that you are sending data to our server
There was a problem hiding this comment.
Do we need this now? I thought this will be enabled in pilots meetings with someone present from our side
There was a problem hiding this comment.
It is possible for any user to inspect the code and access this. So yes, let's add it. It is a small effort and high reward and will be useful long term.
| </div> | ||
| } | ||
| /> | ||
| <Tab |
There was a problem hiding this comment.
This should only show up if the flag is on
Summary by CodeRabbit
New Features
Revert
✏️ Tip: You can customize this high-level summary in your review settings.