Skip to content

Comments

categories#1689

Merged
MrgSub merged 1 commit intostagingfrom
07-08-categories
Jul 8, 2025
Merged

categories#1689
MrgSub merged 1 commit intostagingfrom
07-08-categories

Conversation

@MrgSub
Copy link
Collaborator

@MrgSub MrgSub commented Jul 8, 2025

Refactor Labels API Usage and Optimize Component Performance

Description

This PR refactors how labels are accessed throughout the application by restructuring the useLabels hook to return userLabels and systemLabels separately. It also adds performance optimizations by memoizing component calculations and action handlers in the thread context menu. Additionally, it introduces a new category dropdown component for better label filtering and temporarily disables the categories settings page.

Type of Change

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

Areas Affected

  • User Interface/Experience
  • Data Storage/Management

Testing Done

  • Manual testing performed

Checklist

  • I have performed a self-review of my code
  • My changes generate no new warnings
  • All tests pass locally

Additional Notes

The main changes include:

  1. Restructured useLabels hook to separate user and system labels
  2. Updated all components to use the new label structure
  3. Added memoization to thread context menu actions and calculations
  4. Implemented a new CategoryDropdown component for label filtering
  5. Commented out the categories settings page route
  6. Fixed spacing and alignment issues in the mail layout

Summary by CodeRabbit

  • New Features

    • Introduced a dropdown-based label filter for Google inbox folders, enhancing label selection and accessibility in the mail layout header.
  • Refactor

    • Improved memoization and state management in components related to label handling and thread context menus.
    • Updated label and stats data sourcing in sidebar components for better consistency and performance.
    • Refined the structure and naming of label-related hooks and their returned data for clarity.
  • Chores

    • Disabled the "categories" route and navigation item in settings without removing their code.
    • Adjusted query caching durations for threads and stats to optimize data freshness.
  • Style

    • Made UI and layout adjustments for buttons, dropdowns, and container elements for a more polished appearance.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 8, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This change refactors how label data is handled throughout the mail application. The useLabels hook now returns both userLabels and systemLabels, and components are updated to destructure and use these properties directly. Related UI components and hooks are adjusted for this new structure, with some navigation and category features commented out.

Changes

File(s) Change Summary
apps/mail/hooks/use-labels.ts useLabels now returns { userLabels, systemLabels, ... } instead of raw data; internal destructuring updated.
apps/mail/components/context/command-palette-context.ts
apps/mail/app/(routes)/mail/[folder]/page.tsx
apps/mail/app/(routes)/settings/labels/page.tsx
apps/mail/components/context/thread-context.tsx
apps/mail/components/ui/nav-main.tsx
Updated destructuring of useLabels to use userLabels directly; variable names and prop passing adjusted.
apps/mail/components/ui/sidebar-labels.tsx Refactored to fetch activeAccount and stats internally; prop types updated, interface simplified.
apps/mail/components/mail/mail.tsx Added CategoryDropdown for Google inbox; improved label query caching and refetching; UI tweaks and refactoring.
apps/mail/hooks/use-stats.ts Removed explicit staleTime from useStats query options.
apps/mail/hooks/use-threads.ts Adjusted staleTime for threads queries; fixed/minor comment mismatches.
apps/mail/config/navigation.ts
apps/mail/app/routes.ts
Commented out "categories" navigation and route under settings.
apps/server/src/routes/chat.ts ZeroAgent.getUserLabels now returns all labels (no type === 'user' filtering).

Sequence Diagram(s)

sequenceDiagram
  participant UI_Component
  participant useLabels_Hook
  participant Data_Source

  UI_Component->>useLabels_Hook: Call useLabels()
  useLabels_Hook->>Data_Source: Fetch all labels
  Data_Source-->>useLabels_Hook: Return labels[]
  useLabels_Hook-->>UI_Component: Return { userLabels, systemLabels, ... }
  UI_Component->>UI_Component: Use userLabels/systemLabels as needed
Loading

Possibly related PRs

  • categories #1689: Both PRs modify the destructuring of the useLabels hook's return value in the same files and components, changing from extracting data as userLabels to directly extracting userLabels.
  • Optimistic Fixes #1231: Both PRs modify the destructuring of the useLabels hook and refactor label-related state handling in the thread-context.tsx component.
  • Replace callDriver with direct mail manager methods #1466: Both PRs modify the getUserLabels method in ZeroAgent to change how labels are filtered or returned.

Suggested labels

design

Poem

A bunny hopped through fields of code,
Sorting labels, lightening the load.
With user and system now set apart,
The inbox feels a work of art.
Dropdowns bloom where categories slept—
In the warren of mail, order is kept!
🐇📬✨


📜 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 2f12b81 and bcf1b4b.

📒 Files selected for processing (13)
  • apps/mail/app/(routes)/mail/[folder]/page.tsx (1 hunks)
  • apps/mail/app/(routes)/settings/labels/page.tsx (5 hunks)
  • apps/mail/app/routes.ts (1 hunks)
  • apps/mail/components/context/command-palette-context.tsx (1 hunks)
  • apps/mail/components/context/thread-context.tsx (6 hunks)
  • apps/mail/components/mail/mail.tsx (12 hunks)
  • apps/mail/components/ui/nav-main.tsx (2 hunks)
  • apps/mail/components/ui/sidebar-labels.tsx (1 hunks)
  • apps/mail/config/navigation.ts (1 hunks)
  • apps/mail/hooks/use-labels.ts (1 hunks)
  • apps/mail/hooks/use-stats.ts (0 hunks)
  • apps/mail/hooks/use-threads.ts (2 hunks)
  • apps/server/src/routes/chat.ts (1 hunks)
✨ 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.

Copy link
Collaborator Author

MrgSub commented Jul 8, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@MrgSub MrgSub marked this pull request as ready for review July 8, 2025 22:42
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.

Actionable comments posted: 4

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

1017-1221: Remove dead code from CategorySelect component.

The CategorySelect component has large sections of commented-out code (lines 1037-1087, 1132-1137, 1183-1199) making it essentially non-functional. Since CategoryDropdown appears to replace this functionality, consider:

  1. Remove the component entirely if it's no longer needed
  2. If still needed, remove the commented code and properly refactor
  3. The component still returns UI but with disabled functionality

Either remove the entire component or clean up the dead code. If keeping the component, remove lines 1033-1087, 1132-1137, and 1183-1199.

🧹 Nitpick comments (2)
apps/mail/hooks/use-threads.ts (1)

76-76: Fix comment mismatch.

The comment says "1 minute" but the actual value 1000 * 60 * 60 equals 1 hour.

-        staleTime: 1000 * 60 * 60, // 1 minute
+        staleTime: 1000 * 60 * 60, // 1 hour
apps/mail/hooks/use-labels.ts (1)

12-22: Consider optimizing the label categorization with a single pass.

The current implementation filters the array twice. Consider using a single reduce operation for better performance with large label sets:

  const { userLabels, systemLabels } = useMemo(() => {
    if (!labelQuery.data) return { userLabels: [], systemLabels: [] };
-    return {
-      userLabels: labelQuery.data.filter((label) => label.type === 'user'),
-      systemLabels: labelQuery.data.filter((label) => label.type === 'system'),
-    };
+    return labelQuery.data.reduce(
+      (acc, label) => {
+        if (label.type === 'user') {
+          acc.userLabels.push(label);
+        } else if (label.type === 'system') {
+          acc.systemLabels.push(label);
+        }
+        return acc;
+      },
+      { userLabels: [], systemLabels: [] }
+    );
  }, [labelQuery.data]);
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 2b1e400 and 2f12b81.

📒 Files selected for processing (13)
  • apps/mail/app/(routes)/mail/[folder]/page.tsx (1 hunks)
  • apps/mail/app/(routes)/settings/labels/page.tsx (5 hunks)
  • apps/mail/app/routes.ts (1 hunks)
  • apps/mail/components/context/command-palette-context.tsx (1 hunks)
  • apps/mail/components/context/thread-context.tsx (6 hunks)
  • apps/mail/components/mail/mail.tsx (12 hunks)
  • apps/mail/components/ui/nav-main.tsx (2 hunks)
  • apps/mail/components/ui/sidebar-labels.tsx (1 hunks)
  • apps/mail/config/navigation.ts (1 hunks)
  • apps/mail/hooks/use-labels.ts (1 hunks)
  • apps/mail/hooks/use-stats.ts (0 hunks)
  • apps/mail/hooks/use-threads.ts (2 hunks)
  • apps/server/src/routes/chat.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • apps/mail/hooks/use-stats.ts
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/app/routes.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/config/navigation.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/app/(routes)/mail/[folder]/page.tsx (2)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:331-331
Timestamp: 2025-06-28T03:56:09.376Z
Learning: In apps/server/src/trpc/routes/mail.ts, the user indicated they are not using ISO format for the scheduleAt parameter, despite the frontend code showing toISOString() usage in the ScheduleSendPicker component.
apps/mail/components/context/command-palette-context.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/ui/nav-main.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/hooks/use-threads.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:331-331
Timestamp: 2025-06-28T03:56:09.376Z
Learning: In apps/server/src/trpc/routes/mail.ts, the user indicated they are not using ISO format for the scheduleAt parameter, despite the frontend code showing toISOString() usage in the ScheduleSendPicker component.
apps/mail/hooks/use-labels.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/app/(routes)/settings/labels/page.tsx (3)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:331-331
Timestamp: 2025-06-28T03:56:09.376Z
Learning: In apps/server/src/trpc/routes/mail.ts, the user indicated they are not using ISO format for the scheduleAt parameter, despite the frontend code showing toISOString() usage in the ScheduleSendPicker component.
Learnt from: danteissaias
PR: Mail-0/Zero#902
File: apps/mail/components/connection/add.tsx:77-77
Timestamp: 2025-05-07T16:55:46.513Z
Learning: For the "Upgrade" link in AddConnectionDialog, using a proper <button> element instead of a <span> with onClick is recognized as an accessibility improvement but was deferred as out of scope in PR #902 (CSS variables PR).
apps/mail/components/ui/sidebar-labels.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/context/thread-context.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/mail/mail.tsx (3)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: danteissaias
PR: Mail-0/Zero#902
File: apps/mail/components/connection/add.tsx:77-77
Timestamp: 2025-05-07T16:55:46.513Z
Learning: For the "Upgrade" link in AddConnectionDialog, using a proper <button> element instead of a <span> with onClick is recognized as an accessibility improvement but was deferred as out of scope in PR #902 (CSS variables PR).
Learnt from: Fahad-Dezloper
PR: Mail-0/Zero#1440
File: apps/mail/components/create/ai-chat.tsx:101-113
Timestamp: 2025-06-22T19:23:10.599Z
Learning: On small and medium screens (mobile devices), buttons don't naturally lose focus after being clicked like they do on desktop browsers. This requires manual intervention using setTimeout with additional blur() calls and DOM manipulation to properly clear focus and active states in button click handlers for responsive design.
🧬 Code Graph Analysis (6)
apps/mail/app/(routes)/mail/[folder]/page.tsx (1)
apps/mail/hooks/use-labels.ts (1)
  • useLabels (5-22)
apps/mail/components/context/command-palette-context.tsx (1)
apps/mail/hooks/use-labels.ts (1)
  • useLabels (5-22)
apps/mail/components/ui/nav-main.tsx (1)
apps/mail/hooks/use-labels.ts (1)
  • useLabels (5-22)
apps/mail/app/(routes)/settings/labels/page.tsx (1)
apps/mail/hooks/use-labels.ts (1)
  • useLabels (5-22)
apps/mail/components/ui/sidebar-labels.tsx (2)
apps/mail/hooks/use-stats.ts (1)
  • useStats (5-16)
apps/mail/hooks/use-connections.ts (1)
  • useActiveConnection (10-22)
apps/mail/components/mail/mail.tsx (5)
apps/mail/components/ui/button.tsx (1)
  • Button (71-71)
apps/mail/lib/utils.ts (1)
  • cn (51-51)
apps/mail/hooks/use-labels.ts (1)
  • useLabels (5-22)
apps/mail/components/icons/icons.tsx (3)
  • Mail (791-803)
  • ChevronDown (1200-1216)
  • Check (1632-1647)
apps/mail/components/mail/use-mail.ts (1)
  • useMail (21-23)
🔇 Additional comments (15)
apps/mail/app/(routes)/mail/[folder]/page.tsx (1)

29-29: LGTM: Consistent with useLabels hook refactor.

The change correctly updates the destructuring to use userLabels directly instead of aliasing data, which aligns with the updated useLabels hook that now returns separate userLabels and systemLabels arrays.

apps/mail/components/context/command-palette-context.tsx (1)

200-200: LGTM: Correctly updated for new hook API.

The destructuring change from { data: userLabels = [] } to { userLabels = [] } is consistent with the updated useLabels hook and maintains the appropriate default value.

apps/mail/app/(routes)/settings/labels/page.tsx (1)

42-42: LGTM: Correctly updated for separated label types.

The change from { data: labels, ... } to { userLabels: labels, ... } aligns with the updated useLabels hook that now returns separate userLabels and systemLabels arrays instead of a single data array.

apps/server/src/routes/chat.ts (1)

671-676: Good architectural change to move label filtering to the client.

This change appropriately delegates label categorization to the frontend, providing more flexibility for client-side filtering and reducing server-side logic. The client can now handle both user and system labels as needed.

apps/mail/components/ui/nav-main.tsx (2)

75-75: Correct usage of the updated useLabels hook.

The destructuring now directly extracts userLabels instead of aliasing data, which aligns with the hook's new return structure and improves code clarity.


262-262: Clean simplification of SidebarLabels props.

Good use of the nullish coalescing operator to provide a default empty array. The removal of activeAccount and stats props aligns with the component's refactoring to fetch its own data internally.

apps/mail/hooks/use-labels.ts (1)

25-25: Correct adaptation to the new useLabels structure.

The destructuring properly extracts userLabels and maintains the existing labels variable name for backward compatibility.

apps/mail/components/ui/sidebar-labels.tsx (1)

2-4: Excellent refactoring to use hooks internally.

Moving data fetching into the component using hooks reduces prop drilling and makes the component more self-contained. This follows React best practices for component design.

Also applies to: 12-14

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

61-61: Correct adaptation to useLabels hook structure.

The destructuring properly extracts userLabels and aliases it to labels for backward compatibility with the existing component logic.


146-153: Clean consolidation of hook destructuring.

Consolidating all optimistic actions into a single destructuring statement improves code readability and reduces redundancy.


155-177: Excellent optimization of state derivation.

Consolidating related state computations into a single useMemo hook reduces the number of memoization calls and improves performance. The logic is correctly preserved.


267-292: Great performance optimization with action memoization.

Memoizing the action arrays prevents unnecessary re-creation on each render. The conversion of getActions from a function to a memoized array is appropriate since it doesn't require parameters. All dependencies are correctly specified.

Also applies to: 311-411, 413-446

apps/mail/components/mail/mail.tsx (3)

25-30: LGTM! Import changes align with new dropdown functionality.

The addition of dropdown menu components and the updated useLabels hook import properly support the new CategoryDropdown implementation.

Also applies to: 38-38, 46-46, 67-67


122-132: Good improvements to label data management.

The addition of staleTime reduces unnecessary API calls, and the onSuccess callback ensures labels are refreshed after updates, improving data consistency.


501-501: UI adjustments look good.

The minor height and positioning adjustments improve visual consistency.

Also applies to: 551-551, 568-569, 587-587

route('/general', '(routes)/settings/general/page.tsx'),
route('/labels', '(routes)/settings/labels/page.tsx'),
route('/categories', '(routes)/settings/categories/page.tsx'),
// route('/categories', '(routes)/settings/categories/page.tsx'),
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Consider replacing commented-out route with a feature flag or disabled meta instead of leaving dead code

Commenting out the /categories route works short-term but silently grows dead code and makes future re-enablement harder to track. Prefer a feature-flag guard (e.g., enabled: false) or a // TODO(date): note so tooling can pick it up and avoid accidental rot.

Please confirm there are no deep links or tests still hitting /settings/categories. If unsure, run:


🏁 Script executed:

#!/bin/bash
# Find any references to the removed categories route
rg -n "/settings/categories"

Length of output: 376


Introduce a feature‐flag or disabled meta for the /settings/categories route

Rather than leaving both the route and its navigation entry commented out, encode the intent directly so future tooling and maintainers can detect and re‐enable it safely. For example:

• In apps/mail/app/routes.ts (around line 45):

- // route('/categories', '(routes)/settings/categories/page.tsx'),
+ route('/categories', '(routes)/settings/categories/page.tsx', { disabled: true }),

• In apps/mail/config/navigation.ts (around line 174):

- //     url: '/settings/categories',
+     {
+       url: '/settings/categories',
+       label: t('navigation/settings/categories'),
+       disabled: true,
+     },

This makes it explicit that the feature exists but is currently inactive, and can be surfaced by linters or custom scripts.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/mail/app/routes.ts at line 45, replace the commented-out route for
'/categories' with a route definition that includes a feature flag or a disabled
meta property indicating the route is inactive. This explicit flag should
clearly mark the route as currently disabled but present, enabling future
tooling and maintainers to detect and safely re-enable it without relying on
commented code.

Comment on lines +172 to +176
// {
// title: m['navigation.settings.categories'](),
// url: '/settings/categories',
// icon: Tabs,
// },
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Navigation item removal may desync hotkeys / i18n keys

The sidebar entry for “categories” is now commented out, but:

  1. Hotkey mappings (mail-list-hotkeys.tsx) might still expect the item.
  2. The i18n message key navigation.settings.categories remains referenced and could become unused noise.

Double-check both to prevent orphaned code and translation bloat.


🏁 Script executed:

#!/bin/bash
# Verify hotkey usage and unused i18n key
rg -n "categories" apps/mail/lib/hotkeys | head
rg -n "navigation.settings.categories" --invert-match apps/mail/config/navigation.ts | head

Length of output: 775


🏁 Script executed:

#!/bin/bash
# Find references to the i18n key outside the commented nav config
rg -n "navigation.settings.categories" -g '!apps/mail/config/navigation.ts'

# Find translation definition in JSON files
rg -n '"navigation.settings.categories"' -g '*.json'

# Fallback: check TS/JS translation modules
rg -n '"navigation.settings.categories"' -g '*.{ts,js}'

Length of output: 354


🏁 Script executed:

#!/bin/bash
# Locate translation key in mail locale files (slash-based keys)
rg -n "\"navigation/settings/categories\"" -g "apps/mail/**/locales/*.json" || echo "No JSON definitions found"

# Fallback: search for dot-based key in all translation sources
rg -n "navigation.settings.categories" -g "apps/mail/**/*.{json,ts,js}" || echo "No dot-based keys found"

Length of output: 455


🏁 Script executed:

#!/bin/bash
# Search for slash-based translation key in all JSON translation files across the repo
rg -n '"navigation/settings/categories"' -g '*.json' || echo "Translation key not found in JSON files"

Length of output: 143


Remove dead translation key; hotkeys are unaffected

The sidebar entry for “Categories” has been commented out, but:

  • mail-list-hotkeys.tsx derives its category list from the Categories() API, not from the navigation config—hotkey mappings remain valid.
  • The translation key navigation.settings.categories (slash-based key in i18n.lock at lines 333 & 866) is no longer used anywhere in code or locale JSON.

Please:
• Remove the commented-out nav entry in apps/mail/config/navigation.ts (lines 172–176).
• Delete navigation/settings/categories from your translation sources and regenerate the i18n lock to clear the unused key.

🤖 Prompt for AI Agents
In apps/mail/config/navigation.ts around lines 172 to 176, remove the entire
commented-out navigation entry for "Categories" as it is dead code.
Additionally, delete the translation key "navigation.settings.categories" from
all translation source files and regenerate the i18n lock file to remove this
unused key from the project.

throw new Error('No driver available');
}
return (await this.driver.getUserLabels()).filter((label) => label.type === 'user');
return await this.driver.getUserLabels();
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Method name is now misleading - consider renaming for clarity.

The method getUserLabels now returns all labels instead of filtering by type === 'user'. This creates a semantic mismatch between the method name and its behavior, which could confuse future maintainers.

Consider renaming the method to better reflect its current behavior:

-  async getUserLabels() {
+  async getLabels() {

Alternatively, if the method should remain focused on user labels, consider adding a comment explaining why all labels are returned and how filtering is handled elsewhere.

Verify that all consumers of this method have been updated to handle the new behavior:


🏁 Script executed:

#!/bin/bash
# Description: Find all usages of getUserLabels to verify they handle the new unfiltered response
# Expected: All callers should be updated to handle both user and system labels

# Search for getUserLabels method calls
ast-grep --pattern 'getUserLabels()'

# Search for getUserLabels in method calls and property access
rg -A 3 -B 3 "getUserLabels"

Length of output: 15490


Action Required: Align getUserLabels Name with Its Behavior

The getUserLabels method now returns all labels (user, system, categories), but its name implies it only returns user‐defined labels. This semantic mismatch can confuse maintainers and lead to incorrect assumptions in callers.

Possible remediations:

  • Rename the method (and update all its usages) to getLabels to accurately reflect its behavior.
  • Or keep getUserLabels but add/update JSDoc and method description to state that it returns all labels, and require callers to filter by type === 'user' if needed.

Files/locations that would need updates when renaming:

  • apps/server/src/routes/chat.ts
  • apps/server/src/routes/agent/tools.ts
  • apps/server/src/routes/ai.ts
  • apps/server/src/trpc/routes/label.ts
  • apps/server/src/services/mcp-service/mcp.ts
  • apps/server/src/pipelines.ts
  • apps/mail/lib/prompts.ts
  • apps/server/src/lib/prompts.ts

Example diff for the rename approach:

--- a/apps/server/src/routes/chat.ts
+++ b/apps/server/src/routes/chat.ts
@@ -672,7 +672,7 @@ export class ZeroAgent extends BaseAgent {
-  async getUserLabels() {
+  async getLabels() {
     if (!this.driver) {
       throw new Error('No driver available');
     }
@@ -678,7 +678,7 @@ export class ZeroAgent extends BaseAgent {
     return await this.driver.getUserLabels();
   }

After renaming, update all callers (tools, TRPC routes, pipelines, prompts) to call getLabels().

📝 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 await this.driver.getUserLabels();
async getLabels() {
if (!this.driver) {
throw new Error('No driver available');
}
return await this.driver.getUserLabels();
}
🤖 Prompt for AI Agents
In apps/server/src/routes/chat.ts at line 675, the method getUserLabels returns
all labels, not just user-defined ones, causing a naming mismatch. To fix this,
rename the method from getUserLabels to getLabels across all its definitions and
usages in the listed files, including chat.ts, tools.ts, ai.ts, label.ts,
mcp.ts, pipelines.ts, and prompt files. Alternatively, if renaming is not
preferred, update the JSDoc and method description to clarify it returns all
labels and instruct callers to filter by type 'user' when needed.

Copy link
Collaborator Author

MrgSub commented Jul 8, 2025

bugbot run

@MrgSub MrgSub force-pushed the 07-08-categories branch from 2f12b81 to bcf1b4b Compare July 8, 2025 23:16
Copy link
Collaborator Author

MrgSub commented Jul 8, 2025

Merge activity

  • Jul 8, 11:16 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 8, 11:16 PM UTC: @MrgSub merged this pull request with Graphite.

@MrgSub MrgSub merged commit 7fec116 into staging Jul 8, 2025
4 of 5 checks passed
@MrgSub MrgSub deleted the 07-08-categories branch July 8, 2025 23:16
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.

1 participant