Repository navigation
feat(wework): add priority activity filter - #2365
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDesktopSidebar now provides a priority activity view. It ranks unread, waiting, and active tasks, supports pinned-task preferences and keyboard control, updates notification icons, and includes tests, translations, E2E coverage, and documentation. ChangesPriority activity filtering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant E2EFlow
participant DesktopSidebar
participant RuntimeTask
E2EFlow->>RuntimeTask: Create request-user-input task
RuntimeTask-->>DesktopSidebar: Expose waiting task
DesktopSidebar-->>E2EFlow: Show priority-filtered task
E2EFlow->>DesktopSidebar: Toggle filter shortcut
E2EFlow->>RuntimeTask: Resume and complete task
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
docs/en/wegent/user-guide/coding/priority-activity-filter.md (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the repeated word.
The sentence uses "again" twice.
✏️ Proposed wording
-- Click the button again or use the shortcut again to return to the normal project and task lists. +- Click the button or press the shortcut a second time to return to the normal project and task lists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/en/wegent/user-guide/coding/priority-activity-filter.md` at line 13, Update the sentence in the priority activity filter guide to remove the repeated “again,” while preserving its meaning that either action returns the user to the normal project and task lists.Source: Linters/SAST tools
wework/src/components/layout/DesktopSidebar.tsx (3)
721-744: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClassification order and rank order disagree.
getRuntimeTaskPriorityReasonreturns'active'first, so a task that is running and unread is classified asactive.getRuntimeTaskPriorityRankthen ranks it last (2), behind pure unread and waiting tasks. The documentation states that unread tasks come first. Check the unread key before the running key if unread should win.♻️ Proposed reordering
const address = getRuntimeTaskAddress(workspace, task) const active = runningTaskKeys.has(getRuntimeTaskLifecycleKey(address)) - if (active) return 'active' if (unreadTaskKeys.has(getRuntimeTaskReminderItemKey(workspace, task))) return 'unread' + if (active) return 'active' return isRuntimeTaskWaiting(task) ? 'waiting' : null🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/DesktopSidebar.tsx` around lines 721 - 744, Update getRuntimeTaskPriorityReason to check unreadTaskKeys before runningTaskKeys, so tasks that are both unread and active are classified as 'unread' and receive the highest priority rank from getRuntimeTaskPriorityRank; preserve the existing waiting fallback.
3249-3319: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the priority section into its own component.
DesktopSidebar.tsxis now near 3,990 lines, and this change adds a second full render branch inline. The coding guidelines require splitting files over 1000 lines and favoring cohesive modules. Move the priority section markup, its options menu, and the empty state into aDesktopSidebarPrioritySectioncomponent, and pass the already-derivedpriorityTaskItemsplus the row callbacks. The classification helpers at lines 714-752 can move toruntimeTaskSidebarHelpers.tswith the other task helpers.As per coding guidelines: "Favor cohesive modules, explicit interfaces, and standard practices; split files over 1000 lines."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/DesktopSidebar.tsx` around lines 3249 - 3319, Extract the inline priority-filter render branch into a cohesive DesktopSidebarPrioritySection component, passing priorityTaskItems, priorityShowPinned state, translation helper, and the required RuntimeTaskRow callbacks and selection/read state as explicit props; preserve the options menu, empty state, and row behavior. Move the priority classification helpers near the existing helpers at lines 714-752 into runtimeTaskSidebarHelpers.ts, then replace the inline markup in DesktopSidebar with the new component.Source: Coding guidelines
3011-3028: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRestrict the priority-filter shortcut and add it to the keybindings system.
This
windowlistener callspreventDefault()forAlt+Ctrl/Cmd+Uwithout checking the event target, so it can block AltGr text entry in editable fields. Also add a command/label tokeybindings.tsand expose it throughKeyboardShortcutsSettingsPageinstead of hard-coding the shortcut only in the tooltip.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/DesktopSidebar.tsx` around lines 3011 - 3028, The priority-filter shortcut handler in the DesktopSidebar effect must ignore events originating from editable fields before calling preventDefault, preserving AltGr text entry; add a corresponding command and label in keybindings.ts, then register and expose that command through KeyboardShortcutsSettingsPage so the shortcut is managed by the keybindings system rather than only the tooltip.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/components/layout/DesktopSidebar.tsx`:
- Around line 2765-2772: Update priorityNeedsAttention in DesktopSidebar to
derive from priorityTaskItems.length rather than allPriorityTaskItems.length, so
the attention indicator matches the currently filtered priority list when pinned
tasks are hidden.
In `@wework/src/i18n/locales/en/common.json`:
- Around line 51-55: Align the English feature terminology across the locale
strings and the English priority-activity-filter documentation: replace
“activity view” wording with the established “Priority” view terminology,
including the `priority_filter`, `priority_filter_needs_attention`,
`priority_filter_turn_off`, and `priority_filter_options` values, while
preserving their existing meanings.
---
Nitpick comments:
In `@docs/en/wegent/user-guide/coding/priority-activity-filter.md`:
- Line 13: Update the sentence in the priority activity filter guide to remove
the repeated “again,” while preserving its meaning that either action returns
the user to the normal project and task lists.
In `@wework/src/components/layout/DesktopSidebar.tsx`:
- Around line 721-744: Update getRuntimeTaskPriorityReason to check
unreadTaskKeys before runningTaskKeys, so tasks that are both unread and active
are classified as 'unread' and receive the highest priority rank from
getRuntimeTaskPriorityRank; preserve the existing waiting fallback.
- Around line 3249-3319: Extract the inline priority-filter render branch into a
cohesive DesktopSidebarPrioritySection component, passing priorityTaskItems,
priorityShowPinned state, translation helper, and the required RuntimeTaskRow
callbacks and selection/read state as explicit props; preserve the options menu,
empty state, and row behavior. Move the priority classification helpers near the
existing helpers at lines 714-752 into runtimeTaskSidebarHelpers.ts, then
replace the inline markup in DesktopSidebar with the new component.
- Around line 3011-3028: The priority-filter shortcut handler in the
DesktopSidebar effect must ignore events originating from editable fields before
calling preventDefault, preserving AltGr text entry; add a corresponding command
and label in keybindings.ts, then register and expose that command through
KeyboardShortcutsSettingsPage so the shortcut is managed by the keybindings
system rather than only the tooltip.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 55f880b6-2342-4abe-917a-3adbe635a702
📒 Files selected for processing (7)
docs/en/wegent/user-guide/coding/priority-activity-filter.mddocs/zh/wegent/user-guide/coding/priority-activity-filter.mdwework/README.mdwework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
3cca4b4 to
616a5f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
wework/e2e/desktop/task-flow.e2e.mjs (1)
2331-2393: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
requestReleasednever changes before thefinallyblock, so the guard is dead code.The variable is initialized to
falseand is only assigned inside thefinallyblock itself. The conditionif (!requestReleased)is therefore always true. Remove the flag and release the response directly.♻️ Proposed simplification
- let requestReleased = false try { await control.command('click', '[data-testid="new-chat-button"]') @@ } finally { - if (!requestReleased) { - await withTimeout( - control.releaseRequestUserInputResponse(), - DEFAULT_STEP_TIMEOUT_MS, - 'Timed out releasing the priority-filter request-user-input response' - ) - requestReleased = true - } + await withTimeout( + control.releaseRequestUserInputResponse(), + DEFAULT_STEP_TIMEOUT_MS, + 'Timed out releasing the priority-filter request-user-input response' + ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/e2e/desktop/task-flow.e2e.mjs` around lines 2331 - 2393, Remove the requestReleased flag and its conditional guard around the cleanup in the finally block of the priority-filter test. Invoke control.releaseRequestUserInputResponse() directly through withTimeout, preserving the existing timeout and error message.wework/src/components/layout/DesktopSidebar.tsx (1)
728-756: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClassification order and rank order disagree; timestamp parsing rejects numeric strings.
Two points in these helpers:
getRuntimeTaskPriorityReasonchecksrunningbeforeisRuntimeTaskWaiting, so a task that is both running and waiting is labeledactive.getRuntimeTaskPriorityRankthen ranksactivebelowwaiting. If a waiting task keeps its lifecycle running state, it sorts after every other waiting task. Confirm which state must win for a running task that also waits for input.
getRuntimeTaskTimereturnsstring | number.new Date(value)handles epoch numbers and ISO strings, but returnsInvalid Datefor a numeric string such as'1717171717000', which falls back to0and moves the task to the end of its rank group. Normalize numeric strings before parsing.♻️ Proposed timestamp normalization
function getRuntimeTaskPriorityTime(task: RuntimeTaskSummary): number { const value = getRuntimeTaskTime(task) if (value === undefined) return 0 - const timestamp = new Date(value).getTime() + const numeric = typeof value === 'number' ? value : Number(value) + const timestamp = Number.isFinite(numeric) ? numeric : new Date(value).getTime() return Number.isNaN(timestamp) ? 0 : timestamp }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/DesktopSidebar.tsx` around lines 728 - 756, Update getRuntimeTaskPriorityReason so its classification precedence matches getRuntimeTaskPriorityRank, ensuring tasks that are both running and waiting receive the intended higher-priority waiting state. In getRuntimeTaskPriorityTime, normalize numeric string values to epoch numbers before constructing the Date, while preserving existing handling for numeric values, ISO strings, missing values, and invalid timestamps.wework/src/lib/keybindings.test.ts (1)
43-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a non-macOS assertion for the priority filter shortcut.
This platform-scoped test only covers the macOS default. Add a Windows case, such as
mergeKeybindings([], 'win')[TOGGLE_PRIORITY_FILTER_COMMAND] === 'Control+Alt+U', or another platform-specific case that matches the surrounding test assertions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/lib/keybindings.test.ts` at line 43, Add a platform-specific assertion alongside the existing macOS expectation for TOGGLE_PRIORITY_FILTER_COMMAND, invoking mergeKeybindings with the Windows platform and verifying its default shortcut is Control+Alt+U. Keep the existing macOS assertion unchanged and match the surrounding test structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/classify-wework-desktop-e2e.sh:
- Around line 96-101: The sidebar classifier currently maps DesktopSidebar.tsx
and DesktopSidebarPrioritySection.tsx together, narrowing DesktopSidebar.tsx
coverage incorrectly. In .github/scripts/classify-wework-desktop-e2e.sh lines
96-101, keep DesktopSidebarPrioritySection.tsx mapped only to
core:priority-filter and add a separate DesktopSidebar.tsx case selecting
core:priority-filter, core:core-task-flow, and core:workspace-attachments. In
.github/scripts/test-classify-ci-changes.sh lines 260-266, update the
DesktopSidebar.tsx expected matrix with those additional entries and add
coverage asserting DesktopSidebarPrioritySection.tsx selects only
core:priority-filter.
---
Nitpick comments:
In `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 2331-2393: Remove the requestReleased flag and its conditional
guard around the cleanup in the finally block of the priority-filter test.
Invoke control.releaseRequestUserInputResponse() directly through withTimeout,
preserving the existing timeout and error message.
In `@wework/src/components/layout/DesktopSidebar.tsx`:
- Around line 728-756: Update getRuntimeTaskPriorityReason so its classification
precedence matches getRuntimeTaskPriorityRank, ensuring tasks that are both
running and waiting receive the intended higher-priority waiting state. In
getRuntimeTaskPriorityTime, normalize numeric string values to epoch numbers
before constructing the Date, while preserving existing handling for numeric
values, ISO strings, missing values, and invalid timestamps.
In `@wework/src/lib/keybindings.test.ts`:
- Line 43: Add a platform-specific assertion alongside the existing macOS
expectation for TOGGLE_PRIORITY_FILTER_COMMAND, invoking mergeKeybindings with
the Windows platform and verifying its default shortcut is Control+Alt+U. Keep
the existing macOS assertion unchanged and match the surrounding test structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a3360edd-043b-437c-a846-04fbf51cdeba
📒 Files selected for processing (16)
.github/scripts/classify-wework-desktop-e2e.sh.github/scripts/test-classify-ci-changes.shdocs/en/wegent/user-guide/coding/priority-activity-filter.mddocs/zh/wegent/user-guide/coding/priority-activity-filter.mdwework/README.mdwework/e2e/desktop/checkpoints.mjswework/e2e/desktop/task-flow.e2e.mjswework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopSidebarPrioritySection.tsxwework/src/components/settings/KeyboardShortcutsSettingsPage.test.tsxwework/src/components/settings/KeyboardShortcutsSettingsPage.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/lib/keybindings.test.tswework/src/lib/keybindings.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/zh/wegent/user-guide/coding/priority-activity-filter.md
- wework/src/i18n/locales/zh-CN/common.json
- docs/en/wegent/user-guide/coding/priority-activity-filter.md
- wework/src/i18n/locales/en/common.json
- wework/README.md
- wework/src/components/layout/DesktopSidebar.test.tsx
Summary
Option+Command+Uon macOS (Ctrl+Alt+Uelsewhere).Why
The sidebar did not provide a focused way to find conversations that require attention. The new temporary activity view follows the ChatGPT interaction model without changing task data or normal project organization.
Verification
pnpm --filter wework test— 253 files / 2534 tests passed.pnpm --filter wework exec prettier --check ...— passed.pnpm --filter wework exec eslint ...andpnpm --filter wework run lint:task-lifecycle— passed.pnpm --filter wework typecheck— passed.QA evidence
Captured with
ai:verifyin an isolated Tauri session. Screenshots are retained in the implementation handoff.Summary by CodeRabbit
New Features
Documentation