Conversation
- Delete workspace-view-mode store and ViewModeToggleCompact component - Remove view mode toggle from WorkspaceControls - Simplify ContentView, Sidebar, and WorkspaceView by removing review mode branches - Always show workbench layout with GroupStrip and tabs
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR removes the per-workspace view mode system that toggled between "workbench" and "review" modes. It eliminates the workspace-view-mode store, ViewModeToggleCompact component, and all mode-dependent rendering logic. The sidebar is repositioned as a resizable panel within TabsContent (for the changes view), while new BasePaneWindow and PaneToolbarActions components centralize pane management UI for file viewers and terminal panes. Hotkey bindings are realigned to reflect new shortcuts. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
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/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsx (1)
3-9: LGTM! Clean simplification of component props and render logic.The removal of
workspaceIdand the simplified render guard align well with the PR objective to eliminate per-workspace view mode state.
Minor: Update comment for accuracy.
Line 8's comment mentions "no active workspace" but the guard only checks
worktreePath. Consider updating to:// Don't render if no worktree pathfor precision.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/ViewModeToggleCompact.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/stores/index.tsapps/desktop/src/renderer/stores/workspace-view-mode.ts
💤 Files with no reviewable changes (3)
- apps/desktop/src/renderer/stores/index.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/ViewModeToggleCompact.tsx
- apps/desktop/src/renderer/stores/workspace-view-mode.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
🧬 Code graph analysis (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsx (1)
WorkspaceControls(7-16)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx (1)
apps/desktop/src/shared/changes-types.ts (2)
ChangedFile(22-28)ChangeCategory(15-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (5)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsx (1)
11-15: LGTM! ViewModeToggleCompact correctly removed.This change successfully eliminates the view mode toggle UI component, which is the core objective of this PR. The component now renders only the OpenInMenuButton with proper layout and styling.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx (2)
12-12: LGTM! Prop simplification aligns with component updates.The removal of the
workspaceIdprop from WorkspaceControls correctly aligns with the component's updated signature that now only requiresworktreePath. This simplification is consistent with removing per-workspace view mode logic.
21-21: LGTM! Unconditional rendering simplifies the layout.GroupStrip is now always rendered, eliminating the previous view mode conditional logic. This change consolidates the UI to the workbench-only layout as intended by the PR.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx (1)
64-73: LGTM! Dependency array correctly simplified.The removal of
viewModeandsetWorkspaceViewModefrom the dependency array is correct since the NEW_GROUP hotkey callback no longer includes view mode switching logic. The simplified dependency array now accurately reflects only the values used within the callback (activeWorkspaceIdandaddTab).apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx (1)
12-21: LGTM! Simplified file-open handling removes view mode dependency.The conditional definition of
handleFileOpenbased solely onworkspaceIdpresence correctly removes the previous view mode gating logic.ChangesViewproperly handles the optionalonFileOpenprop using optional chaining (onFileOpen?.(...)) when invoking it, ensuring graceful handling when undefined. The function signature matches the expected parameters for file viewing operations.
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx:
- Around line 11-16: WorkspaceControls is rendered without the required
worktreePath prop in ContentView (trailingAction) so it always returns null;
either remove the <WorkspaceControls /> element from the trailingAction fragment
to eliminate dead code, or pass the appropriate worktreePath prop (from the same
source used by TopBar/OpenInMenuButton) into WorkspaceControls so it can render;
locate the trailingAction JSX in ContentView (the fragment containing
<WorkspaceControls /> and <SidebarControl />) and apply the chosen change,
ensuring no unused imports or references to WorkspaceControls remain if you
remove it.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (1)
4-4: Consider using path alias for cleaner imports.The relative import path
../../../WorkspaceView/ResizableSidebartraverses multiple directory levels. As per the coding guidelines, prefer using the alias defined intsconfig.jsonwhen possible.Suggested change
-import { ResizableSidebar } from "../../../WorkspaceView/ResizableSidebar"; +import { ResizableSidebar } from "renderer/screens/main/components/WorkspaceView/ResizableSidebar";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/desktop/plans/done/20260103-0901-workspace-creation-streaming-ux.mdapps/desktop/plans/done/20260104-1916-restore-group-tabs-sidebar.mdapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ContentHeader/ContentHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ResizableSidebar/ResizableSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx
💤 Files with no reviewable changes (2)
- apps/desktop/plans/done/20260104-1916-restore-group-tabs-sidebar.md
- apps/desktop/plans/done/20260103-0901-workspace-creation-streaming-ux.md
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ContentHeader/ContentHeader.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ResizableSidebar/ResizableSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ResizableSidebar/ResizableSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ResizableSidebar/ResizableSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ResizableSidebar/ResizableSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ResizableSidebar/ResizableSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ResizableSidebar/ResizableSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/{ui,ai-elements,react-flow}/*.tsx : Use kebab-case single files for shadcn/ui components (e.g., button.tsx, base-node.tsx) in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsx
🧬 Code graph analysis (3)
apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/OpenInMenuButton.tsx (1)
OpenInMenuButton(33-210)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceInitializingView/WorkspaceInitializingView.tsx (1)
WorkspaceInitializingView(38-328)apps/desktop/src/main/lib/workspace-init-manager.ts (1)
isInitializing(38-45)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx (1)
ContentView(7-23)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx (3)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsx (1)
WorkspaceControls(7-16)apps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsx (1)
SidebarControl(7-38)apps/desktop/src/renderer/screens/main/components/SidebarControl/index.ts (1)
SidebarControl(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ResizableSidebar/ResizableSidebar.tsx (1)
36-37: LGTM! Drag direction correctly inverted for right-side sidebar.The calculation change from
currentX - startXtostartX - currentXcorrectly handles the inverted drag behavior for a left-edge resize handle. When the sidebar is positioned on the right side of the content area, dragging the handle leftward should increase width, which this implementation achieves.Also applies to: 90-91
apps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsx (1)
13-28: LGTM! Good accessibility and discoverability improvements.The addition of visible "Changes" text alongside the icon improves discoverability, while the dynamic
aria-labelprovides appropriate context for screen readers based on the current sidebar state.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (2)
28-32: LGTM! Proper flex layout with overflow prevention.The
min-w-0class on the TabView container correctly prevents flex children from overflowing their container. The two-column layout cleanly separates the main content from the resizable sidebar.
23-33: Sidebar intentionally hidden when no tabs exist.The
EmptyTabViewcomponent is designed as a standalone empty state UI without sidebar content. TheResizableSidebaris only rendered alongsideTabViewwhen an active tab exists, which is the correct behavior—the sidebar has no meaningful content to display without an active tab.No action needed.
apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx (1)
35-37: LGTM! Appropriate conditional rendering.The
OpenInMenuButtonis correctly rendered only whenactiveWorkspace?.worktreePathexists, ensuring the button only appears for workspaces that have an associated worktree path.apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx (2)
63-72: LGTM! Simplified hotkey dependencies.The
NEW_GROUPhotkey now correctly depends only onactiveWorkspaceIdandaddTab, removing the previously required view mode dependencies as part of this refactor.
171-185: LGTM! Clean layout simplification.The layout has been simplified to a single content area with clear conditional rendering between
WorkspaceInitializingViewandContentView. The removal of the two-column layout withResizableSidebarat this level correctly delegates that responsibility toTabsContent.
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In
@apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsx:
- Around line 5-8: The trpc.workspaces.getActive.useQuery call currently ignores
errors; update the query invocation in WorkspaceControls (the useQuery that
returns data: activeWorkspace) to handle failures by either passing an onError
handler or destructuring the error from the hook and logging it; log using the
prefixed context pattern (e.g. "[workspaces/getActive] <message>") and include
the error object and any relevant contextual info (like user id or params) so
errors are not silently swallowed, then keep the existing worktreePath null
check/early return logic unchanged.
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsx:
- Line 55: PaneToolbarActions is using the terminal-specific hotkey id
CLOSE_TERMINAL in the HotkeyTooltipContent, but this component is generic; add a
new generic hotkey id CLOSE_PANE to the hotkeys definitions (hotkeys.ts) and
replace the hardcoded hotkeyId="CLOSE_TERMINAL" in PaneToolbarActions with
hotkeyId="CLOSE_PANE", or alternatively make PaneToolbarActions accept a prop
(e.g., closeHotkeyId) and pass CLOSE_TERMINAL from TabPane.tsx and CLOSE_PANE
from FileViewerToolbar.tsx; update usages in TabPane.tsx and
FileViewerToolbar.tsx accordingly to ensure terminal-specific semantics remain
when needed.
🧹 Nitpick comments (4)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsx (1)
5-5: Consider handling the loading state for better UX.While the component currently returns
nullduring loading (whendatais undefined), this could cause a brief flicker when the query resolves. Consider explicitly handling the loading state to provide a smoother user experience.🔎 Optional enhancement to handle loading state
- const { data: activeWorkspace, error } = trpc.workspaces.getActive.useQuery(); + const { data: activeWorkspace, error, isLoading } = trpc.workspaces.getActive.useQuery(); const worktreePath = activeWorkspace?.worktreePath; if (error) { console.error("[WorkspaceControls] Failed to fetch active workspace:", error); return null; } + if (isLoading) { + // Return null or a loading skeleton as appropriate + return null; + } if (!worktreePath) return null;apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsx (1)
65-70: Consider memoizing the handlers object to prevent unnecessary toolbar re-renders.The
handlersobject is recreated on every render, causingrenderToolbarto receive a new reference each time. This may trigger unnecessary re-renders of the toolbar component.🔎 Proposed optimization using useMemo
+import { useRef, useMemo } from "react"; -import { useRef } from "react"; export function BasePaneWindow({ paneId, path, tabId, isActive, splitPaneAuto, removePane, setFocusedPane, renderToolbar, children, contentClassName = "w-full h-full overflow-hidden", }: BasePaneWindowProps) { const containerRef = useRef<HTMLDivElement>(null); const splitOrientation = useSplitOrientation(containerRef); const handleFocus = () => { setFocusedPane(tabId, paneId); }; const handleClosePane = (e: React.MouseEvent) => { e.stopPropagation(); removePane(paneId); }; const handleSplitPane = (e: React.MouseEvent) => { e.stopPropagation(); const container = containerRef.current; if (!container) return; const { width, height } = container.getBoundingClientRect(); splitPaneAuto(tabId, paneId, { width, height }, path); }; - const handlers: PaneHandlers = { + const handlers: PaneHandlers = useMemo(() => ({ onFocus: handleFocus, onClosePane: handleClosePane, onSplitPane: handleSplitPane, splitOrientation, - }; + }), [tabId, paneId, splitOrientation]);apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx (2)
132-168: Consider extracting shared pane-update logic.Both
handleToggleLockandswitchToModefollow the same pattern: get panes, check if current pane exists, spread and update. This could be consolidated into a helper to reduce duplication.🔎 Proposed refactor
+const updatePaneFileViewer = ( + updates: Partial<NonNullable<Pane["fileViewer"]>>, +) => { + const panes = useTabsStore.getState().panes; + const currentPane = panes[paneId]; + if (currentPane?.fileViewer) { + useTabsStore.setState({ + panes: { + ...panes, + [paneId]: { + ...currentPane, + fileViewer: { + ...currentPane.fileViewer, + ...updates, + }, + }, + }, + }); + } +}; + const handleToggleLock = () => { - const panes = useTabsStore.getState().panes; - const currentPane = panes[paneId]; - if (currentPane?.fileViewer) { - useTabsStore.setState({ - panes: { - ...panes, - [paneId]: { - ...currentPane, - fileViewer: { - ...currentPane.fileViewer, - isLocked: !currentPane.fileViewer.isLocked, - }, - }, - }, - }); - } + const currentPane = useTabsStore.getState().panes[paneId]; + if (currentPane?.fileViewer) { + updatePaneFileViewer({ isLocked: !currentPane.fileViewer.isLocked }); + } }; const switchToMode = (newMode: FileViewerMode) => { - const panes = useTabsStore.getState().panes; - const currentPane = panes[paneId]; - if (currentPane?.fileViewer) { - useTabsStore.setState({ - panes: { - ...panes, - [paneId]: { - ...currentPane, - fileViewer: { - ...currentPane.fileViewer, - viewMode: newMode, - }, - }, - }, - }); - } + updatePaneFileViewer({ viewMode: newMode }); };
255-273: Redundant wrapper div around FileViewerToolbar.The
<div className="flex h-full w-full">wrapper appears unnecessary sinceFileViewerToolbaralready renders its own<div className="flex h-full w-full items-center justify-between px-2">container (per the relevant code snippet). This creates nested flex containers with the same dimensions.🔎 Proposed simplification
- renderToolbar={(handlers) => ( - <div className="flex h-full w-full"> - <FileViewerToolbar - fileName={fileName} - isDirty={isDirty} - isSaving={isSaving} - viewMode={viewMode} - isLocked={isLocked} - isMarkdown={isMarkdown} - hasDiff={hasDiff} - showEditableBadge={showEditableBadge} - splitOrientation={handlers.splitOrientation} - onViewModeChange={handleViewModeChange} - onSplitPane={handlers.onSplitPane} - onToggleLock={handleToggleLock} - onClosePane={handlers.onClosePane} - /> - </div> - )} + renderToolbar={(handlers) => ( + <FileViewerToolbar + fileName={fileName} + isDirty={isDirty} + isSaving={isSaving} + viewMode={viewMode} + isLocked={isLocked} + isMarkdown={isMarkdown} + hasDiff={hasDiff} + showEditableBadge={showEditableBadge} + splitOrientation={handlers.splitOrientation} + onViewModeChange={handleViewModeChange} + onSplitPane={handlers.onSplitPane} + onToggleLock={handleToggleLock} + onClosePane={handlers.onClosePane} + /> + )}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/hooks/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/hooks/useSplitOrientation/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/hooks/useSplitOrientation/useSplitOrientation.ts
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/index.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/hooks/useSplitOrientation/index.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/{ui,ai-elements,react-flow}/*.tsx : Use kebab-case single files for shadcn/ui components (e.g., button.tsx, base-node.tsx) in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsx
🧬 Code graph analysis (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsx (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsx (1)
BasePaneWindow(32-89)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/index.ts (2)
BasePaneWindow(1-1)PaneToolbarActions(2-2)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsx (1)
PaneToolbarActions(14-60)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsx (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsx (1)
PaneToolbarActions(14-60)packages/ui/src/components/ui/tooltip.tsx (3)
Tooltip(76-76)TooltipTrigger(76-76)TooltipContent(76-76)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsx (1)
BasePaneWindow(32-89)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsx (1)
FileViewerToolbar(29-113)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/UnsavedChangesDialog.tsx (1)
UnsavedChangesDialog(22-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsx (2)
10-11: LGTM! Clean import refactor.The updated imports correctly reflect the component extraction. Moving icon imports to
PaneToolbarActionsimproves encapsulation and reduces coupling.
83-109: Refactor correctly typed and follows DRY principles.The extraction of toolbar actions into
PaneToolbarActionsis a solid refactor that improves code reusability. The type signatures match: bothFileViewerToolbarPropsandPaneToolbarActionsPropscorrectly typeonSplitPaneandonClosePaneas(e: React.MouseEvent) => void, ensuring proper event handling. The lock toggle is appropriately preserved in theleadingActionsslot.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsx (1)
78-117: LGTM! Clean refactoring to BasePaneWindow.The migration from
MosaicWindowtoBasePaneWindowsuccessfully consolidates pane management logic and improves code organization. TherenderToolbarcallback pattern correctly delegates toolbar rendering toPaneToolbarActions, and the rename fromcontainerReftoterminalContainerRefimproves semantic clarity.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx (4)
7-7: LGTM!The import correctly brings in
BasePaneWindowfrom the components barrel, aligning with the refactoring to the new pane composition pattern.
183-215: LGTM!The async save-and-switch handler has proper error handling with contextual logging (
[FileViewerPane] Save failed:), correct state cleanup infinally, and appropriate early return guard. The separation between raw and diff save paths is clear.
113-130: LGTM!The fallback case correctly uses
BasePaneWindowwith all required props, providing a minimal toolbar placeholder and appropriate user feedback message.
14-41: LGTM!The props interface is well-structured with clear typing. The component correctly receives all necessary props for the
BasePaneWindowintegration, including the new pane management callbacks (splitPaneAuto,removePane,setFocusedPane).
| const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery(); | ||
| const worktreePath = activeWorkspace?.worktreePath; | ||
|
|
||
| export function WorkspaceControls({ | ||
| workspaceId, | ||
| worktreePath, | ||
| }: WorkspaceControlsProps) { | ||
| // Don't render if no active workspace with a worktree path | ||
| if (!workspaceId || !worktreePath) return null; | ||
| if (!worktreePath) return null; |
There was a problem hiding this comment.
Add error handling for the tRPC query.
The query result doesn't handle errors, which violates the coding guideline: "Never swallow errors silently - at minimum log them with context". If the query fails, the error will be silently ignored.
🔎 Proposed fix to add error logging
- const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery();
+ const { data: activeWorkspace, error } = trpc.workspaces.getActive.useQuery();
const worktreePath = activeWorkspace?.worktreePath;
- if (!worktreePath) return null;
+ if (error) {
+ console.error("[WorkspaceControls] Failed to fetch active workspace:", error);
+ return null;
+ }
+ if (!worktreePath) return null;Based on coding guidelines: "Never swallow errors silently - at minimum log them with context" and "Use prefixed console logging with context pattern: [domain/operation] message".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery(); | |
| const worktreePath = activeWorkspace?.worktreePath; | |
| export function WorkspaceControls({ | |
| workspaceId, | |
| worktreePath, | |
| }: WorkspaceControlsProps) { | |
| // Don't render if no active workspace with a worktree path | |
| if (!workspaceId || !worktreePath) return null; | |
| if (!worktreePath) return null; | |
| const { data: activeWorkspace, error } = trpc.workspaces.getActive.useQuery(); | |
| const worktreePath = activeWorkspace?.worktreePath; | |
| if (error) { | |
| console.error("[WorkspaceControls] Failed to fetch active workspace:", error); | |
| return null; | |
| } | |
| if (!worktreePath) return null; |
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceControls/WorkspaceControls.tsx
around lines 5-8, The trpc.workspaces.getActive.useQuery call currently ignores
errors; update the query invocation in WorkspaceControls (the useQuery that
returns data: activeWorkspace) to handle failures by either passing an onError
handler or destructuring the error from the hook and logging it; log using the
prefixed context pattern (e.g. "[workspaces/getActive] <message>") and include
the error object and any relevant contextual info (like user id or params) so
errors are not silently swallowed, then keep the existing worktreePath null
check/early return logic unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @apps/desktop/src/renderer/index.tsx:
- Around line 14-17: Add an inline comment above the AppProviders JSX explaining
that React.StrictMode was intentionally removed to avoid Monaco editor
double-rendering/compatibility issues and that this trade-off disables some
development-time checks (like certain deprecation warnings and extra render
checks); reference React.StrictMode and Monaco so future maintainers understand
the rationale and can reconsider if Monaco issues are resolved.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/index.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/index.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/index.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/index.tsx
🧠 Learnings (4)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Applied to files:
apps/desktop/src/renderer/index.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/{ui,ai-elements,react-flow}/*.tsx : Use kebab-case single files for shadcn/ui components (e.g., button.tsx, base-node.tsx) in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/index.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/electron-router-dom.ts : Do not import Node.js modules like node:path or dotenv in electron-router-dom.ts and similar shared files - they run in both main and renderer processes
Applied to files:
apps/desktop/src/renderer/index.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/*.ts : Never import Node.js modules in shared code like electron-router-dom.ts - it runs in both main and renderer processes
Applied to files:
apps/desktop/src/renderer/index.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/index.tsx (5)
apps/desktop/src/renderer/contexts/AppProviders.tsx (1)
AppProviders(12-23)apps/desktop/src/renderer/contexts/index.ts (1)
AppProviders(5-5)apps/desktop/src/renderer/routes.tsx (1)
AppRoutes(71-79)apps/desktop/src/renderer/components/ThemedToaster/index.ts (1)
ThemedToaster(1-1)apps/desktop/src/renderer/components/ThemedToaster/ThemedToaster.tsx (1)
ThemedToaster(4-7)
🔇 Additional comments (1)
apps/desktop/src/renderer/index.tsx (1)
1-18: The automatic JSX transform is properly configured. Your TypeScript configuration usesjsx: react-jsx, you have React 19.2.3, and Vite is configured with the React plugin—all requirements are met for this pattern.
| <AppProviders> | ||
| <AppRoutes /> | ||
| <ThemedToaster /> | ||
| </AppProviders>, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's examine the full index.tsx file to understand the complete context
echo "=== Full content of index.tsx ==="
cat -n apps/desktop/src/renderer/index.tsx
echo -e "\n=== Searching for Monaco editor usage ==="
rg -i 'monaco' --type ts --type tsx -C 2
echo -e "\n=== Searching for comments about StrictMode ==="
rg 'StrictMode' -C 3 --type ts --type tsx
echo -e "\n=== Checking tsconfig.json for JSX configuration ==="
cat tsconfig.json | grep -A 10 '"jsx"' || echo "No jsx config found in tsconfig.json"
echo -e "\n=== Searching for any ADRs or docs about this decision ==="
fd -e md -e txt | xargs grep -l 'StrictMode\|Monaco' 2>/dev/null || echo "No docs found"Repository: superset-sh/superset
Length of output: 1014
🏁 Script executed:
#!/bin/bash
echo "=== Searching for Monaco editor imports with ripgrep (broader search) ==="
rg -i 'monaco' --type-list | head -20
rg -i 'monaco' -r '.' --max-count 50 2>/dev/null | head -50
echo -e "\n=== Searching for @monaco files ==="
fd -e ts -e tsx -e js -e jsx | xargs grep -l '@monaco\|monaco-editor' 2>/dev/null || echo "No @monaco imports found"
echo -e "\n=== Finding tsconfig.json location ==="
find . -name 'tsconfig.json' -type f 2>/dev/null | grep -v node_modules | head -10
echo -e "\n=== Check tsconfig in apps/desktop ==="
if [ -f "apps/desktop/tsconfig.json" ]; then
echo "Found apps/desktop/tsconfig.json:"
cat apps/desktop/tsconfig.json | grep -A 5 'jsx'
fi
echo -e "\n=== Searching for all references to StrictMode in entire codebase ==="
rg 'StrictMode' -i 2>/dev/null | head -20
echo -e "\n=== Check git log for this file to understand the change ==="
git log --oneline -n 20 apps/desktop/src/renderer/index.tsx 2>/dev/null || echo "Git history unavailable"
echo -e "\n=== Search for known Monaco-related issues in comments/PRs ==="
rg -i 'monaco.*strict\|strict.*monaco' 2>/dev/null || echo "No Monaco+StrictMode references found"
</parameter>Repository: superset-sh/superset
Length of output: 9110
🏁 Script executed:
#!/bin/bash
echo "=== Check AppProviders context ==="
cat -n apps/desktop/src/renderer/contexts/index.ts
echo -e "\n=== Check MonacoProvider implementation ==="
head -50 apps/desktop/src/renderer/contexts/MonacoProvider.tsx
echo -e "\n=== Search for any comments about Monaco + StrictMode issues in the codebase ==="
grep -r 'double.*render\|mount.*twice\|StrictMode' apps/desktop/src/renderer --include='*.ts' --include='*.tsx' -B 2 -A 2 2>/dev/null || echo "No related comments found"
echo -e "\n=== Check if there are other entry points that might have StrictMode ==="
fd -e tsx -e ts | xargs grep -l 'createRoot\|render\|ReactDOM' 2>/dev/null | head -10
</parameter>Repository: superset-sh/superset
Length of output: 3248
🏁 Script executed:
#!/bin/bash
echo "=== Check apps/desktop/tsconfig.json for JSX settings ==="
cat apps/desktop/tsconfig.json
echo -e "\n=== Check root tsconfig.json ==="
if [ -f "tsconfig.json" ]; then
cat tsconfig.json | head -40
fi
echo -e "\n=== Check package.json React version ==="
grep -A 2 '"react"' apps/desktop/package.json
echo -e "\n=== Check AppProviders to understand the component structure ==="
cat -n apps/desktop/src/renderer/contexts/AppProviders.tsx
</parameter>Repository: superset-sh/superset
Length of output: 1619
🏁 Script executed:
#!/bin/bash
echo "=== Check if extended TypeScript config has JSX settings ==="
find . -path ./node_modules -prune -o -name 'electron.json' -type f -print 2>/dev/null | xargs cat 2>/dev/null | head -50
echo -e "\n=== Verify React 19 and automatic JSX transform support ==="
grep -r 'jsxImportSource\|jsx.*react-jsx' apps/desktop --include='*.json' 2>/dev/null || echo "No explicit jsx import source found (uses default)"
echo -e "\n=== Search for any GitHub issues or comments referencing Monaco StrictMode ==="
find . -name '*.md' -type f | xargs grep -i 'monaco.*strict\|strict.*monaco' 2>/dev/null || echo "No documentation found"Repository: superset-sh/superset
Length of output: 702
🏁 Script executed:
#!/bin/bash
echo "=== Verify Monaco worker configuration in build ==="
grep -A 10 'monaco' apps/desktop/electron.vite.config.ts | head -20Repository: superset-sh/superset
Length of output: 314
Add a comment explaining the StrictMode removal for future maintainers.
The removal of React.StrictMode is intentional to resolve compatibility issues with the Monaco editor. To help future maintainers understand this decision, add an inline comment explaining the trade-off (e.g., why StrictMode's development-time checks were sacrificed for Monaco compatibility).
Example:
// Note: StrictMode removed to prevent Monaco editor double-rendering issues
// This disables some development-time safety checks (deprecated API warnings, etc.)
<AppProviders>🤖 Prompt for AI Agents
In @apps/desktop/src/renderer/index.tsx around lines 14-17, Add an inline
comment above the AppProviders JSX explaining that React.StrictMode was
intentionally removed to avoid Monaco editor double-rendering/compatibility
issues and that this trade-off disables some development-time checks (like
certain deprecation warnings and extra render checks); reference
React.StrictMode and Monaco so future maintainers understand the rationale and
can reconsider if Monaco issues are resolved.
Kitenite
changed the title
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
Summary by CodeRabbit
Release Notes
Removed Features
UI/UX Changes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.