Conversation
…compute sort keys
…, localize hardcoded strings
|
Warning Review limit reached
More reviews will be available in 39 minutes and 40 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR refactors settings UI across builtin and remote instance pages by introducing reusable helpers, modernizes the download status bar with context.select-based selectors, optimizes download task filtering with caching, and hardens data layer APIs with immutability and invalidation patterns. ChangesSettings UI Helpers & Page Refactoring
Download Status Bar & Filtering Optimization
Data Layer & Service Infrastructure Updates
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/services/instance_manager.dart (1)
161-177:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDefault built-in instance name is still in Chinese here.
Line 75 in
initialize()creates the built-in instance withname: 'Built-in', but_createDefaultInstance()still uses'内建实例'. Depending on which path runs, users see a different display name for the same built-in instance. This also contradicts the intended English migration.🌐 Proposed fix
Aria2Instance( id: 'builtin', - name: '内建实例', + name: 'Built-in', type: InstanceType.builtin,🤖 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 `@lib/services/instance_manager.dart` around lines 161 - 177, The default built-in instance name in _createDefaultInstance() is still Chinese ('内建实例'); update it to match the other creation path by changing the Aria2Instance name to 'Built-in' in _createDefaultInstance() so both paths use the same display name (refer to _createDefaultInstance() and the earlier initialize() creation of the built-in instance).
🧹 Nitpick comments (2)
lib/app.dart (1)
540-560: ⚡ Quick winConsolidate task summary logic to prevent tray/status-bar drift.
Line 540 and Line 931 duplicate the same summary rules in two places. Extract a shared helper so both tray menu and bottom status bar always stay consistent.
Proposed refactor
+({int active, int waiting, int speed}) _summarizeTasks( + List<DownloadTask> tasks, +) { + var active = 0; + var waiting = 0; + var speed = 0; + for (final task in tasks) { + if (task.status == DownloadStatus.active) { + active++; + speed += task.downloadSpeedBytes; + } else if (task.status == DownloadStatus.waiting) { + waiting++; + } + } + return (active: active, waiting: waiting, speed: speed); +} ... - var activeCount = 0; - var waitingCount = 0; - var totalDownloadSpeed = 0; - for (final task in tasks) { ... } + final summary = _summarizeTasks(tasks); + final activeCount = summary.active; + final waitingCount = summary.waiting; + final totalDownloadSpeed = summary.speed; ... - final counts = context.select<DownloadDataService, ({int active, int waiting, int speed})>((service) { - var active = 0; - var waiting = 0; - var speed = 0; - for (final task in service.tasks) { ... } - return (active: active, waiting: waiting, speed: speed); - }); + final counts = context.select< + DownloadDataService, + ({int active, int waiting, int speed}) + >((service) => _summarizeTasks(service.tasks));Also applies to: 931-944
🤖 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 `@lib/app.dart` around lines 540 - 560, The task summary logic that computes activeCount, waitingCount, resumableCount, pausableCount, and totalDownloadSpeed is duplicated (seen around the loop over tasks in lib/app.dart) causing drift between tray/status-bar; extract this into a single helper (e.g., computeTaskSummary or TaskSummary.fromTasks) that accepts the tasks list and returns a struct/object with fields activeCount, waitingCount, resumableCount, pausableCount, and totalDownloadSpeed, replace the inline loop in both places (the current loop and the duplicate at lines ~931–944) with calls to that helper, and ensure callers read the returned fields rather than recomputing.lib/models/settings.dart (1)
97-99: ConfirmgetDefaultDownloadDirectorySync()type-safety in settings
getDefaultDownloadDirectorySync()is defined inlib/utils/default_download_directory.dartand returns a synchronousString, solib/models/settings.dart’sFuture<String> _defaultDownloadDirectory() async { return getDefaultDownloadDirectorySync(); }is type-safe.- No
getDefaultDownloadDirectorysymbol appears to be referenced/defined elsewhere; leaving the redundantasyncwrapper is an optional cleanup.🤖 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 `@lib/models/settings.dart` around lines 97 - 99, The _defaultDownloadDirectory() function currently uses an unnecessary async wrapper around the synchronous getDefaultDownloadDirectorySync(); replace the async function body with a direct Future return to avoid the redundant async: change Future<String> _defaultDownloadDirectory() async { return getDefaultDownloadDirectorySync(); } to Future<String> _defaultDownloadDirectory() { return Future.value(getDefaultDownloadDirectorySync()); } so the method remains async-compatible but no longer uses an unnecessary async/await; reference symbols: _defaultDownloadDirectory and getDefaultDownloadDirectorySync.
🤖 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 @.gitignore:
- Around line 21-24: The .gitignore comment and the actual rule for ".vscode/"
are inconsistent: the comment says the entry is "commented out by default" but
the file currently contains an active ".vscode/" ignore line; update either the
comment or the rule to match policy by either commenting out the ".vscode/" line
or changing the comment to indicate it is intentionally ignored — locate the
".vscode/" entry in the .gitignore and make the comment and the rule consistent
(edit the comment text near ".vscode/" or prepend a "#" to the ".vscode/" line).
In `@lib/pages/components/settings_helpers.dart`:
- Around line 60-73: The current calculation of columns (derived from width) can
exceed the number of sections, causing empty trailing columns and squeezed
content; update the logic in the same block that defines
columns/distributedSections so that you clamp the column count to the number of
sections (e.g., use a clampedColumns = min(columns, max(1, sections.length)) or
equivalent) and then generate distributedSections and the for-loop using that
clampedColumns (ensure index % clampedColumns is used and handle the
zero-sections case by forcing at least 1 column).
In `@lib/pages/download_page/download_page.dart`:
- Around line 332-343: The cache-hit check in download_page.dart fails because
DownloadDataService.tasks returns a new UnmodifiableListView each access so
identical(_cachedTasksRef, tasksRef) is always false; update the cache logic to
use a stable change token/version or a stable reference instead of wrapper
identity: either add and read a changeCounter/version (e.g., expose a
version/getter on DownloadDataService) or cache the underlying list reference
(or a cheap content signature like length + lastModified) and compare that
(replace references to identical(_cachedTasksRef, tasksRef) with the new stable
token/version check), keeping the rest of the predicate (e.g., _cachedFilter,
_cachedCategoryType, _selectedInstanceId, _searchQuery, _sortOption,
_sortDescending, _instanceNames) intact and ensure _cachedTasksRef is set from
the chosen stable token when updating _cachedFilteredTasks.
---
Outside diff comments:
In `@lib/services/instance_manager.dart`:
- Around line 161-177: The default built-in instance name in
_createDefaultInstance() is still Chinese ('内建实例'); update it to match the other
creation path by changing the Aria2Instance name to 'Built-in' in
_createDefaultInstance() so both paths use the same display name (refer to
_createDefaultInstance() and the earlier initialize() creation of the built-in
instance).
---
Nitpick comments:
In `@lib/app.dart`:
- Around line 540-560: The task summary logic that computes activeCount,
waitingCount, resumableCount, pausableCount, and totalDownloadSpeed is
duplicated (seen around the loop over tasks in lib/app.dart) causing drift
between tray/status-bar; extract this into a single helper (e.g.,
computeTaskSummary or TaskSummary.fromTasks) that accepts the tasks list and
returns a struct/object with fields activeCount, waitingCount, resumableCount,
pausableCount, and totalDownloadSpeed, replace the inline loop in both places
(the current loop and the duplicate at lines ~931–944) with calls to that
helper, and ensure callers read the returned fields rather than recomputing.
In `@lib/models/settings.dart`:
- Around line 97-99: The _defaultDownloadDirectory() function currently uses an
unnecessary async wrapper around the synchronous
getDefaultDownloadDirectorySync(); replace the async function body with a direct
Future return to avoid the redundant async: change Future<String>
_defaultDownloadDirectory() async { return getDefaultDownloadDirectorySync(); }
to Future<String> _defaultDownloadDirectory() { return
Future.value(getDefaultDownloadDirectorySync()); } so the method remains
async-compatible but no longer uses an unnecessary async/await; reference
symbols: _defaultDownloadDirectory and getDefaultDownloadDirectorySync.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d23c4ae2-4728-45f2-9c91-e39a35df7200
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.gitignorelib/app.dartlib/models/settings.dartlib/pages/builtin_instance_settings_page.dartlib/pages/components/settings_helpers.dartlib/pages/download_page/download_page.dartlib/pages/download_page/models/download_task.dartlib/pages/remote_instance_settings_page.dartlib/services/aria2_rpc_client.dartlib/services/builtin_instance_service.dartlib/services/download_data_service.dartlib/services/instance_manager.dartlib/utils/default_download_directory.dartpackages/fl_libpubspec.yaml
💤 Files with no reviewable changes (3)
- lib/utils/default_download_directory.dart
- lib/pages/download_page/models/download_task.dart
- lib/services/builtin_instance_service.dart
…edup task summary
This was referenced Jun 2, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Updates
intldependency to ^0.20.2.