Add notification inbox and tab stacks#16
Merged
Conversation
📝 WalkthroughWalkthroughThe PR transforms the app's navigation from explicit route groups to a tab-based structure, introduces a durable, polled notification feed with daemon-record evaluation, and adds notification UI surfaces (bell button, inbox screen, unread badges) throughout the shell. Core changes: Expo Router Tabs with four main tab layouts, project-scoped notification state atoms with polling in MainLayout, notification API functions with relay routing, daemon record evaluation via ChangesNotification system and tab-based navigation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/app/(main)/(tabs)/(dashboard)/_layout.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. app/app/(main)/(tabs)/(dashboard)/agent/[sessionId]/chat.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. app/app/(main)/(tabs)/(dashboard)/index.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/components/NotificationBellButton.tsx (1)
25-26: ⚡ Quick winUse shared tab inference for active state instead of a hardcoded pathname prefix.
Line 25 should derive active state from the same tab mapping used for navigation, so route alias/path changes don’t silently desync the bell highlight behavior.
Proposed change
-import { useMainTabNavigation } from "`@/lib/main-tabs`"; +import { mainTabForPath, useMainTabNavigation } from "`@/lib/main-tabs`"; @@ - const active = pathname.startsWith("/notifications"); + const active = mainTabForPath(pathname) === "inbox";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/NotificationBellButton.tsx` around lines 25 - 26, Replace the hardcoded startsWith check that sets active (const active = pathname.startsWith("/notifications")) with the shared tab inference used by navigation: import and call the shared function/enum (e.g., getActiveTab, inferTabFromPath, or the Tabs enum used elsewhere) with pathname and set active = getActiveTab(pathname) === 'notifications' (or === Tabs.Notifications) so the NotificationBellButton uses the same tab mapping as navigation and won’t desync when routes/aliases change.app/components/NotificationProvider.tsx (1)
31-57: ⚡ Quick winBound per-project seen-ID cache growth.
seenNotificationIdsRefonly grows, so long-running sessions can retain every historical notification ID for a project. Prune stale IDs (or cap size) before processing new records.Proposed adjustment
const currentIds = new Set(notificationFeed.notifications.map((record) => record.id)); const seenIds = seenNotificationIdsRef.current.get(projectScope) ?? new Set<string>(); + for (const id of seenIds) { + if (!currentIds.has(id)) seenIds.delete(id); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/NotificationProvider.tsx` around lines 31 - 57, seenNotificationIdsRef currently only grows per project; before processing in the effect (where projectScope, currentIds, seenIds, and notificationFeed.notifications are used) prune or cap the stored seen IDs to avoid unbounded memory growth: for example, intersect seenIds with currentIds to drop IDs no longer relevant and/or enforce a max size (e.g., keep the most recent N IDs by converting seenIds to an ordered array and slicing) then continue with the existing logic that iterates notificationFeed.notifications, calls evaluateNotificationRecord, and possibly showBrowserNotification; update seenNotificationIdsRef.current.set(projectScope, seenIds) after pruning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/components/NotificationBellButton.tsx`:
- Around line 25-26: Replace the hardcoded startsWith check that sets active
(const active = pathname.startsWith("/notifications")) with the shared tab
inference used by navigation: import and call the shared function/enum (e.g.,
getActiveTab, inferTabFromPath, or the Tabs enum used elsewhere) with pathname
and set active = getActiveTab(pathname) === 'notifications' (or ===
Tabs.Notifications) so the NotificationBellButton uses the same tab mapping as
navigation and won’t desync when routes/aliases change.
In `@app/components/NotificationProvider.tsx`:
- Around line 31-57: seenNotificationIdsRef currently only grows per project;
before processing in the effect (where projectScope, currentIds, seenIds, and
notificationFeed.notifications are used) prune or cap the stored seen IDs to
avoid unbounded memory growth: for example, intersect seenIds with currentIds to
drop IDs no longer relevant and/or enforce a max size (e.g., keep the most
recent N IDs by converting seenIds to an ordered array and slicing) then
continue with the existing logic that iterates notificationFeed.notifications,
calls evaluateNotificationRecord, and possibly showBrowserNotification; update
seenNotificationIdsRef.current.set(projectScope, seenIds) after pruning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ab094c1a-1879-47ea-b348-8d40a69aa6b4
📒 Files selected for processing (29)
app/app/(main)/(tabs)/(dashboard)/_layout.tsxapp/app/(main)/(tabs)/(dashboard)/agent/[sessionId]/chat.tsxapp/app/(main)/(tabs)/(dashboard)/graveyard.tsxapp/app/(main)/(tabs)/(dashboard)/index.tsxapp/app/(main)/(tabs)/(dashboard)/plans/[sessionId].tsxapp/app/(main)/(tabs)/(dashboard)/service/[serviceId].tsxapp/app/(main)/(tabs)/(inbox)/_layout.tsxapp/app/(main)/(tabs)/(inbox)/notifications.tsxapp/app/(main)/(tabs)/(settings)/_layout.tsxapp/app/(main)/(tabs)/(settings)/settings.tsxapp/app/(main)/(tabs)/(threads)/_layout.tsxapp/app/(main)/(tabs)/(threads)/threads.tsxapp/app/(main)/(tabs)/_layout.tsxapp/app/(main)/_layout.tsxapp/app/index.tsxapp/components/AppShell.tsxapp/components/AuthMenu.tsxapp/components/MobileTabBar.tsxapp/components/NotificationBellButton.tsxapp/components/NotificationProvider.tsxapp/components/ProjectSidebar.tsxapp/components/TopBar.tsxapp/lib/api.test.tsapp/lib/api.tsapp/lib/main-tabs.tsapp/lib/notification-policy.test.tsapp/lib/notification-policy.tsapp/stores/notifications.tsdocs/notification-system.md
💤 Files with no reviewable changes (1)
- app/components/AppShell.tsx
This was referenced May 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Summary by CodeRabbit
Release Notes