refactor: deduplicate shortcuts and manage timer completion timeouts#79
Conversation
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThree independent quality improvements: ChangesShortcut Trigger Deduplication
Timer Completion Timeout Lifecycle
VariableScope Unit Tests
Changelog and Audit Log
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 1
🧹 Nitpick comments (1)
src/store/useTimerStore.ts (1)
50-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid side effects inside the
setupdater.
clearCompletionTimeout(t.id)is invoked from within thefilterpredicate that runs inside theset((state) => ...)updater. Zustand expects the updater to be a pure state transform; performing timer-cancellation side effects there is an anti-pattern that can misbehave if the updater is ever re-invoked (e.g., under middleware). Compute the set of expired ids first, clear their timeouts, then return the new state.♻️ Proposed refactor
cleanExpiredTimers: () => { const now = Date.now() - set((state) => ({ - timers: state.timers.filter((t) => { - if (t.status === 'completed' && now - t.endsAt >= COMPLETED_TIMER_CLEANUP_MS) { - clearCompletionTimeout(t.id) - return false - } - return true - }), - })) + const expiredIds = new Set( + useTimerStore + .getState() + .timers.filter( + (t) => t.status === 'completed' && now - t.endsAt >= COMPLETED_TIMER_CLEANUP_MS + ) + .map((t) => t.id) + ) + expiredIds.forEach((id) => clearCompletionTimeout(id)) + set((state) => ({ + timers: state.timers.filter((t) => !expiredIds.has(t.id)), + })) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/store/useTimerStore.ts` around lines 50 - 61, The cleanExpiredTimers updater in useTimerStore mixes state updates with the side effect of calling clearCompletionTimeout inside the filter predicate. Refactor it so the set callback is a pure transform: first derive the expired completed timer ids from state.timers, then clear their timeouts outside the set updater, and finally return the filtered timers array from cleanExpiredTimers without invoking side effects during the state calculation.
🤖 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 `@src/lib/editor/VariableScope.test.ts`:
- Around line 44-53: Update the VariableScope fallback test to assert the real
trim behavior, not just a pre-trimmed input. In VariableScope.test.ts, change
the doc string used with VariableScope.triggerScopeUpdate() so the raw RHS
includes leading/trailing spaces and still expects getNoteScope() to store the
trimmed value. This will verify that triggerScopeUpdate() continues to call
.trim() when expression parsing fails.
---
Nitpick comments:
In `@src/store/useTimerStore.ts`:
- Around line 50-61: The cleanExpiredTimers updater in useTimerStore mixes state
updates with the side effect of calling clearCompletionTimeout inside the filter
predicate. Refactor it so the set callback is a pure transform: first derive the
expired completed timer ids from state.timers, then clear their timeouts outside
the set updater, and finally return the filtered timers array from
cleanExpiredTimers without invoking side effects during the state calculation.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c238c9de-78f3-4656-9026-7758dd021b20
📒 Files selected for processing (5)
AUDIT_LOG.mdCHANGELOG.mdsrc-tauri/src/commands/shortcuts.rssrc/lib/editor/VariableScope.test.tssrc/store/useTimerStore.ts
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
1 participant
Summary by CodeRabbit
Bug Fixes
Tests