Skip to content

Feat/UI #1143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 24 commits into
base: staging
Choose a base branch
from
Open

Feat/UI #1143

wants to merge 24 commits into from

Conversation

Kashyap1ankit
Copy link

@Kashyap1ankit Kashyap1ankit commented May 29, 2025

READ CAREFULLY THEN REMOVE

Remove bullet points that are not relevant.

PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.

  • Pull requests that do not follow these guidelines will be closed without review or comment.
  • If you use AI to write your PR description your pr will be close without review or comment.
  • If you are unsure about anything, feel free to ask for clarification.

Description

-> Added the Sora font family for hero section slogan . Changed the YC badge style . Added proper spacing between different components in hero section

Please provide a clear description of your changes.


Type of Change

Please delete options that are not relevant.

  • 🎨 UI/UX improvement

Areas Affected

Please check all that apply:

  • User Interface/Experience

Testing Done

Describe the tests you've done:

  • Manual testing performed
  • Cross-browser testing (if UI changes)
  • Mobile responsiveness verified (if UI changes)

Security Considerations

For changes involving data or authentication:

  • No sensitive data is exposed
  • Authentication checks are in place
  • Input validation is implemented
  • Rate limiting is considered (if applicable)

Checklist

  • I have read the CONTRIBUTING document
  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in complex areas
  • I have updated the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix/feature works
  • Any dependent changes are merged and published

Additional Notes

Add any other context about the pull request here.

Screenshots/Recordings

Add screenshots or recordings here if applicable.

Screenshot from 2025-05-29 11-59-20


By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.

Summary by CodeRabbit

  • New Features

    • Added support for displaying and downloading all thread-level attachments in email threads.
    • Enhanced optimistic UI updates for marking threads as important and unread.
    • Improved cache refresh and state management for mail actions.
  • Improvements

    • Refined thread context menu and mail list to better reflect optimistic states for starred, important, and unread status.
    • Enhanced typography and animations on the home page for a more polished appearance.
    • Updated font usage and added new Google Fonts for improved visual consistency.
  • Bug Fixes

    • Fixed minor styling and formatting issues in various components.
    • Improved accessibility by removing redundant SVG titles.
  • Chores

    • Simplified authentication logic in server chat routes for streamlined authorization.

MrgSub and others added 24 commits May 27, 2025 21:26
# READ CAREFULLY THEN REMOVE

Remove bullet points that are not relevant.

PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.

- Pull requests that do not follow these guidelines will be closed without review or comment.
- If you use AI to write your PR description your pr will be close without review or comment.
- If you are unsure about anything, feel free to ask for clarification.

## Description

Please provide a clear description of your changes.

---

## Type of Change

Please delete options that are not relevant.

- [ ] 🐛 Bug fix (non-breaking change which fixes an issue)
- [ ] ✨ New feature (non-breaking change which adds functionality)
- [ ] 💥 Breaking change (fix or feature with breaking changes)
- [ ] 📝 Documentation update
- [ ] 🎨 UI/UX improvement
- [ ] 🔒 Security enhancement
- [ ] ⚡ Performance improvement

## Areas Affected

Please check all that apply:

- [ ] Email Integration (Gmail, IMAP, etc.)
- [ ] User Interface/Experience
- [ ] Authentication/Authorization
- [ ] Data Storage/Management
- [ ] API Endpoints
- [ ] Documentation
- [ ] Testing Infrastructure
- [ ] Development Workflow
- [ ] Deployment/Infrastructure

## Testing Done

Describe the tests you've done:

- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] Manual testing performed
- [ ] Cross-browser testing (if UI changes)
- [ ] Mobile responsiveness verified (if UI changes)

## Security Considerations

For changes involving data or authentication:

- [ ] No sensitive data is exposed
- [ ] Authentication checks are in place
- [ ] Input validation is implemented
- [ ] Rate limiting is considered (if applicable)

## Checklist

- [ ] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document
- [ ] My code follows the project's style guidelines
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in complex areas
- [ ] I have updated the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix/feature works
- [ ] All tests pass locally
- [ ] Any dependent changes are merged and published

## Additional Notes

Add any other context about the pull request here.

## Screenshots/Recordings

Add screenshots or recordings here if applicable.

---

_By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
# READ CAREFULLY THEN REMOVE

Remove bullet points that are not relevant.

PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.

- Pull requests that do not follow these guidelines will be closed without review or comment.
- If you use AI to write your PR description your pr will be close without review or comment.
- If you are unsure about anything, feel free to ask for clarification.

## Description

Please provide a clear description of your changes.

---

## Type of Change

Please delete options that are not relevant.

- [ ] 🐛 Bug fix (non-breaking change which fixes an issue)
- [ ] ✨ New feature (non-breaking change which adds functionality)
- [ ] 💥 Breaking change (fix or feature with breaking changes)
- [ ] 📝 Documentation update
- [ ] 🎨 UI/UX improvement
- [ ] 🔒 Security enhancement
- [ ] ⚡ Performance improvement

## Areas Affected

Please check all that apply:

- [ ] Email Integration (Gmail, IMAP, etc.)
- [ ] User Interface/Experience
- [ ] Authentication/Authorization
- [ ] Data Storage/Management
- [ ] API Endpoints
- [ ] Documentation
- [ ] Testing Infrastructure
- [ ] Development Workflow
- [ ] Deployment/Infrastructure

## Testing Done

Describe the tests you've done:

- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] Manual testing performed
- [ ] Cross-browser testing (if UI changes)
- [ ] Mobile responsiveness verified (if UI changes)

## Security Considerations

For changes involving data or authentication:

- [ ] No sensitive data is exposed
- [ ] Authentication checks are in place
- [ ] Input validation is implemented
- [ ] Rate limiting is considered (if applicable)

## Checklist

- [ ] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document
- [ ] My code follows the project's style guidelines
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in complex areas
- [ ] I have updated the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix/feature works
- [ ] All tests pass locally
- [ ] Any dependent changes are merged and published

## Additional Notes

Add any other context about the pull request here.

## Screenshots/Recordings

Add screenshots or recordings here if applicable.

---

_By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
# UI Improvements: Standardized Dark Mode Colors and Enhanced AI Chat Component

## Description

This PR standardizes dark mode colors across the application by replacing hardcoded color values with semantic color variables. It also enhances the AI Chat component with improved tool response rendering and adds a new voice chat capability.

Key changes:
- Replaced hardcoded dark background colors (`#1A1A1A`) with the semantic `bg-panelDark` class
- Replaced hardcoded text colors with semantic classes like `text-muted-foreground`
- Enhanced the AI Chat component with better tool response visualization
- Added tooltips to display tool arguments in AI responses
- Improved email composer editor focus handling
- Added a new Tools enum to better type and organize AI tool capabilities

## Type of Change

- [x] 🎨 UI/UX improvement
- [x] ⚡ Performance improvement

## Areas Affected

- [x] User Interface/Experience

## Testing Done

- [x] Manual testing performed
- [x] Cross-browser testing (if UI changes)
- [x] Mobile responsiveness verified (if UI changes)

## Checklist

- [x] I have performed a self-review of my code
- [x] My changes generate no new warnings
- [x] My code follows the project's style guidelines

## Additional Notes

This PR is part of our ongoing effort to standardize the UI color system and improve the dark mode experience. The changes to the AI Chat component make tool responses more informative and easier to understand.

_By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
# READ CAREFULLY THEN REMOVE

Remove bullet points that are not relevant.

PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.

- Pull requests that do not follow these guidelines will be closed without review or comment.
- If you use AI to write your PR description your pr will be close without review or comment.
- If you are unsure about anything, feel free to ask for clarification.

## Description

Please provide a clear description of your changes.

---

## Type of Change

Please delete options that are not relevant.

- [ ] 🐛 Bug fix (non-breaking change which fixes an issue)
- [ ] ✨ New feature (non-breaking change which adds functionality)
- [ ] 💥 Breaking change (fix or feature with breaking changes)
- [ ] 📝 Documentation update
- [ ] 🎨 UI/UX improvement
- [ ] 🔒 Security enhancement
- [ ] ⚡ Performance improvement

## Areas Affected

Please check all that apply:

- [ ] Email Integration (Gmail, IMAP, etc.)
- [ ] User Interface/Experience
- [ ] Authentication/Authorization
- [ ] Data Storage/Management
- [ ] API Endpoints
- [ ] Documentation
- [ ] Testing Infrastructure
- [ ] Development Workflow
- [ ] Deployment/Infrastructure

## Testing Done

Describe the tests you've done:

- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] Manual testing performed
- [ ] Cross-browser testing (if UI changes)
- [ ] Mobile responsiveness verified (if UI changes)

## Security Considerations

For changes involving data or authentication:

- [ ] No sensitive data is exposed
- [ ] Authentication checks are in place
- [ ] Input validation is implemented
- [ ] Rate limiting is considered (if applicable)

## Checklist

- [ ] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document
- [ ] My code follows the project's style guidelines
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in complex areas
- [ ] I have updated the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix/feature works
- [ ] All tests pass locally
- [ ] Any dependent changes are merged and published

## Additional Notes

Add any other context about the pull request here.

## Screenshots/Recordings

Add screenshots or recordings here if applicable.

---

_By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
## Description

Improved error handling in the ZeroAgent class by providing more specific error messages when authentication fails. The changes include:

1. Enhanced the error message in the execute method to specify whether the failure is due to missing driver or connectionId
2. Improved the error message in onConnect method to clarify when a token is missing
3. Removed redundant driver check in onChatMessage method as this validation is already handled elsewhere
4. Cleaned up imports by removing unused WSMessage type

## Type of Change

- [x] 🐛 Bug fix (non-breaking change which fixes an issue)
- [x] 🔒 Security enhancement

## Areas Affected

- [x] Authentication/Authorization
- [x] API Endpoints

## Testing Done

- [x] Manual testing performed

## Security Considerations

- [x] Authentication checks are in place

## Checklist

- [x] I have performed a self-review of my code
- [x] My changes generate no new warnings

_By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
done:
- adds new hook `useOptimisticAction`
	- in mail-lsit
	- in thread-display
- adds undo action to toast (design needs updating)

wip:
- undo sent email within 10-20 seconds
- update toast design/undo button
- add shortcut to undo 😎
- safari causing slight issues but it works
# READ CAREFULLY THEN REMOVE

Remove bullet points that are not relevant.

PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.

- Pull requests that do not follow these guidelines will be closed without review or comment.
- If you use AI to write your PR description your pr will be close without review or comment.
- If you are unsure about anything, feel free to ask for clarification.

## Description

Please provide a clear description of your changes.

---

## Type of Change

Please delete options that are not relevant.

- [ ] 🐛 Bug fix (non-breaking change which fixes an issue)
- [ ] ✨ New feature (non-breaking change which adds functionality)
- [ ] 💥 Breaking change (fix or feature with breaking changes)
- [ ] 📝 Documentation update
- [ ] 🎨 UI/UX improvement
- [ ] 🔒 Security enhancement
- [ ] ⚡ Performance improvement

## Areas Affected

Please check all that apply:

- [ ] Email Integration (Gmail, IMAP, etc.)
- [ ] User Interface/Experience
- [ ] Authentication/Authorization
- [ ] Data Storage/Management
- [ ] API Endpoints
- [ ] Documentation
- [ ] Testing Infrastructure
- [ ] Development Workflow
- [ ] Deployment/Infrastructure

## Testing Done

Describe the tests you've done:

- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] Manual testing performed
- [ ] Cross-browser testing (if UI changes)
- [ ] Mobile responsiveness verified (if UI changes)

## Security Considerations

For changes involving data or authentication:

- [ ] No sensitive data is exposed
- [ ] Authentication checks are in place
- [ ] Input validation is implemented
- [ ] Rate limiting is considered (if applicable)

## Checklist

- [ ] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document
- [ ] My code follows the project's style guidelines
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in complex areas
- [ ] I have updated the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix/feature works
- [ ] All tests pass locally
- [ ] Any dependent changes are merged and published

## Additional Notes

Add any other context about the pull request here.

## Screenshots/Recordings

Add screenshots or recordings here if applicable.

---

_By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
# Improve Thread Context Menu and Optimistic Updates

This PR refactors the thread context menu and optimistic state handling in the mail application to fix several issues:

1. Simplifies the `ThreadContextMenu` component by removing the redundant `emailId` prop and only using `threadId`
2. Adds support for optimistic marking as unread in the thread context menu
3. Improves the optimistic state handling in the mail list by:
   - Using a consistent thread ID across components
   - Properly displaying optimistic read/unread state
   - Fixing star toggling functionality
4. Enhances the optimistic actions system to invalidate specific queries rather than refetching everything

## Type of Change

- [x] 🐛 Bug fix (non-breaking change which fixes an issue)
- [x] ⚡ Performance improvement

## Areas Affected

- [x] User Interface/Experience
- [x] Data Storage/Management

## Testing Done

- [x] Manual testing performed

## Checklist

- [x] I have performed a self-review of my code
- [x] My changes generate no new warnings

_By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
# READ CAREFULLY THEN REMOVE

Remove bullet points that are not relevant.

PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.

- Pull requests that do not follow these guidelines will be closed without review or comment.
- If you use AI to write your PR description your pr will be close without review or comment.
- If you are unsure about anything, feel free to ask for clarification.

## Description

Please provide a clear description of your changes.

---

## Type of Change

Please delete options that are not relevant.

- [ ] 🐛 Bug fix (non-breaking change which fixes an issue)
- [ ] ✨ New feature (non-breaking change which adds functionality)
- [ ] 💥 Breaking change (fix or feature with breaking changes)
- [ ] 📝 Documentation update
- [ ] 🎨 UI/UX improvement
- [ ] 🔒 Security enhancement
- [ ] ⚡ Performance improvement

## Areas Affected

Please check all that apply:

- [ ] Email Integration (Gmail, IMAP, etc.)
- [ ] User Interface/Experience
- [ ] Authentication/Authorization
- [ ] Data Storage/Management
- [ ] API Endpoints
- [ ] Documentation
- [ ] Testing Infrastructure
- [ ] Development Workflow
- [ ] Deployment/Infrastructure

## Testing Done

Describe the tests you've done:

- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] Manual testing performed
- [ ] Cross-browser testing (if UI changes)
- [ ] Mobile responsiveness verified (if UI changes)

## Security Considerations

For changes involving data or authentication:

- [ ] No sensitive data is exposed
- [ ] Authentication checks are in place
- [ ] Input validation is implemented
- [ ] Rate limiting is considered (if applicable)

## Checklist

- [ ] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document
- [ ] My code follows the project's style guidelines
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in complex areas
- [ ] I have updated the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix/feature works
- [ ] All tests pass locally
- [ ] Any dependent changes are merged and published

## Additional Notes

Add any other context about the pull request here.

## Screenshots/Recordings

Add screenshots or recordings here if applicable.

---

_By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
## Description

Added support for optimistic toggling of "Important" flag in emails. This enhancement allows users to mark emails as important with immediate UI feedback while the action processes in the background. The implementation follows the same pattern as other optimistic actions like starring and marking as read.

Also fixed a few UI issues including removing extra whitespace in the AI chat component and removing debug mode from the Party provider.

## Type of Change

- [x] ✨ New feature (non-breaking change which adds functionality)
- [x] 🐛 Bug fix (non-breaking change which fixes an issue)
- [x] 🎨 UI/UX improvement

## Areas Affected

- [x] User Interface/Experience
- [x] Email Integration (Gmail, IMAP, etc.)

## Testing Done

- [x] Manual testing performed

## Checklist

- [x] I have performed a self-review of my code
- [x] My changes generate no new warnings
- [x] My code follows the project's style guidelines

## Additional Notes

The implementation includes:
1. Adding the Important button to the thread list UI
2. Supporting optimistic state updates for the Important flag
3. Fixing the ZeroAgent to use user ID instead of email for better identification
4. Removing debug mode from Party provider to reduce console noise
Copy link
Contributor

coderabbitai bot commented May 29, 2025

Walkthrough

This set of changes enhances the mail application's UI and optimistic state handling. It introduces improved optimistic updates for thread flags (starred, important, unread), refines context menu and thread list logic, updates font and animation styling, adds thread-level attachment display, and simplifies authentication in the server chat route.

Changes

Files/Groups Change Summary
apps/mail/components/mail/optimistic-thread-state.tsx, apps/mail/store/optimistic-updates.ts, apps/mail/hooks/use-optimistic-actions.ts Extended optimistic state/actions to support "important" status and unread marking, added new action type, improved cache invalidation logic.
apps/mail/components/context/thread-context.tsx, apps/mail/components/mail/mail-list.tsx Integrated optimistic thread state for starred/important flags, updated context menu props and logic, unified thread ID usage, refined UI.
apps/mail/components/mail/mail-display.tsx, apps/mail/components/mail/thread-display.tsx Added support for displaying and downloading thread-level attachments, passed attachment data to first message, updated imports.
apps/mail/components/home/HomeContent.tsx, apps/mail/app/root.tsx, apps/mail/tailwind.config.ts Improved typography (Google Fonts, Tailwind config), enhanced animation and layout, updated external badge, and button structure.
apps/mail/components/home/pixelated-bg.tsx Removed <title> elements from SVGs for cleaner markup.
apps/mail/components/create/ai-chat.tsx Minor CSS class formatting corrections.
apps/mail/components/party.tsx Removed debug property from party socket config.
apps/mail/components/ui/ai-sidebar.tsx Changed agent initialization to use user ID instead of email.
apps/mail/lib/hotkeys/mail-list-hotkeys.tsx Refactored to use optimistic update hooks for bulk actions, removed mutation hooks and direct mutation calls.
apps/server/src/routes/chat.ts Simplified authentication in ZeroAgent by removing token/session logic, now authorizes via agent name only.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI
    participant OptimisticState
    participant Server

    User->>UI: Toggle thread "starred", "important", or "unread"
    UI->>OptimisticState: Apply optimistic update
    OptimisticState-->>UI: Update UI immediately
    UI->>Server: Send mutation request
    Server-->>UI: Respond with success/failure
    UI->>OptimisticState: Reconcile optimistic state with server response
Loading
sequenceDiagram
    participant ThreadDisplay
    participant MailDisplay
    participant Attachments

    ThreadDisplay->>MailDisplay: Pass threadAttachments (all attachments)
    MailDisplay->>Attachments: Render thread-level attachments (if any)
    Attachments-->>User: Allow download/view actions
Loading

Possibly related PRs

  • Staging #1004: Both PRs extend optimistic state handling and update the ThreadContextMenu to support toggling the "important" thread flag.
  • context #595: This PR builds on the initial ThreadContextMenu component, modifying its props and integrating optimistic UI state.
  • thread action buttons #597: Both PRs enhance the ThreadDisplay component, particularly regarding thread action handlers and optimistic updates.

Suggested labels

design

Suggested reviewers

  • ahmetskilinc
  • MrgSub

Poem

In the land of code where emails flow,
Optimism grows with every thread we know.
Attachments now gather, all in one place,
Fonts and animations add style and grace.
With bouncy hops and a tidy UI,
This rabbit’s proud—give these changes a try!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 37de34b and 51826fc.

📒 Files selected for processing (16)
  • apps/mail/app/root.tsx (1 hunks)
  • apps/mail/components/context/thread-context.tsx (7 hunks)
  • apps/mail/components/create/ai-chat.tsx (1 hunks)
  • apps/mail/components/home/HomeContent.tsx (10 hunks)
  • apps/mail/components/home/pixelated-bg.tsx (0 hunks)
  • apps/mail/components/mail/mail-display.tsx (8 hunks)
  • apps/mail/components/mail/mail-list.tsx (11 hunks)
  • apps/mail/components/mail/optimistic-thread-state.tsx (2 hunks)
  • apps/mail/components/mail/thread-display.tsx (15 hunks)
  • apps/mail/components/party.tsx (0 hunks)
  • apps/mail/components/ui/ai-sidebar.tsx (1 hunks)
  • apps/mail/hooks/use-optimistic-actions.ts (9 hunks)
  • apps/mail/lib/hotkeys/mail-list-hotkeys.tsx (5 hunks)
  • apps/mail/store/optimistic-updates.ts (1 hunks)
  • apps/mail/tailwind.config.ts (1 hunks)
  • apps/server/src/routes/chat.ts (2 hunks)
💤 Files with no reviewable changes (2)
  • apps/mail/components/party.tsx
  • apps/mail/components/home/pixelated-bg.tsx
🧰 Additional context used
🧬 Code Graph Analysis (7)
apps/mail/components/ui/ai-sidebar.tsx (1)
apps/server/src/db/schema.ts (1)
  • session (29-40)
apps/mail/lib/hotkeys/mail-list-hotkeys.tsx (1)
apps/mail/hooks/use-optimistic-actions.ts (1)
  • useOptimisticActions (28-412)
apps/mail/components/context/thread-context.tsx (2)
apps/mail/components/mail/optimistic-thread-state.tsx (1)
  • useOptimisticThreadState (7-76)
apps/mail/hooks/use-optimistic-actions.ts (1)
  • useOptimisticActions (28-412)
apps/mail/components/create/ai-chat.tsx (1)
apps/mail/lib/utils.ts (1)
  • cn (51-51)
apps/mail/components/home/HomeContent.tsx (1)
apps/mail/components/icons/icons.tsx (1)
  • ArrowRight (1505-1522)
apps/server/src/routes/chat.ts (3)
apps/server/src/db/index.ts (1)
  • createDb (5-9)
apps/server/src/db/schema.ts (1)
  • connection (87-106)
apps/server/src/lib/server-utils.ts (1)
  • connectionToDriver (37-50)
apps/mail/components/mail/mail-display.tsx (1)
apps/mail/lib/utils.ts (2)
  • getFileIcon (156-163)
  • formatFileSize (148-154)
🪛 Biome (1.9.4)
apps/mail/components/mail/thread-display.tsx

[error] 179-179: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

🔇 Additional comments (33)
apps/mail/components/ui/ai-sidebar.tsx (1)

370-374: LGTM! Client-server alignment improved.

The change from session?.user.email to session?.user.id correctly aligns the client with the server-side authentication changes in apps/server/src/routes/chat.ts. Using the user ID as the agent name ensures consistent identification across the system.

apps/mail/components/create/ai-chat.tsx (1)

390-397: Clean className strings look good.

The redundant spaces have been removed from the wrapper <div> and <EditorContent> component’s className. This is a solid, no-op cleanup with no further action required.

apps/mail/components/home/HomeContent.tsx (5)

309-309: LGTM: Improved spacing for hero section

The increased margin-top provides better visual breathing room on medium+ screens while maintaining the mobile layout.


316-329: LGTM: Improved typography consistency

The restructuring of the Y Combinator badge with the font-sora class enhances typography consistency across the hero section. The separation of text and image elements maintains clean semantic structure.


343-343: LGTM: Consistent typography implementation

The systematic application of font-sora for headings and font-mona for body text establishes a clear and consistent typography hierarchy throughout the hero section.

Also applies to: 353-353, 505-505


410-416: Excellent: Performance-optimized animations

The transition from animate to whileInView with viewport={{ once: true }} is a significant performance improvement. This approach:

  • Prevents animations from running on initial page load when elements are off-screen
  • Triggers animations only when elements enter the viewport
  • Ensures animations run only once with the once: true option
  • Maintains smooth user experience while reducing unnecessary computations

This is a best practice for scroll-triggered animations.

Also applies to: 477-477, 479-485, 514-517, 529-531


400-406: LGTM: Updated badge and restored decorative elements

The Product Hunt badge update includes improved responsive sizing (h-[44px] w-[200px] md:h-[54px] md:w-[250px]) and the uncommenting of decorative border elements enhances the visual design.

The new Product Hunt widget URL appears to be dynamically generated with a timestamp parameter, which should work correctly for displaying the current badge.

Also applies to: 459-461

apps/mail/store/optimistic-updates.ts (1)

8-9: LGTM! Clean addition of IMPORTANT action type.

The new IMPORTANT action type follows the established pattern and maintains type safety. The structure with threadIds array and important boolean is consistent with other optimistic actions.

apps/mail/components/mail/optimistic-thread-state.tsx (3)

29-29: LGTM! Consistent state initialization.

The addition of isImportant: false follows the same pattern as other optimistic state flags.


33-33: LGTM! Proper optimistic state tracking.

The optimisticImportant property follows the established pattern of using boolean | null to track optimistic state values.


62-65: LGTM! Correct IMPORTANT action handling.

The case handling for the IMPORTANT action type properly sets both isImportant and optimisticImportant states, maintaining consistency with other action type handlers.

apps/mail/lib/hotkeys/mail-list-hotkeys.tsx (6)

1-6: Clean import refactor for optimistic actions.

Good replacement of direct mutations with the optimistic actions hook. The import of useOptimisticActions and useParams for folder extraction makes sense.


31-35: LGTM! Proper folder extraction and optimistic actions setup.

The folder extraction from URL params and destructuring of optimistic action functions is well-implemented.


71-82: Excellent refactor to optimistic updates.

The replacement of direct mutation calls with optimisticMarkAsRead improves UX with immediate feedback and undo functionality. The dependency array is correctly updated.


86-97: LGTM! Consistent optimistic unread implementation.

The optimisticMarkAsUnread usage follows the same pattern as the read functionality, maintaining consistency.


101-112: LGTM! Proper archive operation with optimistic updates.

The optimisticMoveThreadsTo call correctly uses the extracted folder parameter and 'archive' destination. Good consistency with the refactor pattern.


4-4:

✅ Verification successful

Verify if queryClient import is still needed.

The useQueryClient import may no longer be necessary since the code has been refactored to use optimistic actions instead of direct mutations.


🏁 Script executed:

#!/bin/bash
# Check if queryClient is used elsewhere in this file
rg -A 5 -B 5 "queryClient" apps/mail/lib/hotkeys/mail-list-hotkeys.tsx

Length of output: 932


No change needed: useQueryClient is still in use
The queryClient instance is used in the invalidateCount function to call queryClient.invalidateQueries(...). Keep the import at the top of apps/mail/lib/hotkeys/mail-list-hotkeys.tsx.

apps/mail/components/context/thread-context.tsx (7)

25-25: LGTM! Good integration of optimistic state.

Adding the useOptimisticThreadState import enables the component to leverage optimistic UI updates.


56-56: LGTM! Cleaner prop interface.

Making threadId mandatory removes ambiguity and ensures the component always has a valid thread identifier.


131-131: LGTM! Proper optimistic state hook usage.

The integration of useOptimisticThreadState enables real-time UI updates during optimistic operations.


138-144: LGTM! Correct optimistic starred state handling.

The logic properly prioritizes optimistic state over thread data, ensuring immediate UI feedback during star operations.


147-153: LGTM! Consistent optimistic important state implementation.

The important state handling follows the same pattern as the starred state, maintaining consistency across optimistic operations.


451-451: Minor styling update noted.

The background color change from a CSS class to a specific hex value #1A1A1A for dark mode is noted.


457-457: Extra separator line noted.

An additional separator has been added. This appears to be a deliberate UI spacing adjustment.

apps/mail/components/mail/mail-list.tsx (2)

67-68: LGTM! Good unification of thread identification.

The memoized idToUse value properly handles the case where a thread might be identified by either the thread ID or message ID, providing a consistent identifier throughout the component.


481-491:

❓ Verification inconclusive

Verify null handling for thread context menu.

The conditional rendering only shows the ThreadContextMenu when idToUse is truthy. While this prevents runtime errors, ensure this doesn't create UI inconsistencies where threads without proper IDs are hidden.


🏁 Script executed:

#!/bin/bash
# Description: Check if there are any cases where threads might not have valid IDs

# Search for thread creation or places where thread IDs might be missing
rg -A 5 "threadId.*null|id.*undefined" --type ts --type tsx

# Look for error handling around thread IDs
ast-grep --pattern 'if (!$ID || !$THREAD) { $$$ }'

Length of output: 4279


Confirm thread context menu null handling

I didn’t find any code paths where idToUse could be null or undefined. However, please double-check that whatever logic assigns idToUse (the thread’s real or optimistic ID) always produces a valid, truthy value—otherwise valid threads would render without a context menu.

apps/mail/hooks/use-optimistic-actions.ts (7)

17-26: LGTM! Good separation of action types.

Adding the 'IMPORTANT' action type provides better semantic clarity and allows for type-specific behavior tracking.


46-48: Good addition for tracking pending actions by type.

Using a Map of Sets provides efficient O(1) lookups and deletions, which is ideal for tracking pending actions.


54-69: Excellent optimization for targeted cache invalidation.

The optional folders parameter enables efficient cache invalidation by targeting only affected folders instead of invalidating all data. Good use of optional chaining and spread operator.


71-79: Smart refresh strategy to avoid race conditions.

Only refreshing when no pending actions of a type remain prevents unnecessary cache invalidations and potential race conditions. Good use of useCallback with proper dependencies.


82-155: Well-implemented action tracking with proper cleanup.

The implementation correctly:

  • Initializes Sets for new action types
  • Maintains consistency by cleaning up in all code paths (success, error, undo)
  • Integrates smoothly with the conditional refresh strategy
  • Prevents memory leaks through proper cleanup

297-298: Good addition for targeted folder invalidation.

Passing both source and destination folders ensures that only the affected folder queries are invalidated, improving performance.


366-402: Consistent implementation of the IMPORTANT action type.

The migration from 'LABEL' to 'IMPORTANT' improves semantic clarity and aligns with the new action type system.

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Kashyap1ankit Kashyap1ankit changed the base branch from main to staging May 29, 2025 06:31
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 4

🔭 Outside diff range comments (1)
apps/mail/components/mail/mail-display.tsx (1)

67-98: ⚠️ Potential issue

Remove duplicate utility functions.

The formatFileSize and getFileIcon functions defined here duplicate the utilities available in apps/mail/lib/utils.ts (lines 148-163). This violates the DRY principle.

Replace the local implementations with imports:

-// Add formatFileSize utility function
-const formatFileSize = (size: number) => {
-  const sizeInMB = (size / (1024 * 1024)).toFixed(2);
-  return sizeInMB === '0.00' ? '' : `${sizeInMB} MB`;
-};
-
-// Add getFileIcon utility function
-const getFileIcon = (filename: string) => {
-  const extension = filename.split('.').pop()?.toLowerCase();
-
-  switch (extension) {
-    case 'pdf':
-      return <PDF className="fill-[#F43F5E]" />;
-    case 'jpg':
-      return <ImageFile />;
-    case 'jpeg':
-      return <ImageFile />;
-    case 'png':
-      return <ImageFile />;
-    case 'gif':
-      return <ImageFile />;
-    case 'docx':
-      return <Docx />;
-    case 'fig':
-      return <Figma />;
-    case 'webp':
-      return <ImageFile />;
-    default:
-      return <FileText className="h-4 w-4 text-[#8B5CF6]" />;
-  }
-};
+import { formatFileSize, getFileIcon as getFileIconFromUtils } from '@/lib/utils';

Note that the imported getFileIcon uses MIME types while the local version uses filename extensions. You'll need to adapt the usage in the ThreadAttachments component to use MIME types or update the utility function to support both approaches.

🧹 Nitpick comments (6)
apps/mail/tailwind.config.ts (1)

93-94: Remove leading whitespace in font family declaration.

The mona font entry currently includes a leading space before sans-serif, which may cause inconsistency with other font definitions. It should be:

- mona: ['Mona Sans', ' sans-serif'],
+ mona: ['Mona Sans', 'sans-serif'],
apps/mail/app/root.tsx (1)

70-74: Prefer <link> over @import for Google Fonts and add preconnect.

Inlining @import inside a <style> block can block rendering. Switching to <link rel="stylesheet"> with preconnect hints improves performance:

-        <style>
-          @import url('https://fonts.googleapis.com/css2?family=Sora:wght@600&display=swap');
-          @import url('https://fonts.googleapis.com/css2?family=Mona+Sans:ital,wght@0,500;1,500&display=swap');
-        </style>
+        <link rel="preconnect" href="https://fonts.googleapis.com" />
+        <link rel="preconnect" href="https://fonts.gstatic.com" crossOrigin="anonymous" />
+        <link
+          href="https://fonts.googleapis.com/css2?family=Sora:wght@600&family=Mona+Sans:ital,wght@0,500;1,500&display=swap"
+          rel="stylesheet"
+        />
apps/mail/components/home/HomeContent.tsx (1)

358-395: LGTM: Improved button layout with minor optimization opportunity

The restructuring successfully separates the primary call-to-action from secondary information, creating better visual hierarchy. The ArrowRight icon addition enhances the "Get Started" button's prominence.

Consider extracting the shared animation properties to reduce code duplication:

+        const buttonAnimation = {
+          initial: { opacity: 0, y: 20 },
+          animate: { opacity: 1, y: 0 },
+          transition: { duration: 0.5, delay: 0.6 }
+        };

         <div className="mt-12 flex flex-row gap-6">
-          <motion.div
-            initial={{ opacity: 0, y: 20 }}
-            animate={{ opacity: 1, y: 0 }}
-            transition={{ duration: 0.5, delay: 0.6 }}
-          >
+          <motion.div {...buttonAnimation}>
             {/* No credit card required button */}
           </motion.div>

-          <motion.div
-            initial={{ opacity: 0, y: 20 }}
-            animate={{ opacity: 1, y: 0 }}
-            transition={{ duration: 0.5, delay: 0.6 }}
-          >
+          <motion.div {...buttonAnimation}>
             {/* Get Started button */}
           </motion.div>
         </div>
apps/mail/components/context/thread-context.tsx (1)

223-229: Simplify the read/unread toggle logic.

The current logic can be simplified and the error handling improved.

-    if (newReadState) {
-      optimisticMarkAsRead(targets);
-    } else if (!newReadState) {
-      optimisticMarkAsUnread(targets);
-    } else {
-      toast.error('Failed to mark as read');
-    }
+    if (newReadState) {
+      optimisticMarkAsRead(targets);
+    } else {
+      optimisticMarkAsUnread(targets);
+    }

The else condition !newReadState will never be reached since newReadState is boolean, and the error case is unreachable.

apps/mail/components/mail/mail-display.tsx (1)

260-312: Consider reusing existing attachment UI patterns.

The ThreadAttachments component creates a new UI pattern for attachments, but there's already an attachment rendering pattern in lines 1401-1428. Consider extracting a shared component to maintain UI consistency.

Additionally, update the component to use the imported utility functions:

-<span className="text-muted-foreground">{getFileIcon(attachment.filename)}</span>
+<span className="text-muted-foreground">{getFileIconFromUtils(attachment.mimeType)}</span>
-<span className="text-muted-foreground">{formatFileSize(attachment.size)}</span>
+<span className="text-muted-foreground">{formatFileSize(attachment.size)}</span>
apps/mail/hooks/use-optimistic-actions.ts (1)

188-217: Clean implementation of mark as unread functionality.

The function correctly mirrors optimisticMarkAsRead with appropriate changes. Consider extracting the common bulk selection logic into a shared helper to reduce duplication between these similar functions.

🛑 Comments failed to post (4)
apps/server/src/routes/chat.ts (3)

42-42: 🛠️ Refactor suggestion

Improve error logging for security monitoring.

The unauthorized access logging should include more context for security monitoring and debugging purposes.

- console.log('Unauthorized no driver or connectionId', connectionId, this.driver);
+ console.error('Unauthorized access attempt:', {
+   connectionId: connectionId || 'missing',
+   hasDriver: !!this.driver,
+   agentName: this.name || 'missing',
+   timestamp: new Date().toISOString(),
+   userAgent: this.connectionContext?.headers?.['user-agent'] || 'unknown'
+ });
📝 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.

          console.error('Unauthorized access attempt:', {
            connectionId: connectionId || 'missing',
            hasDriver: !!this.driver,
            agentName: this.name || 'missing',
            timestamp: new Date().toISOString(),
            userAgent: this.connectionContext?.headers?.['user-agent'] || 'unknown'
          });
🤖 Prompt for AI Agents
In apps/server/src/routes/chat.ts at line 42, the console.log for unauthorized
access lacks sufficient context for security monitoring. Enhance the log message
by including additional relevant details such as timestamps, user identifiers,
request metadata, or any other contextual information available in the scope to
improve traceability and debugging.

83-85: ⚠️ Potential issue

Missing authentication in connection handler.

The onConnect method should validate the client's authentication before setting up the agent. Currently, it directly calls setupAuth() without any preliminary authentication checks.

Add authentication validation:

 async onConnect() {
+  // Extract and validate authentication from connection context
+  const authHeader = this.connectionContext?.headers?.authorization;
+  if (!authHeader) {
+    throw new Error('Unauthorized - missing authentication');
+  }
+  
+  // Validate the session/token
+  const session = await this.validateSession(authHeader);
+  if (!session) {
+    throw new Error('Unauthorized - invalid session');
+  }
+  
+  // Set the authenticated user ID
+  this.name = session.user.id;
+  
   await this.setupAuth();
 }
📝 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.

  async onConnect() {
    // Extract and validate authentication from connection context
    const authHeader = this.connectionContext?.headers?.authorization;
    if (!authHeader) {
      throw new Error('Unauthorized - missing authentication');
    }

    // Validate the session/token
    const session = await this.validateSession(authHeader);
    if (!session) {
      throw new Error('Unauthorized - invalid session');
    }

    // Set the authenticated user ID
    this.name = session.user.id;

    await this.setupAuth();
  }
🤖 Prompt for AI Agents
In apps/server/src/routes/chat.ts around lines 83 to 85, the onConnect method
calls setupAuth() directly without validating client authentication first.
Modify onConnect to include a preliminary authentication check before calling
setupAuth(), ensuring only authenticated clients proceed. Implement the
necessary logic to verify the client's credentials or session and handle
unauthorized access appropriately.

70-81: ⚠️ Potential issue

Critical security issue: Missing authentication validation.

The simplified setupAuth method now trusts the client-provided name (user ID) without any session or token validation. This creates a significant security vulnerability where an attacker could potentially access another user's data by providing their user ID.

Consider implementing one of these solutions:

Option 1: Restore session validation

 private async setupAuth() {
+  // Validate session/token before proceeding
+  const session = await this.validateSession();
+  if (!session) {
+    throw new Error('Unauthorized - invalid session');
+  }
+  
   if (this.name) {
     const db = createDb(env.HYPERDRIVE.connectionString);
     const _connection = await db.query.connection.findFirst({
       where: eq(connection.userId, this.name),
     });
+    // Verify the session user matches the requested connection
+    if (_connection && _connection.userId !== session.user.id) {
+      throw new Error('Unauthorized - user mismatch');
+    }
     if (_connection) {
       await this.ctx.storage.put('connectionId', _connection.id);
       this.driver = connectionToDriver(_connection);
     }
   }
 }

Option 2: Extract user ID from validated session

 private async setupAuth() {
+  const session = await this.validateSession();
+  if (!session) {
+    throw new Error('Unauthorized - invalid session');
+  }
+  
+  const userId = session.user.id;
   const db = createDb(env.HYPERDRIVE.connectionString);
   const _connection = await db.query.connection.findFirst({
-    where: eq(connection.userId, this.name),
+    where: eq(connection.userId, userId),
   });
   if (_connection) {
     await this.ctx.storage.put('connectionId', _connection.id);
     this.driver = connectionToDriver(_connection);
   }
 }
📝 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.

  private async setupAuth() {
+    // 1) Validate session/token before any lookup
+    const session = await this.validateSession();
+    if (!session) {
+      throw new Error('Unauthorized - invalid session');
+    }
+
+    // 2) Derive userId from the trusted session
+    const userId = session.user.id;

    const db = createDb(env.HYPERDRIVE.connectionString);
    const _connection = await db.query.connection.findFirst({
-     where: eq(connection.userId, this.name),
+     where: eq(connection.userId, userId),
    });

    if (_connection) {
      await this.ctx.storage.put('connectionId', _connection.id);
      this.driver = connectionToDriver(_connection);
    }
  }
🤖 Prompt for AI Agents
In apps/server/src/routes/chat.ts around lines 70 to 81, the setupAuth method
currently trusts the client-provided user ID without validating the session or
token, creating a security risk. To fix this, implement proper authentication by
either restoring session validation to verify the user's identity before
querying the database or extract the user ID from a validated session or token
instead of using the client-provided name directly. This ensures only
authenticated users can access their data.
apps/mail/components/mail/thread-display.tsx (1)

174-183: ⚠️ Potential issue

Fix O(n²) performance issue in attachment aggregation.

The spread operator in the reduce function creates a new array on each iteration, resulting in O(n²) time complexity. This could impact performance with threads containing many messages with attachments.

Use array mutation methods instead:

-const allThreadAttachments = useMemo(() => {
-  if (!emailData?.messages) return [];
-  return emailData.messages.reduce<Attachment[]>((acc, message) => {
-    if (message.attachments && message.attachments.length > 0) {
-      return [...acc, ...message.attachments];
-    }
-    return acc;
-  }, []);
-}, [emailData?.messages]);
+const allThreadAttachments = useMemo(() => {
+  if (!emailData?.messages) return [];
+  const attachments: Attachment[] = [];
+  emailData.messages.forEach((message) => {
+    if (message.attachments && message.attachments.length > 0) {
+      attachments.push(...message.attachments);
+    }
+  });
+  return attachments;
+}, [emailData?.messages]);

Or use flatMap for a more functional approach:

-const allThreadAttachments = useMemo(() => {
-  if (!emailData?.messages) return [];
-  return emailData.messages.reduce<Attachment[]>((acc, message) => {
-    if (message.attachments && message.attachments.length > 0) {
-      return [...acc, ...message.attachments];
-    }
-    return acc;
-  }, []);
-}, [emailData?.messages]);
+const allThreadAttachments = useMemo(() => {
+  if (!emailData?.messages) return [];
+  return emailData.messages.flatMap(message => message.attachments || []);
+}, [emailData?.messages]);
🧰 Tools
🪛 Biome (1.9.4)

[error] 179-179: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

🤖 Prompt for AI Agents
In apps/mail/components/mail/thread-display.tsx around lines 174 to 183, the
current useMemo hook uses the spread operator inside a reduce function to
aggregate attachments, causing O(n²) complexity due to array copying on each
iteration. To fix this, replace the reduce with a method that mutates the
accumulator array directly by pushing attachments, or alternatively use the
flatMap method on emailData.messages to collect all attachments in a single
pass, improving performance to O(n).

@BlankParticle BlankParticle force-pushed the staging branch 5 times, most recently from e47318e to a6ec63a Compare May 30, 2025 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants