Fix incomplete UI Automation descendant searches - #842
Nikola Metulev (nmetulev) merged 2 commits into
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Changes recommended
Real WinUI/provider validation and coverage of traversal beyond depth 25 remain missing.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Completes partial UI Automation descendant searches by supplementing bulk results with Control View traversal.
Changes:
- Shares bulk-plus-walker completion across search paths.
- Replaces depth-limited recursion with iterative traversal.
- Adds regression tests for partial results and capped fast paths.
File summaries
| File | Description |
|---|---|
UiAutomationService.cs |
Adds descendant result completion and iterative traversal. |
RealUiAutomationTests.Coverage.cs |
Adds seam-based traversal regressions. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
Quadratic COM-based deduplication can severely degrade non-exact searches on large UI trees.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/winapp-CLI/WinApp.UIAutomation/Services/UiAutomationService.cs:1755
- What is wrong: Deduplication is quadratic and each comparison crosses the UIA COM boundary.
FindSingleElementAsyncpassesint.MaxValue, so even when bulkFindAllis complete, a non-exact selector walks the whole tree and compares the first manual match once, the second twice, and so on.
Show me: If bulk and Control View each return the same 1,000 matches, this loop performs about 500,000 CompareElements calls instead of linear work.
Why it matters: Common text selectors on large application trees can turn an ambiguity check into a long-running or effectively hung action; the reported 128-element fixture does not exercise this growth.
Smallest fix: Use a stable element identity key (for example, a successfully retrieved runtime ID) to deduplicate in constant time, with CompareElements only as a fallback for elements whose identity cannot be read.
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Exact AutomationId searches can return incorrect capped results, and runtime-ID SAFEARRAYs leak during repeated searches.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
src/winapp-CLI/WinApp.UIAutomation/Services/UiAutomationService.cs:478
- Popup searches have the same exact-AutomationId regression. When the main window has no match and a provider cannot evaluate AutomationId substring conditions, an unrelated popup element whose Name matches can fill the remaining cap and prevent the manual walk from finding the exact AutomationId. Restore exact AutomationId lookup ahead of the substring/completion path for owned windows as well.
var condition = BuildCondition(selector);
if (condition is not null)
{
var windowFound = FindAllDescendantMatches(
windowRoot,
condition,
selector.Query,
maxResults - mainResults.Count);
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
Build Metrics ReportBinary Sizes
Test Results✅ 6851 passed, 37 skipped out of 6888 tests in 873.0s (+32 tests, -304.0s vs. baseline) Test Coverage✅ 85.9% line coverage, 79.9% branch coverage · ✅ +0.1% vs. baseline CLI Startup Time63ms median (x64, Try This BuildInstalls the MSIX for your architecture, replacing any previously installed build. Needs the GitHub CLI — the command offers to install it and sign you in if it is missing. & ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) 842Switching between builds often?Put the tool on your PATH once: & ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) -AddToPathThen this build is just: winapp-pr 842Run Updated 2026-09-17 02:01:15 UTC · commit |
There was a problem hiding this comment.
🟡 Changes recommended
The unbounded traversal ignores cancellation, preventing timeouts or Ctrl+C from stopping slow searches.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
|
Copilot review |
There was a problem hiding this comment.
🟡 Changes recommended
Capped substring results can still override an omitted exact AutomationId match.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
e44ffd2 to
27191a4
Compare
Rebase the reviewed traversal fix onto current main while preserving exact AutomationId precedence and complete Control View fallback behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
27191a4 to
b1424b6
Compare
There was a problem hiding this comment.
🟡 Changes recommended
A recovered exact match can retain a stale HWND, causing subsequent UI commands to reject it.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Zach Teutsch (zateutsch)
left a comment
Zach Teutsch (zateutsch)
left a comment
There was a problem hiding this comment.
Review clean, approving.
There was a problem hiding this comment.
🔵 Needs a closer look
Ordinary capped Name searches still perform full traversal, contradicting the stated fast-path behavior.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/winapp-CLI/WinApp.UIAutomation/Services/UiAutomationService.cs:1995
- The cap-satisfied fast path is not preserved for ordinary Name queries. When the exact-ID bulk probe misses and the substring bulk query returns one result for
maxResults = 1, thisint.MaxValuemakesFindAllDescendantMatcheswalk the entire Control View anyway; the newSearchAsync_ExactBulkMiss_UsesOneCompletedSubstringWalkAndPreservesExactPrecedencetest confirms that path. This contradicts the PR's performance claim that completion runs only when bulk results have not satisfied the requested cap, and makes common Name searches pay the measured completion overhead. Either retain the caller's cap here with a narrower exact-ID recovery strategy, or explicitly document and benchmark this always-completed path.
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Copilot review |
There was a problem hiding this comment.
🟢 Approval recommended
The implementation addresses the reported incomplete-provider behavior and the expanded tests cover the previously identified regressions.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Review clean; no blocking findings. The focused real UI Automation fixture passed 137/137 tests. |
Summary
FindAll(TreeScope.Descendants)results with the existing Control View traversalSearchAsync,FindSingleElementAsync, and popup/owned-window lookupsAutomationIdprecedence, including IDs omitted by a provider's bulk descendant queryPerformance and completeness tradeoff
Exact
AutomationIdlookup inFindSingleElementAsyncretains itsFindFirstfast path. Exact-ID bulk searches also retain the cap-satisfied fast path when the provider returns a result.When the exact-ID bulk probe misses, ordinary Name/substring lookup deliberately completes one Control View walk before applying the caller's result cap. This is required because an initialized, visible WebView2 provider can return a nonzero but incomplete bulk result, and a result count at the cap does not prove the omitted exact ID or later substring matches are absent. A node/time budget or WebView-only heuristic would knowingly reintroduce #823; provider metadata did not provide a reliable completeness signal.
Published native binary measurements against the same pre-optimization head remained sub-second while removing the redundant exact-only walk:
Validation
scripts\build-cli.ps1 -SkipTests: passed end to end, including native publish, npm, NuGet, MSIX, schema, and plugin validationFixes #823