Merged
Conversation
📝 WalkthroughWalkthroughThis PR refactors UI state management across the renderer component tree by expanding prop-driven state composition in App.tsx, wiring additional UI state and setters through MainPanel, SessionList, GroupChatPanel and related hooks, while simplifying internal APIs (removing lookbackDays parameter from hasCachedSynopsis) and updating dependent code. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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)
Comment |
Greptile SummaryThis polish PR improves Director's Notes UX and fixes an orphaned file preview bug. The Director's Notes cache now persists regardless of lookback slider changes, old content remains visible and scrollable during regeneration, and the button was renamed from "Refresh" to "Regenerate" for clarity. The orphaned file preview fix adds defensive guards across multiple locations (MainPanel, useRemoteIntegration, useSessionRestoration, agentStore) to ensure Confidence Score: 5/5
Important Files Changed
Last reviewed commit: d4f4d78 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/GroupChatPanel.tsx`:
- Around line 40-41: The GroupChatPanel component has an unused prop named
groups: remove the dead prop from the component props type and from the
destructuring in GroupChatPanel so it is no longer declared or referenced;
specifically, delete the optional groups?: Group[] declaration and remove groups
from the destructured props list in the GroupChatPanel function, leaving
onDraftChange and other used props intact (GroupChatInput already reads groups
from the session store so no forwarding is needed).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/__tests__/renderer/components/DirectorNotes/AIOverviewTab.test.tsxsrc/__tests__/renderer/hooks/useRemoteIntegration.test.tssrc/__tests__/renderer/hooks/useSessionRestoration.test.tssrc/__tests__/renderer/stores/agentStore.test.tssrc/renderer/App.tsxsrc/renderer/components/DirectorNotes/AIOverviewTab.tsxsrc/renderer/components/DirectorNotes/DirectorNotesModal.tsxsrc/renderer/components/GroupChatPanel.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/components/UsageDashboard/AutoRunStats.tsxsrc/renderer/components/UsageDashboard/SessionStats.tsxsrc/renderer/components/UsageDashboard/TasksByHourChart.tsxsrc/renderer/hooks/remote/useRemoteIntegration.tssrc/renderer/hooks/session/useSessionRestoration.tssrc/renderer/stores/agentStore.ts
Comment on lines
+40
to
41
| groups?: Group[]; | ||
| onDraftChange?: (draft: string) => void; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "groups" --type tsx src/renderer/components/GroupChatInput.tsx -A 3 -B 3Repository: RunMaestro/Maestro
Length of output: 89
🏁 Script executed:
# Also check the GroupChatPanel to see the full context of how groups is used/passed
rg -n "groups" --type tsx src/renderer/components/GroupChatPanel.tsx -A 2 -B 2Repository: RunMaestro/Maestro
Length of output: 89
🏁 Script executed:
# Check the interface/type definition of GroupChatInput props
ast-grep --pattern $'interface GroupChatInputProps {
$$$
}'Repository: RunMaestro/Maestro
Length of output: 3070
🏁 Script executed:
cat -n src/renderer/components/GroupChatPanel.tsx | sed -n '35,170p'Repository: RunMaestro/Maestro
Length of output: 4891
🏁 Script executed:
# Check GroupChatHeader and GroupChatMessages props to see if they accept groups
ast-grep --pattern $'interface GroupChatHeaderProps {
$$$
}'Repository: RunMaestro/Maestro
Length of output: 1235
🏁 Script executed:
# Also check for GroupChatMessages props
ast-grep --pattern $'interface GroupChatMessagesProps {
$$$
}'Repository: RunMaestro/Maestro
Length of output: 977
🏁 Script executed:
rg "groups" src/renderer/components/GroupChatPanel.tsxRepository: RunMaestro/Maestro
Length of output: 89
🏁 Script executed:
# Also check if GroupChatInput is being updated elsewhere to accept groups
rg "groups" src/renderer/components/GroupChatInput.tsxRepository: RunMaestro/Maestro
Length of output: 397
groups prop in GroupChatPanel is unused, but not a blocker for @mention features.
groups is destructured at line 91 but not forwarded to any child. However, GroupChatInput does not expect groups as a prop—it reads groups directly from the store via useSessionStore((s) => s.groups). This means the group @mention feature will function correctly despite groups not being passed down. The dead prop is still worth cleaning up, but the feature is not silently broken.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/GroupChatPanel.tsx` around lines 40 - 41, The
GroupChatPanel component has an unused prop named groups: remove the dead prop
from the component props type and from the destructuring in GroupChatPanel so it
is no longer declared or referenced; specifically, delete the optional groups?:
Group[] declaration and remove groups from the destructured props list in the
GroupChatPanel function, leaving onDraftChange and other used props intact
(GroupChatInput already reads groups from the session store so no forwarding is
needed).
Remove 7 console.log calls from useSessionDebounce that fired at high frequency during AI streaming. Replace index-based keys with stable identifiers in 6 components where items can be removed or filtered (staged images, diff tabs, log entries, quit modal agents).
- QuitConfirmModal: use composite key `${name}-${index}` to prevent
duplicate React keys when agents share display names
- extensionColors: early-return accent fallback in colorblind mode so
we never serve non-colorblind-safe colors from EXTENSION_MAP
…y from persistence Users with large working directories (100K+ files) caused sessions.json to balloon to 300MB+. Root cause: the full file tree (FileTreeNode[]) was persisted for every session with no size limits. Three changes: 1. Strip fileTree, fileTreeStats, filePreviewHistory from persistence — these are ephemeral cache data that re-scan automatically on session activation 2. Add configurable local ignore patterns setting (default: .git, node_modules, __pycache__) with UI in Settings > Display, reusing a new generic IgnorePatternsSection component extracted from SshRemoteIgnoreSection 3. Wire local ignore patterns through loadFileTree for local (non-SSH) scans
Add localHonorGitignore setting (default: true) that parses .gitignore files and merges their patterns with the user's local ignore patterns when building file trees. Mirrors the existing SSH remote gitignore support for local filesystem scans.
…s object, and rescan effect - Extract parseGitignoreContent() shared between local/remote gitignore handling - Replace 7 positional params in loadFileTree with LocalFileTreeOptions object - Export DEFAULT_LOCAL_IGNORE_PATTERNS from settingsStore, use in SettingsModal - Fix deprecated onKeyPress -> onKeyDown in IgnorePatternsSection - Add role="checkbox" and aria-checked to honor-gitignore toggle - Add useEffect to auto-rescan file tree when local ignore patterns change - Update tests for new loadFileTree signature
…n plan mode (#447) OpenCode's defaultEnvVars always set OPENCODE_CONFIG_CONTENT with "*":"allow" blanket permissions. In read-only mode, CLI YOLO flags were stripped but the env var granting auto-approval was not. Add readOnlyEnvOverrides to AgentConfig so agents can specify env var overrides for read-only mode. OpenCode's override strips blanket permissions while keeping question tool disabled.
React.memo wraps for 29 components (7 core + 22 UsageDashboard charts) to prevent unnecessary re-renders on parent state changes. Add compound DB indexes (start_time, agent_type/project_path/source) for common query patterns in stats aggregation. Parallelize sequential git rev-parse calls in worktreeInfo and worktreeSetup handlers with Promise.all (~4x fewer round-trips). Cache Date.getTime() results in useFilteredAndSortedSessions sort comparator to avoid repeated date parsing during sort operations. Replace inline debounce in useStats with shared useDebouncedCallback hook, adding proper cleanup on unmount.
Blocked issues were completely non-interactive. Now they can be clicked and keyboard-navigated to explore their documents, while the Start Symphony button remains hidden to prevent starting blocked work.
Group @mentions: typing @ in group chat now shows agent groups in the dropdown (visually differentiated with icon and member count badge). Selecting a group expands to individual @mentions for all member agents. Navigation history: group chats now participate in back/forward breadcrumb navigation. Fixed pre-existing dual-instance bug where useNavigationHistory was instantiated independently in App.tsx and useSessionLifecycle.ts, creating separate stacks that couldn't communicate.
Replace the read-only provider display with a dropdown selector. When provider changes, tabs reset to a fresh session and provider- specific config (path, args, env, model) is cleared. History data persists since it's keyed by Maestro session ID.
* feat: add archive state to group-chats * fix: deduplicate GroupChat return type in global.d.ts, make archive callback optional Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Bug fixes: use tasksTotal for AutoRunStats count, add division-by-zero guard in WeekdayComparisonChart, fix useStats effect cleanup, add v4 stats migration for compound indexes. Sentry compliance: replace console.error with captureException in QRCode, AutoRunStats, LongestAutoRunsTable, TasksByHourChart. Accessibility: add keyboard handlers to AutoRunStats bar items, add target guard and focus-visible ring to SymphonyModal. Performance: memo MetricCard, useMemo for PeakHoursChart hasData, useId for DurationTrendsChart gradient, stable opacities in ChartSkeletons, stable onRemove ref in Toast, consistent variable naming in git handler.
Removed inline blocked info box from the detail header. The footer now shows for blocked issues with a disabled Start Symphony button and a lock-icon message explaining the dependency block, consistent with the normal footer layout.
Replace ad-hoc hardcoded pixel buffers across 6 context menu implementations with a shared useContextMenuPosition hook that measures actual rendered menu size via getBoundingClientRect in useLayoutEffect, clamping position to keep menus fully visible. Two menus (SessionActivityGraph, GroupChatHistoryPanel) previously had no viewport detection at all.
File preview rendering was independent of inputMode, so switching to terminal mode (via authenticateAfterError or remote mode switch) without clearing activeFileTabId left the file preview visible with no tab bar. The corrupted state persisted across app restarts. Guard file preview with inputMode='ai', clear activeFileTabId on all terminal-mode transitions, and auto-heal on session restoration.
- Clear activeFileTabId in onRemoteCommand mode-sync path (same bug as onRemoteSwitchMode, caught by CodeRabbit) - Fix "preserves activeFileTabId" test to use non-null starting value so it actually tests preservation behavior - Add test for onRemoteCommand clearing activeFileTabId on terminal sync
…rector's Notes beta badge - Add z-20 to header container to prevent overlap issues - Dim agent status dots for inactive sessions (0.45 opacity) - Add beta badge to Director's Notes in Encore Features settings
Cache no longer keys on lookbackDays — once generated, notes persist until explicit Regenerate click. During regeneration, old notes stay visible and scrollable with stats bar, timestamp, and Save/Copy buttons remaining enabled. Progress bar only shows on first-ever generation. Renamed "Refresh" to "Regenerate" for clearer intent.
Add ~37 missing props to useMainPanelProps interface, useMemo return, and dependency array. Remove state props from useRightPanelProps call in App.tsx that the refactored hook no longer accepts (RightPanel now reads state directly from Zustand stores).
pedramamini
force-pushed
the
0.15.0-RC-polish-round-two
branch
from
d4f4d78 to
9d0936d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/__tests__/renderer/components/DocumentGraph/NodeContextMenu.test.tsx (1)
337-355:⚠️ Potential issue | 🟡 MinorWrap the test body in try-finally to ensure
console.errorspy is always restored.The
afterEachhook only callsvi.clearAllMocks(), which clears call history but does not restore spies created withvi.spyOn(). If the test throws before reachingconsoleErrorSpy.mockRestore()(e.g., assertion failure), the spy remains active and leaks into subsequent tests. Use try-finally to guarantee restoration even on test failure.Suggested fix
- const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - mockClipboardWriteText.mockRejectedValueOnce(new Error('Clipboard access denied')); - - const onDismiss = vi.fn(); - const props = createProps({ nodeData: mockDocumentNodeData, onDismiss }); - render(<NodeContextMenu {...props} />); - - fireEvent.click(screen.getByRole('button', { name: /copy path/i })); - - await waitFor(() => { - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Failed to copy to clipboard:', - expect.any(Error) - ); - }); - expect(onDismiss).toHaveBeenCalled(); - - consoleErrorSpy.mockRestore(); + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + mockClipboardWriteText.mockRejectedValueOnce(new Error('Clipboard access denied')); + + const onDismiss = vi.fn(); + const props = createProps({ nodeData: mockDocumentNodeData, onDismiss }); + render(<NodeContextMenu {...props} />); + + fireEvent.click(screen.getByRole('button', { name: /copy path/i })); + + await waitFor(() => { + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to copy to clipboard:', + expect.any(Error) + ); + }); + expect(onDismiss).toHaveBeenCalled(); + } finally { + consoleErrorSpy.mockRestore(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/DocumentGraph/NodeContextMenu.test.tsx` around lines 337 - 355, The test creates a spy consoleErrorSpy via vi.spyOn(console, 'error') but only calls consoleErrorSpy.mockRestore() at the end, which can leak the spy if an assertion throws; wrap the test body (the logic using mockClipboardWriteText, render(<NodeContextMenu ...>), fireEvent.click and the waitFor/assertions that expect consoleErrorSpy and onDismiss) inside a try block and call consoleErrorSpy.mockRestore() in a finally block so the spy is always restored even on failure; ensure you keep references to consoleErrorSpy, mockClipboardWriteText, onDismiss and use the same createProps/NodeContextMenu calls unchanged.src/__tests__/renderer/components/DirectorNotes/AIOverviewTab.test.tsx (1)
207-225:⚠️ Potential issue | 🟡 MinorStale comment on line 222: "Click refresh" should be "Click Regenerate".
The test name and button target were updated, but the inline comment still references the old "refresh" label.
Fix stale comment
- // Click refresh + // Click Regenerate🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/DirectorNotes/AIOverviewTab.test.tsx` around lines 207 - 225, Update the stale inline comment in the test for AIOverviewTab: change the comment that currently reads "Click refresh" to "Click Regenerate" so it matches the test name and the button being clicked (the fireEvent targeting the "Regenerate" button and the mockGenerateSynopsis usage in the test).src/renderer/hooks/props/useMainPanelProps.ts (1)
499-501:⚠️ Potential issue | 🟠 Major
mergeResultis hardcoded tonull— merge result UI will never render.
UseMainPanelPropsDepshas nomergeResultfield, sodeps.mergeResultcannot be forwarded. TheMergeResulttype is imported andMainPanelProps.mergeResultexists, but the value is alwaysnull. Any UI insideMainPanelconditioned onmergeResultwill be permanently suppressed.🐛 Proposed fix — add `mergeResult` to deps and wire it through
Add to
UseMainPanelPropsDepsinterface:// Merge progress mergeProgress: GroomingProgress | null; +mergeResult: MergeResult | null; mergeStartTime: number;Update the
useMemobody:-mergeResult: null as MergeResult | null, +mergeResult: deps.mergeResult,Add to the dependency array:
deps.mergeProgress, +deps.mergeResult, deps.mergeStartTime,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/props/useMainPanelProps.ts` around lines 499 - 501, The mergeResult is hardcoded to null so the MainPanel UI never sees it; update the UseMainPanelPropsDeps interface to include mergeResult: MergeResult | null, then change the useMainPanelProps creation to set mergeResult: deps.mergeResult instead of null (look for useMainPanelProps / MainPanelProps construction) and add deps.mergeResult to the useMemo dependency array so memo updates when the merge result changes.src/renderer/components/MainPanel.tsx (1)
887-896:⚠️ Potential issue | 🟡 MinorMissing
onFocushandler — keyboard focus won't update the active focus ring.The outermost panel
<div>only reacts toonClickforsetActiveFocus('main'). When a user keyboard-tabs into the panel, focus is received butactiveFocusis not updated, so the ring (line 888) never renders.SessionListcorrectly handles bothonClickandonFocus.♿ Proposed fix
<div className={`flex-1 flex flex-col min-w-0 relative ${activeFocus === 'main' ? 'ring-1 ring-inset z-10' : ''}`} style={...} + tabIndex={-1} onClick={() => setActiveFocus('main')} + onFocus={() => setActiveFocus('main')} >Based on learnings: "Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus" (applies to
src/renderer/components/**/*.{ts,tsx}).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/MainPanel.tsx` around lines 887 - 896, The outer container div that toggles the focus ring doesn't update activeFocus on keyboard focus—add keyboard focus support by making the div focusable (add tabIndex={0}) and attach an onFocus handler that calls setActiveFocus('main') and optionally onBlur to clear or change focus; update the div containing activeFocus, setActiveFocus, and the ring class so both onClick and onFocus keep behavior consistent with SessionList.src/renderer/components/SessionList.tsx (1)
583-583:⚠️ Potential issue | 🟠 MajorAdd null guards to six unguarded
shortcutsaccesses to prevent runtime crashes.Line 583, 607, 630, 653, 676, and 2985 access
shortcuts.help.keys,shortcuts.settings.keys,shortcuts.systemLogs.keys,shortcuts.processMonitor.keys,shortcuts.usageDashboard.keys, andshortcuts.toggleSidebar.keyswithout checking if the property exists. Sinceshortcutsis typed asRecord<string, Shortcut>, a missing key returnsundefinedand accessing.keysthrows a TypeError. The optional entries likeshortcuts.newInstance(line 491) andshortcuts.openSymphony(line 699) already follow the guard pattern—apply the same approach consistently:Fix pattern
-{formatShortcutKeys(shortcuts.help.keys)} +{shortcuts.help ? formatShortcutKeys(shortcuts.help.keys) : '?'}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SessionList.tsx` at line 583, Multiple places in SessionList.tsx call formatShortcutKeys(shortcuts.<name>.keys) without guarding for missing shortcut entries; add null/undefined guards for the six accesses (shortcuts.help, shortcuts.settings, shortcuts.systemLogs, shortcuts.processMonitor, shortcuts.usageDashboard, shortcuts.toggleSidebar) so you pass a safe value to formatShortcutKeys (e.g., use optional chaining like shortcuts?.help?.keys or check shortcuts.help before accessing .keys, and provide a sensible fallback such as an empty array or undefined). Update the six call sites to mirror the existing guarded pattern used for shortcuts.newInstance and shortcuts.openSymphony to prevent TypeError at runtime.
🧹 Nitpick comments (7)
src/renderer/components/DirectorNotes/DirectorNotesModal.tsx (1)
119-123: Same empty-deps pattern as AIOverviewTab — consider an eslint-disable comment.
cachedis aconstfrom the render scope so it won't go stale, but an explicit comment would document intent and silence lint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/DirectorNotes/DirectorNotesModal.tsx` around lines 119 - 123, In DirectorNotesModal, the useEffect that reads the render-scoped const cached and calls setOverviewGenerating uses an empty deps array and should be documented to satisfy lint; add an inline comment and an eslint-disable-next-line react-hooks/exhaustive-deps above that useEffect (or a concise explanatory comment inside it) stating that cached is intentionally stable for this render so the empty dependency array is deliberate to silence the linter and document intent for future readers.src/renderer/hooks/props/useSessionListProps.ts (3)
153-282: Prop renaming (leftSidebarWidth→leftSidebarWidthState) may confuse maintainers.Lines 163 and 193 rename
deps.leftSidebarWidthtoleftSidebarWidthStateanddeps.setLeftSidebarWidthtosetLeftSidebarWidthStatein the output props. This implicit rename between the deps interface and the returned props creates a subtle mapping that's easy to overlook during maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/props/useSessionListProps.ts` around lines 153 - 282, The returned props from useSessionListProps rename deps.leftSidebarWidth → leftSidebarWidthState and deps.setLeftSidebarWidth → setLeftSidebarWidthState which creates a surprising API mismatch; to fix, keep names consistent by mapping deps.leftSidebarWidth to leftSidebarWidth and deps.setLeftSidebarWidth to setLeftSidebarWidth (or intentionally document and standardize the new names everywhere), updating the return object in useSessionListProps (and any callers) so the prop names leftSidebarWidth and setLeftSidebarWidth match the deps interface and avoid the implicit rename confusion.
1-10: Docstring claim about "primitive values" is misleading.Lines 8-9 state the hook "uses primitive values in dependency arrays" to minimize re-renders, but the dependency array includes many object/array references (
sessions,groups,sortedSessions,shortcuts,groupChats,visibleSessions,participantStates, etc.). The memoization is only effective if these are memoized upstream. Consider updating the comment to reflect this assumption.Clarify the docstring
- * Key optimization: Uses primitive values in dependency arrays (e.g., activeSessionId - * instead of activeSession) to minimize re-renders. + * Key optimization: Centralizes prop computation so SessionList only re-renders + * when dependencies actually change. Callers must ensure non-primitive deps + * (sessions, groups, sortedSessions, etc.) are memoized upstream.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/props/useSessionListProps.ts` around lines 1 - 10, The docstring for useSessionListProps is misleading about using "primitive values in dependency arrays" because the hook's dependency array still contains many non-primitive references (sessions, groups, sortedSessions, shortcuts, groupChats, visibleSessions, participantStates, etc.), so update the comment to state that the hook minimizes re-renders by preferring primitives (e.g., activeSessionId) where possible but relies on those upstream values being memoized/stable for memoization to be effective; mention useSessionListProps and list the non-primitive dependencies (sessions, groups, sortedSessions, shortcuts, groupChats, visibleSessions, participantStates) so readers know the assumption.
283-377: ~95-item dependency array is a maintenance burden.With nearly 95 dependencies, it's easy for a future contributor to add a returned prop but forget to add its source to the dep array (or vice versa). Consider a structural guard — e.g., returning
depsdirectly (if the prop names matched) or using a linter rule to enforce parity between the returned object keys and the dependency list.That said, this is a known tradeoff of the centralized-memoization pattern and works fine as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/props/useSessionListProps.ts` around lines 283 - 377, The huge dependency array in useSessionListProps (the deps[...] list) is brittle; replace the manual 95-item list by using the deps object itself as the dependency source (e.g., useMemo/useCallback with deps as the single dependency or derive dependencies from Object.values(deps)) or add a parity check/linter that asserts returned prop keys match keys in the dependency list; update references to the deps variable and the memo that currently lists deps.theme,...,deps.sidebarContainerRef so it uses the single-source deps approach (or a generated array from deps) to ensure maintainability.src/renderer/components/DirectorNotes/AIOverviewTab.tsx (2)
76-81: Minor:setTimeoutincopyToClipboardnot cleared on unmount.The 2-second timeout to reset
copiedstate can fire after the component unmounts. This is harmless in practice (React 18 swallows the warning), but for completeness you could clear it viamountedRef.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/DirectorNotes/AIOverviewTab.tsx` around lines 76 - 81, The timeout in copyToClipboard (which calls setCopied(true) then setTimeout(() => setCopied(false), 2000)) can run after unmount; store the timeout id (from setTimeout) in a ref (e.g., timeoutRef) when scheduling it inside copyToClipboard and clear that timeout in a useEffect cleanup (or check a mountedRef before calling setCopied(false)); update the copyToClipboard closure to save the id to timeoutRef.current and add a useEffect that returns a cleanup which clearsTimeout(timeoutRef.current) to prevent the callback running after unmount.
134-148: MissinggenerateSynopsisin the useEffect dependency array.The mount effect closes over
generateSynopsisandonSynopsisReadybut declares[]as its dependency array. This is intentional ("only on mount"), but it will trigger an ESLintreact-hooks/exhaustive-depswarning. Consider either suppressing the lint rule explicitly or extracting the mount logic to avoid the stale closure risk ononSynopsisReady.If the caller ever changes
onSynopsisReadybetween renders before the mount effect fires (unlikely but possible with strict mode double-mount), the stale reference would be called.Add eslint-disable to document intent
- }, []); // Only on mount + // eslint-disable-next-line react-hooks/exhaustive-deps -- intentionally run only on mount; generateSynopsis captured once + }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/DirectorNotes/AIOverviewTab.tsx` around lines 134 - 148, The effect closes over generateSynopsis and onSynopsisReady but intentionally runs only on mount, so suppress the eslint warning and prevent a stale callback by making the callbacks stable: add an eslint-disable-next-line react-hooks/exhaustive-deps immediately above the useEffect, and ensure generateSynopsis is stable (wrap it in useCallback) or store onSynopsisReady in a ref (e.g., onSynopsisReadyRef.current) and call that ref inside the effect; reference the generateSynopsis function, onSynopsisReady handler, and mountedRef in your changes.src/renderer/components/MainPanel.tsx (1)
398-446: Remove nine unused props fromMainPanelinterface and destructuring.
_markdownEditMode,_gitDiffPreview,_fileTreeFilterOpen,_setMarkdownEditMode,_setAboutModalOpen,_batchRunState,_showConfirmation,_fileTreeContainerRef, and_fileTreeFilterInputRefare destructured at lines 398–446 but never referenced in the component. They unnecessarily inflate the public API and expand the memoization dependency surface inuseMainPanelProps. Remove these props and their interface entries unless they are deliberately reserved for future use.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/MainPanel.tsx` around lines 398 - 446, Remove the nine unused props from the MainPanel public API and destructuring: _markdownEditMode, _gitDiffPreview, _fileTreeFilterOpen, _setMarkdownEditMode, _setAboutModalOpen, _batchRunState, _showConfirmation, _fileTreeContainerRef, and _fileTreeFilterInputRef; update the MainPanel props interface/type to drop these fields and remove them from the destructuring block in the MainPanel component (the long list that includes markdownEditMode, chatRawTextMode, ...); also remove any references to these symbols from useMainPanelProps dependency lists (so memoization dependencies aren’t unnecessarily expanded) and from any prop forwarding sites so the component signature and usage remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/App.tsx`:
- Around line 424-425: App is currently subscribing to store selectors
showHiddenFiles and setShowHiddenFiles (and similar unused slices referenced
elsewhere) but never uses them, causing unnecessary re-renders; remove these
unused reads from the App component (and the other identical unused selector
usages) or, if they are actually needed, wire them through as props or callbacks
to the components that consume them (reference the showHiddenFiles and
setShowHiddenFiles selector usages inside App and the other identical selector
variables elsewhere and either delete the local reads or forward them to child
components).
In `@src/renderer/components/MainPanel.tsx`:
- Around line 1489-1491: In MainPanel.tsx update the title construction that
uses shortcuts.toggleRightPanel.keys to guard against a missing key the same way
the shortcuts.agentSessions check does; use a conditional or optional chaining
so you only call formatShortcutKeys when shortcuts.toggleRightPanel exists (e.g.
compute a shortcutLabel = shortcuts.toggleRightPanel ?
formatShortcutKeys(shortcuts.toggleRightPanel.keys) : '' and use that in the
title) to prevent accessing shortcuts.toggleRightPanel.keys when undefined.
In `@src/renderer/components/SessionList.tsx`:
- Line 1040: The prop activeFocus is currently typed as string which loses
exhaustiveness checks; change its type to the imported FocusArea (the same union
used by setActiveFocus: (focus: FocusArea) => void and comparisons like
activeFocus === 'sidebar') by replacing "activeFocus: string" with "activeFocus:
FocusArea" so the component uses the FocusArea union everywhere for proper type
safety.
In `@src/renderer/components/UsageDashboard/LongestAutoRunsTable.tsx`:
- Line 16: The fetchData calls in LongestAutoRunsTable (triggered by timeRange
changes and onStatsUpdate) can complete out of order and overwrite newer state;
update fetchData to use a latest-request guard or AbortController: create a
requestId or AbortController per invocation (in fetchData or the effect using
timeRange/onStatsUpdate), store it in a ref, and before calling
setSessions/setLoading only apply results if the current requestId matches (or
the fetch wasn’t aborted); ensure previous requests are cancelled or invalidated
in the cleanup of the effect so stale responses do not clear loading or replace
sessions.
---
Outside diff comments:
In `@src/__tests__/renderer/components/DirectorNotes/AIOverviewTab.test.tsx`:
- Around line 207-225: Update the stale inline comment in the test for
AIOverviewTab: change the comment that currently reads "Click refresh" to "Click
Regenerate" so it matches the test name and the button being clicked (the
fireEvent targeting the "Regenerate" button and the mockGenerateSynopsis usage
in the test).
In `@src/__tests__/renderer/components/DocumentGraph/NodeContextMenu.test.tsx`:
- Around line 337-355: The test creates a spy consoleErrorSpy via
vi.spyOn(console, 'error') but only calls consoleErrorSpy.mockRestore() at the
end, which can leak the spy if an assertion throws; wrap the test body (the
logic using mockClipboardWriteText, render(<NodeContextMenu ...>),
fireEvent.click and the waitFor/assertions that expect consoleErrorSpy and
onDismiss) inside a try block and call consoleErrorSpy.mockRestore() in a
finally block so the spy is always restored even on failure; ensure you keep
references to consoleErrorSpy, mockClipboardWriteText, onDismiss and use the
same createProps/NodeContextMenu calls unchanged.
In `@src/renderer/components/MainPanel.tsx`:
- Around line 887-896: The outer container div that toggles the focus ring
doesn't update activeFocus on keyboard focus—add keyboard focus support by
making the div focusable (add tabIndex={0}) and attach an onFocus handler that
calls setActiveFocus('main') and optionally onBlur to clear or change focus;
update the div containing activeFocus, setActiveFocus, and the ring class so
both onClick and onFocus keep behavior consistent with SessionList.
In `@src/renderer/components/SessionList.tsx`:
- Line 583: Multiple places in SessionList.tsx call
formatShortcutKeys(shortcuts.<name>.keys) without guarding for missing shortcut
entries; add null/undefined guards for the six accesses (shortcuts.help,
shortcuts.settings, shortcuts.systemLogs, shortcuts.processMonitor,
shortcuts.usageDashboard, shortcuts.toggleSidebar) so you pass a safe value to
formatShortcutKeys (e.g., use optional chaining like shortcuts?.help?.keys or
check shortcuts.help before accessing .keys, and provide a sensible fallback
such as an empty array or undefined). Update the six call sites to mirror the
existing guarded pattern used for shortcuts.newInstance and
shortcuts.openSymphony to prevent TypeError at runtime.
In `@src/renderer/hooks/props/useMainPanelProps.ts`:
- Around line 499-501: The mergeResult is hardcoded to null so the MainPanel UI
never sees it; update the UseMainPanelPropsDeps interface to include
mergeResult: MergeResult | null, then change the useMainPanelProps creation to
set mergeResult: deps.mergeResult instead of null (look for useMainPanelProps /
MainPanelProps construction) and add deps.mergeResult to the useMemo dependency
array so memo updates when the merge result changes.
---
Nitpick comments:
In `@src/renderer/components/DirectorNotes/AIOverviewTab.tsx`:
- Around line 76-81: The timeout in copyToClipboard (which calls setCopied(true)
then setTimeout(() => setCopied(false), 2000)) can run after unmount; store the
timeout id (from setTimeout) in a ref (e.g., timeoutRef) when scheduling it
inside copyToClipboard and clear that timeout in a useEffect cleanup (or check a
mountedRef before calling setCopied(false)); update the copyToClipboard closure
to save the id to timeoutRef.current and add a useEffect that returns a cleanup
which clearsTimeout(timeoutRef.current) to prevent the callback running after
unmount.
- Around line 134-148: The effect closes over generateSynopsis and
onSynopsisReady but intentionally runs only on mount, so suppress the eslint
warning and prevent a stale callback by making the callbacks stable: add an
eslint-disable-next-line react-hooks/exhaustive-deps immediately above the
useEffect, and ensure generateSynopsis is stable (wrap it in useCallback) or
store onSynopsisReady in a ref (e.g., onSynopsisReadyRef.current) and call that
ref inside the effect; reference the generateSynopsis function, onSynopsisReady
handler, and mountedRef in your changes.
In `@src/renderer/components/DirectorNotes/DirectorNotesModal.tsx`:
- Around line 119-123: In DirectorNotesModal, the useEffect that reads the
render-scoped const cached and calls setOverviewGenerating uses an empty deps
array and should be documented to satisfy lint; add an inline comment and an
eslint-disable-next-line react-hooks/exhaustive-deps above that useEffect (or a
concise explanatory comment inside it) stating that cached is intentionally
stable for this render so the empty dependency array is deliberate to silence
the linter and document intent for future readers.
In `@src/renderer/components/MainPanel.tsx`:
- Around line 398-446: Remove the nine unused props from the MainPanel public
API and destructuring: _markdownEditMode, _gitDiffPreview, _fileTreeFilterOpen,
_setMarkdownEditMode, _setAboutModalOpen, _batchRunState, _showConfirmation,
_fileTreeContainerRef, and _fileTreeFilterInputRef; update the MainPanel props
interface/type to drop these fields and remove them from the destructuring block
in the MainPanel component (the long list that includes markdownEditMode,
chatRawTextMode, ...); also remove any references to these symbols from
useMainPanelProps dependency lists (so memoization dependencies aren’t
unnecessarily expanded) and from any prop forwarding sites so the component
signature and usage remain consistent.
In `@src/renderer/hooks/props/useSessionListProps.ts`:
- Around line 153-282: The returned props from useSessionListProps rename
deps.leftSidebarWidth → leftSidebarWidthState and deps.setLeftSidebarWidth →
setLeftSidebarWidthState which creates a surprising API mismatch; to fix, keep
names consistent by mapping deps.leftSidebarWidth to leftSidebarWidth and
deps.setLeftSidebarWidth to setLeftSidebarWidth (or intentionally document and
standardize the new names everywhere), updating the return object in
useSessionListProps (and any callers) so the prop names leftSidebarWidth and
setLeftSidebarWidth match the deps interface and avoid the implicit rename
confusion.
- Around line 1-10: The docstring for useSessionListProps is misleading about
using "primitive values in dependency arrays" because the hook's dependency
array still contains many non-primitive references (sessions, groups,
sortedSessions, shortcuts, groupChats, visibleSessions, participantStates,
etc.), so update the comment to state that the hook minimizes re-renders by
preferring primitives (e.g., activeSessionId) where possible but relies on those
upstream values being memoized/stable for memoization to be effective; mention
useSessionListProps and list the non-primitive dependencies (sessions, groups,
sortedSessions, shortcuts, groupChats, visibleSessions, participantStates) so
readers know the assumption.
- Around line 283-377: The huge dependency array in useSessionListProps (the
deps[...] list) is brittle; replace the manual 95-item list by using the deps
object itself as the dependency source (e.g., useMemo/useCallback with deps as
the single dependency or derive dependencies from Object.values(deps)) or add a
parity check/linter that asserts returned prop keys match keys in the dependency
list; update references to the deps variable and the memo that currently lists
deps.theme,...,deps.sidebarContainerRef so it uses the single-source deps
approach (or a generated array from deps) to ensure maintainability.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/__tests__/renderer/components/DirectorNotes/AIOverviewTab.test.tsxsrc/__tests__/renderer/components/DocumentGraph/NodeContextMenu.test.tsxsrc/__tests__/renderer/components/GroupChatInput.test.tsxsrc/main/agents/definitions.tssrc/main/ipc/handlers/git.tssrc/renderer/App.tsxsrc/renderer/components/DirectorNotes/AIOverviewTab.tsxsrc/renderer/components/DirectorNotes/DirectorNotesModal.tsxsrc/renderer/components/DocumentGraph/NodeContextMenu.tsxsrc/renderer/components/GroupChatInput.tsxsrc/renderer/components/GroupChatPanel.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/components/Settings/IgnorePatternsSection.tsxsrc/renderer/components/SymphonyModal.tsxsrc/renderer/components/UsageDashboard/ChartSkeletons.tsxsrc/renderer/components/UsageDashboard/LongestAutoRunsTable.tsxsrc/renderer/components/UsageDashboard/SummaryCards.tsxsrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/hooks/props/useSessionListProps.tssrc/renderer/hooks/remote/useRemoteIntegration.ts
✅ Files skipped from review due to trivial changes (4)
- src/renderer/components/Settings/IgnorePatternsSection.tsx
- src/main/agents/definitions.ts
- src/main/ipc/handlers/git.ts
- src/renderer/components/UsageDashboard/ChartSkeletons.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/hooks/remote/useRemoteIntegration.ts
Comment on lines
+424
to
+425
| showHiddenFiles, | ||
| setShowHiddenFiles, |
There was a problem hiding this comment.
Avoid subscribing App to store slices that are not consumed.
These new reads are not used afterward, but they still subscribe this top-level component and can trigger expensive re-renders on file navigation and Auto Run updates.
♻️ Proposed cleanup (or wire them through if intentionally needed)
@@
- showHiddenFiles,
- setShowHiddenFiles,
@@
- const selectedFileIndex = useFileExplorerStore((s) => s.selectedFileIndex);
@@
- const { setSelectedFileIndex, setFileTreeFilter, setFileTreeFilterOpen } =
- useFileExplorerStore.getState();
+ const { setFileTreeFilter, setFileTreeFilterOpen } = useFileExplorerStore.getState();
@@
- const autoRunIsLoadingDocuments = useBatchStore((s) => s.isLoadingDocuments);
- const autoRunDocumentTaskCounts = useBatchStore((s) => s.documentTaskCounts);Also applies to: 601-602, 631-632, 747-748
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/App.tsx` around lines 424 - 425, App is currently subscribing to
store selectors showHiddenFiles and setShowHiddenFiles (and similar unused
slices referenced elsewhere) but never uses them, causing unnecessary
re-renders; remove these unused reads from the App component (and the other
identical unused selector usages) or, if they are actually needed, wire them
through as props or callbacks to the components that consume them (reference the
showHiddenFiles and setShowHiddenFiles selector usages inside App and the other
identical selector variables elsewhere and either delete the local reads or
forward them to child components).
Comment on lines
1489
to
1491
| className="p-2 rounded hover:bg-white/5" | ||
| title={`Show right panel (${formatShortcutKeys(shortcuts.toggleRightPanel.keys)})`} | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
sed -n '1475,1500p' src/renderer/components/MainPanel.tsxRepository: RunMaestro/Maestro
Length of output: 1056
🏁 Script executed:
rg -n "shortcuts" src/renderer/components/MainPanel.tsx | head -20Repository: RunMaestro/Maestro
Length of output: 461
🏁 Script executed:
rg -n "shortcuts\.[a-zA-Z]+" src/renderer/components/MainPanel.tsx -o | grep -v "shortcuts\?" | head -30Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
grep -n "shortcuts\." src/renderer/components/MainPanel.tsx | grep -v "shortcuts={shortcuts}"Repository: RunMaestro/Maestro
Length of output: 327
🏁 Script executed:
rg -n "type Shortcut|interface Shortcut" src/renderer/components/MainPanel.tsx -A 5Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
head -50 src/renderer/components/MainPanel.tsxRepository: RunMaestro/Maestro
Length of output: 1366
🏁 Script executed:
rg -n "type Shortcut|interface Shortcut" --type-list | head -5Repository: RunMaestro/Maestro
Length of output: 451
🏁 Script executed:
rg "type Shortcut|interface Shortcut" -A 3Repository: RunMaestro/Maestro
Length of output: 890
🏁 Script executed:
rg -B 5 -A 5 "shortcuts.*toggleRightPanel" src/Repository: RunMaestro/Maestro
Length of output: 6049
🏁 Script executed:
rg -n "DEFAULT_SHORTCUTS" src/ -A 20 | head -60Repository: RunMaestro/Maestro
Length of output: 4473
🏁 Script executed:
rg -n "export const DEFAULT_SHORTCUTS" src/renderer/constants/shortcuts.ts -A 50Repository: RunMaestro/Maestro
Length of output: 2870
🏁 Script executed:
rg -n "mergeSettings\|spread.*DEFAULT_SHORTCUTS\|Object.assign.*shortcuts" src/renderer/stores/settingsStore.ts -B 3 -A 3Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
rg -n "shortcuts.*=" src/renderer/stores/settingsStore.ts | head -20Repository: RunMaestro/Maestro
Length of output: 158
🏁 Script executed:
sed -n '1410,1440p' src/renderer/stores/settingsStore.tsRepository: RunMaestro/Maestro
Length of output: 1157
🏁 Script executed:
rg -n "function migrateShortcuts\|const migrateShortcuts" src/renderer/stores/settingsStore.ts -A 30Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
rg -n "migrateShortcuts" src/renderer/stores/settingsStore.ts -B 2 -A 15Repository: RunMaestro/Maestro
Length of output: 2188
Add null guard for shortcuts.toggleRightPanel.keys to match the pattern on line 1479.
The shortcuts parameter is typed as Record<string, Shortcut>, making not all keys guaranteed. Line 1479 guards shortcuts.agentSessions with a conditional check, but line 1490 accesses shortcuts.toggleRightPanel.keys without a guard. This inconsistency creates a potential crash if the key is absent.
🛡️ Proposed fix
-title={`Show right panel (${formatShortcutKeys(shortcuts.toggleRightPanel.keys)})`}
+title={`Show right panel${shortcuts.toggleRightPanel ? ` (${formatShortcutKeys(shortcuts.toggleRightPanel.keys)})` : ''}`}📝 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
| className="p-2 rounded hover:bg-white/5" | |
| title={`Show right panel (${formatShortcutKeys(shortcuts.toggleRightPanel.keys)})`} | |
| > | |
| className="p-2 rounded hover:bg-white/5" | |
| title={`Show right panel${shortcuts.toggleRightPanel ? ` (${formatShortcutKeys(shortcuts.toggleRightPanel.keys)})` : ''}`} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/MainPanel.tsx` around lines 1489 - 1491, In
MainPanel.tsx update the title construction that uses
shortcuts.toggleRightPanel.keys to guard against a missing key the same way the
shortcuts.agentSessions check does; use a conditional or optional chaining so
you only call formatShortcutKeys when shortcuts.toggleRightPanel exists (e.g.
compute a shortcutLabel = shortcuts.toggleRightPanel ?
formatShortcutKeys(shortcuts.toggleRightPanel.keys) : '' and use that in the
title) to prevent accessing shortcuts.toggleRightPanel.keys when undefined.
| activeSessionId: string; | ||
| leftSidebarOpen: boolean; | ||
| leftSidebarWidthState: number; | ||
| activeFocus: string; |
There was a problem hiding this comment.
activeFocus prop typed as string instead of the imported FocusArea.
FocusArea ('sidebar' | 'main' | 'right') is imported at line 50 and used correctly everywhere else (setActiveFocus: (focus: FocusArea) => void at line 1068, comparisons like activeFocus === 'sidebar'). Typing it as string loses exhaustiveness checking.
🔧 Proposed fix
-activeFocus: string;
+activeFocus: FocusArea;📝 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
| activeFocus: string; | |
| activeFocus: FocusArea; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/SessionList.tsx` at line 1040, The prop activeFocus
is currently typed as string which loses exhaustiveness checks; change its type
to the imported FocusArea (the same union used by setActiveFocus: (focus:
FocusArea) => void and comparisons like activeFocus === 'sidebar') by replacing
"activeFocus: string" with "activeFocus: FocusArea" so the component uses the
FocusArea union everywhere for proper type safety.
| */ | ||
|
|
||
| import { memo, useState, useEffect, useMemo, useCallback, useRef } from 'react'; | ||
| import { memo, useState, useEffect, useMemo, useCallback } from 'react'; |
There was a problem hiding this comment.
Guard async fetches to prevent stale data overwrites and loading races.
Line 132/133 and Line 137 now accept results from any completed request. If fetchData() overlaps (rapid timeRange changes or frequent onStatsUpdate), an older request can finish last, overwrite newer sessions, and clear loading too early. Please restore a latest-request guard (or abort/cancel strategy) before setSessions/setLoading.
Proposed fix
-import { memo, useState, useEffect, useMemo, useCallback } from 'react';
+import { memo, useState, useEffect, useMemo, useCallback, useRef } from 'react';
@@
export const LongestAutoRunsTable = memo(function LongestAutoRunsTable({
timeRange,
theme,
}: LongestAutoRunsTableProps) {
const [sessions, setSessions] = useState<AutoRunSession[]>([]);
const [loading, setLoading] = useState(true);
+ const latestRequestIdRef = useRef(0);
+ const mountedRef = useRef(true);
+
+ useEffect(() => {
+ return () => {
+ mountedRef.current = false;
+ };
+ }, []);
const fetchData = useCallback(async () => {
+ const requestId = ++latestRequestIdRef.current;
setLoading(true);
try {
const autoRunSessions = await window.maestro.stats.getAutoRunSessions(timeRange);
- setSessions(autoRunSessions);
+ if (!mountedRef.current || requestId !== latestRequestIdRef.current) return;
+ setSessions(autoRunSessions);
} catch (err) {
captureException(err);
} finally {
- setLoading(false);
+ if (!mountedRef.current || requestId !== latestRequestIdRef.current) return;
+ setLoading(false);
}
}, [timeRange]);Also applies to: 129-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/UsageDashboard/LongestAutoRunsTable.tsx` at line 16,
The fetchData calls in LongestAutoRunsTable (triggered by timeRange changes and
onStatsUpdate) can complete out of order and overwrite newer state; update
fetchData to use a latest-request guard or AbortController: create a requestId
or AbortController per invocation (in fetchData or the effect using
timeRange/onStatsUpdate), store it in a ref, and before calling
setSessions/setLoading only apply results if the current requestId matches (or
the fetch wasn’t aborted); ensure previous requests are cancelled or invalidated
in the cleanup of the effect so stale responses do not clear loading or replace
sessions.
This was referenced Feb 26, 2026
6 tasks
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
2 participants
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
Test plan
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
UX Improvements