Fix incomplete UI Automation descendant searches - #842
Merged
Nikola Metulev (nmetulev) merged 2 commits intoSep 17, 2026
Merged
Nikola Metulev (nmetulev) merged 2 commits into
Nikola Metulev (nmetulev) merged 2 commits into
Conversation
Copilot started reviewing on behalf of
Nikola Metulev (nmetulev)
September 14, 2026 23:19
View session
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
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.
Copilot started reviewing on behalf of
Nikola Metulev (nmetulev)
September 14, 2026 23:50
View session
Contributor
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
Copilot started reviewing on behalf of
Nikola Metulev (nmetulev)
September 15, 2026 00:19
View session
Contributor
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
Contributor
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 |
Copilot started reviewing on behalf of
Nikola Metulev (nmetulev)
September 15, 2026 00:55
View session
Contributor
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 started reviewing on behalf of
Nikola Metulev (nmetulev)
September 15, 2026 01:34
View session
This was referenced Sep 15, 2026
Nikola Metulev (nmetulev)
added this pull request to stack #849
September 15, 2026 06:29
Member
Author
|
Copilot review |
Copilot started reviewing on behalf of
Nikola Metulev (nmetulev)
September 15, 2026 18:46
View session
Copilot stopped work on behalf of
Nikola Metulev (nmetulev) due to an error
September 15, 2026 18:51
Contributor
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
Copilot started reviewing on behalf of
Nikola Metulev (nmetulev)
September 15, 2026 19:02
View session
Nikola Metulev (nmetulev)
force-pushed
the
nmetulev-fix-incomplete-ui-subtree-traversal
branch
from
September 16, 2026 21:12
e44ffd2 to
27191a4
Compare
Copilot started reviewing on behalf of
Nikola Metulev (nmetulev)
September 17, 2026 00:46
View session
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>
Nikola Metulev (nmetulev)
force-pushed
the
nmetulev-fix-incomplete-ui-subtree-traversal
branch
from
September 17, 2026 01:05
27191a4 to
b1424b6
Compare
Copilot started reviewing on behalf of
Nikola Metulev (nmetulev)
September 17, 2026 01:07
View session
Contributor
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
Copilot started reviewing on behalf of
Nikola Metulev (nmetulev)
September 17, 2026 01:25
View session
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Zach Teutsch (zateutsch)
approved these changes
Sep 17, 2026
Zach Teutsch (zateutsch)
left a comment
Zach Teutsch (zateutsch)
left a comment
Contributor
There was a problem hiding this comment.
Review clean, approving.
Contributor
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 started reviewing on behalf of
Nikola Metulev (nmetulev)
September 17, 2026 01:46
View session
Member
Author
|
Copilot review |
Contributor
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
Contributor
Review clean; no blocking findings. The focused real UI Automation fixture passed 137/137 tests. |
Nikola Metulev (nmetulev)
deleted the
nmetulev-fix-incomplete-ui-subtree-traversal
branch
September 17, 2026 02:35
Nikola Metulev (nmetulev)
added a commit
that referenced
this pull request
Sep 17, 2026
Preserve #842 search completion and explicit strict selection while removing the duplicate s_compareElements declaration produced by automatic merging. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Nikola Metulev (nmetulev)
added a commit
that referenced
this pull request
Sep 17, 2026
## Summary Adds composable `--root`, `--type`, and `--class-name` predicates to `ui search`, `get-property`, `get-value`, and `wait-for`. - Root selectors resolve uniquely across app windows with global exact AutomationId precedence. Descendant matches exclude the root, have no inspect display-depth cap, and never spill into popup windows after resolving a root. - Accepts all 41 official UIA control types case-insensitively plus `TextBox` → `Edit` and `TextBlock` → `Text`. Unknown outer/root types are rejected. ClassName matches literal strings, including empty values, exactly and case-insensitively. - Waits re-resolve roots each poll and retry scoped stale-element races during lookup and condition reads. Provider faults and incomplete traversals cannot imply disappearance. - npm wrappers preserve explicitly supplied query arguments. Canonical docs, shipped skill, generated schema/npm API, and shared package type mapping API documentation are updated. ## Stack Depends on #842 and reuses its complete traversal helpers. Native stack **849** has order **#842 → #848**, ultimate base `main`. No batching, persistent handles, explicit-action changes, or formatting changes. ## Validation and performance Normal build, generated schema/docs, npm, and published ARM64/x64 real-app integrations passed. Regression coverage includes partial provider results, global root uniqueness/precedence, deep boundaries, all types/aliases, exact/empty class names, root replacement, explicit HWND/popup isolation, and identity/property/traversal failures. Actionable review regressions were demonstrated failing before fixes. Final real-app measurement (20 iterations after 3 warmups): median baseline **217.865ms**, type **154.439ms**, class **148.711ms**, root+type+class **150.149ms**. Fixture measurements are not a universal performance guarantee. ## Pre-human-review gate complete At head `bc46bda29e48c9893b00ea178cc0f98d009ce1b6`: - Fresh Copilot follow-up review **5208275219**: **Approval recommended**, "No unresolved defects were found." Reviewed 29/29 files, zero new comments. This is the review's recommendation, not a substitute for required human approval. - All **24** review threads answered and resolved, including Code Quality feedback. - **30 successful checks**, one neutral CodeQL aggregate; required `build-and-package` and real-app `e2e-test-ui` both passed. - No conflicts; owning working tree clean. Neither PR merged. Fixes #819 --------- Co-authored-by: Nikola Metulev <711864+nmetulev@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Nikola Metulev (nmetulev)
added a commit
that referenced
this pull request
Sep 17, 2026
## Description
Read `FontWeight`, `FontName`, `FontSize`, `ForegroundColor`,
`IsItalic`, and `StrikethroughStyle` across the element's whole
TextPattern document. Uniform values are invariant strings; UIA's mixed
and reserved-not-supported COM tokens become `Mixed` and `NotSupported`,
while elements without TextPattern return `Unavailable`. Provider
failures retain the existing error contract.
Unknown case-sensitive property names now fail with `invalid_arguments`
before querying the app. Omitting `--property` includes all six
attributes. The published `elementId` and string-valued `properties`
JSON envelope and existing geometry formats are unchanged.
## Usage Example
```powershell
winapp ui get-property Document -a myapp --property FontWeight --json
# {"elementId":"Document","properties":{"FontWeight":"700"}}
winapp ui get-property Document -a myapp --json
```
## Related Issue
Fixes #822. Independent of #841, #842, and #844. No query predicates,
explicit actions, selection/caret/range APIs, batch mode, or
persistence.
## Type of Change
- New feature
- Test update
- Documentation
## Checklist
- [x] New unit, CLI, and real-provider tests
- [x] Tested locally on Windows: 26 formatting/property library tests
and 28 CLI/property/public-API tests pass
- [x] `scripts/build-cli.ps1 -SkipTests` completes NativeAOT x64/arm64
publishing, npm/NuGet/MSIX packaging, and generated docs/schema
- [x] Canonical UI automation docs and shipped skill/reference updated
## Additional Notes
Real-app coverage reads native RichEdit uniform and mixed formatting
independently of selection, a Button without TextPattern, and an
HWND-based provider returning UIAutomationCore's actual
reserved-not-supported token through the repository COM projection.
Background fixtures assert they never activate. Unit tests cover
invariant conversion, actual COM sentinel identity, unknown names, and
provider-error propagation; CLI tests pin string serialization and
scrubbed errors.
Pre-human-review gate is pending automated review and CI; this PR is not
being merged by the implementation session.
---------
Co-authored-by: Nikola Metulev <711864+nmetulev@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Zach Teutsch <88554871+zateutsch@users.noreply.github.com>
Nikola Metulev (nmetulev)
added a commit
that referenced
this pull request
Sep 18, 2026
## Description Adds `--action invoke|select|toggle|toggle-on|toggle-off|expand|collapse` to `winapp ui invoke`. Explicit actions use only their corresponding UIA pattern on the selected element, with no pattern or ancestor fallback. Explicit resolution also refuses to rebind a removed element to a same-name sibling. Toggle-on/off read first, leave an already-correct state untouched, and verify each transition. Initially indeterminate controls allow at most two transitions; other starting states allow one. Omitted `--action` preserves automatic behavior. JSON includes `requestedAction` and `performedAction`, including `none` for an idempotent no-op. The package API, npm forwarding surface, canonical docs, shipped skill, and generated schema are updated together. ## Usage Example ```powershell winapp ui invoke SettingsCategory -a myapp --action select winapp ui invoke AgreeCheckbox -a myapp --action toggle-on --json winapp ui invoke SizeComboBox -a myapp --action collapse ``` ## Related Issue Fixes #821. Independent of #841, #842, and #844; no batching or persistent handles. ## Type of Change - New feature - Documentation - Test update ## Checklist - [x] Unit and real-app regression coverage added - [x] Tested locally on Windows - [x] Canonical UI automation documentation and shipped skill updated - [x] CLI schema and npm surface regenerated through the normal build ## Validation - 64 explicit-action unit/live cases passed, including strict identity and same-name surviving siblings. - 49 CLI/API/cancellation targeted cases passed. - `scripts\build-cli.ps1 -SkipTests` passed: NativeAOT x64/arm64, npm, NuGet, MSIX, and generated docs. - Published CLI argument/error checks and `validate-llm-docs.ps1 -FailOnDrift` passed. - Full `scripts\build-cli.ps1` was run using a session-local corporate NuGet mirror. It reached 5,440 CLI and 453 UI successes plus 63 analyzer successes. A missing new-type API allowlist entry was fixed and retested. Remaining unrelated failures: two recording `DirectoryNotFoundException` cases, x64 dump analysis under the ARM64 host, and a process-cancellation timing case (the latter passed in isolation). ## Additional Notes Automated review and required CI gates are being monitored. This PR must not be merged by the agent. --------- Co-authored-by: Nikola Metulev <711864+nmetulev@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Zachary Teutsch <zteutsch@microsoft.com> Co-authored-by: Zach Teutsch <88554871+zateutsch@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
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
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