perf(desktop): reduce polling, persistence, and render churn#2045
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a generic debounced-value hook, debounced/deduplicated tRPC storage writes with per-entry pending snapshots and TTL, multiple UI search debounces, RAF batching for resize updates, workspace-scoped state selection, granular store selectors, conditional polling based on UI activity, and several small optimizations and API tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Renderer UI
participant Hook as useDebouncedValue
participant Storage as TrpcStorage
participant Local as localStorage
participant TRPC as tRPC Service
User->>UI: type/search input or resize
UI->>Hook: update raw value
Hook-->>UI: debounced value (after delay)
UI->>Storage: setItem(pending snapshot) (staged)
Storage->>Local: write pending snapshot sidecar + version (localStorage)
Storage->>Storage: schedule flush timer (writeDebounceMs) or immediate
Storage->>TRPC: flush pending snapshot to server
TRPC-->>Storage: ack / error
Storage->>Local: update lastFlushed marker / clear pending snapshot
UI->>UI: batch width updates -> requestAnimationFrame -> flush on mouseup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 16 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/lib/trpc-storage.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/trpc-storage.ts:56">
P2: A stale debounce timer can survive while a write is in-flight, causing premature/duplicate flush attempts and undermining the debounce optimization.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx (1)
96-106: Consider deriving a workspace-indexed status selector to reduce repeated full-tab iterations per row.The selector at lines 96-106 executes on every tabs store update for each
WorkspaceListIteminstance in the list, performing a fullstate.tabsscan and pane ID extraction. With multiple workspaces rendered, this compounds torows × tabswork per update. Instead, compute aworkspaceStatusByIdmap once in the store (when tabs change) and selectworkspaceStatusById[id]here, reducing overhead to a single O(tabs) pass per update plus O(1) selections per row.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx` around lines 96 - 106, The current WorkspaceListItem uses useTabsStore with a per-instance generator that scans state.tabs and extracts pane IDs to compute getHighestPriorityStatus on every store update, causing O(rows × tabs) work; instead add a derived map in the tabs store (e.g., workspaceStatusById) that computes the highest-priority status per workspace whenever tabs/panes/layouts change, and then change WorkspaceListItem to select useTabsStore(state => state.workspaceStatusById[id]) (or a selector that returns workspaceStatusById[id]); reference getHighestPriorityStatus, extractPaneIdsFromLayout, useTabsStore and WorkspaceListItem when locating the code and ensure the derived map updates only on relevant mutations to keep per-row selection O(1).apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsx (1)
90-90: Consider aligning debounce delay with other search hooks.This uses 120ms while
useKeywordSearchanduseFileSearchuse 150ms. While a shorter delay for autocomplete can feel more responsive, consider whether consistency across the codebase is preferred for maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsx` at line 90, The debounce delay for the mention autocomplete is set to 120ms via useDebouncedValue(searchQuery.trim(), 120) which is inconsistent with other search hooks; change the 120ms to 150ms to match useKeywordSearch and useFileSearch so debouncedSearchQuery uses the same debounce interval for consistency across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/renderer/lib/trpc-storage.ts`:
- Around line 48-51: The current sequence writes the sidecar version to
localStorage before awaiting the tRPC write, which can cause hydrate to skip
migrations if config.set(parsed.state) fails; move the
localStorage.setItem(`${name}:version`, String(parsed.version)) call to after
the await config.set(parsed.state) resolves (i.e., only after config.set
succeeds), keeping assignment to lastFlushedValue unchanged and ensuring any
thrown errors prevent the version bump; update the code around the
config.set(parsed.state), localStorage.setItem, and lastFlushedValue references
accordingly.
In
`@apps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsx`:
- Around line 113-118: The cleanup currently discards pendingWidthRef by setting
it to null which can lose the final drag width; instead, in the ResizablePanel
cleanup (where rafIdRef and pendingWidthRef are handled) flush any pending
width: if pendingWidthRef.current !== null, call the same width-apply/save
routine used on mouseup (the function that persists/sets sidebar width—refer to
pendingWidthRef, rafIdRef, and the component's width apply handler) to persist
that value, then cancelAnimationFrame(rafIdRef.current) and clear
rafIdRef.current and pendingWidthRef.current; this ensures the last dragged
width is saved on unmount/navigation.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/ChangesContent.tsx`:
- Around line 12-14: The selector for isChangesSidebarVisible is incorrectly
using s.isSidebarOpen (left-sidebar flag) via useSidebarStore; update the
selector to use the right-sidebar visibility source instead (e.g.,
s.isRightSidebarOpen if it exists, or the appropriate right-sidebar
store/selector) combined with s.rightSidebarTab === RightSidebarTab.Changes so
the condition accurately reflects the right Changes panel; then adjust any logic
that disables polling/focus-refetch (the code using isChangesSidebarVisible) to
rely on this corrected boolean.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx`:
- Around line 96-106: The current WorkspaceListItem uses useTabsStore with a
per-instance generator that scans state.tabs and extracts pane IDs to compute
getHighestPriorityStatus on every store update, causing O(rows × tabs) work;
instead add a derived map in the tabs store (e.g., workspaceStatusById) that
computes the highest-priority status per workspace whenever tabs/panes/layouts
change, and then change WorkspaceListItem to select useTabsStore(state =>
state.workspaceStatusById[id]) (or a selector that returns
workspaceStatusById[id]); reference getHighestPriorityStatus,
extractPaneIdsFromLayout, useTabsStore and WorkspaceListItem when locating the
code and ensure the derived map updates only on relevant mutations to keep
per-row selection O(1).
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsx`:
- Line 90: The debounce delay for the mention autocomplete is set to 120ms via
useDebouncedValue(searchQuery.trim(), 120) which is inconsistent with other
search hooks; change the 120ms to 150ms to match useKeywordSearch and
useFileSearch so debouncedSearchQuery uses the same debounce interval for
consistency across the codebase.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c57352ec-c94e-48f0-bfbd-d03cfc6dcb28
📒 Files selected for processing (16)
apps/desktop/src/renderer/hooks/useDebouncedValue.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/screens/main/components/KeywordSearch/useKeywordSearch.tsapps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsxapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileSearch/useFileSearch.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsxapps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/renderer/stores/sidebar-state.ts
Comment on lines
+48
to
+51
| // Persist version in localStorage, bare state via tRPC. | ||
| localStorage.setItem(`${name}:version`, String(parsed.version)); | ||
| await config.set(parsed.state); | ||
| lastFlushedValue = valueToFlush; |
Contributor
There was a problem hiding this comment.
Persist the sidecar version only after the tRPC write succeeds.
Line 49 updates ${name}:version before config.set(parsed.state) resolves. If the tRPC write fails, hydrate can read old state with a newer version and skip required migrations.
🔧 Suggested fix
- // Persist version in localStorage, bare state via tRPC.
- localStorage.setItem(`${name}:version`, String(parsed.version));
- await config.set(parsed.state);
+ // Persist bare state via tRPC first, then advance sidecar version.
+ await config.set(parsed.state);
+ localStorage.setItem(`${name}:version`, String(parsed.version));📝 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.
Suggested change
| // Persist version in localStorage, bare state via tRPC. | |
| localStorage.setItem(`${name}:version`, String(parsed.version)); | |
| await config.set(parsed.state); | |
| lastFlushedValue = valueToFlush; | |
| // Persist bare state via tRPC first, then advance sidecar version. | |
| await config.set(parsed.state); | |
| localStorage.setItem(`${name}:version`, String(parsed.version)); | |
| lastFlushedValue = valueToFlush; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/lib/trpc-storage.ts` around lines 48 - 51, The
current sequence writes the sidecar version to localStorage before awaiting the
tRPC write, which can cause hydrate to skip migrations if
config.set(parsed.state) fails; move the localStorage.setItem(`${name}:version`,
String(parsed.version)) call to after the await config.set(parsed.state)
resolves (i.e., only after config.set succeeds), keeping assignment to
lastFlushedValue unchanged and ensuring any thrown errors prevent the version
bump; update the code around the config.set(parsed.state), localStorage.setItem,
and lastFlushedValue references accordingly.
Comment on lines
+113
to
118
| if (rafIdRef.current !== null) { | ||
| cancelAnimationFrame(rafIdRef.current); | ||
| rafIdRef.current = null; | ||
| } | ||
| pendingWidthRef.current = null; | ||
| }; |
Contributor
There was a problem hiding this comment.
Flush pending width in cleanup instead of discarding it.
On Line 117, clearing pendingWidthRef can drop the last drag width if unmount/navigation happens before mouseup. That can leave persisted sidebar width stale relative to what the user just dragged to.
💡 Suggested fix
return () => {
document.removeEventListener("mousemove", handleMouseMove);
document.removeEventListener("mouseup", handleMouseUp);
document.body.style.userSelect = "";
document.body.style.cursor = "";
if (rafIdRef.current !== null) {
cancelAnimationFrame(rafIdRef.current);
rafIdRef.current = null;
}
- pendingWidthRef.current = null;
+ flushPendingWidth();
};
-}, [isResizing, handleMouseMove, handleMouseUp]);
+}, [isResizing, handleMouseMove, handleMouseUp, flushPendingWidth]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsx`
around lines 113 - 118, The cleanup currently discards pendingWidthRef by
setting it to null which can lose the final drag width; instead, in the
ResizablePanel cleanup (where rafIdRef and pendingWidthRef are handled) flush
any pending width: if pendingWidthRef.current !== null, call the same
width-apply/save routine used on mouseup (the function that persists/sets
sidebar width—refer to pendingWidthRef, rafIdRef, and the component's width
apply handler) to persist that value, then
cancelAnimationFrame(rafIdRef.current) and clear rafIdRef.current and
pendingWidthRef.current; this ensures the last dragged width is saved on
unmount/navigation.
Comment on lines
+12
to
+14
| const isChangesSidebarVisible = useSidebarStore( | ||
| (s) => s.isSidebarOpen && s.rightSidebarTab === RightSidebarTab.Changes, | ||
| ); |
Contributor
There was a problem hiding this comment.
Use right-sidebar visibility source; current flag reads left-sidebar state.
On Line 13, s.isSidebarOpen is from apps/desktop/src/renderer/stores/sidebar-state.ts and represents left sidebar visibility. That makes isChangesSidebarVisible inaccurate, and then Lines 23-24 can disable polling/focus refetch when the right Changes panel is not actually visible, causing stale data risk.
Suggested fix
- const isChangesSidebarVisible = useSidebarStore(
- (s) => s.isSidebarOpen && s.rightSidebarTab === RightSidebarTab.Changes,
- );
+ const isChangesSidebarVisible = useSidebarStore(
+ (s) => s.isRightSidebarOpen && s.rightSidebarTab === RightSidebarTab.Changes,
+ );If there is no isRightSidebarOpen in this store, derive the condition from the actual right-sidebar visibility source instead of isSidebarOpen.
Also applies to: 23-24
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/ChangesContent.tsx`
around lines 12 - 14, The selector for isChangesSidebarVisible is incorrectly
using s.isSidebarOpen (left-sidebar flag) via useSidebarStore; update the
selector to use the right-sidebar visibility source instead (e.g.,
s.isRightSidebarOpen if it exists, or the appropriate right-sidebar
store/selector) combined with s.rightSidebarTab === RightSidebarTab.Changes so
the condition accurately reflects the right Changes panel; then adjust any logic
that disables polling/focus-refetch (the code using isChangesSidebarVisible) to
rely on this corrected boolean.
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/lib/trpc-storage.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/trpc-storage.ts:118">
P2: Handle `localStorage.setItem` errors when writing pending snapshots; an exception here aborts flush scheduling and can cause unhandled promise rejections.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/renderer/lib/trpc-storage.ts (1)
50-53:⚠️ Potential issue | 🟠 MajorMove the sidecar version write after tRPC persistence succeeds.
Line 51 bumps
${name}:versionbefore Line 52 awaitsconfig.set(parsed.state). If the mutate fails, hydrate can read a newer version with older state and skip required migrations.🔧 Suggested fix
- // Persist version in localStorage, bare state via tRPC. - localStorage.setItem(`${name}:version`, String(parsed.version)); - await config.set(parsed.state); + // Persist bare state via tRPC first, then advance sidecar version. + await config.set(parsed.state); + localStorage.setItem(`${name}:version`, String(parsed.version));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/trpc-storage.ts` around lines 50 - 53, The version write to the sidecar is happening before the tRPC persist succeeds; move the localStorage.setItem(`${name}:version`, String(parsed.version)) call so it runs only after await config.set(parsed.state) resolves successfully (i.e., after the mutate returns), and keep lastFlushedValue assignment after both succeed; update the block around localStorage.setItem, config.set, lastFlushedValue and ensure any error path does not update the version.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.ts (1)
26-30: Consider addingonErrorhandler for subscription resilience.With the fallback interval now 12x longer (30s vs 2.5s), a silently failing subscription could leave users with stale port data for an extended period. Adding an
onErrorcallback would allow logging or triggering an immediate refetch if the subscription disconnects unexpectedly.🛡️ Suggested enhancement for error handling
electronTrpc.ports.subscribe.useSubscription(undefined, { onData: () => { utils.ports.getAll.invalidate(); }, + onError: (error) => { + console.error("[usePortsData] Subscription error:", error); + // Trigger immediate refetch on subscription failure + utils.ports.getAll.invalidate(); + }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.ts` around lines 26 - 30, The subscription call to electronTrpc.ports.subscribe.useSubscription currently only supplies onData; add an onError callback to handle disconnects by logging the error and triggering an immediate refetch via utils.ports.getAll.invalidate (or another refresh action) so stale port data is avoided when the subscription fails; update the useSubscription call to include onError that accepts the error, calls processLogger/error or console.error, and then calls utils.ports.getAll.invalidate().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsx`:
- Around line 90-113: The query is rendering stale fileResults while
debouncedSearchQuery trails immediateSearchQuery; update the
chatServiceTrpc.workspace.searchFiles.useQuery call (and related logic around
debouncedSearchQuery and immediateSearchQuery) to avoid returning previous
results during the debounce window—either set keepPreviousData: false in the
query options or change placeholderData to return [] when immediateSearchQuery
!== debouncedSearchQuery so fileResults is empty until the debounced query
resolves (adjust usage of fileResults / files accordingly).
---
Duplicate comments:
In `@apps/desktop/src/renderer/lib/trpc-storage.ts`:
- Around line 50-53: The version write to the sidecar is happening before the
tRPC persist succeeds; move the localStorage.setItem(`${name}:version`,
String(parsed.version)) call so it runs only after await
config.set(parsed.state) resolves successfully (i.e., after the mutate returns),
and keep lastFlushedValue assignment after both succeed; update the block around
localStorage.setItem, config.set, lastFlushedValue and ensure any error path
does not update the version.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.ts`:
- Around line 26-30: The subscription call to
electronTrpc.ports.subscribe.useSubscription currently only supplies onData; add
an onError callback to handle disconnects by logging the error and triggering an
immediate refetch via utils.ports.getAll.invalidate (or another refresh action)
so stale port data is avoided when the subscription fails; update the
useSubscription call to include onError that accepts the error, calls
processLogger/error or console.error, and then calls
utils.ports.getAll.invalidate().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b342298d-8c37-4896-a0cd-79769ed84b71
📒 Files selected for processing (3)
apps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsx
Comment on lines
+90
to
+113
| const immediateSearchQuery = searchQuery.trim(); | ||
| const debouncedSearchQuery = useDebouncedValue(immediateSearchQuery, 120); | ||
|
|
||
| // File search via chatService (IPC to main process) | ||
| const { data: fileResults } = chatServiceTrpc.workspace.searchFiles.useQuery( | ||
| { | ||
| rootPath: cwd, | ||
| query: searchQuery, | ||
| query: debouncedSearchQuery, | ||
| includeHidden: false, | ||
| limit: MAX_RESULTS, | ||
| }, | ||
| { | ||
| enabled: open && searchQuery.length > 0 && !!cwd, | ||
| enabled: | ||
| open && | ||
| immediateSearchQuery.length > 0 && | ||
| debouncedSearchQuery.length > 0 && | ||
| !!cwd, | ||
| staleTime: 1000, | ||
| placeholderData: (previous) => previous ?? [], | ||
| }, | ||
| ); | ||
|
|
||
| const files = fileResults ?? []; | ||
| const files = | ||
| open && immediateSearchQuery.length > 0 ? (fileResults ?? []) : []; |
Contributor
There was a problem hiding this comment.
Prevent stale file results while debounce is still settling.
fileResults can briefly represent the previous query because debouncedSearchQuery lags immediateSearchQuery. With the current gate, those stale results are still rendered for ~120ms during edits/deletes.
💡 Suggested fix
const immediateSearchQuery = searchQuery.trim();
const debouncedSearchQuery = useDebouncedValue(immediateSearchQuery, 120);
+const isDebounceSettled = immediateSearchQuery === debouncedSearchQuery;
// File search via chatService (IPC to main process)
const { data: fileResults } = chatServiceTrpc.workspace.searchFiles.useQuery(
{
rootPath: cwd,
query: debouncedSearchQuery,
includeHidden: false,
limit: MAX_RESULTS,
},
{
enabled:
open &&
+ isDebounceSettled &&
immediateSearchQuery.length > 0 &&
debouncedSearchQuery.length > 0 &&
!!cwd,
staleTime: 1000,
placeholderData: (previous) => previous ?? [],
},
);
-const files =
- open && immediateSearchQuery.length > 0 ? (fileResults ?? []) : [];
+const files =
+ open && isDebounceSettled && immediateSearchQuery.length > 0
+ ? (fileResults ?? [])
+ : [];📝 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.
Suggested change
| const immediateSearchQuery = searchQuery.trim(); | |
| const debouncedSearchQuery = useDebouncedValue(immediateSearchQuery, 120); | |
| // File search via chatService (IPC to main process) | |
| const { data: fileResults } = chatServiceTrpc.workspace.searchFiles.useQuery( | |
| { | |
| rootPath: cwd, | |
| query: searchQuery, | |
| query: debouncedSearchQuery, | |
| includeHidden: false, | |
| limit: MAX_RESULTS, | |
| }, | |
| { | |
| enabled: open && searchQuery.length > 0 && !!cwd, | |
| enabled: | |
| open && | |
| immediateSearchQuery.length > 0 && | |
| debouncedSearchQuery.length > 0 && | |
| !!cwd, | |
| staleTime: 1000, | |
| placeholderData: (previous) => previous ?? [], | |
| }, | |
| ); | |
| const files = fileResults ?? []; | |
| const files = | |
| open && immediateSearchQuery.length > 0 ? (fileResults ?? []) : []; | |
| const immediateSearchQuery = searchQuery.trim(); | |
| const debouncedSearchQuery = useDebouncedValue(immediateSearchQuery, 120); | |
| const isDebounceSettled = immediateSearchQuery === debouncedSearchQuery; | |
| // File search via chatService (IPC to main process) | |
| const { data: fileResults } = chatServiceTrpc.workspace.searchFiles.useQuery( | |
| { | |
| rootPath: cwd, | |
| query: debouncedSearchQuery, | |
| includeHidden: false, | |
| limit: MAX_RESULTS, | |
| }, | |
| { | |
| enabled: | |
| open && | |
| isDebounceSettled && | |
| immediateSearchQuery.length > 0 && | |
| debouncedSearchQuery.length > 0 && | |
| !!cwd, | |
| staleTime: 1000, | |
| placeholderData: (previous) => previous ?? [], | |
| }, | |
| ); | |
| const files = | |
| open && isDebounceSettled && immediateSearchQuery.length > 0 | |
| ? (fileResults ?? []) | |
| : []; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsx`
around lines 90 - 113, The query is rendering stale fileResults while
debouncedSearchQuery trails immediateSearchQuery; update the
chatServiceTrpc.workspace.searchFiles.useQuery call (and related logic around
debouncedSearchQuery and immediateSearchQuery) to avoid returning previous
results during the debounce window—either set keepPreviousData: false in the
query options or change placeholderData to return [] when immediateSearchQuery
!== debouncedSearchQuery so fileResults is empty until the debounced query
resolves (adjust usage of fileResults / files accordingly).
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/lib/trpc-storage.ts (1)
50-53:⚠️ Potential issue | 🟠 MajorPersist sidecar version only after the tRPC write succeeds.
On Line 51,
${name}:versionis advanced before Line 52await config.set(parsed.state). If the mutate fails, hydrate can observe a newer version with older backend state and skip needed migrations.🔧 Proposed fix
- // Persist version in localStorage, bare state via tRPC. - localStorage.setItem(`${name}:version`, String(parsed.version)); - await config.set(parsed.state); + // Persist bare state via tRPC first, then advance sidecar version. + await config.set(parsed.state); + localStorage.setItem(`${name}:version`, String(parsed.version)); lastFlushedValue = valueToFlush;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/trpc-storage.ts` around lines 50 - 53, The code persists the sidecar version to localStorage before the tRPC write completes, which can cause hydrate to see a newer version while the backend state update (config.set) failed; fix by moving the localStorage.setItem(`${name}:version`, String(parsed.version)) so it runs only after await config.set(parsed.state) resolves successfully; update the block around config.set, lastFlushedValue and valueToFlush to set version after config.set returns (and only then update lastFlushedValue) to ensure version advancement only occurs on successful backend write.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/lib/trpc-storage.ts (1)
26-75: Consider adding an explicit flush path for app/window shutdown.With debounced writes, a close during the debounce window can leave the latest state only in
${name}:pendinguntil a future launch rehydrates and flushes. Givenapps/desktop/src/renderer/stores/tabs/store.ts:1839-1860currently has no explicit shutdown flush hook, exposing a flush handle from this adapter (or wiring a close-time flush trigger) would reduce stale backend windows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/trpc-storage.ts` around lines 26 - 75, The adapter currently uses debounced writes (createTrpcStorageAdapter / flushPendingWrite) so a shutdown during debounce can leave the latest state only in `${name}:pending`; add an explicit synchronous flush API and wire it to shutdown: implement a public async flush(name?: string) function that clears any flushTimer, cancels debounce, and immediately calls the same logic as flushPendingWrite (or delegates to it) to write pendingValue -> config.set, update `${name}:version`, and remove the pending snapshot via getPendingSnapshotKey; return this flush function (or attach it to the returned StateStorage object) from createTrpcStorageAdapter and call it from the app/window shutdown hook (e.g., the tabs store shutdown path) so pending state is persisted before exit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/desktop/src/renderer/lib/trpc-storage.ts`:
- Around line 50-53: The code persists the sidecar version to localStorage
before the tRPC write completes, which can cause hydrate to see a newer version
while the backend state update (config.set) failed; fix by moving the
localStorage.setItem(`${name}:version`, String(parsed.version)) so it runs only
after await config.set(parsed.state) resolves successfully; update the block
around config.set, lastFlushedValue and valueToFlush to set version after
config.set returns (and only then update lastFlushedValue) to ensure version
advancement only occurs on successful backend write.
---
Nitpick comments:
In `@apps/desktop/src/renderer/lib/trpc-storage.ts`:
- Around line 26-75: The adapter currently uses debounced writes
(createTrpcStorageAdapter / flushPendingWrite) so a shutdown during debounce can
leave the latest state only in `${name}:pending`; add an explicit synchronous
flush API and wire it to shutdown: implement a public async flush(name?: string)
function that clears any flushTimer, cancels debounce, and immediately calls the
same logic as flushPendingWrite (or delegates to it) to write pendingValue ->
config.set, update `${name}:version`, and remove the pending snapshot via
getPendingSnapshotKey; return this flush function (or attach it to the returned
StateStorage object) from createTrpcStorageAdapter and call it from the
app/window shutdown hook (e.g., the tabs store shutdown path) so pending state
is persisted before exit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8ad103b-1fd2-472d-b4df-27785e1516dc
📒 Files selected for processing (4)
apps/desktop/src/lib/trpc/routers/changes/branches.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/hooks/useGitChangesStatus/useGitChangesStatus.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx
Contributor
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.ts:5">
P2: The fallback polling interval was tightened to 10s, which increases redundant background refetching despite having subscription invalidation; this is a performance regression in this path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| /** Matches the port scanner's scan cycle in port-manager.ts */ | ||
| const PORTS_REFETCH_INTERVAL_MS = 2500; | ||
| const PORTS_FALLBACK_REFETCH_INTERVAL_MS = 10_000; |
Contributor
There was a problem hiding this comment.
P2: The fallback polling interval was tightened to 10s, which increases redundant background refetching despite having subscription invalidation; this is a performance regression in this path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.ts, line 5:
<comment>The fallback polling interval was tightened to 10s, which increases redundant background refetching despite having subscription invalidation; this is a performance regression in this path.</comment>
<file context>
@@ -2,7 +2,7 @@ import { useMemo } from "react";
import type { EnrichedPort } from "shared/types";
-const PORTS_FALLBACK_REFETCH_INTERVAL_MS = 30_000;
+const PORTS_FALLBACK_REFETCH_INTERVAL_MS = 10_000;
export interface WorkspacePortGroup {
</file context>
Suggested change
| const PORTS_FALLBACK_REFETCH_INTERVAL_MS = 10_000; | |
| const PORTS_FALLBACK_REFETCH_INTERVAL_MS = 30_000; |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsx (1)
103-114:⚠️ Potential issue | 🟡 MinorPrevent stale file results while debounce is still settling.
At Line 109 and Line 113, previous-query results can still render during the debounce window (
immediateSearchQuery !== debouncedSearchQuery). That briefly shows stale matches for the wrong input.💡 Suggested fix
const immediateSearchQuery = searchQuery.trim(); const debouncedSearchQuery = useDebouncedValue(immediateSearchQuery, 120); +const isDebounceSettled = immediateSearchQuery === debouncedSearchQuery; // File search via chatService (IPC to main process) const { data: fileResults, isFetching: isSearchFetching } = chatServiceTrpc.workspace.searchFiles.useQuery( @@ { enabled: open && immediateSearchQuery.length > 0 && debouncedSearchQuery.length > 0 && !!cwd, staleTime: 1000, - placeholderData: (previous) => previous ?? [], + placeholderData: () => (isDebounceSettled ? undefined : []), }, ); const files = - open && immediateSearchQuery.length > 0 ? (fileResults ?? []) : []; + open && isDebounceSettled && immediateSearchQuery.length > 0 + ? (fileResults ?? []) + : [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsx` around lines 103 - 114, The current logic can surface stale fileResults while the debounce settles because immediateSearchQuery and debouncedSearchQuery can differ; update the query gating so both the query hook "enabled" option and the derived "files" value only use results when immediateSearchQuery === debouncedSearchQuery. Concretely, add an equality check for immediateSearchQuery === debouncedSearchQuery to the enabled condition inside the query options and change the files assignment (the variable computed from fileResults) to return [] unless open && immediateSearchQuery.length > 0 && immediateSearchQuery === debouncedSearchQuery && fileResults is present, so stale previous results are not shown during the debounce window.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsx`:
- Around line 103-114: The current logic can surface stale fileResults while the
debounce settles because immediateSearchQuery and debouncedSearchQuery can
differ; update the query gating so both the query hook "enabled" option and the
derived "files" value only use results when immediateSearchQuery ===
debouncedSearchQuery. Concretely, add an equality check for immediateSearchQuery
=== debouncedSearchQuery to the enabled condition inside the query options and
change the files assignment (the variable computed from fileResults) to return
[] unless open && immediateSearchQuery.length > 0 && immediateSearchQuery ===
debouncedSearchQuery && fileResults is present, so stale previous results are
not shown during the debounce window.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53643baa-a7cc-4eb6-9b51-b9da4b62c59f
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/KeywordSearch/useKeywordSearch.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileSearch/useFileSearch.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/KeywordSearch/useKeywordSearch.ts
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/lib/trpc-storage.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/trpc-storage.ts:154">
P1: Backend failure now prevents pending snapshot recovery. Moving `config.get()` before the localStorage read means any tRPC/IPC error causes the catch to return `null`, silently skipping crash-recovery data that exists in localStorage. The old code checked localStorage first, so a backend failure didn't block recovery. Consider isolating the `config.get()` in its own try-catch so pending snapshot reads still proceed when the backend is unavailable.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return { | ||
| getItem: async (name: string): Promise<string | null> => { | ||
| try { | ||
| const state = await config.get(); |
Contributor
There was a problem hiding this comment.
P1: Backend failure now prevents pending snapshot recovery. Moving config.get() before the localStorage read means any tRPC/IPC error causes the catch to return null, silently skipping crash-recovery data that exists in localStorage. The old code checked localStorage first, so a backend failure didn't block recovery. Consider isolating the config.get() in its own try-catch so pending snapshot reads still proceed when the backend is unavailable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/lib/trpc-storage.ts, line 154:
<comment>Backend failure now prevents pending snapshot recovery. Moving `config.get()` before the localStorage read means any tRPC/IPC error causes the catch to return `null`, silently skipping crash-recovery data that exists in localStorage. The old code checked localStorage first, so a backend failure didn't block recovery. Consider isolating the `config.get()` in its own try-catch so pending snapshot reads still proceed when the backend is unavailable.</comment>
<file context>
@@ -77,33 +151,55 @@ function createTrpcStorageAdapter(config: TrpcStorageConfig): StateStorage {
getItem: async (name: string): Promise<string | null> => {
try {
- // Prefer the latest pending snapshot to avoid dropping state on fast exit.
+ const state = await config.get();
+ const version = Number.parseInt(
+ localStorage.getItem(`${name}:version`) ?? "0",
</file context>
Suggested change
| const state = await config.get(); | |
| let state: unknown; | |
| try { | |
| state = await config.get(); | |
| } catch { | |
| state = null; | |
| } |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/renderer/lib/trpc-storage.ts (1)
122-125:⚠️ Potential issue | 🔴 CriticalWrite canonical state before version sidecar, and don’t let localStorage failures block persistence.
Line 123 updates
${name}:versionbefore Line 124 persists state via tRPC. Also, iflocalStorage.setItemthrows,config.setis skipped because both are in the sametry, causing data loss and version skew.Suggested fix
try { const parsed = JSON.parse(valueToFlush) as { state: unknown; version: number; }; - // Persist version in localStorage, bare state via tRPC. - localStorage.setItem(`${name}:version`, String(parsed.version)); - await config.set(parsed.state); + // Persist bare state first; sidecar version is best-effort. + await config.set(parsed.state); + try { + localStorage.setItem(`${name}:version`, String(parsed.version)); + } catch (error) { + console.error("[trpc-storage] Failed to persist version sidecar:", error); + } lastFlushedValue = valueToFlush;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/trpc-storage.ts` around lines 122 - 125, Persist the canonical state first by calling config.set(parsed.state) before touching the version sidecar, then update localStorage with `${name}:version`; wrap the localStorage.setItem call in its own try/catch so any localStorage error is swallowed and does not prevent the tRPC persistence, and only set lastFlushedValue = valueToFlush after config.set succeeds (ensure lastFlushedValue is still updated even if localStorage.setItem throws). Use the existing symbols config.set, localStorage.setItem, lastFlushedValue, and valueToFlush to locate and change the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/renderer/lib/trpc-storage.ts`:
- Around line 153-205: The catch currently returns null even when config.get()
succeeded and canonicalSnapshot exists; hoist or predeclare canonicalSnapshot
(e.g., let canonicalSnapshot: string | null = null) and assign it from
JSON.stringify({ state, version }) after awaiting config.get(), then in the
catch block return canonicalSnapshot if it's non-null (and still log the error)
instead of unconditionally returning null so canonical state is preserved on
localStorage errors; reference config.get(), canonicalSnapshot, and the catch
that currently console.error(...); return null.
---
Duplicate comments:
In `@apps/desktop/src/renderer/lib/trpc-storage.ts`:
- Around line 122-125: Persist the canonical state first by calling
config.set(parsed.state) before touching the version sidecar, then update
localStorage with `${name}:version`; wrap the localStorage.setItem call in its
own try/catch so any localStorage error is swallowed and does not prevent the
tRPC persistence, and only set lastFlushedValue = valueToFlush after config.set
succeeds (ensure lastFlushedValue is still updated even if localStorage.setItem
throws). Use the existing symbols config.set, localStorage.setItem,
lastFlushedValue, and valueToFlush to locate and change the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96e39a98-de18-4ccb-b8bb-7021c217a2ce
📒 Files selected for processing (1)
apps/desktop/src/renderer/lib/trpc-storage.ts
Comment on lines
153
to
205
| try { | ||
| const state = await config.get(); | ||
| if (!state) return null; | ||
| // Version is stored in localStorage as a sidecar since the | ||
| // tRPC backend validates bare state and rejects envelopes. | ||
| const version = Number.parseInt( | ||
| localStorage.getItem(`${name}:version`) ?? "0", | ||
| 10, | ||
| ); | ||
| return JSON.stringify({ state, version }); | ||
| const canonicalSnapshot = state | ||
| ? JSON.stringify({ state, version }) | ||
| : null; | ||
|
|
||
| const pendingSnapshot = localStorage.getItem( | ||
| getPendingSnapshotKey(name), | ||
| ); | ||
| const pendingUpdatedAt = Number.parseInt( | ||
| localStorage.getItem(getPendingSnapshotUpdatedAtKey(name)) ?? "0", | ||
| 10, | ||
| ); | ||
| const pendingAgeMs = | ||
| Number.isFinite(pendingUpdatedAt) && pendingUpdatedAt > 0 | ||
| ? Date.now() - pendingUpdatedAt | ||
| : Number.POSITIVE_INFINITY; | ||
| const isPendingFresh = pendingAgeMs <= PENDING_SNAPSHOT_TTL_MS; | ||
|
|
||
| if (pendingSnapshot) { | ||
| if (!canonicalSnapshot) { | ||
| if (isPendingFresh) { | ||
| scheduleImmediateFlush(name, pendingSnapshot); | ||
| return pendingSnapshot; | ||
| } | ||
| clearPendingSnapshot(name); | ||
| return null; | ||
| } | ||
|
|
||
| if (pendingSnapshot === canonicalSnapshot) { | ||
| clearPendingSnapshot(name); | ||
| return canonicalSnapshot; | ||
| } | ||
|
|
||
| // Only trust pending snapshots that are very recent; otherwise | ||
| // canonical appState remains the source of truth. | ||
| if (isPendingFresh) { | ||
| scheduleImmediateFlush(name, pendingSnapshot); | ||
| return pendingSnapshot; | ||
| } | ||
|
|
||
| clearPendingSnapshot(name); | ||
| return canonicalSnapshot; | ||
| } | ||
|
|
||
| return canonicalSnapshot; | ||
| } catch (error) { | ||
| console.error("[trpc-storage] Failed to get state:", error); | ||
| return null; |
Contributor
There was a problem hiding this comment.
getItem should still return canonical state when localStorage access fails.
A localStorage exception inside this block currently drops to return null, even if config.get() already succeeded. That breaks hydration fallback and can surface as data loss under storage errors.
Suggested fix
getItem: async (name: string): Promise<string | null> => {
try {
const state = await config.get();
- const version = Number.parseInt(
- localStorage.getItem(`${name}:version`) ?? "0",
- 10,
- );
+ let version = 0;
+ try {
+ version = Number.parseInt(
+ localStorage.getItem(`${name}:version`) ?? "0",
+ 10,
+ );
+ } catch (error) {
+ console.error("[trpc-storage] Failed to read version sidecar:", error);
+ }
const canonicalSnapshot = state
? JSON.stringify({ state, version })
: null;
- const pendingSnapshot = localStorage.getItem(
- getPendingSnapshotKey(name),
- );
- const pendingUpdatedAt = Number.parseInt(
- localStorage.getItem(getPendingSnapshotUpdatedAtKey(name)) ?? "0",
- 10,
- );
+ let pendingSnapshot: string | null = null;
+ let pendingUpdatedAt = 0;
+ try {
+ pendingSnapshot = localStorage.getItem(getPendingSnapshotKey(name));
+ pendingUpdatedAt = Number.parseInt(
+ localStorage.getItem(getPendingSnapshotUpdatedAtKey(name)) ?? "0",
+ 10,
+ );
+ } catch (error) {
+ console.error("[trpc-storage] Failed to read pending snapshot:", error);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/lib/trpc-storage.ts` around lines 153 - 205, The
catch currently returns null even when config.get() succeeded and
canonicalSnapshot exists; hoist or predeclare canonicalSnapshot (e.g., let
canonicalSnapshot: string | null = null) and assign it from JSON.stringify({
state, version }) after awaiting config.get(), then in the catch block return
canonicalSnapshot if it's non-null (and still log the error) instead of
unconditionally returning null so canonical state is preserved on localStorage
errors; reference config.get(), canonicalSnapshot, and the catch that currently
console.error(...); return null.
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
This PR follows up a desktop-only performance debugging pass and lands targeted improvements for hard-to-spot renderer and IPC bottlenecks.
Included changes
uiState.tabs.set) to reduce IPC + lowdb write pressure.Why
Before these changes, desktop had multiple compounding sources of overhead:
Validation
bunx biome checkon changed files.bun run typecheck --filter=@superset/desktopsrc/lib/trpc/routers/external/helpers.ts(missingwindsurfkey in typed record).Commits
eb38913a7perf(desktop): debounce and dedupe tabs state persistencecf1a59f46perf(desktop): stabilize hotkey listeners and narrow workspace subscriptions0cd932f95perf(desktop): reduce sidebar resize and render churnb1b6c1d23perf(desktop): debounce expensive workspace search queries13d2dd42dperf(desktop): avoid global tab subscriptions per workspace row413338a0eperf(desktop): remove redundant ports polling interval78990a87ffix(desktop): use supported zustand selector signaturesSummary by cubic
Cuts desktop renderer and IPC overhead by debouncing persistence/search, throttling resize, and trimming polling/subscriptions. Adds crash-safe tabs persistence with debounced local snapshots and stale-replay protection, visibility-aware polling with branch metadata syncing, and prevents false empty states during debounced searches.
Performance
Refactors
Written for commit c611a65. Summary will update on new commits.
Summary by CodeRabbit
New Features
User-facing Improvements
Performance