Skip to content

fix: address 4 medium-priority findings (PERF-005/007, CI-001, TEST-005) #298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laurentiu021 merged 1 commit into from
May 13, 2026
Merged

fix: address 4 medium-priority findings (PERF-005/007, CI-001, TEST-005) #298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions .github/workflows/auto-release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,12 @@ jobs:
FIRST_LINE=$(echo "$COMMIT_MSG" | head -n1)
echo "commit_msg=$FIRST_LINE"

if echo "$FIRST_LINE" | grep -qiE '^feat(\(.*\))?!?:'; then
# CI-001: Check for breaking changes (! suffix) before minor/patch
if echo "$FIRST_LINE" | grep -qiE '^(feat|fix)(\(.*\))?!:'; then
echo "type=major" >> "$GITHUB_OUTPUT"
elif echo "$FIRST_LINE" | grep -qiE '^feat(\(.*\))?:'; then
echo "type=minor" >> "$GITHUB_OUTPUT"
elif echo "$FIRST_LINE" | grep -qiE '^fix(\(.*\))?!?:'; then
elif echo "$FIRST_LINE" | grep -qiE '^fix(\(.*\))?:'; then
echo "type=patch" >> "$GITHUB_OUTPUT"
else
echo "type=none" >> "$GITHUB_OUTPUT"
Expand All @@ -64,7 +67,11 @@ jobs:
IFS='.' read -r MAJOR MINOR PATCH <<< "$VERSION"

BUMP="${{ steps.bump.outputs.type }}"
if [ "$BUMP" = "minor" ]; then
if [ "$BUMP" = "major" ]; then
MAJOR=$((MAJOR + 1))
MINOR=0
PATCH=0
elif [ "$BUMP" = "minor" ]; then
MINOR=$((MINOR + 1))
PATCH=0
else
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,19 @@ jobs:
run: dotnet build SysManager/SysManager.UITests/SysManager.UITests.csproj -c Release

- name: Run UI tests
id: ui-tests
run: >
dotnet test SysManager/SysManager.UITests/SysManager.UITests.csproj
-c Release --no-build
--logger "trx;LogFileName=ui-results.trx"
--results-directory TestResults
continue-on-error: true

# TEST-005: Surface UI test failures as a visible PR annotation
- name: Annotate UI test failures
if: steps.ui-tests.outcome == 'failure'
run: echo "::warning::UI automation tests failed — review test results artifact for details."

- name: Upload UI test results
if: always()
uses: actions/upload-artifact@v7
Expand Down
12 changes: 12 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

## [0.50.0] - 2026-05-13

### Fixed
- **Performance: ConsoleViewModel** — fix O(n²) trim by removing from index 0
forward instead of reverse-order removal (PERF-005).
Comment on lines +12 to +13

@coderabbitai coderabbitai Bot May 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Correct the complexity claim wording.

Line 12 says this change “fix[es] O(n²),” but removing from index 0 repeatedly is still O(n) per removal and can still be quadratic for larger trims. Please adjust the release note to describe the behavioral fix (oldest-first trimming) without claiming an O(n²) elimination.

Suggested wording 
-- **Performance: ConsoleViewModel** — fix O(n²) trim by removing from index 0
-  forward instead of reverse-order removal (PERF-005).
+- **Performance: ConsoleViewModel** — fix trim order to remove oldest lines
+  first (index 0), preserving the most recent output (PERF-005).
📝 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.

Suggested change
- **Performance: ConsoleViewModel** — fix O(n²) trim by removing from index 0
forward instead of reverse-order removal (PERF-005).
- **Performance: ConsoleViewModel** — fix trim order to remove oldest lines
first (index 0), preserving the most recent output (PERF-005).
🤖 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 `@CHANGELOG.md` around lines 12 - 13, Update the CHANGELOG entry for
ConsoleViewModel (PERF-005) to remove the explicit "fix O(n²)" complexity claim
and instead describe the behavioral change: state that trimming now removes
oldest entries first instead of repeatedly removing from index 0, improving
trimming behavior and avoiding the previous inefficient removal pattern; mention
ConsoleViewModel and PERF-005 so the edit is easy to locate.

- **Performance: ProcessManagerViewModel** — move icon extraction and process
description lookup to background thread to prevent UI freezes (PERF-007).
- **CI: auto-release** — detect breaking change commits (feat!:/fix!:) and
bump major version instead of treating them as minor/patch (CI-001).
- **CI: ci.yml** — add warning annotation when UI automation tests fail so
failures are visible on PRs without blocking merge (TEST-005).

## [0.49.0] - 2026-05-13

### Fixed
Expand Down
6 changes: 4 additions & 2 deletions SysManager/SysManager/ViewModels/ConsoleViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ public void Append(PowerShellLine line)
Lines.Add(line);
if (Lines.Count > MaxLines)
{
// PERF-005: Remove excess items from front (index 0). Each removal
// is O(n) but unavoidable with ObservableCollection.
var excess = Lines.Count - MaxLines;
for (int i = excess - 1; i >= 0; i--)
Lines.RemoveAt(i);
for (int i = 0; i < excess; i++)
Lines.RemoveAt(0);
}
}
}
Expand Down
40 changes: 24 additions & 16 deletions SysManager/SysManager/ViewModels/ProcessManagerViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,26 +52,34 @@ private async Task RefreshAsync()

try
{
var snapshot = await _service.SnapshotAsync();
Processes.Clear();
foreach (var p in snapshot)
// PERF-007: Perform snapshot, icon extraction, and description lookup
// on a background thread to avoid UI freezes on slow processes.
var enriched = await Task.Run(() =>
{
p.Icon = IconExtractorService.GetProcessIcon(p.FilePath, p.Name);
var dbEntry = ProcessDescriptionService.Instance.Lookup(p.Name);
if (dbEntry != null)
{
p.PlainDescription = dbEntry.Description;
p.Category = dbEntry.Category;
p.SafetyLevel = dbEntry.Safety.ToString();
}
else
var snapshot = _service.SnapshotAsync().GetAwaiter().GetResult();
foreach (var p in snapshot)
{
p.PlainDescription = p.Description; // fallback to FileVersionInfo description
p.Category = "Unknown";
p.SafetyLevel = "Unknown";
p.Icon = IconExtractorService.GetProcessIcon(p.FilePath, p.Name);
var dbEntry = ProcessDescriptionService.Instance.Lookup(p.Name);
if (dbEntry != null)
{
p.PlainDescription = dbEntry.Description;
p.Category = dbEntry.Category;
p.SafetyLevel = dbEntry.Safety.ToString();
}
else
{
p.PlainDescription = p.Description;
p.Category = "Unknown";
p.SafetyLevel = "Unknown";
}
}
return snapshot;
});

Processes.Clear();
foreach (var p in enriched)
Processes.Add(p);
}

ApplyFilter();
StatusMessage = $"Loaded {ProcessCount} processes.";
Expand Down
Loading