feat: Tab Description as Encore Feature#469
Open
felipeggv wants to merge 36 commits intoRunMaestro:mainfrom
Open
feat: Tab Description as Encore Feature#469felipeggv wants to merge 36 commits intoRunMaestro:mainfrom
felipeggv wants to merge 36 commits intoRunMaestro:mainfrom
Conversation
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>
…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>
…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>
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>
…'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>
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>
…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>
processInput already adds the user log entry, so the optimistic push in handleAgentInboxQuickReply was creating duplicates. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5 tasks
|
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 lazy-loaded Unified Agent Inbox (list + focus views), data hook and types, modal/store/shortcut wiring, settings feature flags, per-tab descriptions and handlers, TabBar inline editing UI, keyboard/menu integrations, accessibility and scrolling fixes, and prop/hook threading across app shell components. Changes
Sequence Diagram(s)sequenceDiagram
participant User as rgba(54,117,136,0.5) User
participant App as rgba(19,90,60,0.5) AppShell
participant ModalStore as rgba(120,20,140,0.5) ModalStore
participant AgentInbox as rgba(200,80,30,0.5) AgentInbox
participant Sessions as rgba(10,90,180,0.5) SessionManager
participant AIEngine as rgba(180,40,100,0.5) AIEngine
User->>App: trigger Unified Inbox (shortcut / menu)
App->>ModalStore: setAgentInboxOpen(true)
ModalStore-->>App: agentInboxOpen = true
App->>AgentInbox: lazy-load & render (passes callbacks)
AgentInbox->>Sessions: useAgentInbox(sessions, groups) / request items
Sessions-->>AgentInbox: inbox items
User->>AgentInbox: select item / quick reply / open-and-reply
AgentInbox->>Sessions: onNavigateToSession(sessionId)
AgentInbox->>AIEngine: processInput(text) (quick reply)
AIEngine-->>Sessions: response / log entry
AgentInbox->>ModalStore: setAgentInboxOpen(false) (optional close)
ModalStore-->>App: agentInboxOpen = false
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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 PR adds inline-editable tab descriptions as a new Encore Feature, allowing users to add context notes to individual AI tabs. The implementation is clean and well-structured: Key Implementation Details:
Code Quality:
The feature is production-ready with no bugs or logical issues found. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant TabBar
participant Tab
participant Handler
participant Store
User->>TabBar: Hover over AI tab
TabBar->>Tab: Show overlay with description
alt Description exists
Tab->>User: Display description (3-line clamp)
else No description
Tab->>User: Show "Add description..." placeholder
end
User->>Tab: Click description button
Tab->>Tab: Enter edit mode, focus textarea
alt User presses Enter (no Shift)
User->>Tab: Press Enter
Tab->>Tab: Set skipNextBlurSaveRef = true
Tab->>Handler: handleSaveDescription()
Handler->>Store: Update tab.description (trimmed)
Tab->>Tab: Exit edit mode
else User presses Escape
User->>Tab: Press Escape
Tab->>Tab: Set skipNextBlurSaveRef = true
Tab->>Tab: Restore original description
Tab->>Tab: Exit edit mode
else User clicks away (blur)
User->>Tab: Blur textarea
Tab->>Handler: handleSaveDescription()
Handler->>Store: Update tab.description (trimmed)
Tab->>Tab: Exit edit mode
end
Last reviewed commit: e3e7a07 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/renderer/App.tsx
Outdated
Comment on lines
2436
to
2437
| // Keep inbox processInput ref in sync with the latest processInput | ||
| inboxProcessInputRef.current = processInput; |
There was a problem hiding this comment.
inboxProcessInputRef is not defined anywhere in the codebase - this will throw a ReferenceError at runtime
Suggested change
| // Keep inbox processInput ref in sync with the latest processInput | |
| inboxProcessInputRef.current = processInput; | |
| // Remove this line - inboxProcessInputRef does not exist |
src/renderer/components/TabBar.tsx
Outdated
| className="w-3.5 h-3.5 mt-0.5 shrink-0" | ||
| style={{ color: theme.colors.textDim }} | ||
| /> | ||
| <span className={tab.description ? 'line-clamp-3 break-all' : 'italic'}> |
There was a problem hiding this comment.
break-all can break mid-word awkwardly - use break-words instead for better readability
Suggested change
| <span className={tab.description ? 'line-clamp-3 break-all' : 'italic'}> | |
| <span className={tab.description ? 'line-clamp-3 break-words' : 'italic'}> |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 2436-2438: The code assigns to inboxProcessInputRef.current but
inboxProcessInputRef is not declared; add a declaration for inboxProcessInputRef
inside the same component (near other refs) using React.useRef / useRef with an
appropriate type that matches processInput (e.g., a MutableRefObject holding the
function or null) and initialize it (e.g., useRef<typeof processInput |
null>(null)) so the assignment inboxProcessInputRef.current = processInput is
valid at runtime.
In `@src/renderer/components/TabBar.tsx`:
- Around line 383-394: The Escape key handler in handleDescriptionKeyDown
currently sets isEditingDescription to false but does not prevent the input's
onBlur from calling handleSaveDescription, so cancel can still trigger a
blur-save; fix by adding a transient suppression flag (e.g.,
ignoreNextDescriptionBlurRef) that handleDescriptionKeyDown sets to true when
Escape (and optionally Enter when you explicitly saved) is pressed, and then
have the description input's onBlur check that flag before calling
handleSaveDescription (clear the flag after blur or via setTimeout to re-enable
normal behavior). Update handleDescriptionKeyDown, handleSaveDescription usage,
and the onBlur handler to consult ignoreNextDescriptionBlurRef to avoid
duplicate or unwanted saves.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/renderer/App.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/components/TabBar.tsxsrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/hooks/tabs/useTabHandlers.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/index.ts
felipeggv
force-pushed
the
feat/tab-description
branch
from
ba08bcb to
d61340a
Compare
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (1)
src/renderer/components/TabBar.tsx (1)
377-395:⚠️ Potential issue | 🟠 MajorPrevent blur-save from overriding Escape cancel (and double-saving on Enter).
At Line 391 you exit edit mode, but Line 743 still runs
handleSaveDescriptionon blur. Escape can still persist edits, and Enter can cause a duplicate save via blur.💡 Suggested fix
const [isEditingDescription, setIsEditingDescription] = useState(false); const [descriptionDraft, setDescriptionDraft] = useState(''); const descriptionInputRef = useRef<HTMLTextAreaElement>(null); + const skipNextBlurSaveRef = useRef(false); @@ const handleSaveDescription = useCallback(() => { const trimmed = descriptionDraft.trim(); + skipNextBlurSaveRef.current = true; onUpdateDescription?.(tabId, trimmed); setIsEditingDescription(false); }, [onUpdateDescription, tabId, descriptionDraft]); + + const handleDescriptionBlur = useCallback(() => { + if (skipNextBlurSaveRef.current) { + skipNextBlurSaveRef.current = false; + return; + } + handleSaveDescription(); + }, [handleSaveDescription]); @@ } else if (e.key === 'Escape') { e.preventDefault(); + skipNextBlurSaveRef.current = true; + setDescriptionDraft(tab.description || ''); setIsEditingDescription(false); } }, - [handleSaveDescription] + [handleSaveDescription, tab.description] ); @@ - onBlur={handleSaveDescription} + onBlur={handleDescriptionBlur}Also applies to: 743-744
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/TabBar.tsx` around lines 377 - 395, The blur handler is currently always calling handleSaveDescription which lets Escape or Enter-triggered saves be overwritten or doubled; add a mutable ref (e.g., suppressBlurRef) and set suppressBlurRef.current = true inside handleDescriptionKeyDown when handling Enter (after calling handleSaveDescription) and when handling Escape (to prevent save), then in the input/textarea onBlur handler check suppressBlurRef.current and skip calling handleSaveDescription if true; finally clear suppressBlurRef (e.g., via setTimeout(() => suppressBlurRef.current = false, 0) or after state updates) so normal blurs continue to save. Ensure references to handleSaveDescription, handleDescriptionKeyDown and setIsEditingDescription are used consistently.
🧹 Nitpick comments (5)
src/renderer/hooks/useAgentInbox.ts (1)
5-5: Align truncation length with theInboxItem.lastMessagecontract.This file truncates to 250 chars, while
src/renderer/types/agent-inbox.tsdocumentslastMessageas truncated to 90. Consider making this consistent to avoid UI/data expectation drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/useAgentInbox.ts` at line 5, The truncation constant MAX_MESSAGE_LENGTH in useAgentInbox.ts is set to 250 while the InboxItem.lastMessage contract expects a 90-character truncation; update the truncation to match the contract by changing MAX_MESSAGE_LENGTH to 90 (or derive its value from the inbox type/contract if available) so the truncation logic that produces InboxItem.lastMessage is consistent with the documented contract.src/renderer/types/agent-inbox.ts (1)
35-42: StrengthenSTATUS_COLORStyping to theme keys.Typing this map as
stringloses compile-time validation. Preferkeyof ThemeColorsso invalid color keys are caught immediately.🔧 Suggested typing improvement
-import type { SessionState } from './index'; +import type { SessionState, ThemeColors } from './index'; @@ -export const STATUS_COLORS: Record<SessionState, string> = { +export const STATUS_COLORS: Record<SessionState, keyof ThemeColors> = { idle: 'success', waiting_input: 'warning', busy: 'info', connecting: 'textMuted', error: 'error', };🤖 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 35 - 42, STATUS_COLORS is currently typed as Record<SessionState, string> which loses compile-time validation; change its type to Record<SessionState, keyof ThemeColors> (or keyof Theme['colors'] if you prefer) so only valid theme color keys are allowed, update the export to use this type for STATUS_COLORS, ensure ThemeColors (or Theme) is imported/available in the module, and verify the existing values (idle, waiting_input, busy, connecting, error -> 'success','warning','info','textMuted','error') exactly match keys on ThemeColors, adjusting any mismatches.src/renderer/stores/modalStore.ts (1)
141-146: Consider documenting the intentional loose typing.The
filterModeandsortModeare typed asstringrather than their specific union types (InboxFilterMode/InboxSortMode). This appears intentional to avoid circular dependencies withtypes/agent-inbox.ts. A brief comment noting this design choice would help future maintainers understand why the types are looser here.💡 Suggested documentation
/** Agent Inbox modal data (persisted filter/sort/expand state) */ +// Note: filterMode/sortMode are string here to avoid circular import with types/agent-inbox.ts +// Consumers should cast to InboxFilterMode/InboxSortMode at usage point export interface AgentInboxModalData { filterMode?: string; sortMode?: string; isExpanded?: boolean; }🤖 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 141 - 146, Add a short inline comment above the AgentInboxModalData interface explaining that filterMode and sortMode are intentionally typed as string (rather than InboxFilterMode/InboxSortMode) to avoid a circular dependency with types/agent-inbox.ts; reference the specific fields (filterMode, sortMode) and the intended union types (InboxFilterMode/InboxSortMode) so future maintainers understand the deliberate loose typing and rationale.src/renderer/components/AgentInbox/index.tsx (1)
94-104: Consider memoizing the frozen item lookup for better performance.The
resolveFrozenItemsfunction performs a nested lookup (liveItems.find()) for each frozen item, resulting in O(n*m) complexity. For typical inbox sizes this is fine, but if the inbox grows large, consider building a lookup Map for O(1) access.💡 Performance optimization suggestion
const resolveFrozenItems = useCallback( (frozen: { sessionId: string; tabId: string }[]): InboxItem[] => { + // Build lookup map for O(1) access + const liveMap = new Map<string, InboxItem>(); + for (const item of liveItems) { + liveMap.set(`${item.sessionId}:${item.tabId}`, item); + } const resolved: InboxItem[] = []; for (const key of frozen) { - const live = liveItems.find((i) => i.sessionId === key.sessionId && i.tabId === key.tabId); + const live = liveMap.get(`${key.sessionId}:${key.tabId}`); if (live) resolved.push(live); } return resolved.length > 0 ? resolved : liveItems; }, [liveItems] );🤖 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 94 - 104, resolveFrozenItems currently does an O(n*m) nested find over liveItems for each frozen entry; create a memoized lookup Map (e.g., useMemo(() => new Map(liveItems.map(i => [`${i.sessionId}:${i.tabId}`, i])), [liveItems])) keyed by sessionId+tabId, then change resolveFrozenItems to O(m) by using that Map for O(1) lookups of frozen entries; keep the existing fallback to return liveItems when no resolved entries are found and ensure you reference resolveFrozenItems, liveItems, and the frozen keys when making the change.src/renderer/components/AgentInbox/FocusModeView.tsx (1)
680-685: Consider using requestAnimationFrame instead of setTimeout for focus.The 200ms delay for focusing the reply input is a timing-based workaround. Using
requestAnimationFramewould be more reliable as it fires after the browser has completed layout.💡 Suggested improvement
// Auto-focus reply input when entering focus mode or switching items useEffect(() => { - const timer = setTimeout(() => { - replyInputRef.current?.focus(); - }, 200); - return () => clearTimeout(timer); + // Use double RAF to ensure layout is complete before focusing + const raf1 = requestAnimationFrame(() => { + const raf2 = requestAnimationFrame(() => { + replyInputRef.current?.focus(); + }); + // Store raf2 for cleanup + (raf1 as any).__inner = raf2; + }); + return () => { + cancelAnimationFrame(raf1); + if ((raf1 as any).__inner) cancelAnimationFrame((raf1 as any).__inner); + }; }, [item.sessionId, item.tabId]);🤖 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 680 - 685, Replace the setTimeout-based focus workaround in the useEffect with a requestAnimationFrame approach: in the useEffect that depends on item.sessionId and item.tabId, schedule replyInputRef.current?.focus() inside requestAnimationFrame instead of setTimeout(200), store the returned raf id in a variable, and ensure the cleanup function calls cancelAnimationFrame(rafId) to avoid leaked callbacks; keep the same dependency array and behavior in the FocusModeView component so focus still occurs after layout but without an arbitrary timeout.
🤖 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-07.md`:
- Around line 56-60: The gate text contradicts the earlier note about a
remaining lint error; update the Gate section to accurately reflect the expected
lint outcome by either removing "npm run lint passes" or clarifying that lint
passes except for the known pre-existing error (onOpenUnifiedInbox in
AppModalsProps). Reference the symbols AppModalsProps, onOpenUnifiedInbox and
setAgentInboxOpen (and the conditional render in SessionList) to ensure the gate
explicitly documents that lint should succeed except for the pre-existing
onOpenUnifiedInbox error or, alternatively, require fixing that error before
marking the gate as passed.
In `@playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-14.md`:
- Around line 54-56: Update the contradictory test gate wording: replace the
bullet "- `npm run test` passes (all tests green)" with wording that reflects
the documented known failures (e.g., "- `npm run test` passes (except 3 known
failing tests)") so the gate aligns with the note that "3 known failing tests"
is recorded elsewhere in the doc; ensure any other test-gate bullets referencing
pass/green status (such as "`npm run lint`" / "`npm run lint:eslint`") remain
accurate or are similarly qualified if they have known exceptions.
In `@src/renderer/App.tsx`:
- Around line 6165-6172: The AgentInbox modal is missing props for several
action handlers so its UI cannot trigger inbox behaviors; pass the implemented
callbacks into the <AgentInbox> component: provide handleAgentInboxQuickReply,
handleAgentInboxOpenAndReply, handleAgentInboxMarkAsRead, and
handleAgentInboxToggleThinking as props (alongside existing theme, sessions,
groups, enterToSendAI, onClose, onNavigateToSession) so the modal uses those
handler functions when users quick-reply, open-and-reply, mark-as-read, or
toggle thinking.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 467-533: The sidebar item div (key
`${itm.sessionId}-${itm.tabId}`) needs keyboard accessibility: add tabIndex={0}
and role="button", implement an onKeyDown handler that calls onNavigateItem(idx)
when Enter or Space is pressed, and add onFocus/onBlur handlers that mirror the
current onMouseEnter/onMouseLeave behavior (apply the same
backgroundColor/border styles when focused and remove them on blur); keep using
currentRowRef and isCurrent logic as-is so keyboard focus works the same as
click/hover.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 412-414: The label text for the high-context metric doesn't match
the implemented condition: the code computes highContext using items.filter((i)
=> i.contextUsage !== undefined && i.contextUsage >= 80).length but the UI label
reads "Context >80%"; update either the condition or the label so they
match—preferably change the label to "Context ≥80%" (or "Context >=80%")
wherever the label is rendered next to highContext, or if you prefer strict
greater-than change the filter predicate to i.contextUsage > 80 to match the
existing ">" wording.
- Around line 1150-1158: The inline style block in InboxListView.tsx is using
broad shorthand properties (border: 'none' and background: 'transparent') which
override the specific styles for borderBottom, borderLeft and backgroundColor
used to indicate selection (isRowSelected); remove or relocate these shorthand
properties so they do not override those specific rules—specifically, delete or
conditionally apply the 'border' and 'background' shorthands (or replace them
with non-conflicting values) in the style object that uses borderBottom,
borderLeft and backgroundColor for the row rendering logic tied to isRowSelected
and theme.colors so the selected/collapsed visuals can show correctly.
- Around line 1058-1063: The div with role="listbox" is not keyboard-focusable
and lacks key handlers; add tabIndex={0} to make it focusable and wire an
onKeyDown={handleListboxKeyDown} plus onFocus/onBlur handlers as needed, then
implement handleListboxKeyDown in the same component to handle
ArrowUp/ArrowDown/Home/End/Enter (moving the active item and updating the
aria-activedescendant via selectedItemId) and call existing selection/updating
logic so keyboard navigation works the same as mouse selection.
In `@src/renderer/components/SettingsModal.tsx`:
- Around line 3657-3665: The toggle buttons in SettingsModal are stateful but
don't expose their on/off state to assistive tech; update the button that calls
setEncoreFeatures and reads encoreFeatures.unifiedInbox to include an ARIA state
(e.g., add aria-pressed={encoreFeatures.unifiedInbox} or convert to
role="switch" aria-checked={encoreFeatures.unifiedInbox}) so screen readers can
announce the current state, and apply the same change to the other similar
toggle button in this component that flips another encoreFeatures property
(ensure you reference the same encoreFeatures property used in its onClick).
In `@src/renderer/components/TabBar.tsx`:
- Around line 734-773: The tab-description UI is currently reachable only via
hover; make it keyboard-accessible by adding tabIndex and focus/keyboard
handlers: add tabIndex={0} to the description button (the element that currently
renders when isEditingDescription is false) and to the surrounding description
container if needed, wire onFocus to call handleStartEditDescription (or open
the editor/overlay) and add onKeyDown to the same element to open the editor on
Enter/Space; ensure the textarea (descriptionInputRef) retains focus when
entering edit mode and keep existing handlers handleSaveDescription and
handleDescriptionKeyDown for keyboard save/escape behavior; also add appropriate
aria-label/role to the button for screen readers and use theme/tab.description
variables unchanged.
In `@src/renderer/hooks/tabs/useTabHandlers.ts`:
- Around line 1077-1086: The handler handleUpdateTabDescription stores
whitespace-only descriptions; before updating, trim the incoming description and
if the trimmed string is empty set description to undefined. Inside the
setSessions callback used from useSessionStore.getState(), normalize the value
passed into the aiTabs mapping (use a local const like normalized =
description?.trim(); then use normalized ? normalized : undefined) so aiTabs
entries get undefined for empty/whitespace-only descriptions instead of a
whitespace string.
In `@src/renderer/hooks/useAgentInbox.ts`:
- Around line 149-172: Grouping currently uses item.sessionName as the Map key
(agentGroups and the local variable key), which merges distinct sessions that
share a name; change the grouping key to be identity-safe by using
item.sessionId (or a composite like `${item.sessionId}:${item.sessionName}`)
when creating agentGroups and when pushing to groupMeta, while keeping
sessionName available for display and alphabetical fallbacks in the final sort;
update references to key in groupMeta.sort to compare a.sessionName /
b.sessionName (or parse the composite) so unread counts and per-session item
lists remain correct and isolated per session.
- Line 24: The current read-filter in useAgentInbox.ts (the return expression
that uses hasUnread and sessionState) incorrectly excludes items that are read
but in other session states; change the logic in the read branch so it only
depends on hasUnread (i.e., treat "read" as !hasUnread) and remove the
sessionState condition from the expression (update the return in the filter
function that currently references hasUnread and sessionState).
---
Duplicate comments:
In `@src/renderer/components/TabBar.tsx`:
- Around line 377-395: The blur handler is currently always calling
handleSaveDescription which lets Escape or Enter-triggered saves be overwritten
or doubled; add a mutable ref (e.g., suppressBlurRef) and set
suppressBlurRef.current = true inside handleDescriptionKeyDown when handling
Enter (after calling handleSaveDescription) and when handling Escape (to prevent
save), then in the input/textarea onBlur handler check suppressBlurRef.current
and skip calling handleSaveDescription if true; finally clear suppressBlurRef
(e.g., via setTimeout(() => suppressBlurRef.current = false, 0) or after state
updates) so normal blurs continue to save. Ensure references to
handleSaveDescription, handleDescriptionKeyDown and setIsEditingDescription are
used consistently.
---
Nitpick comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 680-685: Replace the setTimeout-based focus workaround in the
useEffect with a requestAnimationFrame approach: in the useEffect that depends
on item.sessionId and item.tabId, schedule replyInputRef.current?.focus() inside
requestAnimationFrame instead of setTimeout(200), store the returned raf id in a
variable, and ensure the cleanup function calls cancelAnimationFrame(rafId) to
avoid leaked callbacks; keep the same dependency array and behavior in the
FocusModeView component so focus still occurs after layout but without an
arbitrary timeout.
In `@src/renderer/components/AgentInbox/index.tsx`:
- Around line 94-104: resolveFrozenItems currently does an O(n*m) nested find
over liveItems for each frozen entry; create a memoized lookup Map (e.g.,
useMemo(() => new Map(liveItems.map(i => [`${i.sessionId}:${i.tabId}`, i])),
[liveItems])) keyed by sessionId+tabId, then change resolveFrozenItems to O(m)
by using that Map for O(1) lookups of frozen entries; keep the existing fallback
to return liveItems when no resolved entries are found and ensure you reference
resolveFrozenItems, liveItems, and the frozen keys when making the change.
In `@src/renderer/hooks/useAgentInbox.ts`:
- Line 5: The truncation constant MAX_MESSAGE_LENGTH in useAgentInbox.ts is set
to 250 while the InboxItem.lastMessage contract expects a 90-character
truncation; update the truncation to match the contract by changing
MAX_MESSAGE_LENGTH to 90 (or derive its value from the inbox type/contract if
available) so the truncation logic that produces InboxItem.lastMessage is
consistent with the documented contract.
In `@src/renderer/stores/modalStore.ts`:
- Around line 141-146: Add a short inline comment above the AgentInboxModalData
interface explaining that filterMode and sortMode are intentionally typed as
string (rather than InboxFilterMode/InboxSortMode) to avoid a circular
dependency with types/agent-inbox.ts; reference the specific fields (filterMode,
sortMode) and the intended union types (InboxFilterMode/InboxSortMode) so future
maintainers understand the deliberate loose typing and rationale.
In `@src/renderer/types/agent-inbox.ts`:
- Around line 35-42: STATUS_COLORS is currently typed as Record<SessionState,
string> which loses compile-time validation; change its type to
Record<SessionState, keyof ThemeColors> (or keyof Theme['colors'] if you prefer)
so only valid theme color keys are allowed, update the export to use this type
for STATUS_COLORS, ensure ThemeColors (or Theme) is imported/available in the
module, and verify the existing values (idle, waiting_input, busy, connecting,
error -> 'success','warning','info','textMuted','error') exactly match keys on
ThemeColors, adjusting any mismatches.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-02.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-04.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-05.mdplaybooks/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.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-12.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-14.mdsrc/renderer/App.tsxsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/components/AppModals.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/components/TabBar.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/constants/shortcuts.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/hooks/props/useSessionListProps.tssrc/renderer/hooks/tabs/useTabHandlers.tssrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/agent-inbox.tssrc/renderer/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/types/index.ts
- src/renderer/components/MainPanel.tsx
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-07.md
Show resolved
Hide resolved
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-14.md
Show resolved
Hide resolved
Comment on lines
467
to
533
| 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'; | ||
| }} | ||
| > | ||
| {/* Status dot */} | ||
| <span | ||
| className="flex-shrink-0" | ||
| style={{ | ||
| width: 6, | ||
| height: 6, | ||
| borderRadius: '50%', | ||
| backgroundColor: statusColor, | ||
| }} | ||
| /> | ||
| {/* Name + preview vertical stack */} | ||
| <div className="flex-1 flex flex-col gap-0.5 min-w-0"> | ||
| <span | ||
| className="text-xs truncate" | ||
| style={{ | ||
| color: isCurrent ? theme.colors.textMain : theme.colors.textDim, | ||
| fontWeight: isCurrent ? 600 : 400, | ||
| }} | ||
| > | ||
| {itm.tabName || 'Tab'} | ||
| </span> | ||
| {previewText && ( | ||
| <span | ||
| className="text-[10px] truncate" | ||
| style={{ color: theme.colors.textDim, opacity: 0.5 }} | ||
| > | ||
| {previewText} | ||
| </span> | ||
| )} | ||
| </div> | ||
| {/* Indicators: unread */} | ||
| {itm.hasUnread && ( | ||
| <span | ||
| className="flex-shrink-0" | ||
| style={{ | ||
| width: 6, | ||
| height: 6, | ||
| borderRadius: '50%', | ||
| backgroundColor: theme.colors.accent, | ||
| alignSelf: 'flex-start', | ||
| marginTop: 2, | ||
| }} | ||
| /> | ||
| )} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Add keyboard accessibility to sidebar items.
The sidebar item rows use onClick for selection but lack tabIndex and keyboard event handlers. Users relying on keyboard navigation cannot access these items. As per coding guidelines, components needing keyboard focus should include tabIndex and focus event handlers.
♿ Suggested accessibility fix
return (
<div
key={`${itm.sessionId}-${itm.tabId}`}
ref={isCurrent ? currentRowRef : undefined}
onClick={() => onNavigateItem(idx)}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ onNavigateItem(idx);
+ }
+ }}
+ tabIndex={0}
+ role="option"
+ aria-selected={isCurrent}
className="flex items-center gap-2 px-3 cursor-pointer transition-colors"
style={{📝 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
| 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'; | |
| }} | |
| > | |
| {/* Status dot */} | |
| <span | |
| className="flex-shrink-0" | |
| style={{ | |
| width: 6, | |
| height: 6, | |
| borderRadius: '50%', | |
| backgroundColor: statusColor, | |
| }} | |
| /> | |
| {/* Name + preview vertical stack */} | |
| <div className="flex-1 flex flex-col gap-0.5 min-w-0"> | |
| <span | |
| className="text-xs truncate" | |
| style={{ | |
| color: isCurrent ? theme.colors.textMain : theme.colors.textDim, | |
| fontWeight: isCurrent ? 600 : 400, | |
| }} | |
| > | |
| {itm.tabName || 'Tab'} | |
| </span> | |
| {previewText && ( | |
| <span | |
| className="text-[10px] truncate" | |
| style={{ color: theme.colors.textDim, opacity: 0.5 }} | |
| > | |
| {previewText} | |
| </span> | |
| )} | |
| </div> | |
| {/* Indicators: unread */} | |
| {itm.hasUnread && ( | |
| <span | |
| className="flex-shrink-0" | |
| style={{ | |
| width: 6, | |
| height: 6, | |
| borderRadius: '50%', | |
| backgroundColor: theme.colors.accent, | |
| alignSelf: 'flex-start', | |
| marginTop: 2, | |
| }} | |
| /> | |
| )} | |
| </div> | |
| ); | |
| return ( | |
| <div | |
| key={`${itm.sessionId}-${itm.tabId}`} | |
| ref={isCurrent ? currentRowRef : undefined} | |
| onClick={() => onNavigateItem(idx)} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| onNavigateItem(idx); | |
| } | |
| }} | |
| tabIndex={0} | |
| role="option" | |
| aria-selected={isCurrent} | |
| 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'; | |
| }} | |
| > | |
| {/* Status dot */} | |
| <span | |
| className="flex-shrink-0" | |
| style={{ | |
| width: 6, | |
| height: 6, | |
| borderRadius: '50%', | |
| backgroundColor: statusColor, | |
| }} | |
| /> | |
| {/* Name + preview vertical stack */} | |
| <div className="flex-1 flex flex-col gap-0.5 min-w-0"> | |
| <span | |
| className="text-xs truncate" | |
| style={{ | |
| color: isCurrent ? theme.colors.textMain : theme.colors.textDim, | |
| fontWeight: isCurrent ? 600 : 400, | |
| }} | |
| > | |
| {itm.tabName || 'Tab'} | |
| </span> | |
| {previewText && ( | |
| <span | |
| className="text-[10px] truncate" | |
| style={{ color: theme.colors.textDim, opacity: 0.5 }} | |
| > | |
| {previewText} | |
| </span> | |
| )} | |
| </div> | |
| {/* Indicators: unread */} | |
| {itm.hasUnread && ( | |
| <span | |
| className="flex-shrink-0" | |
| style={{ | |
| width: 6, | |
| height: 6, | |
| borderRadius: '50%', | |
| backgroundColor: theme.colors.accent, | |
| alignSelf: 'flex-start', | |
| marginTop: 2, | |
| }} | |
| /> | |
| )} | |
| </div> | |
| ); |
🤖 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 467 - 533,
The sidebar item div (key `${itm.sessionId}-${itm.tabId}`) needs keyboard
accessibility: add tabIndex={0} and role="button", implement an onKeyDown
handler that calls onNavigateItem(idx) when Enter or Space is pressed, and add
onFocus/onBlur handlers that mirror the current onMouseEnter/onMouseLeave
behavior (apply the same backgroundColor/border styles when focused and remove
them on blur); keep using currentRowRef and isCurrent logic as-is so keyboard
focus works the same as click/hover.
Comment on lines
3657
to
3665
| <button | ||
| className="w-full flex items-center justify-between p-4 text-left" | ||
| onClick={() => | ||
| setEncoreFeatures({ | ||
| ...encoreFeatures, | ||
| unifiedInbox: !encoreFeatures.unifiedInbox, | ||
| }) | ||
| } | ||
| > |
There was a problem hiding this comment.
Expose toggle state to assistive tech.
These are stateful toggle controls, but they don’t expose on/off state via ARIA. Add aria-pressed (or role="switch" + aria-checked) so screen readers can announce current state.
♿ Suggested update
- <button
+ <button
className="w-full flex items-center justify-between p-4 text-left"
+ aria-pressed={encoreFeatures.unifiedInbox}
onClick={() =>
setEncoreFeatures({
...encoreFeatures,
unifiedInbox: !encoreFeatures.unifiedInbox,
})
}
>
@@
- <button
+ <button
className="w-full flex items-center justify-between p-4 text-left"
+ aria-pressed={encoreFeatures.tabDescription}
onClick={() =>
setEncoreFeatures({
...encoreFeatures,
tabDescription: !encoreFeatures.tabDescription,
})
}
>Also applies to: 3740-3747
🤖 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 3657 - 3665, The
toggle buttons in SettingsModal are stateful but don't expose their on/off state
to assistive tech; update the button that calls setEncoreFeatures and reads
encoreFeatures.unifiedInbox to include an ARIA state (e.g., add
aria-pressed={encoreFeatures.unifiedInbox} or convert to role="switch"
aria-checked={encoreFeatures.unifiedInbox}) so screen readers can announce the
current state, and apply the same change to the other similar toggle button in
this component that flips another encoreFeatures property (ensure you reference
the same encoreFeatures property used in its onClick).
…gation for Unified Inbox Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…11y in Unified Inbox
- FocusModeView: Add resizeCleanupRef + useEffect unmount guard so
mousemove/mouseup listeners are removed if component unmounts mid-drag
- useAgentInbox: Group byAgent items by sessionId (unique) instead of
sessionName (display-only) to prevent merging different agents with
the same name
- InboxListView: Add tabIndex={0} and onKeyDown to the listbox div so
it is keyboard-focusable and navigable
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d Inbox - Replace "session" with "agent" in all user-facing labels (empty states, ARIA labels, menu text) - Align MAX_MESSAGE_LENGTH (250→90) with documented InboxItem.lastMessage truncation - Fix read filter to show all non-unread items regardless of session state - Add role="switch" and aria-checked to Encore feature toggle buttons - Guard agentInbox keyboard shortcut on setAgentInboxOpen callback availability - Add keyboard a11y (tabIndex, role="option", aria-selected, onKeyDown) to FocusModeView sidebar - Add onFocus/onBlur handlers to dialog container per focus guidelines - Fix context threshold label (>80% → ≥80%) to match >= 80 condition - Remove background/border shorthand overrides from group header styles - Fix playbook doc contradictions (ENCORE-INBOX-07, -11, -14) 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>
Add maxWidth 320px and break-words/break-all rules to tab hover overlay to prevent layout overflow with long tab names, session IDs, and descriptions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Added tabIndex={0}, aria-label, and onKeyDown handler for Enter/Space
on the description edit button in the tab hover overlay. Widened
handleStartEditDescription to accept KeyboardEvent in addition to
MouseEvent.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
felipeggv
force-pushed
the
feat/tab-description
branch
from
d61340a to
e3e7a07
Compare
Move onSelectedLevelsChange call from inside setSelectedLevelsState updater to a dedicated useEffect, preventing Zustand store set() from firing during React state update. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/renderer/components/SettingsModal.tsx (1)
3744-3751:⚠️ Potential issue | 🟡 MinorExpose Tab Description toggle state to assistive tech.
This toggle is stateful but does not expose on/off state, so screen readers won’t announce it properly.
Suggested fix
<button + role="switch" + aria-checked={encoreFeatures.tabDescription} className="w-full flex items-center justify-between p-4 text-left" onClick={() => setEncoreFeatures({ ...encoreFeatures, tabDescription: !encoreFeatures.tabDescription, }) } >🤖 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 3744 - 3751, The toggle button for tabDescription in SettingsModal lacks accessible state; update the element that currently calls setEncoreFeatures (the button wrapping the tabDescription toggle) to expose its on/off state to assistive tech by adding an appropriate ARIA attribute—e.g., aria-pressed={encoreFeatures.tabDescription} if keeping role=button, or role="switch" with aria-checked={encoreFeatures.tabDescription}—so screen readers will announce the current state when users interact with setEncoreFeatures and encoreFeatures.tabDescription.src/renderer/components/TabBar.tsx (1)
585-603:⚠️ Potential issue | 🟠 MajorDescription editing is still not keyboard-reachable from the tab itself.
Line 597 and Line 598 only open the overlay on hover, so keyboard users still can’t reach the Line 768 description edit control unless the overlay is already mouse-opened.
♿ Suggested fix (focus-open path for the overlay entry point)
<div ref={setTabRef} data-tab-id={tab.id} @@ onMouseEnter={handleMouseEnter} onMouseLeave={handleMouseLeave} + tabIndex={0} + onFocus={handleMouseEnter} + onBlur={handleMouseLeave} draggable onDragStart={handleTabDragStart}As per coding guidelines:
src/renderer/components/**/*.{ts,tsx}: Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus.Also applies to: 677-703, 746-793
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/TabBar.tsx` around lines 585 - 603, The tab element currently only exposes the overlay on hover, making the description edit unreachable via keyboard; add keyboard focus handling by giving the tab container a tabIndex (e.g., tabIndex={0}) and wire focus/blur handlers (e.g., onFocus={handleMouseEnter} and onBlur={handleMouseLeave} or dedicated handleFocus/handleBlur) and a key handler (onKeyDown) to open the overlay with Enter/Space; update the element that uses setTabRef and handlers (the div with ref={setTabRef}, onMouseEnter/onMouseLeave, onClick, etc.) so keyboard users can open the same overlay that handleMouseEnter currently opens and reach the description edit control.src/renderer/components/AgentInbox/InboxListView.tsx (1)
1058-1065:⚠️ Potential issue | 🟠 MajorFocusable listbox has no visible focus state.
Line 1064 removes the outline, but no replacement focus style is applied on the listbox itself, making keyboard focus location unclear.
♿ Suggested fix (restore explicit focus styling)
<div role="listbox" tabIndex={0} onKeyDown={handleKeyDown} + onFocus={(e) => { + e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`; + e.currentTarget.style.outlineOffset = '-2px'; + }} + onBlur={(e) => { + e.currentTarget.style.outline = 'none'; + }} aria-activedescendant={selectedItemId} aria-label="Inbox items" className="outline-none"As per coding guidelines:
src/renderer/components/**/*.{ts,tsx}: Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus.🤖 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 1058 - 1065, The listbox div currently removes the default outline via className="outline-none" leaving no visible focus indicator; restore a visible focus style and add focus handlers: remove or replace "outline-none" on the element in InboxListView (the div with role="listbox", tabIndex={0}, onKeyDown={handleKeyDown}, aria-activedescendant={selectedItemId}) with a focus-visible style (e.g., a ring/outline class or inline focus style such as focus:ring-2/focus:ring-offset and a contrasting color) and wire up onFocus and onBlur handlers (e.g., handleListboxFocus / handleListboxBlur or small inline handlers) to manage focus state if needed per accessibility guidelines so keyboard users can see where focus is.src/renderer/components/AgentInbox/FocusModeView.tsx (1)
482-509:⚠️ Potential issue | 🟡 MinorFocusable sidebar items still miss
onFocus/onBlurparity with hover styles.These rows are keyboard-focusable and keyboard-activatable, but keyboard focus still lacks equivalent visual treatment.
♿ Suggested fix
onMouseEnter={(e) => { if (!isCurrent) e.currentTarget.style.backgroundColor = `${theme.colors.accent}08`; }} onMouseLeave={(e) => { if (!isCurrent) e.currentTarget.style.backgroundColor = 'transparent'; }} + onFocus={(e) => { + if (!isCurrent) + e.currentTarget.style.backgroundColor = `${theme.colors.accent}08`; + }} + onBlur={(e) => { + if (!isCurrent) e.currentTarget.style.backgroundColor = 'transparent'; + }} >As per coding guidelines
src/renderer/components/**/*.{ts,tsx}: Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus.🤖 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 482 - 509, The focusable row div handling click/keyboard lacks onFocus/onBlur to mirror the hover visuals; add onFocus and onBlur handlers on the same element (the element keyed by `${itm.sessionId}-${itm.tabId}` and using currentRowRef/onNavigateItem) so that when focused (and not isCurrent) you set the same backgroundColor as onMouseEnter (`${theme.colors.accent}08`) and on blur revert to 'transparent' (and ensure the isCurrent styling still wins), or preferably reuse the same logic used in onMouseEnter/onMouseLeave to keep hover and keyboard focus visuals consistent while respecting the selected (isCurrent) state.
🧹 Nitpick comments (1)
src/renderer/hooks/useAgentInbox.ts (1)
69-72: PreferAITab['logs']typing for summary/timestamp helpers.These helpers operate on tab logs, so typing them as
Session['aiLogs']is looser than needed and can drift over time.Suggested refactor
-import type { Session, Group } from '../types'; +import type { Session, Group, AITab } from '../types'; @@ export function generateSmartSummary( - logs: Session['aiLogs'] | undefined, + logs: AITab['logs'] | undefined, sessionState: Session['state'] ): string { @@ -function deriveTimestamp(logs: Session['aiLogs'] | undefined, tabCreatedAt: number): number { +function deriveTimestamp(logs: AITab['logs'] | undefined, tabCreatedAt: number): number {Also applies to: 113-113
🤖 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 69 - 72, The helper functions (e.g., generateSmartSummary) are typed too loosely as Session['aiLogs']; change their parameter types to AITab['logs'] (and any other helper on the same file noted in the review around the second occurrence) so they operate on tab log shapes directly; update the function signatures (generateSmartSummary and the helper at the other noted location) to accept AITab['logs'] and adjust any internal references if necessary to the more specific shape.
🤖 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 1543-1576: The current handleAgentInboxQuickReply uses hardcoded
setTimeouts (150ms/100ms) to switch sessions which races under variable renders;
remove those time-based switches and instead route the message deterministically
by either (A) making inboxProcessInputRef accept an explicit target
(sessionId/tabId) and calling inboxProcessInputRef.current(text, { sessionId,
tabId }) after updating sessions via setSessions, or (B) implement a
pendingSendRef + useEffect that watches activeSessionId and, once the state
update from setActiveSessionId completes, invokes
inboxProcessInputRef.current(text) and then restores previousActiveSessionId—do
not rely on setTimeouts; update handleAgentInboxQuickReply, inboxProcessInputRef
usage, and any related state restoration logic accordingly.
In `@src/renderer/components/AgentInbox/index.tsx`:
- Around line 53-68: The current useEffect cleanup cancels the
requestAnimationFrame stored in rafIdRef which prevents the focus-restoration
scheduled in handleClose from running; change the cleanup to stop calling
cancelAnimationFrame (or only null the ref) so the RAF scheduled by handleClose
can execute and focus triggerRef, i.e., update the useEffect cleanup that
references rafIdRef to not call cancelAnimationFrame(rafIdRef.current) (or add a
guard so it only cancels unrelated RAFs) while leaving handleClose’s
requestAnimationFrame logic intact so onClose -> requestAnimationFrame -> focus
restoration runs as intended.
---
Duplicate comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 482-509: The focusable row div handling click/keyboard lacks
onFocus/onBlur to mirror the hover visuals; add onFocus and onBlur handlers on
the same element (the element keyed by `${itm.sessionId}-${itm.tabId}` and using
currentRowRef/onNavigateItem) so that when focused (and not isCurrent) you set
the same backgroundColor as onMouseEnter (`${theme.colors.accent}08`) and on
blur revert to 'transparent' (and ensure the isCurrent styling still wins), or
preferably reuse the same logic used in onMouseEnter/onMouseLeave to keep hover
and keyboard focus visuals consistent while respecting the selected (isCurrent)
state.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 1058-1065: The listbox div currently removes the default outline
via className="outline-none" leaving no visible focus indicator; restore a
visible focus style and add focus handlers: remove or replace "outline-none" on
the element in InboxListView (the div with role="listbox", tabIndex={0},
onKeyDown={handleKeyDown}, aria-activedescendant={selectedItemId}) with a
focus-visible style (e.g., a ring/outline class or inline focus style such as
focus:ring-2/focus:ring-offset and a contrasting color) and wire up onFocus and
onBlur handlers (e.g., handleListboxFocus / handleListboxBlur or small inline
handlers) to manage focus state if needed per accessibility guidelines so
keyboard users can see where focus is.
In `@src/renderer/components/SettingsModal.tsx`:
- Around line 3744-3751: The toggle button for tabDescription in SettingsModal
lacks accessible state; update the element that currently calls
setEncoreFeatures (the button wrapping the tabDescription toggle) to expose its
on/off state to assistive tech by adding an appropriate ARIA attribute—e.g.,
aria-pressed={encoreFeatures.tabDescription} if keeping role=button, or
role="switch" with aria-checked={encoreFeatures.tabDescription}—so screen
readers will announce the current state when users interact with
setEncoreFeatures and encoreFeatures.tabDescription.
In `@src/renderer/components/TabBar.tsx`:
- Around line 585-603: The tab element currently only exposes the overlay on
hover, making the description edit unreachable via keyboard; add keyboard focus
handling by giving the tab container a tabIndex (e.g., tabIndex={0}) and wire
focus/blur handlers (e.g., onFocus={handleMouseEnter} and
onBlur={handleMouseLeave} or dedicated handleFocus/handleBlur) and a key handler
(onKeyDown) to open the overlay with Enter/Space; update the element that uses
setTabRef and handlers (the div with ref={setTabRef}, onMouseEnter/onMouseLeave,
onClick, etc.) so keyboard users can open the same overlay that handleMouseEnter
currently opens and reach the description edit control.
---
Nitpick comments:
In `@src/renderer/hooks/useAgentInbox.ts`:
- Around line 69-72: The helper functions (e.g., generateSmartSummary) are typed
too loosely as Session['aiLogs']; change their parameter types to AITab['logs']
(and any other helper on the same file noted in the review around the second
occurrence) so they operate on tab log shapes directly; update the function
signatures (generateSmartSummary and the helper at the other noted location) to
accept AITab['logs'] and adjust any internal references if necessary to the more
specific shape.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-07.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-11.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-14.mdsrc/renderer/App.tsxsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/components/TabBar.tsxsrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/hooks/tabs/useTabHandlers.tssrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/index.ts
✅ Files skipped from review due to trivial changes (1)
- playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-14.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/renderer/components/MainPanel.tsx
- src/renderer/hooks/tabs/useTabHandlers.ts
- src/renderer/stores/settingsStore.ts
Comment on lines
+53
to
+68
| useEffect(() => { | ||
| return () => { | ||
| if (rafIdRef.current !== null) { | ||
| cancelAnimationFrame(rafIdRef.current); | ||
| } | ||
| }; | ||
| }, []); | ||
|
|
||
| const handleClose = useCallback(() => { | ||
| onClose(); | ||
| rafIdRef.current = requestAnimationFrame(() => { | ||
| rafIdRef.current = null; | ||
| if (triggerRef.current && triggerRef.current instanceof HTMLElement) { | ||
| triggerRef.current.focus(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Unmount cleanup currently cancels the close-time focus restoration.
At Line 53, the unmount cleanup cancels the same requestAnimationFrame scheduled in handleClose, so focus restoration often never runs after modal close.
💡 Suggested fix
- const triggerRef = useRef<Element | null>(document.activeElement);
- const rafIdRef = useRef<number | null>(null);
- useEffect(() => {
- return () => {
- if (rafIdRef.current !== null) {
- cancelAnimationFrame(rafIdRef.current);
- }
- };
- }, []);
+ const triggerRef = useRef<HTMLElement | null>(
+ document.activeElement instanceof HTMLElement ? document.activeElement : null
+ );
const handleClose = useCallback(() => {
- onClose();
- rafIdRef.current = requestAnimationFrame(() => {
- rafIdRef.current = null;
- if (triggerRef.current && triggerRef.current instanceof HTMLElement) {
- triggerRef.current.focus();
- }
- });
+ const trigger = triggerRef.current;
+ onClose();
+ requestAnimationFrame(() => {
+ trigger?.focus();
+ });
}, [onClose]);🤖 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 53 - 68, The
current useEffect cleanup cancels the requestAnimationFrame stored in rafIdRef
which prevents the focus-restoration scheduled in handleClose from running;
change the cleanup to stop calling cancelAnimationFrame (or only null the ref)
so the RAF scheduled by handleClose can execute and focus triggerRef, i.e.,
update the useEffect cleanup that references rafIdRef to not call
cancelAnimationFrame(rafIdRef.current) (or add a guard so it only cancels
unrelated RAFs) while leaving handleClose’s requestAnimationFrame logic intact
so onClose -> requestAnimationFrame -> focus restoration runs as intended.
Author
|
All review feedback addressed: Escape/blur save race condition (skipNextBlurSaveRef), break-all to break-words, whitespace trim for descriptions, keyboard accessibility for edit action. Inherits all Inbox fixes from #468 via rebase. Requesting re-review. @coderabbitai review @greptile review |
|
Starting the review now. ✅ Actions performedReview triggered.
|
…indicators - Replace non-deterministic setTimeout with requestAnimationFrame in handleAgentInboxQuickReply; use activeSessionIdRef to avoid stale closures - Add role="switch" + aria-checked to tabDescription Encore toggle for consistency with unifiedInbox toggle - Add onFocus/onBlur handlers to FocusModeView sidebar items mirroring onMouseEnter/onMouseLeave hover background - Add visible focus outline to InboxListView listbox via onFocus/onBlur inline handlers (keeps outline-none class to prevent double-outline) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/renderer/components/AgentInbox/FocusModeView.tsx (1)
971-989: Consider adding keyboard accessibility to the resize handle.The sidebar resize handle only supports mouse drag. For full keyboard accessibility, consider allowing users to resize with keyboard (e.g., focus the handle and use arrow keys). This is a low-priority enhancement since resize handles are commonly mouse-only.
🤖 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 971 - 989, The resize handle only supports mouse dragging; add keyboard accessibility by making the div focusable (tabIndex=0), giving it an appropriate role/aria-label (e.g., role="separator" aria-orientation="vertical" aria-valuenow/aria-valuemin/aria-valuemax tied to the sidebar width), and adding an onKeyDown handler that adjusts the sidebar width state (the same state mutated by handleResizeStart/drag logic — e.g., sidebarWidth/setSidebarWidth) in response to ArrowLeft/ArrowRight (and Home/End/PageUp/PageDown if desired); also add visible focus styles and ensure isResizingRef logic continues to prevent hover-style resets while keyboard resizing.src/renderer/components/SettingsModal.tsx (2)
3646-3729: Unified Inbox Encore card: solid a11y + clear gating UI.
role="switch"+aria-checkedon the header button is consistent and should read well in assistive tech (Line 3659-3662).Two optional nits:
- Add
type="button"for robustness.- Consider clarifying the shortcut hint to include Linux if applicable (currently “Alt+I (Windows)”).
Proposed diff (optional)
<button role="switch" aria-checked={encoreFeatures.unifiedInbox} + type="button" className="w-full flex items-center justify-between p-4 text-left" onClick={() => setEncoreFeatures({ ...encoreFeatures, unifiedInbox: !encoreFeatures.unifiedInbox, }) } > @@ {encoreFeatures.unifiedInbox && ( <div className="px-4 pb-4"> <p className="text-xs" style={{ color: theme.colors.textDim }}> - Access via Option+I (Mac) or Alt+I (Windows). Aggregates status updates, + Access via Option+I (Mac) or Alt+I (Windows/Linux). Aggregates status updates, errors, and action items from all running agents into a single keyboard-navigable view. </p> </div> )}🤖 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 3646 - 3729, The header toggle for the Unified Inbox (the button that reads and sets encoreFeatures.unifiedInbox) should include an explicit type attribute and a Linux shortcut mention: add type="button" to the button element that currently has role="switch" and aria-checked={encoreFeatures.unifiedInbox}, and update the shortcut text shown when encoreFeatures.unifiedInbox is true (the paragraph text under the Unified Inbox Info) to mention Linux (e.g., "Option+I (Mac), Alt+I (Windows) or Alt+I (Linux)").
3731-3801: Tab Descriptions Encore card: consistent switch semantics + clear user value.
Same optional hardening here: addtype="button"on the header button.Proposed diff (optional)
<button role="switch" aria-checked={encoreFeatures.tabDescription} + type="button" className="w-full flex items-center justify-between p-4 text-left" onClick={() => setEncoreFeatures({ ...encoreFeatures, tabDescription: !encoreFeatures.tabDescription, }) } >🤖 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 3731 - 3801, The header toggle button for the Tab Descriptions feature lacks an explicit type, which can cause unintended form submits; update the button in SettingsModal (the button using aria-checked={encoreFeatures.tabDescription} and onClick that calls setEncoreFeatures({... , tabDescription: !encoreFeatures.tabDescription})) to include type="button" so it behaves as a non-submitting toggle while preserving the existing aria and style behavior.
🤖 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 1551-1560: The handlers handleAgentInboxQuickReply and
handleAgentInboxOpenAndReply activate a session/tab via setSessions but do not
ensure the tab is in AI input mode or clear file focus, so replies can be routed
through terminal/file execution; update the setSessions update for the matching
session/tab (same block that sets activeTabId and clears hasUnread) to also set
inputMode: 'ai' and activeFileTabId: null on the returned session object (use
the sessionId, tabId, aiTabs, inputMode and activeFileTabId symbols to locate
the logic), ensuring both handlers perform the same mutation so quick-reply
flows always force AI mode and clear file tab focus.
---
Nitpick comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 971-989: The resize handle only supports mouse dragging; add
keyboard accessibility by making the div focusable (tabIndex=0), giving it an
appropriate role/aria-label (e.g., role="separator" aria-orientation="vertical"
aria-valuenow/aria-valuemin/aria-valuemax tied to the sidebar width), and adding
an onKeyDown handler that adjusts the sidebar width state (the same state
mutated by handleResizeStart/drag logic — e.g., sidebarWidth/setSidebarWidth) in
response to ArrowLeft/ArrowRight (and Home/End/PageUp/PageDown if desired); also
add visible focus styles and ensure isResizingRef logic continues to prevent
hover-style resets while keyboard resizing.
In `@src/renderer/components/SettingsModal.tsx`:
- Around line 3646-3729: The header toggle for the Unified Inbox (the button
that reads and sets encoreFeatures.unifiedInbox) should include an explicit type
attribute and a Linux shortcut mention: add type="button" to the button element
that currently has role="switch" and aria-checked={encoreFeatures.unifiedInbox},
and update the shortcut text shown when encoreFeatures.unifiedInbox is true (the
paragraph text under the Unified Inbox Info) to mention Linux (e.g., "Option+I
(Mac), Alt+I (Windows) or Alt+I (Linux)").
- Around line 3731-3801: The header toggle button for the Tab Descriptions
feature lacks an explicit type, which can cause unintended form submits; update
the button in SettingsModal (the button using
aria-checked={encoreFeatures.tabDescription} and onClick that calls
setEncoreFeatures({... , tabDescription: !encoreFeatures.tabDescription})) to
include type="button" so it behaves as a non-submitting toggle while preserving
the existing aria and style behavior.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/renderer/App.tsxsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/LogViewer.tsxsrc/renderer/components/SettingsModal.tsx
Comment on lines
+1551
to
+1560
| // Activate the target tab and mark as read (processInput adds the user log entry) | ||
| setSessions((prev) => | ||
| prev.map((s) => { | ||
| if (s.id !== sessionId) return s; | ||
| return { | ||
| ...s, | ||
| activeTabId: tabId, | ||
| aiTabs: s.aiTabs.map((t) => (t.id === tabId ? { ...t, hasUnread: false } : t)), | ||
| }; | ||
| }) |
There was a problem hiding this comment.
Force AI mode in inbox reply flows before writing/sending text.
handleAgentInboxQuickReply and handleAgentInboxOpenAndReply switch session/tab, but they do not force inputMode: 'ai' and activeFileTabId: null. If the target session is in terminal mode, quick-reply can route text through the wrong execution path.
🐛 Proposed fix
@@
setSessions((prev) =>
prev.map((s) => {
if (s.id !== sessionId) return s;
return {
...s,
activeTabId: tabId,
+ activeFileTabId: null,
+ inputMode: 'ai' as const,
aiTabs: s.aiTabs.map((t) => (t.id === tabId ? { ...t, hasUnread: false } : t)),
};
})
);
@@
setSessions((prev) =>
prev.map((s) => {
if (s.id !== sessionId) return s;
return {
...s,
activeTabId: tabId,
+ activeFileTabId: null,
+ inputMode: 'ai' as const,
aiTabs: s.aiTabs.map((t) =>
t.id === tabId ? { ...t, inputValue: text, hasUnread: false } : t
),
};
})
);Also applies to: 1586-1590
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/App.tsx` around lines 1551 - 1560, The handlers
handleAgentInboxQuickReply and handleAgentInboxOpenAndReply activate a
session/tab via setSessions but do not ensure the tab is in AI input mode or
clear file focus, so replies can be routed through terminal/file execution;
update the setSessions update for the matching session/tab (same block that sets
activeTabId and clears hasUnread) to also set inputMode: 'ai' and
activeFileTabId: null on the returned session object (use the sessionId, tabId,
aiTabs, inputMode and activeFileTabId symbols to locate the logic), ensuring
both handlers perform the same mutation so quick-reply flows always force AI
mode and clear file tab focus.
- Replace O(n*m) .find() loop in resolveFrozenItems with O(n+m) Map lookup - Replace setTimeout(200) with double requestAnimationFrame for reply focus - Add circular dependency comment to AgentInboxModalData loose string types - Add comment explaining STATUS_COLORS cannot use keyof ThemeColors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/renderer/components/AgentInbox/index.tsx (1)
291-292: Remove empty event handlers.The empty
onFocusandonBlurhandlers serve no purpose and add minor noise to the code.🧹 Remove empty handlers
onKeyDownCapture={handleCaptureKeyDown} onKeyDown={handleShellKeyDown} -onFocus={() => {}} -onBlur={() => {}} >🤖 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 291 - 292, Remove the no-op event handlers by deleting the empty onFocus and onBlur props from the JSX in the AgentInbox component (remove the onFocus={() => {}} and onBlur={() => {}} occurrences) so the element no longer includes unused handlers and behavior is unchanged.src/renderer/stores/modalStore.ts (1)
666-669: Consider tightening the type forupdateAgentInboxData.The
Record<string, unknown>type is looser thanPartial<AgentInboxModalData>. While this avoids the circular dependency, it also allows invalid keys to be passed without type errors.💡 Alternative: inline the known keys
-updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data), +updateAgentInboxData: (data: { filterMode?: string; sortMode?: string; isExpanded?: boolean }) => + 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 666 - 669, The updateAgentInboxData function currently accepts a loose Record<string, unknown>; tighten it to Partial<AgentInboxModalData> to catch invalid keys by changing the parameter type on updateAgentInboxData to Partial<AgentInboxModalData>. If importing AgentInboxModalData would create a circular dependency, instead declare a small inline type (e.g. AgentInboxModalPayload with the known keys used by the modal) and use Partial<AgentInboxModalPayload> for updateAgentInboxData to preserve type safety while avoiding the cycle; keep the function name updateAgentInboxData and the modal id 'agentInbox' unchanged.src/renderer/components/AgentInbox/FocusModeView.tsx (1)
978-996: Consider adding keyboard accessibility to the resize handle.The resize handle responds to mouse events but lacks keyboard support. Users relying on keyboard navigation cannot resize the sidebar. While not blocking, adding keyboard controls would improve accessibility.
♿ Optional keyboard resize support
<div onMouseDown={handleResizeStart} + tabIndex={0} + role="separator" + aria-orientation="vertical" + aria-label="Resize sidebar" + onKeyDown={(e) => { + if (e.key === 'ArrowLeft') { + e.preventDefault(); + setSidebarWidth((w) => Math.max(160, w - 10)); + } else if (e.key === 'ArrowRight') { + e.preventDefault(); + setSidebarWidth((w) => Math.min(400, w + 10)); + } + }} style={{ width: 4, cursor: 'col-resize',🤖 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 978 - 996, The resize handle div currently only supports mouse events; make it keyboard accessible by adding tabIndex={0} and an onKeyDown handler on the same element to adjust the sidebar width when ArrowLeft/ArrowRight (and optionally Home/End) are pressed, invoking the same resize logic used by handleResizeStart (or factoring the width-update logic into a shared function) and respecting isResizingRef state; also add focus/blur handlers to mirror the mouseEnter/mouseLeave visual feedback using theme colors so keyboard users see the active state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 978-996: The resize handle div currently only supports mouse
events; make it keyboard accessible by adding tabIndex={0} and an onKeyDown
handler on the same element to adjust the sidebar width when
ArrowLeft/ArrowRight (and optionally Home/End) are pressed, invoking the same
resize logic used by handleResizeStart (or factoring the width-update logic into
a shared function) and respecting isResizingRef state; also add focus/blur
handlers to mirror the mouseEnter/mouseLeave visual feedback using theme colors
so keyboard users see the active state.
In `@src/renderer/components/AgentInbox/index.tsx`:
- Around line 291-292: Remove the no-op event handlers by deleting the empty
onFocus and onBlur props from the JSX in the AgentInbox component (remove the
onFocus={() => {}} and onBlur={() => {}} occurrences) so the element no longer
includes unused handlers and behavior is unchanged.
In `@src/renderer/stores/modalStore.ts`:
- Around line 666-669: The updateAgentInboxData function currently accepts a
loose Record<string, unknown>; tighten it to Partial<AgentInboxModalData> to
catch invalid keys by changing the parameter type on updateAgentInboxData to
Partial<AgentInboxModalData>. If importing AgentInboxModalData would create a
circular dependency, instead declare a small inline type (e.g.
AgentInboxModalPayload with the known keys used by the modal) and use
Partial<AgentInboxModalPayload> for updateAgentInboxData to preserve type safety
while avoiding the cycle; keep the function name updateAgentInboxData and the
modal id 'agentInbox' unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/stores/modalStore.tssrc/renderer/types/agent-inbox.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/types/agent-inbox.ts
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>
…rd handler Extract shared filterAndSortSlashCommands() utility to ensure both InputArea (visual dropdown) and useInputKeyDown (Enter/Tab handler) produce identically ordered command lists. Previously, InputArea sorted by relevance score while useInputKeyDown kept original array order, causing the wrong command to be inserted when pressing Enter/Tab. Also fixes SessionList group auto-expansion to use fuzzyMatch() consistently instead of substring .includes(), matching the main sessionCategories filter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prefix match compared lowercased cmdName against potentially mixed-case searchTerm, making it case-sensitive while fuzzyMatchWithScore was case-insensitive. Now lowercases searchTerm once at function entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Clamp selectedSlashCommandIndex in useInputKeyDown before accessing filteredCommands, preventing silent no-op when index exceeds list length after the user continues typing - Align SessionList group-expansion useEffect with sessionCategories memo: add worktree children branch/name matching and parentSessionId filtering, fix missing sessions and worktreeChildrenByParentId dependencies - Add 11 tests for filterAndSortSlashCommands covering mode filtering, prefix vs fuzzy ranking, case insensitivity, description matching, empty results, and generic type preservation - Remove duplicate SlashCommand interface from InputArea.tsx, import SlashCommandEntry from search.ts instead Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When filteredCommands.length === 0, ArrowDown/ArrowUp/Enter/Tab handlers would compute invalid indices (e.g. Math.min(prev+1, -1)). Added early return guard that only allows Escape when no commands match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The 'filters out aiOnly commands in terminal mode' test used '/r' as input, which with fuzzy matching now matches '/clear' (the 'r' in 'clear'). Changed to '/rn' which only fuzzy-matches '/run', correctly testing that aiOnly commands are excluded in terminal mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
Adds inline-editable tab descriptions as an Encore Feature.
tabDescriptionflag inEncoreFeatureFlags— disabled by default, toggle in Settings > Encore tabdescriptionproperty onAITabinterfaceChanges (tab-description only)
5 commits, 8 files, ~210 lines added:
types/index.ts—tabDescriptionflag +descriptionfield onAITabsettingsStore.ts— defaulttabDescription: falseSettingsModal.tsx— Tab Description toggle card in Encore tabuseTabHandlers.ts—handleUpdateTabDescriptionhandlerTabBar.tsx— inline-editable description in hover overlayApp.tsx/MainPanel.tsx/useMainPanelProps.ts— prop threadingDepends on
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes & Accessibility