Refactor core UI rendering, search matching, and service maintenance loops - #4906
vyas-devgna wants to merge 10 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe changes update literal search behavior, UI rendering and filtering, system and package-manager checks, tweak execution loops, update progress handling, architecture documentation, and related Pester coverage. ChangesPowerShell workflows
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 4
🧹 Nitpick comments (1)
functions/private/Find-TweaksByNameOrDescription.ps1 (1)
69-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
$matchesto avoid PowerShell's automatic variable. Use$isMatch; PSScriptAnalyzer flags assignments to$matches, which stores regex results.🤖 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 `@functions/private/Find-TweaksByNameOrDescription.ps1` around lines 69 - 72, Rename the local `$matches` variable in `Find-TweaksByNameOrDescription` to `$isMatch`, updating both its assignment and the subsequent conditional reference. Preserve the existing search logic and matching behavior.
🤖 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 `@functions/private/Find-TweaksByNameOrDescription.ps1`:
- Around line 34-44: Update the reset branch in Find-TweaksByNameOrDescription
so it restores each category according to its collapsed-label prefix instead of
making every item visible. Reuse the approach from Find-AppsByNameOrDescription:
detect labels starting with "+", keep those category items collapsed, and ensure
the label prefix remains consistent with item visibility.
In `@functions/private/Invoke-WinUtilCurrentSystem.ps1`:
- Around line 104-110: Update the service validation logic in
Invoke-WinUtilCurrentSystem so a missing service returned by Get-Service is
treated as a mismatch. Clear $allMatch and exit the validation loop whenever
$Service is null or its StartType differs from $tweak.StartupType; preserve the
existing matching behavior for present services with the expected startup type.
In `@functions/private/Invoke-WinUtilISO.ps1`:
- Around line 62-67: Update the timeout handling in the ISO mount flow around
Mount-DiskImage and the $mountElapsed check to dismount the mounted ISO before
raising the timeout error. Invoke the existing dismount mechanism for the target
image, then throw the same timeout message, while preserving normal mounting
behavior when the drive letter appears.
In `@functions/private/Test-WinUtilPackageManager.ps1`:
- Around line 18-24: Update the package-manager selection and validation in
Test-WinUtilPackageManager so passing both -winget and -choco cannot silently
check only winget; either reject the mutually exclusive combination or evaluate
both requested managers and return "installed" only when all requested commands
are available, while preserving the existing single-switch results.
---
Nitpick comments:
In `@functions/private/Find-TweaksByNameOrDescription.ps1`:
- Around line 69-72: Rename the local `$matches` variable in
`Find-TweaksByNameOrDescription` to `$isMatch`, updating both its assignment and
the subsequent conditional reference. Preserve the existing search logic and
matching behavior.
🪄 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: fb0f9660-cec9-48e4-81b6-214661337bc8
📒 Files selected for processing (12)
functions/private/Find-AppsByNameOrDescription.ps1functions/private/Find-TweaksByNameOrDescription.ps1functions/private/Invoke-WinUtilCurrentSystem.ps1functions/private/Invoke-WinUtilISO.ps1functions/private/Invoke-WinUtilTweaks.ps1functions/private/Show-CustomDialog.ps1functions/private/Test-WinUtilPackageManager.ps1functions/private/Write-WinUtilLog.ps1functions/public/Invoke-WPFFixesUpdate.ps1functions/public/Invoke-WPFUIElements.ps1pester/sanity.Tests.ps1scripts/main.ps1
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af9d24ea5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ChrisTitusTech
left a comment
ChrisTitusTech
left a comment
There was a problem hiding this comment.
Feedback needs to be resolved or this PR will be closed out.
- Find-TweaksByNameOrDescription: respect collapsed category state on search reset (mirrors Find-AppsByNameOrDescription); rename $matches to $isMatch to avoid shadowing the PS automatic variable - Invoke-WinUtilCurrentSystem: treat a missing service as a mismatch instead of silently passing validation - Invoke-WinUtilISO: dismount ISO before throwing timeout error to prevent stale mounts; restore per-workdir log file for diagnostics - Test-WinUtilPackageManager: check both managers when both -winget and -choco switches are passed - Invoke-WPFFixesUpdate: restore per-service PercentComplete in the Stop-Service loop; abort on failure instead of silently continuing
|
All review feedback addressed in b4b04e6: CodeRabbit + Codex feedback:
@mewclouds feedback:
|
coderabbitai
Bot
added
bug
|
Thanks! Will review shortly. |
|
Could the ISO log write directly to |
Both fair, thanks. The workdir log is still gated on Win11ISOWorkDir which is only set after the modify run succeeds, so I will point Log at the workDir the runspace already receives and create the folder before the first line, and since the failure path deletes workDir I will preserve the log before cleanup. For the test, the empty search case builds the category as "+ Privacy" and still expects everything visible, so I will update it to keep collapsed categories collapsed and add a "- Privacy" case for the other branch. Pushing shortly. |
|
Both fixed in d20279d. The log now goes to The empty-search tweak test now covers both branches — Thanks for the catch. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pester/search-filter.Tests.ps1 (1)
395-414: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the category markers after resetting the search.
The fixture uses
+ Privacyand- Explorerto represent collapsed and expanded states, but the test checks only visibility. A regression that changes either marker can pass. Add content assertions for both categories.Proposed test assertions
$collapsedCategory.Label.Visibility | Should -Be ([Windows.Visibility]::Visible) + $collapsedCategory.Label.Content | Should -Be "+ Privacy" $collapsedItem.Visibility | Should -Be ([Windows.Visibility]::Collapsed) $expandedCategory.Border.Visibility | Should -Be ([Windows.Visibility]::Visible) $expandedCategory.Label.Visibility | Should -Be ([Windows.Visibility]::Visible) + $expandedCategory.Label.Content | Should -Be "- Explorer" $expandedItem.Visibility | Should -Be ([Windows.Visibility]::Visible)🤖 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 `@pester/search-filter.Tests.ps1` around lines 395 - 414, Update the test “restores category labels and respects collapsed category state for empty search” to assert each category label’s Content after Find-TweaksByNameOrDescription resets the search. Verify the collapsed category retains “+ Privacy” and the expanded category retains “- Explorer”, alongside the existing visibility assertions.
🤖 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 `@pester/search-filter.Tests.ps1`:
- Around line 395-414: Update the test “restores category labels and respects
collapsed category state for empty search” to assert each category label’s
Content after Find-TweaksByNameOrDescription resets the search. Verify the
collapsed category retains “+ Privacy” and the expanded category retains “-
Explorer”, alongside the existing visibility assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 15635d3a-b994-4486-b1aa-ad71e104143d
📒 Files selected for processing (3)
docs/src/content/docs/code-reference/architecture.mdxfunctions/private/Invoke-WinUtilISO.ps1pester/search-filter.Tests.ps1
🚧 Files skipped from review as they are similar to previous changes (1)
- functions/private/Invoke-WinUtilISO.ps1
coderabbitai
Bot
added
ui update
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b9a01e78d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ce loops - Replace O(N^2) array concatenations with generic lists in GUI item rendering and tweak checks - Convert slow pipeline loops (ForEach-Object) to direct foreach runtime enumeration - Replace wildcard regex matches in app and tweak search with fast string index lookups - Add timeout protection and batch file cleanup in ISO mounting workflows - Prevent file-locking exceptions when logging within an active transcript session - Streamline Windows Update service repair routines and throttle progress updates during DLL reregistration - Ensure command quote resilience in sanity tests when invoking nested Windows PowerShell parsers
- Find-TweaksByNameOrDescription: respect collapsed category state on search reset (mirrors Find-AppsByNameOrDescription); rename $matches to $isMatch to avoid shadowing the PS automatic variable - Invoke-WinUtilCurrentSystem: treat a missing service as a mismatch instead of silently passing validation - Invoke-WinUtilISO: dismount ISO before throwing timeout error to prevent stale mounts; restore per-workdir log file for diagnostics - Test-WinUtilPackageManager: check both managers when both -winget and -choco switches are passed - Invoke-WPFFixesUpdate: restore per-service PercentComplete in the Stop-Service loop; abort on failure instead of silently continuing
Write the modify log to <workDir>.log in %TEMP% instead of a file gated on $sync["Win11ISOWorkDir"], which is only assigned after a successful run. The log now starts at the first line, before the work directory is created, and survives the cleanup that removes that directory, so early failures leave a diagnostic behind. Update the empty-search tweak test for the collapsed-category reset behavior and cover the expanded branch as well.
The reset branch leaves Label.Content alone while the search branch rewrites "+ X" to "- X". Without these assertions a reset that started rewriting the marker would desync the label from its collapsed items and still pass.
3b9a01e to
3e0fa36
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e0fa360bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@functions/private/Find-TweaksByNameOrDescription.ps1`:
- Around line 106-108: Update the category expansion logic around
$categoryLabel.Content to record categories expanded by search in $sync before
changing the "+ " prefix to "- ". When the search is cleared, use that shared
state to restore each affected category’s "+ " prefix and collapsed items, while
preserving categories that were already expanded and using $sync for the shared
state and UI references.
🪄 Autofix
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: c7600339-7466-4ce4-88d1-905adee87230
📒 Files selected for processing (8)
docs/src/content/docs/code-reference/architecture.mdxfunctions/private/Find-AppsByNameOrDescription.ps1functions/private/Find-TweaksByNameOrDescription.ps1functions/private/Invoke-WinUtilCurrentSystem.ps1functions/private/Invoke-WinUtilTweaks.ps1functions/public/Invoke-WPFUIElements.ps1pester/search-filter.Tests.ps1scripts/main.ps1
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/src/content/docs/code-reference/architecture.mdx
- pester/search-filter.Tests.ps1
- scripts/main.ps1
- functions/public/Invoke-WPFUIElements.ps1
- functions/private/Invoke-WinUtilCurrentSystem.ps1
- functions/private/Invoke-WinUtilTweaks.ps1
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
- Find-TweaksByNameOrDescription: respect collapsed category state on search reset (mirrors Find-AppsByNameOrDescription); rename $matches to $isMatch to avoid shadowing the PS automatic variable - Invoke-WinUtilCurrentSystem: treat a missing service as a mismatch instead of silently passing validation - Invoke-WinUtilISO: dismount ISO before throwing timeout error to prevent stale mounts; restore per-workdir log file for diagnostics - Test-WinUtilPackageManager: check both managers when both -winget and -choco switches are passed - Invoke-WPFFixesUpdate: restore per-service PercentComplete in the Stop-Service loop; abort on failure instead of silently continuing
This branch has not been deployed
3 participants
Type of Change
Description
Reduce repeated allocations and pipeline overhead in UI rendering, tweak application, and installed-state detection while preserving the current job layer and startup flow.
Validation
.\Compile.ps1succeeds.Import-Module Pester -RequiredVersion 5.8.0 -Force; Invoke-Pester -Path 'pester/*.Tests.ps1' -Output Detailed -CI: 842 passed, 0 failed, 1 existing skip because the compiled headless test would open a UAC prompt.Add-Type -AssemblyName PresentationFrameworkfollowed by the search-filter Pester suite: 17 passed with real WPF controls, including direct StackPanel and ScrollViewer category layouts.lint/PSScriptAnalyser.ps1: no errors; existing convention/test-double warnings remain.git diff origin/main --check: clean. Generated tweak/feature reference pages, ISO orchestration, logging, and startup are unchanged from main.Optional-service detection retains main's behavior: missing optional services are neutral in normal UI detection, while strict environment-report collection marks them as mismatches. This supersedes the earlier conflicting review suggestions without changing main's apply/detect behavior.
Issue related to PR