feat(web): port session bell filter from desktop Left Bar#884
feat(web): port session bell filter from desktop Left Bar#884pedramamini merged 2 commits intoRunMaestro:rcfrom
Conversation
Mirrors the desktop `useSortedSessions.passesUnreadFilter` so the web LeftPanel can restrict the agent list to sessions that are active, busy, have unread tabs, or contain busy/unread worktree children. The bell button sits in the panel header, shows an accent dot when any session has unread activity, and auto-disables if the unread set empties. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughLifts the mobile "bell filter" state to the app, adds Changes
Sequence Diagram(s)(No sequence diagrams generated — changes are UI/state lifting within a small set of components and do not introduce a multi-component sequential flow that requires visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR ports the desktop session bell filter to the web Confidence Score: 5/5Safe to merge — all findings are P2 style/UX suggestions with no correctness or data-integrity concerns. The filter logic is correct and faithfully mirrors the desktop implementation. Two P2 findings (connecting state omission and ephemeral filter state) are minor UX considerations that don't break any user path. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[sessions prop] --> B[buildWorktreeChildrenMap\nworktreeChildrenByParent]
A --> C{showUnreadOnly?}
C -- No --> D[visibleSessions = sessions]
C -- Yes --> E["Filter: keep all children\n+ parents that passesUnreadFilter"]
B --> E
E --> D
D --> F[groupSessions\nvisibleSessions]
F --> G[grouped / worktreeChildrenMap]
G --> H{grouped.length === 0\n&& showUnreadOnly\n&& sessions.length > 0?}
H -- Yes --> I[Empty state:\n'No active or unread agents']
H -- No --> J[Render session groups]
A --> K[hasUnreadAgents]
K --> L{Bell button indicator dot\nshown when hasUnreadAgents\n&& !showUnreadOnly}
K --> M[useEffect auto-disable\nif showUnreadOnly && !hasUnreadAgents]
Reviews (1): Last reviewed commit: "feat(web): port session bell filter from..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function sessionHasUnreadActivity(session: Session): boolean { | ||
| return (session.aiTabs?.some((tab) => tab.hasUnread) ?? false) || session.state === 'busy'; | ||
| } |
There was a problem hiding this comment.
connecting state excluded from bell filter
sessionHasUnreadActivity only tests state === 'busy', so a session in the connecting state is invisible to the bell filter even though getGroupStatus (line 46) and the status-color helper (line 54) both treat connecting as an active/busy state. A user will see the orange pulsing dot but the session won't appear when the bell filter is on, making the filter feel inconsistent with the visual indicators.
Note: this intentionally diverges from the desktop passesUnreadFilter (which also omits connecting), so accepting the divergence is also a valid choice — worth an explicit decision either way.
| function sessionHasUnreadActivity(session: Session): boolean { | |
| return (session.aiTabs?.some((tab) => tab.hasUnread) ?? false) || session.state === 'busy'; | |
| } | |
| function sessionHasUnreadActivity(session: Session): boolean { | |
| return (session.aiTabs?.some((tab) => tab.hasUnread) ?? false) || session.state === 'busy' || session.state === 'connecting'; | |
| } |
There was a problem hiding this comment.
Intentional parity with desktop useSortedSessions.passesUnreadFilter — desktop also omits connecting. connecting is a transient state (seconds), and including it would cause sessions to flash in and out of the filtered list during connection. Keeping the divergence from getGroupStatus as-is: the bell answers "what needs my attention", which connecting sessions don't (yet). Happy to revisit as a cross-platform change if the desktop side changes.
| [setCollapsedGroups] | ||
| ); | ||
|
|
There was a problem hiding this comment.
showUnreadOnly is local component state, so it resets to false every time the panel is unmounted and remounted (common on mobile). By contrast, collapsedGroups is deliberately lifted to props with the comment "persists across panel open/close." If the bell filter should behave similarly, it should be lifted to the parent and passed in the same way — otherwise users on mobile will lose their filter preference every time they close the panel.
There was a problem hiding this comment.
Good catch — fixed in 6372d3c. Lifted showUnreadOnly to App.tsx using the same pattern as collapsedGroups so the filter survives LeftPanel unmount/remount.
`LeftPanel` is conditionally rendered under `{showLeftPanel && ...}`,
so it remounts on every open/close. The bell filter was local state
and reset every time, which is noticeable on mobile. Lifts
`showUnreadOnly` to `App.tsx` using the same pattern as
`collapsedGroups`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/web/mobile/LeftPanel.tsx (1)
680-684: Nit: includesetShowUnreadOnlyin the effect deps.
setShowUnreadOnlyis now a prop rather than a localuseStatesetter, so it's not guaranteed to be referentially stable for future callers (though in practiceMobileApppasses a stableuseStatesetter today). Adding it to the dependency array will also satisfyreact-hooks/exhaustive-depswithout changing behavior.♻️ Proposed tweak
useEffect(() => { if (showUnreadOnly && !hasUnreadAgents) { setShowUnreadOnly(false); } - }, [showUnreadOnly, hasUnreadAgents]); + }, [showUnreadOnly, hasUnreadAgents, setShowUnreadOnly]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/mobile/LeftPanel.tsx` around lines 680 - 684, The useEffect that checks showUnreadOnly and hasUnreadAgents omits the prop setter setShowUnreadOnly from its dependency array; update the useEffect deps to include setShowUnreadOnly so the effect depends on the prop setter as well (locate the useEffect block referencing showUnreadOnly, hasUnreadAgents and call setShowUnreadOnly) — this keeps behavior identical but ensures correct hooks dependencies and satisfies react-hooks/exhaustive-deps.
🤖 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/web/mobile/LeftPanel.tsx`:
- Around line 680-684: The useEffect that checks showUnreadOnly and
hasUnreadAgents omits the prop setter setShowUnreadOnly from its dependency
array; update the useEffect deps to include setShowUnreadOnly so the effect
depends on the prop setter as well (locate the useEffect block referencing
showUnreadOnly, hasUnreadAgents and call setShowUnreadOnly) — this keeps
behavior identical but ensures correct hooks dependencies and satisfies
react-hooks/exhaustive-deps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: abc74406-3b18-4bbb-bfb7-618fb1117709
📒 Files selected for processing (3)
src/__tests__/web/mobile/App.test.tsxsrc/web/mobile/App.tsxsrc/web/mobile/LeftPanel.tsx
✅ Files skipped from review due to trivial changes (1)
- src/tests/web/mobile/App.test.tsx
|
Thanks for the contribution, @chr1syy! 🙏 Nice clean port — the web No merge conflicts, approving. One optional follow-up from CodeRabbit worth considering: adding Approving ✅ |
- LeftPanel (mobile): include setShowUnreadOnly in the auto-disable
effect's deps so React's exhaustive-deps lint stays satisfied now
that the setter arrives via props.
- web-server-factory: collapse the inline `import('../../shared/...').
Shortcut` to the existing top-level type import.
- agents.test (SSH detect): tighten the call-count check to
AGENT_DEFINITIONS.length so a regression that silently skips agents
is caught instead of just dropping below the prior `> 0` threshold.
2 participants
Summary
LeftPanel: a bell button in the panel header that toggles the agent list to show only sessions that are active (busy), have unread tabs, or contain busy/unread worktree children.src/renderer/hooks/session/useSortedSessions.tspassesUnreadFilterexactly so web and desktop stay in sync.No server / hook / props-interface changes —
AITabData.hasUnreadwas already flowing through the existing WebSocket contract (src/web/hooks/useWebSocket.ts).Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit