fix: re-enable batch state debouncing to resolve Auto Run performance regression#465
Closed
felipeggv wants to merge 24 commits intoRunMaestro:mainfrom
Closed
fix: re-enable batch state debouncing to resolve Auto Run performance regression#465felipeggv wants to merge 24 commits intoRunMaestro:mainfrom
felipeggv wants to merge 24 commits intoRunMaestro:mainfrom
Conversation
… regression Debug code was left in useBatchProcessor that bypassed the 200ms debounce mechanism for batch state updates. This caused every progress event to trigger immediate synchronous React re-renders and state broadcasts, resulting in massive UI lag during Auto Run execution. Changes: - Restore updateBatchStateAndBroadcast to delegate to scheduleDebouncedUpdate instead of dispatching directly (was ~70 lines of inline debug bypass) - Remove 7 console.log statements from hot paths (dispatch, mount/unmount, debounce fire, stop, kill) that added overhead during batch processing - Clean up orphaned prevStates/newStates variables from removed debug block The existing useSessionDebounce hook (200ms per-session batching with composed updater functions) was already implemented but unused — the scheduleUpdate binding was prefixed with underscore (_scheduleDebouncedUpdate). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 new Agent Inbox feature (types, hook, list/focus UI, modal integration and priority), a Settings toggle guarded by a new unifiedInbox feature flag (types and default), modal store actions/data for persisting inbox state, and minor refactors to a batch processor hook (debounced update scheduling and logging cleanup). Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes a critical performance regression in Auto Run by removing debug code that completely bypassed the 200ms debouncing mechanism for batch state updates. The debug code in Key changes:
The Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 4c9f951 |
Adds src/renderer/types/agent-inbox.ts with InboxItem, InboxFilterMode, InboxSortMode, InboxViewMode, STATUS_LABELS, and STATUS_COLORS. All imports resolve against current codebase (SessionState at types/index.ts:53). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Register Unified Inbox as an Encore Feature with type flag (boolean) and default value (false) in both EncoreFeatureFlags interface and DEFAULT_ENCORE_FEATURES constant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/renderer/types/agent-inbox.ts (1)
1-10: PreferToolTypeoverstringforInboxItem.toolType.Line 10 currently loosens type guarantees and can allow invalid values to propagate into inbox UI logic.
♻️ Suggested type-tightening diff
-import type { SessionState } from './index'; +import type { SessionState, ToolType } from './index'; @@ - toolType: string; + toolType: ToolType;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/types/agent-inbox.ts` around lines 1 - 10, The InboxItem interface currently types the toolType field as string; change it to the stricter ToolType to prevent invalid values leaking into the inbox UI: import the ToolType symbol (e.g., alongside SessionState from './index' or the module that exports ToolType) and update the InboxItem definition so toolType: ToolType instead of string (refer to the InboxItem interface and the ToolType export to locate the change).
🤖 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/SettingsModal.tsx`:
- Around line 3656-3712: The toggle button in SettingsModal (the button that
reads Unified Inbox and uses encoreFeatures.unifiedInbox) lacks accessible
toggle semantics; update that button to expose state to assistive tech by adding
appropriate ARIA attributes and role — for example set role="switch" and
aria-checked={encoreFeatures.unifiedInbox} (or aria-pressed if you prefer a
button semantic) and include a clear accessible name via aria-label or ensure
the visible label is programmatically associated; keep the existing onClick
handler and visual styles but ensure keyboard focus/activation and state updates
remain unchanged.
- Around line 3718-3720: The SettingsModal documentation text advertises a
non-existent shortcut "Option+I (Mac) or Alt+I (Windows)" for the Unified Inbox;
either remove the shortcut claim from the copy in SettingsModal.tsx (the string
containing "Access via Option+I (Mac) or Alt+I (Windows). Aggregates status
updates...") or implement the shortcut binding: add a constant in
src/renderer/constants/shortcuts.ts (e.g. UNIFIED_INBOX_SHORTCUT) and register a
keyboard handler in the app's keyboard event system that invokes the existing
Unified Inbox action/handler (the code path that opens the Unified Inbox UI) so
the documented shortcut actually triggers that action. Ensure the text in
SettingsModal.tsx and the shortcuts/constants/handler stay consistent.
---
Nitpick comments:
In `@src/renderer/types/agent-inbox.ts`:
- Around line 1-10: The InboxItem interface currently types the toolType field
as string; change it to the stricter ToolType to prevent invalid values leaking
into the inbox UI: import the ToolType symbol (e.g., alongside SessionState from
'./index' or the module that exports ToolType) and update the InboxItem
definition so toolType: ToolType instead of string (refer to the InboxItem
interface and the ToolType export to locate the change).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-02.mdsrc/renderer/components/SettingsModal.tsxsrc/renderer/stores/settingsStore.tssrc/renderer/types/agent-inbox.tssrc/renderer/types/index.ts
Comment on lines
+3656
to
+3712
| <button | ||
| className="w-full flex items-center justify-between p-4 text-left" | ||
| onClick={() => | ||
| setEncoreFeatures({ | ||
| ...encoreFeatures, | ||
| unifiedInbox: !encoreFeatures.unifiedInbox, | ||
| }) | ||
| } | ||
| > | ||
| <div className="flex items-center gap-3"> | ||
| <Inbox | ||
| className="w-5 h-5" | ||
| style={{ | ||
| color: encoreFeatures.unifiedInbox | ||
| ? theme.colors.accent | ||
| : theme.colors.textDim, | ||
| }} | ||
| /> | ||
| <div> | ||
| <div | ||
| className="text-sm font-bold flex items-center gap-2" | ||
| style={{ color: theme.colors.textMain }} | ||
| > | ||
| Unified Inbox | ||
| <span | ||
| className="px-1.5 py-0.5 rounded text-[9px] font-bold uppercase" | ||
| style={{ | ||
| backgroundColor: theme.colors.warning + '30', | ||
| color: theme.colors.warning, | ||
| }} | ||
| > | ||
| Beta | ||
| </span> | ||
| </div> | ||
| <div className="text-xs mt-0.5" style={{ color: theme.colors.textDim }}> | ||
| Cross-session notification center with keyboard-first navigation | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <div | ||
| className={`relative w-10 h-5 rounded-full transition-colors ${encoreFeatures.unifiedInbox ? '' : 'opacity-50'}`} | ||
| style={{ | ||
| backgroundColor: encoreFeatures.unifiedInbox | ||
| ? theme.colors.accent | ||
| : theme.colors.border, | ||
| }} | ||
| > | ||
| <div | ||
| className="absolute top-0.5 w-4 h-4 rounded-full bg-white transition-transform" | ||
| style={{ | ||
| transform: encoreFeatures.unifiedInbox | ||
| ? 'translateX(22px)' | ||
| : 'translateX(2px)', | ||
| }} | ||
| /> | ||
| </div> | ||
| </button> |
There was a problem hiding this comment.
Expose toggle state to assistive technologies.
The Unified Inbox toggle is visually stateful, but Line 3656’s button does not expose pressed/checked state. Add toggle semantics so screen readers can announce enabled/disabled state.
♿ Suggested accessibility fix
<button
className="w-full flex items-center justify-between p-4 text-left"
onClick={() =>
setEncoreFeatures({
...encoreFeatures,
unifiedInbox: !encoreFeatures.unifiedInbox,
})
}
+ aria-pressed={encoreFeatures.unifiedInbox}
+ aria-label={`Unified Inbox ${encoreFeatures.unifiedInbox ? 'enabled' : 'disabled'}`}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/SettingsModal.tsx` around lines 3656 - 3712, The
toggle button in SettingsModal (the button that reads Unified Inbox and uses
encoreFeatures.unifiedInbox) lacks accessible toggle semantics; update that
button to expose state to assistive tech by adding appropriate ARIA attributes
and role — for example set role="switch" and
aria-checked={encoreFeatures.unifiedInbox} (or aria-pressed if you prefer a
button semantic) and include a clear accessible name via aria-label or ensure
the visible label is programmatically associated; keep the existing onClick
handler and visual styles but ensure keyboard focus/activation and state updates
remain unchanged.
Comment on lines
+3718
to
+3720
| Access via Option+I (Mac) or Alt+I (Windows). Aggregates status updates, | ||
| errors, and action items from all running agents into a single | ||
| keyboard-navigable view. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the shortcut mentions in TypeScript/TSX files
echo "=== Searching for Option+I / Alt+I mentions ==="
rg -n --type=ts --type=tsx "Option\+I|Alt\+I|Unified Inbox" | head -20
echo ""
echo "=== Searching for keyboard event handlers related to shortcuts ==="
rg -n --type=ts --type=tsx "KeyI|shortcut.*[iI]|hotkey.*[iI]" | head -30
echo ""
echo "=== Searching for inbox open/toggle handlers ==="
rg -n --type=ts --type=tsx "(altKey|metaKey).*(keyCode|key).*73|key.*73.*(altKey|metaKey)" | head -20
echo ""
echo "=== Searching for keyboard event handling in components ==="
rg -n --type=ts --type=tsx "onKeyDown|onKeyUp|addEventListener.*key" | grep -i "component\|modal" | head -20Repository: RunMaestro/Maestro
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Searching for Option+I / Alt+I mentions ==="
rg -n "Option\+I|Alt\+I|Unified Inbox" --include="*.ts" --include="*.tsx" | head -30
echo ""
echo "=== Searching for keyboard handler patterns ==="
rg -n "KeyI|keyCode.*73|key.*I.*alt|key.*I.*meta" --include="*.ts" --include="*.tsx" | head -30
echo ""
echo "=== Searching for shortcut/hotkey bindings ==="
rg -n "shortcut|hotkey|keybind" --include="*.ts" --include="*.tsx" -i | head -40
echo ""
echo "=== Check SettingsModal specifically for context ==="
rg -n "Option|Alt|shortcut" src/renderer/components/SettingsModal.tsx | head -20Repository: RunMaestro/Maestro
Length of output: 568
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Searching for Option+I / Alt+I mentions ==="
fd -e ts -e tsx | xargs rg -n "Option\+I|Alt\+I|Unified Inbox" | head -30
echo ""
echo "=== Searching for keyboard handlers with key code 73 (I key) ==="
fd -e ts -e tsx | xargs rg -n "keyCode.*73|key.*[\"']i[\"']" | head -30
echo ""
echo "=== Check SettingsModal file for context around line 3718 ==="
cat -n src/renderer/components/SettingsModal.tsx | sed -n '3710,3730p'
echo ""
echo "=== Search for keyboard event listeners in codebase ==="
fd -e ts -e tsx | xargs rg -n "onKeyDown|onKeyPress|addEventListener.*key" | head -20
echo ""
echo "=== Search for shortcuts/keybindings mention ==="
fd -e ts -e tsx | xargs rg -n "shortcut|keybind|hotkey" -i | head -20Repository: RunMaestro/Maestro
Length of output: 576
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== View context around line 3718 ==="
cat -n src/renderer/components/SettingsModal.tsx | sed -n '3710,3730p'
echo ""
echo "=== Search for Unified Inbox open/toggle handlers ==="
fd -e ts -e tsx | xargs rg -n "Unified.*Inbox|unifiedInbox|unified_inbox" -i | head -20
echo ""
echo "=== Search for any Alt key handlers ==="
fd -e ts -e tsx | xargs rg -n "altKey|metaKey" | head -20
echo ""
echo "=== Search for keyboard shortcuts/command palette ==="
fd -e ts -e tsx | xargs rg -n "registerShortcut|registerKeybinding|keymap|shortcut.*map" -i | head -20
echo ""
echo "=== Search for event listeners that might handle Alt+I ==="
fd -e ts -e tsx | xargs rg -n "addEventListener|on.*key|handleKey" -i | head -30Repository: RunMaestro/Maestro
Length of output: 10811
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for where Unified Inbox is actually opened/triggered ==="
fd -e ts -e tsx | xargs rg -n "openUnifiedInbox|showUnifiedInbox|unifiedInbox.*open|inbox.*trigger" -i | head -20
echo ""
echo "=== Search for shortcut definitions/registrations ==="
fd -e ts -e tsx | xargs rg -n "Alt.*I|Option.*I" | head -20
echo ""
echo "=== Check if shortcuts are defined in constants ==="
fd -e ts -e tsx | xargs rg -n "const.*SHORTCUTS|const.*SHORTCUT" -A5 | head -40
echo ""
echo "=== Search in ShortcutEditor or shortcuts config ==="
cat -n src/renderer/components/ShortcutEditor.tsx 2>/dev/null | head -50
echo ""
echo "=== Look for where shortcuts are consumed in handlers ==="
fd -e ts -e tsx | xargs rg -n "handleKeyDown|handleKeyPress" | grep -v test | head -20Repository: RunMaestro/Maestro
Length of output: 131
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== List files that might contain shortcuts ==="
fd -e ts -e tsx | xargs grep -l "shortcut" | head -20
echo ""
echo "=== Search more broadly for Inbox opening ==="
fd -e ts -e tsx -e js | xargs grep -l "inbox" -i | head -20
echo ""
echo "=== Check main app files ==="
ls -la src/renderer/ | head -20
echo ""
echo "=== Look at App.tsx or main entry ==="
fd -name "App.tsx" -o -name "main.tsx" -o -name "index.tsx"Repository: RunMaestro/Maestro
Length of output: 993
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check App.tsx for keyboard handlers ==="
wc -l src/renderer/App.tsx
echo ""
head -100 src/renderer/App.tsx
echo ""
echo "=== Search for keyboard event listener in App.tsx ==="
grep -n "onKeyDown\|onKeyPress\|addEventListener\|shortcut" src/renderer/App.tsx | head -20
echo ""
echo "=== Check settingsStore for shortcut definitions ==="
grep -n "unifiedInbox\|Alt.*I\|Option.*I" src/renderer/stores/settingsStore.ts | head -20
echo ""
echo "=== Search for keyboard handler in AppModals or AppOverlays ==="
grep -n "onKeyDown\|handleKey\|keyboard" src/renderer/components/AppModals.tsx | head -10
grep -n "onKeyDown\|handleKey\|keyboard" src/renderer/components/AppOverlays.tsx | head -10Repository: RunMaestro/Maestro
Length of output: 5089
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find useMainKeyboardHandler ==="
fd -e ts -e tsx | xargs grep -l "useMainKeyboardHandler" | head -10
echo ""
echo "=== Search for Alt+I in useMainKeyboardHandler ==="
fd -e ts -e tsx | xargs grep -l "useMainKeyboardHandler" | xargs grep -l "Alt.*I\|Option.*I"
echo ""
echo "=== Find the hook definition ==="
fd -name "*useMainKeyboardHandler*"
echo ""
echo "=== Search for where shortcut handlers are defined ==="
fd -e ts -e tsx | xargs grep -l "shortcuts\[" | head -10
echo ""
echo "=== Look for keyboard shortcuts configuration ==="
fd -name "*shortcut*" -type f | head -20Repository: RunMaestro/Maestro
Length of output: 425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read useMainKeyboardHandler.ts ==="
wc -l src/renderer/hooks/keyboard/useMainKeyboardHandler.ts
echo ""
cat -n src/renderer/hooks/keyboard/useMainKeyboardHandler.ts | head -150
echo ""
echo "=== Search for shortcut matching/handling logic ==="
grep -n "shortcut\|Alt\|Option\|metaKey\|altKey" src/renderer/hooks/keyboard/useMainKeyboardHandler.ts | head -40Repository: RunMaestro/Maestro
Length of output: 11816
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Continue reading useMainKeyboardHandler.ts (rest of file) ==="
cat -n src/renderer/hooks/keyboard/useMainKeyboardHandler.ts | tail -n +150 | head -100
echo ""
echo "=== Search entire codebase for Alt+I handler ==="
fd -e ts -e tsx | xargs grep -n "altKey.*keyLower.*===.*'i'\|key.*===.*'i'.*altKey\|keyCode.*73" | head -20
echo ""
echo "=== Search for where unifiedInbox shortcut would be handled ==="
fd -e ts -e tsx | xargs grep -n "unifiedInbox" | head -20
echo ""
echo "=== Check if Alt+I appears anywhere as a shortcut ==="
fd -e ts -e tsx | xargs grep -n "'i'" | grep -i "alt\|shift.*i" | head -20Repository: RunMaestro/Maestro
Length of output: 4535
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for unifiedInbox in keyboard handler ==="
grep -n "unifiedInbox" src/renderer/hooks/keyboard/useMainKeyboardHandler.ts
echo ""
echo "=== Check entire codebase for Alt+I ==="
fd -e ts -e tsx | xargs grep -i "alt.*i\|option.*i" | grep -v "option.*interactive\|option.*int\|optional\|altitude\|alternativ\|allergen" | head -30
echo ""
echo "=== Search for where inbox is opened ==="
fd -e ts -e tsx | xargs grep -n "inbox.*open\|open.*inbox\|show.*inbox" -i | head -20
echo ""
echo "=== Search for all keyboard shortcuts mapped ==="
grep -n "ctx.isShortcut(e," src/renderer/hooks/keyboard/useMainKeyboardHandler.ts | head -40Repository: RunMaestro/Maestro
Length of output: 113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check entire codebase for Alt+I or Option+I ==="
fd -e ts -e tsx | xargs grep "Alt.*I\|Option.*I" 2>/dev/null | head -20
echo ""
echo "=== List all shortcuts defined via isShortcut ==="
grep -n "ctx.isShortcut(e," src/renderer/hooks/keyboard/useMainKeyboardHandler.ts
echo ""
echo "=== Check SettingsModal line 3718 in context again ==="
sed -n '3710,3730p' src/renderer/components/SettingsModal.tsx
echo ""
echo "=== Search for shortcuts object definition ==="
fd -e ts | xargs grep -l "shortcuts.*=" | head -10
echo ""
echo "=== Check if there's a default shortcuts object ==="
fd -e ts | xargs grep -n "const.*shortcuts.*=.*{" | head -20Repository: RunMaestro/Maestro
Length of output: 2115
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for shortcuts configuration file ==="
fd -e ts | xargs grep -l "toggleSidebar\|toggleRightPanel" | head -10
echo ""
echo "=== Check settingsStore for all default shortcuts ==="
grep -n -A 50 "const.*shortcuts\|export.*shortcuts" src/renderer/stores/settingsStore.ts | head -80
echo ""
echo "=== Look for keyboard shortcut definitions ==="
fd -e ts -e json | xargs grep -l "shortcut.*Key\|keyboard.*map" | grep -v test | head -10Repository: RunMaestro/Maestro
Length of output: 2390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read shortcuts.ts ==="
wc -l src/renderer/constants/shortcuts.ts
echo ""
cat -n src/renderer/constants/shortcuts.ts
echo ""
echo "=== Search for unifiedInbox in shortcuts ==="
grep -n "unifiedInbox\|Alt.*I\|Option.*I" src/renderer/constants/shortcuts.tsRepository: RunMaestro/Maestro
Length of output: 8813
Remove misleading keyboard shortcut documentation; the documented shortcut is not actually implemented.
Line 3718 advertises "Option+I (Mac) or Alt+I (Windows)" to access Unified Inbox, but this shortcut does not exist in the codebase. It is not defined in src/renderer/constants/shortcuts.ts and has no handler in the keyboard event system. Either implement the shortcut binding or remove the shortcut claim from the documentation to avoid misleading users.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/SettingsModal.tsx` around lines 3718 - 3720, The
SettingsModal documentation text advertises a non-existent shortcut "Option+I
(Mac) or Alt+I (Windows)" for the Unified Inbox; either remove the shortcut
claim from the copy in SettingsModal.tsx (the string containing "Access via
Option+I (Mac) or Alt+I (Windows). Aggregates status updates...") or implement
the shortcut binding: add a constant in src/renderer/constants/shortcuts.ts
(e.g. UNIFIED_INBOX_SHORTCUT) and register a keyboard handler in the app's
keyboard event system that invokes the existing Unified Inbox action/handler
(the code path that opens the Unified Inbox UI) so the documented shortcut
actually triggers that action. Ensure the text in SettingsModal.tsx and the
shortcuts/constants/handler stay consistent.
…re branch Port 4 files from feature/inbox-focus-polish into current architecture: - useAgentInbox.ts: data aggregation hook (filter, sort, smart summaries) - AgentInbox/index.tsx: modal shell with list/focus view switching - AgentInbox/InboxListView.tsx: virtualized list (react-window → @tanstack/react-virtual) - AgentInbox/FocusModeView.tsx: focus mode detail view with reply input Also adds: - AGENT_INBOX priority (555) in modalPriorities.ts - agentInbox ModalId, AgentInboxModalData, and modal actions in modalStore.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…re gating - Add agentInboxOpen reactive state to useModalActions() in modalStore.ts - Add lazy import for AgentInbox component in App.tsx - Destructure agentInboxOpen/setAgentInboxOpen from useModalActions() - Create handleAgentInboxNavigateToSession callback (close modal + switch session) - Add gated modal render: encoreFeatures.unifiedInbox && agentInboxOpen - Wire conditional setters for SessionList and QuickActions/AppModals - Add setAgentInboxOpen to keyboard handler ref for future keybinding support - 2 expected TS errors for props not yet updated (phases 06-08) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/renderer/hooks/useAgentInbox.ts (1)
5-50: AlignlastMessagetruncation length with the shared inbox contract.Line 5 uses a 250-char cap, while
InboxItem.lastMessageis documented as truncated to 90 chars insrc/renderer/types/agent-inbox.ts. Keeping one canonical limit avoids inconsistent rendering assumptions.Suggested change
-const MAX_MESSAGE_LENGTH = 250; +const MAX_MESSAGE_LENGTH = 90;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/useAgentInbox.ts` around lines 5 - 50, MAX_MESSAGE_LENGTH is set to 250 and used by truncate(), but InboxItem.lastMessage is specified as truncated to 90 chars; change the canonical limit to 90 to match the shared inbox contract by updating MAX_MESSAGE_LENGTH to 90 and ensure truncate(text: string) uses that constant; verify references to MAX_MESSAGE_LENGTH and truncate (in useAgentInbox.ts and any callers) remain consistent with the 90-char contract.
🤖 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/AgentInbox/FocusModeView.tsx`:
- Around line 692-707: The reply handlers handleQuickReply and
handleOpenAndReply send replies but never invoke the explicit read-flow; after
calling onQuickReply(item.sessionId, item.tabId, text) and
onOpenAndReply(item.sessionId, item.tabId, text) respectively, call
onMarkAsRead(item.sessionId, item.tabId) if onMarkAsRead is provided (and then
clear reply state in both paths as appropriate—e.g., setReplyText('') in
handleOpenAndReply as well); update the dependency arrays to include
onMarkAsRead so the callbacks stay current.
In `@src/renderer/components/AgentInbox/index.tsx`:
- Around line 207-216: The keyboard handler handleCaptureKeyDown only captures
'[' and ']' with meta/ctrl but the UI shows ⌘←/⌘→, so update
handleCaptureKeyDown to also treat ArrowLeft and ArrowRight (with e.metaKey ||
e.ctrlKey) as aliases for '[' and ']' respectively (and then call setFocusIndex
the same way), or alternatively update the UI hints to reflect '['/']' if you
prefer; adjust the conditional inside handleCaptureKeyDown and any related logic
that sets focus (setFocusIndex) so arrow keys move focus the same way as the
bracket keys.
In `@src/renderer/stores/modalStore.ts`:
- Around line 665-667: setAgentInboxOpen may drop the first update because
updateModalData bails if current.data is undefined; update setAgentInboxOpen to
accept an optional data?: Record<string, unknown> and ensure the modal is
created with initial data or that updateModalData will initialize data when the
modal exists: either pass the data into openModal('agentInbox') when opening, or
change updateModalData to no longer return early but to initialize current.data
= {} (then merge) when current exists but data is undefined; reference
setAgentInboxOpen, openModal, updateAgentInboxData (updateModalData) and
updateModalData's current.data check when applying the fix.
---
Nitpick comments:
In `@src/renderer/hooks/useAgentInbox.ts`:
- Around line 5-50: MAX_MESSAGE_LENGTH is set to 250 and used by truncate(), but
InboxItem.lastMessage is specified as truncated to 90 chars; change the
canonical limit to 90 to match the shared inbox contract by updating
MAX_MESSAGE_LENGTH to 90 and ensure truncate(text: string) uses that constant;
verify references to MAX_MESSAGE_LENGTH and truncate (in useAgentInbox.ts and
any callers) remain consistent with the 90-char contract.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-04.mdsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.ts
Comment on lines
+692
to
+707
| const handleQuickReply = useCallback(() => { | ||
| const text = replyText.trim(); | ||
| if (!text) return; | ||
| if (onQuickReply) { | ||
| onQuickReply(item.sessionId, item.tabId, text); | ||
| } | ||
| setReplyText(''); | ||
| }, [replyText, item, onQuickReply]); | ||
|
|
||
| const handleOpenAndReply = useCallback(() => { | ||
| const text = replyText.trim(); | ||
| if (!text) return; | ||
| if (onOpenAndReply) { | ||
| onOpenAndReply(item.sessionId, item.tabId, text); | ||
| } | ||
| }, [replyText, item, onOpenAndReply]); |
There was a problem hiding this comment.
Reply actions never trigger the explicit “mark as read” path.
Lines 692-707 send replies but never invoke onMarkAsRead, even though Line 723 states unread should clear on explicit interaction. This can keep replied threads stuck as unread.
Suggested change
const handleQuickReply = useCallback(() => {
const text = replyText.trim();
if (!text) return;
if (onQuickReply) {
onQuickReply(item.sessionId, item.tabId, text);
}
+ onMarkAsRead?.(item.sessionId, item.tabId);
setReplyText('');
- }, [replyText, item, onQuickReply]);
+ }, [replyText, item, onQuickReply, onMarkAsRead]);
const handleOpenAndReply = useCallback(() => {
const text = replyText.trim();
if (!text) return;
if (onOpenAndReply) {
onOpenAndReply(item.sessionId, item.tabId, text);
}
- }, [replyText, item, onOpenAndReply]);
+ onMarkAsRead?.(item.sessionId, item.tabId);
+ }, [replyText, item, onOpenAndReply, onMarkAsRead]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx` around lines 692 - 707,
The reply handlers handleQuickReply and handleOpenAndReply send replies but
never invoke the explicit read-flow; after calling onQuickReply(item.sessionId,
item.tabId, text) and onOpenAndReply(item.sessionId, item.tabId, text)
respectively, call onMarkAsRead(item.sessionId, item.tabId) if onMarkAsRead is
provided (and then clear reply state in both paths as appropriate—e.g.,
setReplyText('') in handleOpenAndReply as well); update the dependency arrays to
include onMarkAsRead so the callbacks stay current.
Comment on lines
+207
to
+216
| const handleCaptureKeyDown = useCallback( | ||
| (e: React.KeyboardEvent) => { | ||
| if (viewMode !== 'focus') return; | ||
| if ((e.metaKey || e.ctrlKey) && (e.key === '[' || e.key === ']')) { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| if (items.length > 1) { | ||
| setFocusIndex((prev) => | ||
| e.key === '[' ? Math.max(prev - 1, 0) : Math.min(prev + 1, items.length - 1) | ||
| ); |
There was a problem hiding this comment.
Focus navigation shortcuts don’t match the UI hints.
Line 210 only handles Cmd/Ctrl + [ and ], while Focus Mode surfaces ⌘←/⌘→ hints in controls. Add arrow-key aliases here (or update the labels) to avoid mismatch.
Suggested change
- if ((e.metaKey || e.ctrlKey) && (e.key === '[' || e.key === ']')) {
+ if (
+ (e.metaKey || e.ctrlKey) &&
+ (e.key === '[' || e.key === ']' || e.key === 'ArrowLeft' || e.key === 'ArrowRight')
+ ) {
e.preventDefault();
e.stopPropagation();
if (items.length > 1) {
setFocusIndex((prev) =>
- e.key === '[' ? Math.max(prev - 1, 0) : Math.min(prev + 1, items.length - 1)
+ e.key === '[' || e.key === 'ArrowLeft'
+ ? Math.max(prev - 1, 0)
+ : Math.min(prev + 1, items.length - 1)
);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/index.tsx` around lines 207 - 216, The
keyboard handler handleCaptureKeyDown only captures '[' and ']' with meta/ctrl
but the UI shows ⌘←/⌘→, so update handleCaptureKeyDown to also treat ArrowLeft
and ArrowRight (with e.metaKey || e.ctrlKey) as aliases for '[' and ']'
respectively (and then call setFocusIndex the same way), or alternatively update
the UI hints to reflect '['/']' if you prefer; adjust the conditional inside
handleCaptureKeyDown and any related logic that sets focus (setFocusIndex) so
arrow keys move focus the same way as the bracket keys.
Comment on lines
+665
to
+667
| setAgentInboxOpen: (open: boolean) => | ||
| open ? openModal('agentInbox') : closeModal('agentInbox'), | ||
| updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data), |
There was a problem hiding this comment.
Agent Inbox state updates can be dropped on first open.
Line 666 opens agentInbox with no data. Because updateModalData bails when current.data is undefined (Line 366), Line 667 can no-op and never persist filter/sort/expand state.
Suggested fix
// Agent Inbox Modal
setAgentInboxOpen: (open: boolean) =>
- open ? openModal('agentInbox') : closeModal('agentInbox'),
- updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data),
+ open
+ ? openModal('agentInbox', {
+ filterMode: 'unread',
+ sortMode: 'newest',
+ isExpanded: false,
+ })
+ : closeModal('agentInbox'),
+ updateAgentInboxData: (data: Record<string, unknown>) => {
+ const current = useModalStore.getState().getData('agentInbox');
+ if (!current) {
+ openModal('agentInbox', data);
+ return;
+ }
+ updateModalData('agentInbox', data);
+ },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/stores/modalStore.ts` around lines 665 - 667, setAgentInboxOpen
may drop the first update because updateModalData bails if current.data is
undefined; update setAgentInboxOpen to accept an optional data?: Record<string,
unknown> and ensure the modal is created with initial data or that
updateModalData will initialize data when the modal exists: either pass the data
into openModal('agentInbox') when opening, or change updateModalData to no
longer return early but to initialize current.data = {} (then merge) when
current exists but data is undefined; reference setAgentInboxOpen, openModal,
updateAgentInboxData (updateModalData) and updateModalData's current.data check
when applying the fix.
…ncore gate Added agentInbox shortcut definition (Alt+I) to shortcuts.ts and wired the keyboard handler in useMainKeyboardHandler.ts, gated behind encoreFeatures.unifiedInbox. App.tsx context already had setAgentInboxOpen wired from Phase 05. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wire setAgentInboxOpen through all prop interfaces (HamburgerMenuContentProps, SessionListProps, UseSessionListPropsDeps) and add conditionally-rendered Inbox button after Director's Notes in the hamburger menu. Button only appears when the Encore feature gate provides the setter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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/App.tsx`:
- Line 5239: Add the missing unified inbox props to the appropriate TypeScript
interfaces: extend UseSessionListPropsDeps with an optional setAgentInboxOpen?:
(open: boolean) => void; following the existing setDirectorNotesOpen pattern,
and add onOpenUnifiedInbox?: () => void; to AppModalsProps following the
onOpenDirectorNotes pattern so the props passed at setAgentInboxOpen:
encoreFeatures.unifiedInbox ? setAgentInboxOpen : undefined and the modal-open
handler align with the declared interfaces.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-05.mdsrc/renderer/App.tsxsrc/renderer/stores/modalStore.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/stores/modalStore.ts
| setUsageDashboardOpen, | ||
| setSymphonyModalOpen, | ||
| setDirectorNotesOpen: encoreFeatures.directorNotes ? setDirectorNotesOpen : undefined, | ||
| setAgentInboxOpen: encoreFeatures.unifiedInbox ? setAgentInboxOpen : undefined, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that new App.tsx props are present in dependent type surfaces.
# SessionList props/deps side
rg -n -C3 "UseSessionListPropsDeps|setAgentInboxOpen" src -g "*.{ts,tsx}"
# AppModals props side
rg -n -C3 "AppModalsProps|onOpenUnifiedInbox" src -g "*.{ts,tsx}"
# App.tsx call sites for quick comparison
rg -n -C2 "setAgentInboxOpen: encoreFeatures\\.unifiedInbox|onOpenUnifiedInbox" src/renderer/App.tsxRepository: RunMaestro/Maestro
Length of output: 6922
🏁 Script executed:
#!/bin/bash
# Check UseSessionListPropsDeps full definition
sed -n '29,100p' src/renderer/hooks/props/useSessionListProps.ts
# Check the component being called at line 5655
sed -n '5645,5660p' src/renderer/App.tsx
# Check SessionList component props definition
rg -n "interface.*SessionListProps|type.*SessionListProps" src -g "*.{ts,tsx}" -A 30Repository: RunMaestro/Maestro
Length of output: 8245
🏁 Script executed:
#!/bin/bash
# Search for setAgentInboxOpen in the full UseSessionListPropsDeps
rg -n "setAgentInboxOpen" src/renderer/hooks/props/useSessionListProps.ts
# Search for onOpenUnifiedInbox in SessionListProps
rg -n "onOpenUnifiedInbox" src/renderer/components/SessionList.tsx
# Check AppModalsProps for these props
rg -n "onOpenUnifiedInbox|setAgentInboxOpen" src/renderer/components/AppModals.tsx
# Get the component name at line 5655 in App.tsx
sed -n '5640,5665p' src/renderer/App.tsx | head -30Repository: RunMaestro/Maestro
Length of output: 1234
🏁 Script executed:
#!/bin/bash
# Find what component is being rendered around line 5655
sed -n '5620,5670p' src/renderer/App.tsx | grep -B 30 "onOpenUnifiedInbox" | head -40
# Explicitly check for setAgentInboxOpen in UseSessionListPropsDeps
grep -n "setAgentInboxOpen" src/renderer/hooks/props/useSessionListProps.ts
# Check full AppModalsProps interface definition
sed -n '1765,1850p' src/renderer/components/AppModals.tsxRepository: RunMaestro/Maestro
Length of output: 4235
🏁 Script executed:
#!/bin/bash
# Find the component name before the onOpenUnifiedInbox prop
sed -n '5600,5680p' src/renderer/App.tsx | grep -A 2 -B 50 "onOpenUnifiedInbox" | tail -60
# Check what component expects onOpenUnifiedInbox
rg -n "onOpenUnifiedInbox" src/renderer/components -g "*.{ts,tsx}" -B 5Repository: RunMaestro/Maestro
Length of output: 2517
🏁 Script executed:
#!/bin/bash
# Find the opening tag for this component
sed -n '5500,5680p' src/renderer/App.tsx | grep -E "^[[:space:]]*<[A-Z]" | tail -5
# More comprehensive search - find the component opening tag
sed -n '5620,5700p' src/renderer/App.tsx | head -100Repository: RunMaestro/Maestro
Length of output: 3636
🏁 Script executed:
#!/bin/bash
# Find opening tag - look backwards from line 5655
sed -n '5500,5655p' src/renderer/App.tsx | grep -n "^[[:space:]]*<[A-Z]" | tail -3
# Or search directly in a smaller window
sed -n '5600,5620p' src/renderer/App.tsxRepository: RunMaestro/Maestro
Length of output: 1051
🏁 Script executed:
#!/bin/bash
# Search from line 5400 to find the component opening tag
sed -n '5400,5620p' src/renderer/App.tsx | grep -n "^[[:space:]]*<[A-Z]" | tail -5
# Alternative: look for closing tags to identify what component has all these props
sed -n '5500,5620p' src/renderer/App.tsx | head -20Repository: RunMaestro/Maestro
Length of output: 1014
🏁 Script executed:
#!/bin/bash
# Check full AppModalsProps interface for onOpenUnifiedInbox
sed -n '1765,2100p' src/renderer/components/AppModals.tsx | grep -n "onOpenUnifiedInbox\|onOpenDirectorNotes"
# Check complete UseSessionListPropsDeps for setAgentInboxOpen
sed -n '29,150p' src/renderer/hooks/props/useSessionListProps.ts | grep -n "setAgentInboxOpen\|setDirectorNotesOpen"Repository: RunMaestro/Maestro
Length of output: 152
🏁 Script executed:
#!/bin/bash
# Get complete AppModalsProps - search for onOpenUnifiedInbox explicitly
rg -n "onOpenUnifiedInbox" src/renderer/components/AppModals.tsx
# Get complete UseSessionListPropsDeps - search for setAgentInboxOpen explicitly
rg -n "setAgentInboxOpen" src/renderer/hooks/props/useSessionListProps.ts
# Check the line numbers where these interfaces end
sed -n '1765,1900p' src/renderer/components/AppModals.tsx | grep -n "^}" | head -1
sed -n '29,150p' src/renderer/hooks/props/useSessionListProps.ts | grep -n "^}" | head -1Repository: RunMaestro/Maestro
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Confirm onOpenUnifiedInbox is NOT in AppModalsProps
grep -c "onOpenUnifiedInbox" src/renderer/components/AppModals.tsx
# Confirm setAgentInboxOpen is NOT in UseSessionListPropsDeps
grep -c "setAgentInboxOpen" src/renderer/hooks/props/useSessionListProps.ts
# Show the context around onOpenDirectorNotes in AppModalsProps to see the pattern
sed -n '2000,2030p' src/renderer/components/AppModals.tsx
# Show the context around setDirectorNotesOpen in UseSessionListPropsDeps to see the pattern
sed -n '90,110p' src/renderer/hooks/props/useSessionListProps.tsRepository: RunMaestro/Maestro
Length of output: 1977
🏁 Script executed:
#!/bin/bash
# Show where onOpenDirectorNotes appears in AppModalsProps context
sed -n '2010,2020p' src/renderer/components/AppModals.tsx
# Show where setDirectorNotesOpen appears in UseSessionListPropsDeps
sed -n '97,107p' src/renderer/hooks/props/useSessionListProps.tsRepository: RunMaestro/Maestro
Length of output: 869
🏁 Script executed:
#!/bin/bash
# Check what component is rendered around line 5239
sed -n '5200,5250p' src/renderer/App.tsx | grep -B 50 "setAgentInboxOpen: encoreFeatures" | head -60Repository: RunMaestro/Maestro
Length of output: 1084
🏁 Script executed:
#!/bin/bash
# Find what this object is being passed to
sed -n '5150,5200p' src/renderer/App.tsxRepository: RunMaestro/Maestro
Length of output: 1202
🏁 Script executed:
#!/bin/bash
# Confirm AppModalsProps location and structure
sed -n '1765,1770p' src/renderer/components/AppModals.tsx
# Verify the component at line 5655 is AppModals
sed -n '5454,5460p' src/renderer/App.tsxRepository: RunMaestro/Maestro
Length of output: 429
Add setAgentInboxOpen to UseSessionListPropsDeps and onOpenUnifiedInbox to AppModalsProps.
The code at lines 5239 and 5655–5657 passes these Unified Inbox props to dependent components, but the corresponding prop interfaces do not include them. This will cause TypeScript errors (TS2353/TS2322).
Add setAgentInboxOpen?: (open: boolean) => void; to UseSessionListPropsDeps (following the setDirectorNotesOpen pattern) and onOpenUnifiedInbox?: () => void; to AppModalsProps (following the onOpenDirectorNotes pattern).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/App.tsx` at line 5239, Add the missing unified inbox props to
the appropriate TypeScript interfaces: extend UseSessionListPropsDeps with an
optional setAgentInboxOpen?: (open: boolean) => void; following the existing
setDirectorNotesOpen pattern, and add onOpenUnifiedInbox?: () => void; to
AppModalsProps following the onOpenDirectorNotes pattern so the props passed at
setAgentInboxOpen: encoreFeatures.unifiedInbox ? setAgentInboxOpen : undefined
and the modal-open handler align with the declared interfaces.
Added onOpenUnifiedInbox prop to QuickActionsModal and threaded it through AppModals (both AppUtilityModalsProps and AppModalsProps interfaces). The action entry appears conditionally after Director's Notes, matching the existing Encore gate pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prefix onMarkAsRead with _ since it's destructured but not yet used in the FocusModeView component body. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace unconditional scroll-to-bottom on every log update with smart proximity detection: only auto-scroll when user is within 150px of bottom or when navigating to a new item. Prevents scroll hijacking while reading earlier messages during active agent output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lection sync - Bug 1: Changed group header <div> to <button type="button"> with onKeyDown handler for Enter/Space keyboard accessibility - Bug 2: Scoped collapse filtering to only apply when sortMode is 'grouped' or 'byAgent', preventing hidden items in other sort modes - Bug 3: Added guard useEffect to reset parent selectedIndex when it points to a collapsed-away item Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Overlay animation: duration-150 → duration-100 - InboxListView header: px-6 → px-4 py-3, text-base → text-lg, added Bot icon (w-5 h-5, accent) - FocusModeView header: added py-3, removed fixed height, text-sm font-bold → text-lg font-semibold - Footer padding aligned to px-4 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/renderer/components/AgentInbox/FocusModeView.tsx (1)
568-569:⚠️ Potential issue | 🟠 MajorReply actions should execute explicit read-state updates.
onMarkAsReadis available but never called, so replied items can remain unread. Also,handleOpenAndReplydoesn’t clear the draft.Suggested fix
- onMarkAsRead: _onMarkAsRead, + onMarkAsRead, @@ const handleQuickReply = useCallback(() => { const text = replyText.trim(); if (!text) return; if (onQuickReply) { onQuickReply(item.sessionId, item.tabId, text); } + onMarkAsRead?.(item.sessionId, item.tabId); setReplyText(''); - }, [replyText, item, onQuickReply]); + }, [replyText, item, onQuickReply, onMarkAsRead]); const handleOpenAndReply = useCallback(() => { const text = replyText.trim(); if (!text) return; if (onOpenAndReply) { onOpenAndReply(item.sessionId, item.tabId, text); } - }, [replyText, item, onOpenAndReply]); + onMarkAsRead?.(item.sessionId, item.tabId); + setReplyText(''); + }, [replyText, item, onOpenAndReply, onMarkAsRead]);Also applies to: 704-719
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AgentInbox/FocusModeView.tsx` around lines 568 - 569, The reply flow currently never calls the provided onMarkAsRead handler and leaves drafts uncleared; update the reply actions (where onMarkAsRead: _onMarkAsRead and onToggleThinking are passed) to explicitly invoke _onMarkAsRead for the item being replied to and then clear the draft state inside handleOpenAndReply (or the reply submit handler used in the 704-719 block), ensuring you pass the correct item id/context to _onMarkAsRead and reset the draft variable or state after sending the reply so the message is marked read and the draft is emptied.
🧹 Nitpick comments (1)
src/renderer/components/AgentInbox/InboxListView.tsx (1)
1132-1144: Avoid per-header forward scans inbyAgentmode.Line 1136+ scans subsequent rows for every visible header. With many rows/groups this becomes quadratic work during render. Precompute
{toolType, unreadCount}by group once in auseMemoand read by key during row render.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AgentInbox/InboxListView.tsx` around lines 1132 - 1144, The per-header forward scan in InboxListView (the block using sortMode === 'byAgent', virtualRow.index, rows, agentToolType, unreadCount) causes quadratic render cost; compute the toolType and unreadCount per group once using a useMemo keyed by the group id (or header key) before rendering, producing a map from groupKey-> {toolType, unreadCount}, then in the row render (where virtualRow and rows are used) look up the precomputed values by the header's group key instead of scanning subsequent rows. Ensure the useMemo depends on rows and sortMode and reference the same group identifier used by headers/items so the lookup is O(1) during render.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-11.md`:
- Around line 20-27: Update the useMemo guard in the rows computation so
collapsing also applies when sortMode is 'byAgent'; replace the current
condition (if (sortMode !== 'grouped' || collapsedGroups.size === 0) return
allRows;) with a guard that returns allRows unless sortMode is 'grouped' or
'byAgent' (e.g. if (!['grouped','byAgent'].includes(sortMode) ||
collapsedGroups.size === 0) return allRows;), keeping the rest of the filter
logic that checks row.type === 'header' and item.groupName.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 426-487: The header and item clickable divs in FocusModeView (the
element with key `header-${row.groupName}-${rowIdx}` that toggles
setCollapsedGroups and the item div keyed `${itm.sessionId}-${itm.tabId}` that
calls onNavigateItem) are not keyboard-accessible; make them focusable and
activate on Enter/Space by adding tabIndex={0}, role="button", and an onKeyDown
handler that calls the same toggle logic for headers and onNavigateItem(idx) for
items when key is Enter or Space, and ensure onFocus/onBlur mirror the mouse
hover visual behavior so keyboard focus shows the same highlight.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 1167-1175: The inline style block in InboxListView (the style
object for the row render) unintentionally resets earlier border and background
fields because of the shorthand 'border' and 'background' entries; remove or
replace those two shorthands so they don't override borderBottom, borderLeft and
backgroundColor — specifically delete or conditionally set the 'border: "none"'
and 'background: "transparent"' keys in the style object used by the row (the
object that uses isRowSelected, theme.colors.border/accent and backgroundColor)
or change them to side-specific/transparent-only values that don't clobber
borderBottom/borderLeft (e.g., remove 'border' or use borderStyle/borderWidth
only for sides you intend).
- Around line 48-51: Update all user-facing strings and aria labels that use
"session"/"sessions" to instead use "agent"/"agents" for UI consistency (while
keeping the underlying Session interface and variable names unchanged).
Specifically, change entries in the empty-state map (e.g., the objects with
texts "No active sessions to show.", "No unread sessions.", etc.) to use
"agent(s)" and update aria-labels like "Sort sessions" and "Filter sessions" to
"Sort agents" and "Filter agents"; also make the same replacements around the
referenced block (1059-1067) where user-facing wording appears. Leave internal
code identifiers (Session, session variables) as-is per guideline.
- Around line 1147-1221: The header button in InboxListView (the <button> keyed
by `header-${row.groupName}` that calls toggleGroup) removes the native focus
ring via className="outline-none" and has no visible keyboard focus state;
restore accessibility by removing the outline-none className, add tabIndex={0}
(if not already focusable) and implement onFocus/onBlur handlers that toggle a
small piece of state (e.g., headerFocused or rowHeaderFocused) to apply a
visible focus style (outline or box-shadow and/or changed border color)
alongside the existing selected styling (isRowSelected) so keyboard users see a
clear focus ring when the header receives focus while preserving current
click/keyboard activation logic in toggleGroup.
In `@src/renderer/components/QuickActionsModal.tsx`:
- Around line 1045-1048: Update the user-facing subtext for the Unified Inbox
entry in QuickActionsModal: replace "Cross-session notification center" with
agent-focused wording such as "Cross-agent notification center" (or similar) so
the UI uses "agent" language; locate the object with label: 'Unified Inbox' (the
entry that uses shortcut: shortcuts.agentInbox and action: () => { ... }) and
change its subtext value accordingly to use "agent" instead of "session".
---
Duplicate comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 568-569: The reply flow currently never calls the provided
onMarkAsRead handler and leaves drafts uncleared; update the reply actions
(where onMarkAsRead: _onMarkAsRead and onToggleThinking are passed) to
explicitly invoke _onMarkAsRead for the item being replied to and then clear the
draft state inside handleOpenAndReply (or the reply submit handler used in the
704-719 block), ensuring you pass the correct item id/context to _onMarkAsRead
and reset the draft variable or state after sending the reply so the message is
marked read and the draft is emptied.
---
Nitpick comments:
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 1132-1144: The per-header forward scan in InboxListView (the block
using sortMode === 'byAgent', virtualRow.index, rows, agentToolType,
unreadCount) causes quadratic render cost; compute the toolType and unreadCount
per group once using a useMemo keyed by the group id (or header key) before
rendering, producing a map from groupKey-> {toolType, unreadCount}, then in the
row render (where virtualRow and rows are used) look up the precomputed values
by the header's group key instead of scanning subsequent rows. Ensure the
useMemo depends on rows and sortMode and reference the same group identifier
used by headers/items so the lookup is O(1) during render.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-07.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-08.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-10.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-11.mdsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/AppModals.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/constants/shortcuts.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/props/useSessionListProps.ts
Comment on lines
+20
to
+27
| const rows = useMemo(() => { | ||
| if (sortMode !== 'grouped' || collapsedGroups.size === 0) return allRows; | ||
| return allRows.filter(row => { | ||
| if (row.type === 'header') return true; | ||
| const itemGroup = row.item.groupName ?? 'Ungrouped'; | ||
| return !collapsedGroups.has(itemGroup); | ||
| }); | ||
| }, [allRows, collapsedGroups, sortMode]); |
There was a problem hiding this comment.
Embedded snippet is stale relative to the stated fix.
The snippet still shows if (sortMode !== 'grouped' || collapsedGroups.size === 0), while the task note says collapse should also support byAgent mode. Please update the snippet to match the implemented guard.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-11.md`
around lines 20 - 27, Update the useMemo guard in the rows computation so
collapsing also applies when sortMode is 'byAgent'; replace the current
condition (if (sortMode !== 'grouped' || collapsedGroups.size === 0) return
allRows;) with a guard that returns allRows unless sortMode is 'grouped' or
'byAgent' (e.g. if (!['grouped','byAgent'].includes(sortMode) ||
collapsedGroups.size === 0) return allRows;), keeping the rest of the filter
logic that checks row.type === 'header' and item.groupName.
Comment on lines
+426
to
+487
| <div | ||
| key={`header-${row.groupName}-${rowIdx}`} | ||
| onClick={() => { | ||
| setCollapsedGroups((prev) => { | ||
| const next = new Set(prev); | ||
| if (next.has(row.groupName)) next.delete(row.groupName); | ||
| else next.add(row.groupName); | ||
| return next; | ||
| }); | ||
| }} | ||
| className="flex items-center gap-1.5 px-3 py-1.5 text-[10px] uppercase tracking-wider cursor-pointer" | ||
| style={{ | ||
| color: theme.colors.textDim, | ||
| fontWeight: 600, | ||
| backgroundColor: theme.colors.bgSidebar, | ||
| }} | ||
| > | ||
| {collapsedGroups.has(row.groupName) ? ( | ||
| <ChevronRight className="w-3 h-3" /> | ||
| ) : ( | ||
| <ChevronDown className="w-3 h-3" /> | ||
| )} | ||
| {row.groupName} | ||
| <span style={{ color: theme.colors.textDim, opacity: 0.5, marginLeft: 'auto' }}> | ||
| {row.type === 'header' ? row.count : 0} | ||
| </span> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // Skip items in collapsed groups | ||
| if (activeGroup && collapsedGroups.has(activeGroup)) return null; | ||
|
|
||
| const itm = row.item; | ||
| const idx = row.index; | ||
| const isCurrent = idx === currentIndex; | ||
| const statusColor = resolveStatusColor(itm.state, theme); | ||
|
|
||
| const previewText = itm.lastMessage | ||
| ? itm.lastMessage.replace(/[#*`>]/g, '').slice(0, 60) | ||
| : ''; | ||
|
|
||
| return ( | ||
| <div | ||
| key={`${itm.sessionId}-${itm.tabId}`} | ||
| ref={isCurrent ? currentRowRef : undefined} | ||
| onClick={() => onNavigateItem(idx)} | ||
| className="flex items-center gap-2 px-3 cursor-pointer transition-colors" | ||
| style={{ | ||
| height: 48, | ||
| backgroundColor: isCurrent ? `${theme.colors.accent}15` : 'transparent', | ||
| borderLeft: isCurrent | ||
| ? `2px solid ${theme.colors.accent}` | ||
| : '2px solid transparent', | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| if (!isCurrent) | ||
| e.currentTarget.style.backgroundColor = `${theme.colors.accent}08`; | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| if (!isCurrent) e.currentTarget.style.backgroundColor = 'transparent'; | ||
| }} |
There was a problem hiding this comment.
Sidebar interactive rows/headers are not keyboard-accessible.
The clickable div elements are mouse-only right now. They should be keyboard-focusable and handle Enter/Space activation.
Suggested fix
- <div
+ <div
key={`header-${row.groupName}-${rowIdx}`}
+ tabIndex={0}
+ onFocus={(e) => {
+ e.currentTarget.style.outline = `1px solid ${theme.colors.accent}`;
+ }}
+ onBlur={(e) => {
+ e.currentTarget.style.outline = 'none';
+ }}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ setCollapsedGroups((prev) => {
+ const next = new Set(prev);
+ if (next.has(row.groupName)) next.delete(row.groupName);
+ else next.add(row.groupName);
+ return next;
+ });
+ }
+ }}
onClick={() => {
@@
- <div
+ <div
key={`${itm.sessionId}-${itm.tabId}`}
ref={isCurrent ? currentRowRef : undefined}
+ tabIndex={0}
+ onFocus={(e) => {
+ e.currentTarget.style.backgroundColor = `${theme.colors.accent}08`;
+ }}
+ onBlur={(e) => {
+ if (!isCurrent) e.currentTarget.style.backgroundColor = 'transparent';
+ }}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ onNavigateItem(idx);
+ }
+ }}
onClick={() => onNavigateItem(idx)}📝 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
| <div | |
| key={`header-${row.groupName}-${rowIdx}`} | |
| onClick={() => { | |
| setCollapsedGroups((prev) => { | |
| const next = new Set(prev); | |
| if (next.has(row.groupName)) next.delete(row.groupName); | |
| else next.add(row.groupName); | |
| return next; | |
| }); | |
| }} | |
| className="flex items-center gap-1.5 px-3 py-1.5 text-[10px] uppercase tracking-wider cursor-pointer" | |
| style={{ | |
| color: theme.colors.textDim, | |
| fontWeight: 600, | |
| backgroundColor: theme.colors.bgSidebar, | |
| }} | |
| > | |
| {collapsedGroups.has(row.groupName) ? ( | |
| <ChevronRight className="w-3 h-3" /> | |
| ) : ( | |
| <ChevronDown className="w-3 h-3" /> | |
| )} | |
| {row.groupName} | |
| <span style={{ color: theme.colors.textDim, opacity: 0.5, marginLeft: 'auto' }}> | |
| {row.type === 'header' ? row.count : 0} | |
| </span> | |
| </div> | |
| ); | |
| } | |
| // Skip items in collapsed groups | |
| if (activeGroup && collapsedGroups.has(activeGroup)) return null; | |
| const itm = row.item; | |
| const idx = row.index; | |
| const isCurrent = idx === currentIndex; | |
| const statusColor = resolveStatusColor(itm.state, theme); | |
| const previewText = itm.lastMessage | |
| ? itm.lastMessage.replace(/[#*`>]/g, '').slice(0, 60) | |
| : ''; | |
| return ( | |
| <div | |
| key={`${itm.sessionId}-${itm.tabId}`} | |
| ref={isCurrent ? currentRowRef : undefined} | |
| onClick={() => onNavigateItem(idx)} | |
| className="flex items-center gap-2 px-3 cursor-pointer transition-colors" | |
| style={{ | |
| height: 48, | |
| backgroundColor: isCurrent ? `${theme.colors.accent}15` : 'transparent', | |
| borderLeft: isCurrent | |
| ? `2px solid ${theme.colors.accent}` | |
| : '2px solid transparent', | |
| }} | |
| onMouseEnter={(e) => { | |
| if (!isCurrent) | |
| e.currentTarget.style.backgroundColor = `${theme.colors.accent}08`; | |
| }} | |
| onMouseLeave={(e) => { | |
| if (!isCurrent) e.currentTarget.style.backgroundColor = 'transparent'; | |
| }} | |
| <div | |
| key={`header-${row.groupName}-${rowIdx}`} | |
| tabIndex={0} | |
| onFocus={(e) => { | |
| e.currentTarget.style.outline = `1px solid ${theme.colors.accent}`; | |
| }} | |
| onBlur={(e) => { | |
| e.currentTarget.style.outline = 'none'; | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| setCollapsedGroups((prev) => { | |
| const next = new Set(prev); | |
| if (next.has(row.groupName)) next.delete(row.groupName); | |
| else next.add(row.groupName); | |
| return next; | |
| }); | |
| } | |
| }} | |
| onClick={() => { | |
| setCollapsedGroups((prev) => { | |
| const next = new Set(prev); | |
| if (next.has(row.groupName)) next.delete(row.groupName); | |
| else next.add(row.groupName); | |
| return next; | |
| }); | |
| }} | |
| className="flex items-center gap-1.5 px-3 py-1.5 text-[10px] uppercase tracking-wider cursor-pointer" | |
| style={{ | |
| color: theme.colors.textDim, | |
| fontWeight: 600, | |
| backgroundColor: theme.colors.bgSidebar, | |
| }} | |
| > | |
| {collapsedGroups.has(row.groupName) ? ( | |
| <ChevronRight className="w-3 h-3" /> | |
| ) : ( | |
| <ChevronDown className="w-3 h-3" /> | |
| )} | |
| {row.groupName} | |
| <span style={{ color: theme.colors.textDim, opacity: 0.5, marginLeft: 'auto' }}> | |
| {row.type === 'header' ? row.count : 0} | |
| </span> | |
| </div> | |
| ); | |
| } | |
| // Skip items in collapsed groups | |
| if (activeGroup && collapsedGroups.has(activeGroup)) return null; | |
| const itm = row.item; | |
| const idx = row.index; | |
| const isCurrent = idx === currentIndex; | |
| const statusColor = resolveStatusColor(itm.state, theme); | |
| const previewText = itm.lastMessage | |
| ? itm.lastMessage.replace(/[#*`>]/g, '').slice(0, 60) | |
| : ''; | |
| return ( | |
| <div | |
| key={`${itm.sessionId}-${itm.tabId}`} | |
| ref={isCurrent ? currentRowRef : undefined} | |
| tabIndex={0} | |
| onFocus={(e) => { | |
| e.currentTarget.style.backgroundColor = `${theme.colors.accent}08`; | |
| }} | |
| onBlur={(e) => { | |
| if (!isCurrent) e.currentTarget.style.backgroundColor = 'transparent'; | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| onNavigateItem(idx); | |
| } | |
| }} | |
| onClick={() => onNavigateItem(idx)} | |
| className="flex items-center gap-2 px-3 cursor-pointer transition-colors" | |
| style={{ | |
| height: 48, | |
| backgroundColor: isCurrent ? `${theme.colors.accent}15` : 'transparent', | |
| borderLeft: isCurrent | |
| ? `2px solid ${theme.colors.accent}` | |
| : '2px solid transparent', | |
| }} | |
| onMouseEnter={(e) => { | |
| if (!isCurrent) | |
| e.currentTarget.style.backgroundColor = `${theme.colors.accent}08`; | |
| }} | |
| onMouseLeave={(e) => { | |
| if (!isCurrent) e.currentTarget.style.backgroundColor = 'transparent'; | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx` around lines 426 - 487,
The header and item clickable divs in FocusModeView (the element with key
`header-${row.groupName}-${rowIdx}` that toggles setCollapsedGroups and the item
div keyed `${itm.sessionId}-${itm.tabId}` that calls onNavigateItem) are not
keyboard-accessible; make them focusable and activate on Enter/Space by adding
tabIndex={0}, role="button", and an onKeyDown handler that calls the same toggle
logic for headers and onNavigateItem(idx) for items when key is Enter or Space,
and ensure onFocus/onBlur mirror the mouse hover visual behavior so keyboard
focus shows the same highlight.
Comment on lines
+48
to
+51
| all: { text: 'No active sessions to show.', showIcon: true }, | ||
| unread: { text: 'No unread sessions.', showIcon: false }, | ||
| read: { text: 'No read sessions with activity.', showIcon: false }, | ||
| starred: { text: 'No starred sessions.', showIcon: false }, |
There was a problem hiding this comment.
User-facing wording should use “agent” instead of “session”.
Strings like “No active sessions to show.” and aria labels such as “Sort sessions” / “Filter sessions” should use agent terminology for UI consistency.
As per coding guidelines, "Use the Session interface in code to represent agents (due to historical naming), but use 'agent' in user-facing language and 'session' for provider-level conversation contexts".
Also applies to: 1059-1067
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/InboxListView.tsx` around lines 48 - 51,
Update all user-facing strings and aria labels that use "session"/"sessions" to
instead use "agent"/"agents" for UI consistency (while keeping the underlying
Session interface and variable names unchanged). Specifically, change entries in
the empty-state map (e.g., the objects with texts "No active sessions to show.",
"No unread sessions.", etc.) to use "agent(s)" and update aria-labels like "Sort
sessions" and "Filter sessions" to "Sort agents" and "Filter agents"; also make
the same replacements around the referenced block (1059-1067) where user-facing
wording appears. Leave internal code identifiers (Session, session variables)
as-is per guideline.
Comment on lines
+1147
to
+1221
| <button | ||
| type="button" | ||
| key={`header-${row.groupName}`} | ||
| className="outline-none" | ||
| style={{ | ||
| position: 'absolute', | ||
| top: 0, | ||
| left: 0, | ||
| width: '100%', | ||
| height: `${virtualRow.size}px`, | ||
| transform: `translateY(${virtualRow.start}px)`, | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| paddingLeft: 16, | ||
| paddingRight: 16, | ||
| fontSize: 13, | ||
| fontWeight: 600, | ||
| color: isRowSelected ? theme.colors.accent : theme.colors.textDim, | ||
| letterSpacing: '0.5px', | ||
| textTransform: 'uppercase', | ||
| borderBottom: `2px solid ${theme.colors.border}40`, | ||
| borderLeft: isRowSelected | ||
| ? `3px solid ${theme.colors.accent}` | ||
| : '3px solid transparent', | ||
| backgroundColor: isRowSelected ? `${theme.colors.accent}10` : 'transparent', | ||
| cursor: 'pointer', | ||
| background: 'transparent', | ||
| border: 'none', | ||
| textAlign: 'left', | ||
| }} | ||
| onClick={() => toggleGroup(row.groupName)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| toggleGroup(row.groupName); | ||
| } | ||
| }} | ||
| > | ||
| {isCollapsed ? ( | ||
| <ChevronRight | ||
| style={{ width: 14, height: 14, marginRight: 4, flexShrink: 0 }} | ||
| /> | ||
| ) : ( | ||
| <ChevronDown | ||
| style={{ width: 14, height: 14, marginRight: 4, flexShrink: 0 }} | ||
| /> | ||
| )} | ||
| {row.groupName} | ||
| {sortMode === 'byAgent' && agentToolType && ( | ||
| <span | ||
| style={{ | ||
| fontSize: 11, | ||
| color: theme.colors.textDim, | ||
| fontWeight: 400, | ||
| marginLeft: 4, | ||
| }} | ||
| > | ||
| ({TOOL_TYPE_LABELS[agentToolType] ?? agentToolType}) | ||
| </span> | ||
| )} | ||
| {sortMode === 'byAgent' && unreadCount > 0 && ( | ||
| <span | ||
| style={{ | ||
| fontSize: 11, | ||
| marginLeft: 'auto', | ||
| padding: '1px 6px', | ||
| borderRadius: 10, | ||
| backgroundColor: theme.colors.warning + '20', | ||
| color: theme.colors.warning, | ||
| }} | ||
| > | ||
| {unreadCount} unread | ||
| </span> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
Missing visible focus state on keyboard-focusable group header button.
className="outline-none" removes default focus indication, and there is no replacement focus handler on this button. Keyboard users can land on it without a visible focus ring (Line 1150).
♿ Suggested fix
<button
type="button"
key={`header-${row.groupName}`}
className="outline-none"
@@
onClick={() => toggleGroup(row.groupName)}
+ onFocus={(e) => {
+ e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`;
+ e.currentTarget.style.outlineOffset = '-2px';
+ }}
+ onBlur={(e) => {
+ e.currentTarget.style.outline = 'none';
+ }}
onKeyDown={(e) => {As per coding guidelines, "Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus".
📝 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
| <button | |
| type="button" | |
| key={`header-${row.groupName}`} | |
| className="outline-none" | |
| style={{ | |
| position: 'absolute', | |
| top: 0, | |
| left: 0, | |
| width: '100%', | |
| height: `${virtualRow.size}px`, | |
| transform: `translateY(${virtualRow.start}px)`, | |
| display: 'flex', | |
| alignItems: 'center', | |
| paddingLeft: 16, | |
| paddingRight: 16, | |
| fontSize: 13, | |
| fontWeight: 600, | |
| color: isRowSelected ? theme.colors.accent : theme.colors.textDim, | |
| letterSpacing: '0.5px', | |
| textTransform: 'uppercase', | |
| borderBottom: `2px solid ${theme.colors.border}40`, | |
| borderLeft: isRowSelected | |
| ? `3px solid ${theme.colors.accent}` | |
| : '3px solid transparent', | |
| backgroundColor: isRowSelected ? `${theme.colors.accent}10` : 'transparent', | |
| cursor: 'pointer', | |
| background: 'transparent', | |
| border: 'none', | |
| textAlign: 'left', | |
| }} | |
| onClick={() => toggleGroup(row.groupName)} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| toggleGroup(row.groupName); | |
| } | |
| }} | |
| > | |
| {isCollapsed ? ( | |
| <ChevronRight | |
| style={{ width: 14, height: 14, marginRight: 4, flexShrink: 0 }} | |
| /> | |
| ) : ( | |
| <ChevronDown | |
| style={{ width: 14, height: 14, marginRight: 4, flexShrink: 0 }} | |
| /> | |
| )} | |
| {row.groupName} | |
| {sortMode === 'byAgent' && agentToolType && ( | |
| <span | |
| style={{ | |
| fontSize: 11, | |
| color: theme.colors.textDim, | |
| fontWeight: 400, | |
| marginLeft: 4, | |
| }} | |
| > | |
| ({TOOL_TYPE_LABELS[agentToolType] ?? agentToolType}) | |
| </span> | |
| )} | |
| {sortMode === 'byAgent' && unreadCount > 0 && ( | |
| <span | |
| style={{ | |
| fontSize: 11, | |
| marginLeft: 'auto', | |
| padding: '1px 6px', | |
| borderRadius: 10, | |
| backgroundColor: theme.colors.warning + '20', | |
| color: theme.colors.warning, | |
| }} | |
| > | |
| {unreadCount} unread | |
| </span> | |
| )} | |
| </button> | |
| <button | |
| type="button" | |
| key={`header-${row.groupName}`} | |
| className="outline-none" | |
| style={{ | |
| position: 'absolute', | |
| top: 0, | |
| left: 0, | |
| width: '100%', | |
| height: `${virtualRow.size}px`, | |
| transform: `translateY(${virtualRow.start}px)`, | |
| display: 'flex', | |
| alignItems: 'center', | |
| paddingLeft: 16, | |
| paddingRight: 16, | |
| fontSize: 13, | |
| fontWeight: 600, | |
| color: isRowSelected ? theme.colors.accent : theme.colors.textDim, | |
| letterSpacing: '0.5px', | |
| textTransform: 'uppercase', | |
| borderBottom: `2px solid ${theme.colors.border}40`, | |
| borderLeft: isRowSelected | |
| ? `3px solid ${theme.colors.accent}` | |
| : '3px solid transparent', | |
| backgroundColor: isRowSelected ? `${theme.colors.accent}10` : 'transparent', | |
| cursor: 'pointer', | |
| background: 'transparent', | |
| border: 'none', | |
| textAlign: 'left', | |
| }} | |
| onClick={() => toggleGroup(row.groupName)} | |
| onFocus={(e) => { | |
| e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`; | |
| e.currentTarget.style.outlineOffset = '-2px'; | |
| }} | |
| onBlur={(e) => { | |
| e.currentTarget.style.outline = 'none'; | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| toggleGroup(row.groupName); | |
| } | |
| }} | |
| > | |
| {isCollapsed ? ( | |
| <ChevronRight | |
| style={{ width: 14, height: 14, marginRight: 4, flexShrink: 0 }} | |
| /> | |
| ) : ( | |
| <ChevronDown | |
| style={{ width: 14, height: 14, marginRight: 4, flexShrink: 0 }} | |
| /> | |
| )} | |
| {row.groupName} | |
| {sortMode === 'byAgent' && agentToolType && ( | |
| <span | |
| style={{ | |
| fontSize: 11, | |
| color: theme.colors.textDim, | |
| fontWeight: 400, | |
| marginLeft: 4, | |
| }} | |
| > | |
| ({TOOL_TYPE_LABELS[agentToolType] ?? agentToolType}) | |
| </span> | |
| )} | |
| {sortMode === 'byAgent' && unreadCount > 0 && ( | |
| <span | |
| style={{ | |
| fontSize: 11, | |
| marginLeft: 'auto', | |
| padding: '1px 6px', | |
| borderRadius: 10, | |
| backgroundColor: theme.colors.warning + '20', | |
| color: theme.colors.warning, | |
| }} | |
| > | |
| {unreadCount} unread | |
| </span> | |
| )} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/InboxListView.tsx` around lines 1147 -
1221, The header button in InboxListView (the <button> keyed by
`header-${row.groupName}` that calls toggleGroup) removes the native focus ring
via className="outline-none" and has no visible keyboard focus state; restore
accessibility by removing the outline-none className, add tabIndex={0} (if not
already focusable) and implement onFocus/onBlur handlers that toggle a small
piece of state (e.g., headerFocused or rowHeaderFocused) to apply a visible
focus style (outline or box-shadow and/or changed border color) alongside the
existing selected styling (isRowSelected) so keyboard users see a clear focus
ring when the header receives focus while preserving current click/keyboard
activation logic in toggleGroup.
Comment on lines
+1167
to
+1175
| borderBottom: `2px solid ${theme.colors.border}40`, | ||
| borderLeft: isRowSelected | ||
| ? `3px solid ${theme.colors.accent}` | ||
| : '3px solid transparent', | ||
| backgroundColor: isRowSelected ? `${theme.colors.accent}10` : 'transparent', | ||
| cursor: 'pointer', | ||
| background: 'transparent', | ||
| border: 'none', | ||
| textAlign: 'left', |
There was a problem hiding this comment.
Header border/background styles are unintentionally overridden.
At Line 1174 (border: 'none') and Line 1173 (background: 'transparent'), shorthand properties override the earlier borderBottom, borderLeft, and backgroundColor values.
🎨 Suggested fix
borderBottom: `2px solid ${theme.colors.border}40`,
borderLeft: isRowSelected
? `3px solid ${theme.colors.accent}`
: '3px solid transparent',
backgroundColor: isRowSelected ? `${theme.colors.accent}10` : 'transparent',
cursor: 'pointer',
- background: 'transparent',
- border: 'none',
textAlign: 'left',📝 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
| borderBottom: `2px solid ${theme.colors.border}40`, | |
| borderLeft: isRowSelected | |
| ? `3px solid ${theme.colors.accent}` | |
| : '3px solid transparent', | |
| backgroundColor: isRowSelected ? `${theme.colors.accent}10` : 'transparent', | |
| cursor: 'pointer', | |
| background: 'transparent', | |
| border: 'none', | |
| textAlign: 'left', | |
| borderBottom: `2px solid ${theme.colors.border}40`, | |
| borderLeft: isRowSelected | |
| ? `3px solid ${theme.colors.accent}` | |
| : '3px solid transparent', | |
| backgroundColor: isRowSelected ? `${theme.colors.accent}10` : 'transparent', | |
| cursor: 'pointer', | |
| textAlign: 'left', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/InboxListView.tsx` around lines 1167 -
1175, The inline style block in InboxListView (the style object for the row
render) unintentionally resets earlier border and background fields because of
the shorthand 'border' and 'background' entries; remove or replace those two
shorthands so they don't override borderBottom, borderLeft and backgroundColor —
specifically delete or conditionally set the 'border: "none"' and 'background:
"transparent"' keys in the style object used by the row (the object that uses
isRowSelected, theme.colors.border/accent and backgroundColor) or change them to
side-specific/transparent-only values that don't clobber borderBottom/borderLeft
(e.g., remove 'border' or use borderStyle/borderWidth only for sides you
intend).
Comment on lines
+1045
to
+1048
| label: 'Unified Inbox', | ||
| shortcut: shortcuts.agentInbox, | ||
| subtext: 'Cross-session notification center', | ||
| action: () => { |
There was a problem hiding this comment.
Use agent terminology in this user-facing subtext.
“Cross-session notification center” should follow the UI vocabulary rule and use “agent” wording.
As per coding guidelines, "Use the Session interface in code to represent agents (due to historical naming), but use 'agent' in user-facing language and 'session' for provider-level conversation contexts".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/QuickActionsModal.tsx` around lines 1045 - 1048,
Update the user-facing subtext for the Unified Inbox entry in QuickActionsModal:
replace "Cross-session notification center" with agent-focused wording such as
"Cross-agent notification center" (or similar) so the UI uses "agent" language;
locate the object with label: 'Unified Inbox' (the entry that uses shortcut:
shortcuts.agentInbox and action: () => { ... }) and change its subtext value
accordingly to use "agent" instead of "session".
…'s Notes tokens Consistent px-4 horizontal padding across header, info bar, content scroll area, and reply input. Info bar uses py-2 padding instead of fixed height. Reply input uses py-3 matching the header. Scroll area uses uniform p-4 padding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…SessionStorage timeouts documented Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…10 integration checks green Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Star indicators were oversized and redundant — the "Starred" filter already communicates starred state. Removed star icon from inbox list rows, focus sidebar items, focus info bar badge, and unicode star from filter pill label. Cleaned up unused Star imports from lucide-react. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…coreFeatureFlags Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…iption handler Adds the Encore Feature toggle card in Settings > Encore Features for Tab Descriptions, following the Unified Inbox pattern. Also adds the handler in useTabHandlers.ts for updating tab descriptions (empty string clears). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…line-editable overlay UI Thread handleUpdateTabDescription from App.tsx through useMainPanelProps, MainPanel, and TabBar with Encore feature gating. Add inline-editable description section in the tab hover overlay between header and actions, with textarea editing, Enter to save, Shift+Enter for newlines, Escape to cancel, and auto-reset on overlay close. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The agent filter was using exact substring matching (.includes()), making it impossible to find agents with non-contiguous character queries. Now uses the existing fuzzyMatch() utility for consistent behavior with File Search Modal and Tab Switcher. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Slash command search was using startsWith() prefix matching, requiring exact contiguous character input. Now uses fuzzyMatchWithScore() with 3-tier ranking (prefix > fuzzy name > fuzzy description) in InputArea and fuzzyMatch() in useInputKeyDown for consistent keyboard navigation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…train tab overlay width - Update useInputKeyDown test to expect /clear fuzzy match for '/r' input - Remove duplicate optimistic user log from handleSendToTab (processInput handles it) - Add maxWidth 320px and word-break rules to tab hover overlay for long text Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
|
Split into 3 focused PRs:
The debounce fix (4c9f951) is already on main. |
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
useBatchProcessor.tscompletely bypassed the 200ms debounce for batch state updates, causing every Auto Run progress event to trigger immediate synchronous React re-rendersconsole.logstatements from hot paths that added additional overheadRoot Cause
The
updateBatchStateAndBroadcastfunction had ~70 lines of inline debug code that replaced the debounced update path with direct synchronous dispatches. The existinguseSessionDebouncehook (200ms per-session batching with composed updater functions) was already implemented but intentionally unused — thescheduleUpdatebinding was prefixed with underscore (_scheduleDebouncedUpdate).The debug code also included
console.logon every dispatch, every debounce fire, mount/unmount lifecycle, and stop/kill operations — hundreds of log calls per second during batch processing.Changes
updateBatchStateAndBroadcastscheduleDebouncedUpdate_scheduleDebouncedUpdatescheduleDebouncedUpdateconsole.login hot pathsdispatchwrapperprevStates/newStatesfor loggingNet change: +4 / -115 lines (single file)
Test plan
npm run lint)npm run buildsucceeds🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Refactor