fix: deduplicate FormatSize (CQ-002) and batch ConsoleViewModel trim (CQ-008)#254
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
📝 WalkthroughWalkthroughConsolidates size-formatting into CleanupCategory.HumanSize across DiskUsageEntry, InstalledApp, and ProcessEntry, updates an associated test and CHANGELOG, and changes ConsoleViewModel.Append to batch-remove excess lines instead of repeated head removals. ChangesCode Deduplication and Performance Optimization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
SysManager/SysManager/ViewModels/ConsoleViewModel.cs (1)
39-44: 🏗️ Heavy liftThis still does O(n) head-removal in steady state.
Because
Appendinserts one line at a time,excessis typically 1, so Line 43 is effectivelyRemoveAt(0)per append. That keeps the same shifting cost onObservableCollection. If amortized O(1) is the goal, use a ring-buffer/queue-backed store and project the latestMaxLinesto the UI.🤖 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 `@SysManager/SysManager/ViewModels/ConsoleViewModel.cs` around lines 39 - 44, The current Append logic removes from the head of ObservableCollection (Lines.RemoveAt(i)) which causes O(n) shifts per append; replace the head-removal approach with a ring-buffer or queue-backed store and have the ObservableCollection only reflect the latest window: add a private Queue<T> or circular buffer field (e.g. _buffer) in ConsoleViewModel, on Append enqueue the new line and while _buffer.Count > MaxLines dequeue from _buffer (O(1) ops), then update Lines to project the _buffer contents (replace the ObservableCollection contents in one update or by efficient batch/ReplaceRange) instead of calling RemoveAt(0); reference the existing symbols Lines, MaxLines and the current head-removal loop to locate where to swap in the queue-backed logic.
🤖 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.
Nitpick comments:
In `@SysManager/SysManager/ViewModels/ConsoleViewModel.cs`:
- Around line 39-44: The current Append logic removes from the head of
ObservableCollection (Lines.RemoveAt(i)) which causes O(n) shifts per append;
replace the head-removal approach with a ring-buffer or queue-backed store and
have the ObservableCollection only reflect the latest window: add a private
Queue<T> or circular buffer field (e.g. _buffer) in ConsoleViewModel, on Append
enqueue the new line and while _buffer.Count > MaxLines dequeue from _buffer
(O(1) ops), then update Lines to project the _buffer contents (replace the
ObservableCollection contents in one update or by efficient batch/ReplaceRange)
instead of calling RemoveAt(0); reference the existing symbols Lines, MaxLines
and the current head-removal loop to locate where to swap in the queue-backed
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9851504b-1e64-4e9b-8a10-3d61c2d12fb6
📒 Files selected for processing (5)
CHANGELOG.mdSysManager/SysManager/Models/DiskUsageEntry.csSysManager/SysManager/Models/InstalledApp.csSysManager/SysManager/Models/ProcessEntry.csSysManager/SysManager/ViewModels/ConsoleViewModel.cs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (csharp)
- GitHub Check: Build & unit tests
🔇 Additional comments (3)
SysManager/SysManager/Models/DiskUsageEntry.cs (1)
24-24: Good deduplication to shared formatter.Centralizing size formatting through
CleanupCategory.HumanSizeimproves consistency and maintainability.SysManager/SysManager/Models/ProcessEntry.cs (1)
36-36: Nice consistency win across models.Using
CleanupCategory.HumanSizehere removes duplicate formatting logic and keeps output uniform.SysManager/SysManager/Models/InstalledApp.cs (1)
26-26: Looks good—deduplicated without changing the fallback behavior.The shared formatter plus existing
"—"guard keeps display behavior clear.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced May 13, 2026
laurentiu021
added a commit
that referenced
this pull request
May 22, 2026
## Summary Replace standalone sort buttons/dropdowns with native DataGrid clickable column headers across 5 tabs, consistent with Windows Task Manager behavior. ## Changes ### Process Manager (#266) - Converted ListView to DataGrid with sortable columns: PID, Name, Memory, CPU%, Threads, Status - Removed toolbar sort buttons (Memory, CPU, Name, PID) - Removed SortBy property and sort commands from ViewModel ### Uninstaller (#254) - Converted ListView to DataGrid with sortable columns: Name, Size, Version, Publisher, Source, Status - Removed toolbar sort buttons (Name, Size, Publisher) - Removed SortBy property and sort commands from ViewModel ### Services - Removed redundant Sort ComboBox from toolbar (DataGrid already has CanUserSortColumns) - Added SortMemberPath on all columns - Removed SortBy/SortOptions properties from ViewModel ### Startup Manager - Converted ListView to DataGrid with sortable columns: Name, Publisher, Status ### App Updates - Added CanUserSortColumns=True and SortMemberPath on all columns ## Behavior Click any column header to sort ascending. Click again to sort descending. ## Tests - Updated ProcessManagerViewModelTests and UninstallerViewModelTests for removed sort commands Closes #266, Closes #254, Closes #250 Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
laurentiu021
added a commit
that referenced
this pull request
May 22, 2026
…(CQ-008) (#254) * fix: deduplicate FormatSize (CQ-002) and batch ConsoleViewModel trim (CQ-008) * test: update MemoryDisplay expected format to match HumanSize --------- Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
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
2 participants
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
Two low-effort code quality fixes from code review.
Build: 0 errors
Summary by CodeRabbit