feat: Dashboard Health Score card with color-coded gauge#266
Conversation
- HealthScoreService: aggregates disk SMART, RAM usage, uptime, and battery wear into a weighted 0-100 score - HealthScoreResult model: score, color hex, label, component breakdown, up to 3 actionable recommendations - DashboardViewModel: auto-loads health score on init, refreshes with Scan system button - DashboardView: circular gauge ring with score number + label, component breakdown, recommendation list with severity colors - Weights: disk 35%, RAM 25%, uptime 20%, battery 20% (redistributed on desktops without battery) - 28 new unit tests for scoring logic and model properties - README, ARCHITECTURE, CHANGELOG updated Closes #259
|
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 as they are similar to previous 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)
📝 WalkthroughWalkthroughAdds a HealthScoreService (models, logic, tests), wires it into DashboardViewModel, and renders a Health Score card and loading state in DashboardView; documentation and changelog updated. ChangesHealth Score Implementation and Dashboard Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
SysManager/SysManager.Tests/HealthScoreServiceTests.cs (2)
10-13: 🏗️ Heavy liftAdd explicit end-to-end aggregation tests for objective-critical behavior.
Line 11–12 says aggregation is validated, but this file currently focuses on per-component/static mappings. Please add
ComputeAsync-level tests for weighted rollup, desktop weight redistribution (no battery), and “top 3 recommendations” cap to lock down the dashboard contract.🤖 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.Tests/HealthScoreServiceTests.cs` around lines 10 - 13, Add end-to-end tests that call HealthScoreService.ComputeAsync to validate aggregation behavior: (1) a weighted rollup test that supplies multiple component inputs with known weights and asserts the final aggregated score matches the weighted sum; (2) a desktop weight redistribution test where Battery component is absent to ensure its weight is redistributed as per service logic and the resulting score reflects that redistribution; and (3) a recommendations limit test that feeds many failing checks and asserts only the top 3 recommendations (by severity or score impact) are returned. Use the HealthScoreService constructor or factory used in existing tests and assert both the aggregated score and the returned Recommendations collection length/content to lock down the dashboard contract.
160-207: ⚡ Quick winCover malformed battery telemetry (zero/invalid design capacity).
Please add a boundary test for
DesignCapacityMWh <= 0so battery scoring behavior is guarded against divide-by-zero or invalid-data regressions.Proposed test addition
+ [Fact] + public void ComputeBatteryScore_ZeroDesignCapacity_DoesNotThrow_AndReturnsValidScore() + { + var battery = new BatteryInfo + { + HasBattery = true, + DesignCapacityMWh = 0, + FullChargeCapacityMWh = 22000 + }; + + var score = HealthScoreService.ComputeBatteryScore(battery); + Assert.InRange(score, 0, 100); + }🤖 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.Tests/HealthScoreServiceTests.cs` around lines 160 - 207, Add tests covering malformed/zero/negative design capacity for HealthScoreService.ComputeBatteryScore: create BatteryInfo instances with HasBattery = true and DesignCapacityMWh set to 0 and to a negative value (e.g., -1000) and assert ComputeBatteryScore returns the safe default (match existing behavior for missing battery, e.g., 100). Ensure the new tests reference BatteryInfo.DesignCapacityMWh and HealthScoreService.ComputeBatteryScore and are added alongside the other battery tests (use descriptive test names like ComputeBatteryScore_ZeroDesignCapacity_Returns100 and ComputeBatteryScore_NegativeDesignCapacity_Returns100).
🤖 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 `@SysManager/SysManager/Services/HealthScoreService.cs`:
- Around line 199-203: The current selection of the worst disk in
HealthScoreService orders only by d.HealthPercent ?? 100, which treats null
SMART percentages as 100 and can miss disks with HealthStatus indicating
problems; update the selection logic used when building `worst` (the variable
assigned from `disks.OrderBy(...)`) to compute a fallback numeric percent when
`HealthPercent` is null by mapping `d.HealthStatus` to a conservative score (for
example: Unhealthy => 0, Degraded => 50, Healthy => 100) and order by that
computed value instead, then keep using `worst?.FriendlyName` when creating the
HealthRecommendation so the recommendation targets the actual problematic disk.
In `@SysManager/SysManager/ViewModels/DashboardViewModel.cs`:
- Around line 58-84: The fire-and-forget call to LoadHealthScoreAsync can leak
unobserved exceptions and the method only catches ManagementException and
InvalidOperationException; fix by (1) making the caller observe the Task
returned from LoadHealthScoreAsync instead of discarding it — e.g. store it in a
field like Task _healthScoreLoadTask = LoadHealthScoreAsync() and attach a
continuation (ContinueWith) that logs any faulted exceptions so they are
observed, and (2) broaden LoadHealthScoreAsync’s catch block to include a final
catch (Exception ex) that logs unexpected errors (using Log.Error) so failures
from _healthScore.ComputeAsync and other runtime issues are captured (keep
existing IsHealthScoreLoading toggling and symbols HealthResult, HasHealthScore,
_healthScore).
---
Nitpick comments:
In `@SysManager/SysManager.Tests/HealthScoreServiceTests.cs`:
- Around line 10-13: Add end-to-end tests that call
HealthScoreService.ComputeAsync to validate aggregation behavior: (1) a weighted
rollup test that supplies multiple component inputs with known weights and
asserts the final aggregated score matches the weighted sum; (2) a desktop
weight redistribution test where Battery component is absent to ensure its
weight is redistributed as per service logic and the resulting score reflects
that redistribution; and (3) a recommendations limit test that feeds many
failing checks and asserts only the top 3 recommendations (by severity or score
impact) are returned. Use the HealthScoreService constructor or factory used in
existing tests and assert both the aggregated score and the returned
Recommendations collection length/content to lock down the dashboard contract.
- Around line 160-207: Add tests covering malformed/zero/negative design
capacity for HealthScoreService.ComputeBatteryScore: create BatteryInfo
instances with HasBattery = true and DesignCapacityMWh set to 0 and to a
negative value (e.g., -1000) and assert ComputeBatteryScore returns the safe
default (match existing behavior for missing battery, e.g., 100). Ensure the new
tests reference BatteryInfo.DesignCapacityMWh and
HealthScoreService.ComputeBatteryScore and are added alongside the other battery
tests (use descriptive test names like
ComputeBatteryScore_ZeroDesignCapacity_Returns100 and
ComputeBatteryScore_NegativeDesignCapacity_Returns100).
🪄 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: 8aaf5b27-3579-48f7-987f-5699225206cb
📒 Files selected for processing (9)
ARCHITECTURE.mdCHANGELOG.mdREADME.mdSysManager/SysManager.Tests/DashboardViewModelTests.csSysManager/SysManager.Tests/HealthScoreServiceTests.csSysManager/SysManager/Models/HealthScoreResult.csSysManager/SysManager/Services/HealthScoreService.csSysManager/SysManager/ViewModels/DashboardViewModel.csSysManager/SysManager/Views/DashboardView.xaml
📜 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: Build & unit tests
- GitHub Check: Analyze (csharp)
🔇 Additional comments (1)
SysManager/SysManager.Tests/HealthScoreServiceTests.cs (1)
28-156: Nice threshold coverage across disk/RAM/uptime and UI mapping bands.Good job locking boundary values (e.g., 79/80, 49/50) and severity color mapping—this reduces regression risk in both scoring and presentation.
Also applies to: 211-248
| if (diskScore < 80 && disks != null) | ||
| { | ||
| var worst = disks.OrderBy(d => d.HealthPercent ?? 100).FirstOrDefault(); | ||
| string diskName = worst?.FriendlyName ?? "Disk"; | ||
| recs.Add(new HealthRecommendation |
There was a problem hiding this comment.
Disk recommendation can target the wrong drive when SMART percent is missing.
At Line 201, worst-disk selection ignores HealthStatus fallback logic, so a disk with HealthPercent == null and status "Unhealthy" may be ranked as 100 and not picked as the problematic disk.
Suggested fix
- var worst = disks.OrderBy(d => d.HealthPercent ?? 100).FirstOrDefault();
+ static int ScoreForRecommendation(DiskHealthReport d) =>
+ d.HealthPercent ?? d.HealthStatus switch
+ {
+ "Healthy" => 100,
+ "Warning" => 50,
+ "Unhealthy" => 20,
+ _ => 80
+ };
+
+ var worst = disks.OrderBy(ScoreForRecommendation).FirstOrDefault();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (diskScore < 80 && disks != null) | |
| { | |
| var worst = disks.OrderBy(d => d.HealthPercent ?? 100).FirstOrDefault(); | |
| string diskName = worst?.FriendlyName ?? "Disk"; | |
| recs.Add(new HealthRecommendation | |
| if (diskScore < 80 && disks != null) | |
| { | |
| static int ScoreForRecommendation(DiskHealthReport d) => | |
| d.HealthPercent ?? d.HealthStatus switch | |
| { | |
| "Healthy" => 100, | |
| "Warning" => 50, | |
| "Unhealthy" => 20, | |
| _ => 80 | |
| }; | |
| var worst = disks.OrderBy(ScoreForRecommendation).FirstOrDefault(); | |
| string diskName = worst?.FriendlyName ?? "Disk"; | |
| recs.Add(new HealthRecommendation |
🤖 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/Services/HealthScoreService.cs` around lines 199 - 203,
The current selection of the worst disk in HealthScoreService orders only by
d.HealthPercent ?? 100, which treats null SMART percentages as 100 and can miss
disks with HealthStatus indicating problems; update the selection logic used
when building `worst` (the variable assigned from `disks.OrderBy(...)`) to
compute a fallback numeric percent when `HealthPercent` is null by mapping
`d.HealthStatus` to a conservative score (for example: Unhealthy => 0, Degraded
=> 50, Healthy => 100) and order by that computed value instead, then keep using
`worst?.FriendlyName` when creating the HealthRecommendation so the
recommendation targets the actual problematic disk.
| _ = LoadHealthScoreAsync(); | ||
| } | ||
|
|
||
| // ── Health Score ─────────────────────────────────────────────────── | ||
|
|
||
| private async Task LoadHealthScoreAsync() | ||
| { | ||
| IsHealthScoreLoading = true; | ||
| try | ||
| { | ||
| HealthResult = await _healthScore.ComputeAsync(); | ||
| HasHealthScore = true; | ||
| Log.Information("Health Score computed: {Score}", HealthResult.Score); | ||
| } | ||
| catch (System.Management.ManagementException ex) | ||
| { | ||
| Log.Warning("Health Score failed: {Error}", ex.Message); | ||
| } | ||
| catch (InvalidOperationException ex) | ||
| { | ||
| Log.Warning("Health Score failed: {Error}", ex.Message); | ||
| } | ||
| finally | ||
| { | ||
| IsHealthScoreLoading = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fire-and-forget health load can leak unobserved exceptions.
Line 58 starts LoadHealthScoreAsync() without awaiting, but the method only catches two exception types. Any other runtime failure can escape that task and destabilize the VM lifecycle.
Suggested fix
private async Task LoadHealthScoreAsync()
{
IsHealthScoreLoading = true;
try
{
HealthResult = await _healthScore.ComputeAsync();
HasHealthScore = true;
Log.Information("Health Score computed: {Score}", HealthResult.Score);
}
catch (System.Management.ManagementException ex)
{
Log.Warning("Health Score failed: {Error}", ex.Message);
+ HasHealthScore = false;
}
catch (InvalidOperationException ex)
{
Log.Warning("Health Score failed: {Error}", ex.Message);
+ HasHealthScore = false;
+ }
+ catch (Exception ex)
+ {
+ Log.Error(ex, "Health Score failed unexpectedly");
+ HasHealthScore = false;
}
finally
{
IsHealthScoreLoading = false;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _ = LoadHealthScoreAsync(); | |
| } | |
| // ── Health Score ─────────────────────────────────────────────────── | |
| private async Task LoadHealthScoreAsync() | |
| { | |
| IsHealthScoreLoading = true; | |
| try | |
| { | |
| HealthResult = await _healthScore.ComputeAsync(); | |
| HasHealthScore = true; | |
| Log.Information("Health Score computed: {Score}", HealthResult.Score); | |
| } | |
| catch (System.Management.ManagementException ex) | |
| { | |
| Log.Warning("Health Score failed: {Error}", ex.Message); | |
| } | |
| catch (InvalidOperationException ex) | |
| { | |
| Log.Warning("Health Score failed: {Error}", ex.Message); | |
| } | |
| finally | |
| { | |
| IsHealthScoreLoading = false; | |
| } | |
| } | |
| private async Task LoadHealthScoreAsync() | |
| { | |
| IsHealthScoreLoading = true; | |
| try | |
| { | |
| HealthResult = await _healthScore.ComputeAsync(); | |
| HasHealthScore = true; | |
| Log.Information("Health Score computed: {Score}", HealthResult.Score); | |
| } | |
| catch (System.Management.ManagementException ex) | |
| { | |
| Log.Warning("Health Score failed: {Error}", ex.Message); | |
| HasHealthScore = false; | |
| } | |
| catch (InvalidOperationException ex) | |
| { | |
| Log.Warning("Health Score failed: {Error}", ex.Message); | |
| HasHealthScore = false; | |
| } | |
| catch (Exception ex) | |
| { | |
| Log.Error(ex, "Health Score failed unexpectedly"); | |
| HasHealthScore = false; | |
| } | |
| finally | |
| { | |
| IsHealthScoreLoading = false; | |
| } | |
| } |
🤖 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/DashboardViewModel.cs` around lines 58 - 84,
The fire-and-forget call to LoadHealthScoreAsync can leak unobserved exceptions
and the method only catches ManagementException and InvalidOperationException;
fix by (1) making the caller observe the Task returned from LoadHealthScoreAsync
instead of discarding it — e.g. store it in a field like Task
_healthScoreLoadTask = LoadHealthScoreAsync() and attach a continuation
(ContinueWith) that logs any faulted exceptions so they are observed, and (2)
broaden LoadHealthScoreAsync’s catch block to include a final catch (Exception
ex) that logs unexpected errors (using Log.Error) so failures from
_healthScore.ComputeAsync and other runtime issues are captured (keep existing
IsHealthScoreLoading toggling and symbols HealthResult, HasHealthScore,
_healthScore).
| if (hasBattery) | ||
| { | ||
| overall = (int)Math.Round( | ||
| diskScore * 0.35 + | ||
| ramScore * 0.25 + | ||
| uptimeScore * 0.20 + | ||
| batteryScore * 0.20); | ||
| } | ||
| else | ||
| { | ||
| // No battery — redistribute weight: disk 40%, RAM 30%, uptime 30% | ||
| overall = (int)Math.Round( | ||
| diskScore * 0.40 + | ||
| ramScore * 0.30 + | ||
| uptimeScore * 0.30); | ||
| } |
| foreach (var d in disks) | ||
| { | ||
| int score = d.HealthPercent ?? d.HealthStatus switch | ||
| { | ||
| "Healthy" => 100, | ||
| "Warning" => 50, | ||
| "Unhealthy" => 20, | ||
| _ => 80 | ||
| }; | ||
| if (score < worstScore) worstScore = score; | ||
| } |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
## 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>
* feat: Dashboard Health Score card with color-coded gauge - HealthScoreService: aggregates disk SMART, RAM usage, uptime, and battery wear into a weighted 0-100 score - HealthScoreResult model: score, color hex, label, component breakdown, up to 3 actionable recommendations - DashboardViewModel: auto-loads health score on init, refreshes with Scan system button - DashboardView: circular gauge ring with score number + label, component breakdown, recommendation list with severity colors - Weights: disk 35%, RAM 25%, uptime 20%, battery 20% (redistributed on desktops without battery) - 28 new unit tests for scoring logic and model properties - README, ARCHITECTURE, CHANGELOG updated Closes #259 * fix: correct HealthScore test expectation for Warning disk (60 not 50) --------- Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
Summary
Adds an overall system health score (0-100) card to the Dashboard, displayed as a color-coded circular gauge with component breakdown and actionable recommendations.
How it works
Components (weighted):
Visual:
Behavior:
Files added
Files modified
Tests (28 new)
Docs
Closes #259
Summary by CodeRabbit
New Features
Documentation
Tests