[codex] move v2 toggle to experimental settings#3748
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRemoved the TopBar version toggle and fully deleted the VersionToggle component. Added a new "Experimental" settings page and ExperimentalSettings component (with V2 toggle) integrated into routing, search, and settings state. Replaced the v2-local-override store's Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant UI as "ExperimentalSettings UI"
participant Flags as "useIsV2CloudEnabled (feature flag)"
participant Store as "v2-local-override store"
User->>UI: Open Experimental settings page
UI->>Flags: read isV2CloudEnabled, isRemoteV2Enabled
Flags-->>UI: return enablement state
UI->>User: render toggle (enabled/disabled) and messages
User->>UI: toggle switch
UI->>Store: call setForceV1(!currentForceV1)
Store-->>UI: updated forceV1 state
UI->>User: reflect new toggle state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR moves the Superset V2 toggle from the dashboard top bar into a new Settings > Experimental page, making the feature more discoverable as a persistent setting rather than an ephemeral toolbar button. All wiring — sidebar entry, route registration, section ordering, search metadata, and the Confidence Score: 5/5Safe to merge — all changes are consistent with existing patterns and the only finding is a minor dead-code cleanup. All P2 findings. The sole issue is the now-unused toggle() method left in the store after VersionToggle was deleted. Every other aspect of the implementation — route registration, sidebar wiring, search metadata, store action, and switch logic — is correct and mirrors existing section patterns. apps/desktop/src/renderer/stores/v2-local-override.ts — unused toggle method can be removed.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/ExperimentalSettings.tsx | New component exposing the V2 toggle as a Switch in the Experimental settings section; logic correctly mirrors the old VersionToggle behavior using setForceV1. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx | Removes VersionToggle import and its render site; also drops the now-unused isRemoteV2Enabled destructure from useIsV2CloudEnabled. |
| apps/desktop/src/renderer/stores/v2-local-override.ts | Adds setForceV1 action; toggle() is now dead code with no callers after VersionToggle was deleted. |
| apps/desktop/src/renderer/routes/_authenticated/settings/experimental/page.tsx | New TanStack Router page for /settings/experimental, consistent with the pattern used by other settings pages. |
| apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx | Registers 'experimental' in SECTION_ORDER, getSectionFromPath, and getPathFromSection — fully consistent with other sections. |
| apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts | Adds EXPERIMENTAL_SUPERSET_V2 item ID and its SETTINGS_ITEMS entry with appropriate keywords for search. |
| apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/GeneralSettings.tsx | Adds Experimental sidebar entry after Models using HiOutlineBeaker icon; route type union and section group updated correctly. |
| apps/desktop/src/renderer/stores/settings-state.ts | Adds 'experimental' to the SettingsSection union type. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User opens Settings] --> B[SettingsSidebar]
B --> C[Experimental entry\n/settings/experimental]
C --> D[ExperimentalSettingsPage]
D --> E{searchQuery?}
E -- yes --> F[getMatchingItemsForSection\n'experimental']
E -- no --> G[visibleItems = null\nshow all]
F --> H[ExperimentalSettings\nvisibleItems=filtered]
G --> H
H --> I{isRemoteV2Enabled?}
I -- no --> J[Switch disabled\nchecked=false]
I -- yes --> K[Switch enabled\nchecked=isV2CloudEnabled]
K --> L{onCheckedChange}
L -- enable --> M[setForceV1 false]
L -- disable --> N[setForceV1 true]
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/stores/v2-local-override.ts
Line: 4-9
Comment:
**Unused `toggle` function is now dead code**
The `toggle()` method was only called from the deleted `VersionToggle.tsx` component. Now that `VersionToggle` is removed and `ExperimentalSettings` uses `setForceV1` directly, `toggle` has no remaining callers and can be removed from both the interface and the store implementation.
```suggestion
interface V2LocalOverrideState {
/** When true, forces v1 mode locally even though v2 is enabled remotely. */
forceV1: boolean;
setForceV1: (forceV1: boolean) => void;
}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "move v2 toggle to experimental settings" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
4
to
9
| interface V2LocalOverrideState { | ||
| /** When true, forces v1 mode locally even though v2 is enabled remotely. */ | ||
| forceV1: boolean; | ||
| setForceV1: (forceV1: boolean) => void; | ||
| toggle: () => void; | ||
| } |
There was a problem hiding this comment.
toggle function is now dead code
The toggle() method was only called from the deleted VersionToggle.tsx component. Now that VersionToggle is removed and ExperimentalSettings uses setForceV1 directly, toggle has no remaining callers and can be removed from both the interface and the store implementation.
Suggested change
| interface V2LocalOverrideState { | |
| /** When true, forces v1 mode locally even though v2 is enabled remotely. */ | |
| forceV1: boolean; | |
| setForceV1: (forceV1: boolean) => void; | |
| toggle: () => void; | |
| } | |
| interface V2LocalOverrideState { | |
| /** When true, forces v1 mode locally even though v2 is enabled remotely. */ | |
| forceV1: boolean; | |
| setForceV1: (forceV1: boolean) => void; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/stores/v2-local-override.ts
Line: 4-9
Comment:
**Unused `toggle` function is now dead code**
The `toggle()` method was only called from the deleted `VersionToggle.tsx` component. Now that `VersionToggle` is removed and `ExperimentalSettings` uses `setForceV1` directly, `toggle` has no remaining callers and can be removed from both the interface and the store implementation.
```suggestion
interface V2LocalOverrideState {
/** When true, forces v1 mode locally even though v2 is enabled remotely. */
forceV1: boolean;
setForceV1: (forceV1: boolean) => void;
}
```
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
This was referenced Apr 26, 2026
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 26, 2026
3 PR (#442, #443, #444) で取り込み済みの 9 commits を git 履歴上もマージ済みに記録する。 内容差分は無し (merge -s ours)。 取り込み内容: - 6b96acd Improve sidebar group management UX (superset-sh#3745) - a4079e7 Update dashboard sidebar workspace icons (superset-sh#3755) - d3753d0 [codex] Use dynamic footer copyright years (superset-sh#3754) - ce606be Handle browser passthrough during v2 resize (superset-sh#3744) - b1e1eb7 Refactor v2 workspace page (superset-sh#3747) - 8693869 [codex] move v2 toggle to experimental settings (superset-sh#3748) - ef3f381 Revert "fix(desktop): refit v2 terminal after font settle (superset-sh#3742)" (superset-sh#3750) - 手動移植 (vibrancy patch 維持) - 25b2d52 Show terminal sessions from all workspaces in dropdown (superset-sh#3751) - 62737db fix v1 terminal resize repaint (superset-sh#3756)
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.
Summary
Validation
bun run --cwd apps/desktop typecheckbunx @biomejs/biome@2.4.2 check apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/GeneralSettings.tsx apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts apps/desktop/src/renderer/stores/settings-state.ts apps/desktop/src/renderer/stores/v2-local-override.ts apps/desktop/src/renderer/routes/_authenticated/settings/experimental/page.tsx apps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/ExperimentalSettings.tsx apps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/index.tsSummary by cubic
Moved the Superset V2 switch from the dashboard top bar to Settings > System > Experimental to centralize control and declutter the UI. Keeps the local override behavior via
setForceV1, adds settings search, and removes the unused top bar toggle and references.New Features
Refactors
VersionToggleand deleted its component files.isRemoteV2Enabled.togglewithsetForceV1inuseV2LocalOverrideStoreand wired the switch to it.Written for commit fca3582. Summary will update on new commits.
Summary by CodeRabbit
New Features
UI Changes