fix: DevFlow IView visibility/bounds resolution for Comet

[davidortinau]

Extracts DevFlow-only changes from PR #68 / feature/comet so they can ship independently of the Comet app framework.

Commits

1. 6f7efa29 — fix: DevFlow IView visibility/bounds resolution

   • CometViewResolver (new) bridges IView → handler-native view via reflection (no compile-time Comet ref)
   • VisualTreeWalker uses it for visibility/bounds when Element lookup yields nothing

2. 93101e0a — feat(devflow): Yoga layout inspection across agent, driver, and CLI

   • New ?layout=1 query param on /api/v1/tree (and --layout CLI flag, layout MCP param)
   • LayoutInspectorAdapter reflects against Comet.Layout.IYogaLayoutInspector so the agent picks up Yoga snapshots when Comet is loaded — zero hard dependency on Comet
   • Adds LayoutInfo DTOs to both Agent.Core and Driver
   • Cached per-Type LayoutManager PropertyInfo lookup (walks the type chain) for low overhead

Scope notes

• DevFlow is an independent product in this monorepo; it must not block on Comet shipping
• Two further DevFlow commits remain on feature/comet (_app→BoundApplication migration + Comet view-resolution / IView scroll support). They have non-trivial conflicts with BLE work that landed on main post-branch, so they're deferred to a later DevFlow PR with build verification — they don't block this one
• Closes the DevFlow concerns of Fix remaining DevFlow support for Comet apps #68

Verification

• Local: cherry-picks of cae8b05e and 0d033ed6; conflicts resolved manually keeping main's BLE/status/dispatcher work intact
• Build: relies on CI (Arcade SDK + sandbox network restrictions prevent local build here)

[Copilot]

Pull request overview

Adds improved DevFlow visual-tree inspection support for Comet (and other IView implementations that don’t derive from VisualElement) by resolving bounds/visibility via the handler’s native platform view, plus a reflection-based Comet text extraction fallback.

Changes:

• Extend VisualTreeWalker (Core) to compute visibility, window-bounds, and native-info for IView elements via new overridable hooks.
• Implement platform-specific IView window-bounds/visibility/native-info resolution in the Agent walker using native view APIs.
• Add CometViewResolver.TryExtractText reflection fallback for Comet views that don’t surface text through standard MAUI interfaces.

Show a summary per file

File	Description
src/DevFlow/Microsoft.Maui.DevFlow.Agent/VisualTreeWalker.cs	Implements platform-native bounds/visibility/native-info extraction for IView via Handler.PlatformView.
src/DevFlow/Microsoft.Maui.DevFlow.Agent.Core/VisualTreeWalker.cs	Adds IView fallthrough flow (visibility/bounds/native info + richer text extraction).
src/DevFlow/Microsoft.Maui.DevFlow.Agent.Core/CometViewResolver.cs	Adds reflection-based text extraction fallback (TryExtractText) for Comet views.

Copilot's findings

Comments suppressed due to low confidence (1)

src/DevFlow/Microsoft.Maui.DevFlow.Agent.Core/VisualTreeWalker.cs:1284

• Fallback visibility check only treats Visibility.Collapsed as not visible. If IView.Visibility can also be Hidden, this would currently report hidden (non-displayed) views as visible. Consider using iView.Visibility == Visibility.Visible (or explicitly excluding both Hidden and Collapsed) when falling back to the MAUI visibility value.

            var platformVisible = ResolveIViewPlatformVisibility(iView);
            info.IsVisible = platformVisible ?? (iView.Visibility != Visibility.Collapsed);

• Files reviewed: 3/3 changed files
• Comments generated: 8

[Copilot]

The zero-size visibility check uses Width <= 0 && Height <= 0 (iOS and Android paths). This will treat views with one dimension <= 0 as visible. If the intent is to treat any zero dimension as not visible, use || (or align with the > 0 && > 0 checks used elsewhere in this file).

[Copilot]

break here stops trying other GetEnvironment overloads even when the current call returns null/empty. Consider continuing the loop unless a non-empty string was found, so additional overloads can be attempted.

Suggested change

	break;
	continue;

[Copilot]

New IView visibility/window-bounds behavior isn’t covered by existing unit tests. Consider adding a net10.0 test that walks a minimal IVisualTreeElement implementing IView with a test VisualTreeWalker subclass overriding the new virtuals, to verify (1) platform visibility precedence vs IView.Visibility, and (2) bounds/window-bounds handling for IView elements.

[Copilot]

This #if block handles iOS/Android/Windows but not MACOS, even though the Agent targets net10.0-macos and this file already has AppKit implementations elsewhere. As a result, IView/Comet visibility on macOS will always fall back to the base behavior. Consider adding an AppKit NSView visibility/alpha/size check here.

Suggested change

	}
	}
	#elif MACOS
	if (platformView is NSView nsView)
	{
	if (nsView.Hidden) return false;
	if (nsView.AlphaValue <= 0) return false;
	if (nsView.Bounds.Width <= 0 && nsView.Bounds.Height <= 0) return false;
	return true;
	}

[Copilot]

This native-info population for IView doesn’t include a #elif MACOS branch. On macOS, Comet/IView elements will miss native type/property details even though VisualElement native info is supported on macOS elsewhere in this file. Consider adding an AppKit NSView case here for parity.

[Copilot]

This preprocessor block lacks a #elif MACOS case, so IView/Comet elements won’t get window-absolute bounds on macOS even though ResolveWindowBounds(VisualElement) supports AppKit above. Consider adding an NSView implementation here for parity.

Suggested change

	}
	}
	#elif MACOS
	if (platformView is NSView nsView && nsView.Window != null)
	{
	var windowRect = nsView.ConvertRectToView(nsView.Bounds, null);
	return new BoundsInfo
	{
	X = windowRect.X,
	Y = windowRect.Y,
	Width = windowRect.Width,
	Height = windowRect.Height
	};
	}

[Copilot]

The comment says this “falls back to IView.Visibility if platform check is unavailable”, but the implementation will also override a collapsed IView with true when ResolveIViewPlatformVisibility returns true. Consider combining both signals (treat Visibility.Collapsed as not visible regardless of platform result), or adjust the comment to match the intended precedence.

This issue also appears on line 1283 of the same file.

Suggested change

	// Falls back to IView.Visibility if platform check is unavailable
	var platformVisible = ResolveIViewPlatformVisibility(iView);
	info.IsVisible = platformVisible ?? (iView.Visibility != Visibility.Collapsed);
	// Combine it with IView.Visibility so collapsed views are always treated as not visible.
	var isCollapsed = iView.Visibility == Visibility.Collapsed;
	var platformVisible = ResolveIViewPlatformVisibility(iView);
	info.IsVisible = !isCollapsed && (platformVisible ?? true);

[Copilot]

This method does multiple per-call reflection lookups (e.g., GetProperty("Value") / GetProperty("Text")). Since it may run across many elements during a tree walk, consider caching reflection metadata per Comet view type to reduce overhead.

[github-actions]

Expert Code Review — IView Visibility/Bounds Resolution for Comet

Methodology: 3 independent reviewers with adversarial consensus. Findings included only when ≥2/3 reviewers agreed (disputed findings verified via targeted follow-up).

CI Status

Builds are in progress (macOS + Windows matrix). CLA check passed. No test failures observed yet.

Test Coverage Assessment

No new tests added for the 3 new virtual methods or TryExtractText. The ResolveIViewWindowBounds, ResolveIViewPlatformVisibility, and PopulateIViewNativeInfo overrides require platform-specific testing (devices/simulators), which is noted as out of scope per repo conventions. The TryExtractText reflection logic in CometViewResolver could benefit from unit tests with mock objects.

---

Findings (ranked by severity)

#	Severity	Consensus	File	Finding
1	🟡 MODERATE	3/3	Agent/VisualTreeWalker.cs	Missing #elif MACOS in all 3 new IView overrides — macOS Comet views lose bounds, visibility, and native properties
2	🟡 MODERATE	3/3 (after follow-up)	Agent.Core/VisualTreeWalker.cs:1284	Visibility.Hidden incorrectly reported as visible — fallback uses != Collapsed instead of == Visible
3	🟡 MODERATE	3/3 (after follow-up)	Agent/VisualTreeWalker.cs:360-373	Windows PopulateIViewNativeInfo extracts only 1 of 10 native properties — automationId, isPassword, etc. silently omitted
4	🟡 MODERATE	2/3	Agent.Core/CometViewResolver.cs:398	Premature break in GetEnvironment loop skips valid overloads when first match has unknown param count
5	🟡 MODERATE	2/3	Agent/VisualTreeWalker.cs:291,299	Zero-size bounds used as visibility proxy — causes false negatives during layout transitions on iOS/Android
6	🟢 MINOR	3/3 (after follow-up)	Agent.Core/CometViewResolver.cs:379-381	Uncached reflection + LINQ allocation per TryExtractText call (rest of class caches reflection metadata)
7	🟢 MINOR	2/3	Agent.Core/CometViewResolver.cs:377	TryExtractText is public static but doesn't guard IsCometAvailable() — fragile for future callers

Discarded Findings (single-reviewer only, no consensus)

• Near-identical code duplication between VisualElement and IView platform methods (1/3)
• Generic Binding<T> properties unwrapped via .ToString() without verifying T == string (1/3)

Generated by Expert Code Review (auto) for issue #174 · ● 8.9M

[github-actions]

🟢 MINOR · 3/3 reviewers (after follow-up) · Uncached reflection + LINQ allocation per call

Every call to TryExtractText for a Comet view executes GetMethods() + .Where().ToList(). The rest of CometViewResolver caches reflection metadata in static fields (_cometViewType, _getViewMethod, _builtViewProperty, etc.) initialized once in IsCometAvailable(). During a full tree walk with many Comet views, this creates unnecessary GC pressure.

Suggestion: Cache the GetEnvironment method lookup in a static field (e.g., private static MethodInfo[]? _getEnvironmentMethods;) initialized alongside the other cached members.

[github-actions]

🟡 MODERATE · 3/3 reviewers (after follow-up) · Windows PopulateIViewNativeInfo omits most native properties

The existing PopulateNativeInfo for Windows (line ~81) extracts 10 properties: automationId, automationName, helpText, name, visibility, isHitTestVisible, isEnabled, isTabStop, isReadOnly, isPassword. This new method only extracts automationName.

The iOS and Android blocks in PopulateIViewNativeInfo closely mirror their PopulateNativeInfo counterparts, but the Windows block was left sparse. Comet IView elements on Windows will silently lose diagnostically important data like automationId (critical for test automation targeting), isPassword, and isReadOnly.

Suggestion: Extract the same set of Windows properties that PopulateNativeInfo does, or factor the Windows property extraction into a shared helper that both methods call.

[github-actions]

🟡 MODERATE · 3/3 reviewers · Missing #elif MACOS branch in all three new IView overrides

ResolveIViewWindowBounds, ResolveIViewPlatformVisibility, and PopulateIViewNativeInfo handle iOS/Catalyst, Android, and Windows — but none include a #elif MACOS branch. The existing ResolveWindowBounds(VisualElement) (line ~205) and PopulateNativeInfo (line ~55) both have macOS/AppKit handling with NSView coordinate flipping and accessibility extraction.

On macOS, Comet IView elements will get null WindowBounds, fall back to the Visibility.Collapsed-only check (missing NSView.Hidden/AlphaValue), and emit no native properties.

Suggestion: Add #elif MACOS branches mirroring the existing NSView logic, e.g.:

#elif MACOS
if (platformView is AppKit.NSView nsView && nsView.Window?.ContentView != null)
{
    var windowRect = nsView.ConvertRectToView(nsView.Bounds, nsView.Window.ContentView);
    var contentHeight = nsView.Window.ContentView.Bounds.Height;
    return new BoundsInfo
    {
        X = windowRect.X,
        Y = contentHeight - windowRect.Y - windowRect.Height, // flip Y axis
        Width = windowRect.Width,
        Height = windowRect.Height
    };
}

[github-actions]

🟡 MODERATE · 2/3 reviewers · Zero-size bounds used as visibility proxy causes false negatives

On iOS, uiView.Bounds gives the view's own coordinate space — a correctly laid-out view can report zero Bounds before layoutSubviews completes. On Android (line 299), Width/Height are 0 until onMeasure/onLayout runs. A freshly-added IView will be transiently reported as invisible even though it's logically visible. Windows (line 305) doesn't have this check, creating cross-platform inconsistency.

The Hidden/Alpha checks are reliable visibility signals; the zero-size check adds false negatives during layout transitions.

Suggestion: Remove the zero-size checks from this method — restrict to actual visibility flags (Hidden, Alpha, Visibility). Geometry checks belong in bounds/hit-test logic, not visibility determination.

[github-actions]

🟡 MODERATE · 2/3 reviewers · Premature break skips valid GetEnvironment overloads

If Comet adds a 3+ parameter GetEnvironment overload that sorts first in the reflection list, MakeGenericMethod succeeds but paramCount matches neither == 1 nor == 2, leaving result as null. The break then exits the loop — the 1- or 2-parameter overload that would have returned text is never tried.

Suggestion: Add an else continue for unrecognized parameter counts:

if (paramCount == 1)
    result = genericMethod.Invoke(cometView, new object[] { "Text.Value" });
else if (paramCount == 2)
    result = genericMethod.Invoke(cometView, new object[] { "Text.Value", false });
else
    continue; // unknown overload shape; try the next one

if (result is string envText && !string.IsNullOrEmpty(envText))
    return envText;
break; // invocation succeeded, no need to try others

[github-actions]

🟢 MINOR · 2/3 reviewers · TryExtractText doesn't guard IsCometAvailable()

This method is public static and uses _cometViewType (set by IsCometAvailable()). The current call site is safe (guarded by cometResolved != null), but any future caller that skips TryResolveCometView will silently skip the environment-based text path since _cometViewType will be null.

Suggestion: Add if (!IsCometAvailable()) return null; at the top, matching the defensive pattern used in TryResolveCometView (line ~124) and GetEnvironmentKeys (line ~420).

[github-actions]

🟡 MODERATE · 3/3 reviewers (after follow-up) · Visibility.Hidden incorrectly reported as visible

The fallback iView.Visibility != Visibility.Collapsed evaluates to true for Visibility.Hidden, but Hidden means the view is not rendered (only occupies layout space). The VisualElement path (line ~1256) uses ve.IsVisible, which returns false for both Hidden and Collapsed.

When platform visibility returns null (no handler, macOS, GTK), any Comet view with Visibility.Hidden will be erroneously reported as visible — inconsistent with the VisualElement path.

Suggestion:

info.IsVisible = platformVisible ?? (iView.Visibility == Visibility.Visible);