-
Notifications
You must be signed in to change notification settings - Fork 21
feat(tasks): add target type filters to task list #101
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
Conversation
Implements Phase 3.1 subtask: Add filters for target types (PR/email/tweet/stdout) Changes: - Added Filter icon import - Added filter state management (selectedTargetTypes, showFilters) - Implemented filtering logic in sortedTasks useMemo - Added filter toggle button in Tasks header - Created filter UI with chips for each target type (All, PR, Email, Tweet, Stdout) - Filter buttons show selected state and toggle on click - 'All' option shows all tasks, other filters can be combined The filtering allows users to quickly focus on tasks by their target type, improving task queue navigation and management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
Adds comprehensive target type filtering functionality to the task list in UnifiedSidebar, allowing users to filter tasks by PR, Email, Tweet, or Stdout types.
Key Changes:
- Added filter toggle button with
Filtericon next to the "Create Task" button - Implemented collapsible filter UI with chip-style buttons for each target type
- Filter state managed using React
useStatewith aSet<string>for efficient lookups - Filtering logic integrated into existing
sortedTasksuseMemo, maintaining status-based sorting - Supports multi-select filtering (e.g., show only PR and Email tasks)
- "All" filter acts as exclusive selection, automatically selected when no specific types are chosen
Implementation Quality:
- Clean state management with proper React hooks
- Efficient filtering using Set data structure
- Proper memoization dependencies to prevent unnecessary recalculations
- Good UX with visual feedback (default vs outline button variants)
- Filter state persists during session but resets on page reload (as documented)
Confidence Score: 5/5
- This PR is safe to merge with no issues found
- The implementation is well-structured with proper state management, correct filtering logic, and appropriate React patterns. The filter logic handles edge cases correctly (empty selection resets to 'all'), useMemo dependencies are complete, and the UI integration is clean. TypeScript compilation and build passed successfully.
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| src/components/UnifiedSidebar.tsx | 5/5 | Added target type filtering for tasks with collapsible filter UI, proper state management, and correct filtering logic |
Sequence Diagram
sequenceDiagram
participant User
participant FilterButton
participant FilterUI
participant State
participant useMemo
participant TaskList
User->>FilterButton: Click filter icon
FilterButton->>State: Toggle showFilters
State->>FilterUI: Render filter chips
User->>FilterUI: Click "PR" filter
FilterUI->>State: Update selectedTargetTypes
Note over State: Remove 'all', add 'pr'
State->>useMemo: Trigger recomputation
useMemo->>useMemo: Filter tasks by target_type
useMemo->>useMemo: Sort by status
useMemo->>TaskList: Return filtered tasks
TaskList->>User: Display filtered tasks
User->>FilterUI: Click "Email" filter
FilterUI->>State: Update selectedTargetTypes
Note over State: Add 'email' to set
State->>useMemo: Trigger recomputation
useMemo->>TaskList: Return tasks matching PR or Email
User->>FilterUI: Deselect "PR" & "Email"
FilterUI->>State: Update selectedTargetTypes
Note over State: Set is empty, reset to ['all']
State->>useMemo: Trigger recomputation
useMemo->>TaskList: Return all tasks
1 file reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to d8dda1c in 1 minute and 45 seconds. Click for details.
- Reviewed
116lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/UnifiedSidebar.tsx:118
- Draft comment:
Using a Set for selectedTargetTypes works, but consider adding a comment explaining the choice and ensuring that task.target_type is consistently normalized (e.g. lower-case) to match the filter strings. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. src/components/UnifiedSidebar.tsx:140
- Draft comment:
In the filtering logic, ensure that task.target_type values exactly match the strings used in the filter buttons (e.g. 'pr', 'email', etc.). Consider normalizing these values if needed. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment is asking the author to "ensure" and "consider" something, which are red flags according to the rules. The comment is speculative - it suggests there might be a mismatch between task.target_type values and the filter strings, but provides no evidence that such a mismatch exists. Without seeing the Task type definition or actual task data, this is purely hypothetical. The comment doesn't point to a definite bug - it's asking the author to verify/double-check their work. This violates the rule about not asking authors to confirm their intention or ensure things work correctly. Could there be a legitimate concern here? Perhaps the automated tool has knowledge of the Task type that shows target_type uses different casing or values? Maybe this is a common bug pattern the tool has seen before? Even if this is a common bug pattern, without concrete evidence in the diff or visible code that there's a mismatch, this is still speculative. The rules explicitly state not to make speculative comments or ask authors to verify/ensure things. If there was a real type mismatch, it would likely be caught by TypeScript or runtime testing. This comment should be deleted. It's asking the author to "ensure" something works correctly and to "consider" normalizing values, without providing any evidence of an actual problem. This is speculative and violates the rule against asking authors to verify or ensure their code works as intended.
3. src/components/UnifiedSidebar.tsx:208
- Draft comment:
The onClick handler for the filter chips contains inline logic to update the Set. Consider refactoring this into a separate helper function to improve readability and maintainability. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. src/components/UnifiedSidebar.tsx:136
- Draft comment:
Sorting tasks by status using a statusOrder mapping is clear. Just verify that the ordering matches the intended UX (active/pending shown first). - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_vlKSoo3dOSM4yzgy
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Implements Phase 3.1: Web UI Integration - Add filters for target types
Changes
Implementation Details
Testing
Phase 3.1 Status
All subtasks complete:
Related: Task migrate-legacy-queue-script Phase 3.1
Important
Adds target type filtering to task list in
UnifiedSidebar.tsx, allowing users to filter tasks by type with session persistence.UnifiedSidebar.tsx.useState.sortedTasksusinguseMemo.Buttoncomponent fromshadcn/ui.This description was created by
for d8dda1c. You can customize this summary. It will automatically update as commits are pushed.