[WinUI][CV2] Fix for Null Item Handling - follow-ups from CollectionView2 Windows handler (#34600)

[SuthiYuvaraj]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Issue Description

Null-item false selection

RootCause
UpdateVisualStates was incorrectly computing selection state for null items. A null item could appear visually selected or deselected incorrectly because the container-to-item lookup did not account for null values.

Fix Description
Added a null guard so null items are checked by container identity rather than item equality.

Grouped null-Replace desync

RootCause
When a grouped source fired an INCC Replace with a null new-item, HandleGroupItemsReplace skipped creating a new ItemTemplateContext2 for that slot. This left the flat-list mirror out of sync with the source.

Fix Description
Fixed by always creating a fresh ItemTemplateContext2(template, null) for null replacement items instead of skipping.

Null drag stale index

RootCause
During drag-and-drop reorder, if GetContainerIndex failed to resolve the dragged container (returning -1), the stale index was used in subsequent drop calculations causing an incorrect reorder or index out of range.

Fix Description
Added a reset fallback: if _draggedSourceIndex < 0 the drag state is cleaned up rather than proceeding with a bad index.

Issues Fixed

Fixes #36068

Tested the behaviour in the following platforms

• Android
• Windows
• iOS
• Mac

Output Screenshot

Before Issue Fix	After Issue Fix
BeforenullItem.mov	CV2AfterFix.mov

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36170

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36170"

[MauiBot]

Expert Review — 4 findings

See inline comments for details.

[MauiBot]

[moderate] Handler Mapper and Property Patterns — _ignoreVirtualSelectionChange and the temporary SelectionChanged unsubscribe are not restored in a finally block. If ItemsView.SelectedItem = selectedItem throws from a bindable-property callback, validation path, or user PropertyChanged handler, the handler stays with SelectionChanged detached and _ignoreVirtualSelectionChange == true, so subsequent virtual/platform selection changes are silently suppressed for the handler lifetime. Wrap the assignment in try/finally that always reattaches VirtualSelectionChanged and resets _ignoreVirtualSelectionChange.

[MauiBot]

[major] Logic and Correctness — Treating any blank ItemContainer as bound to null makes a null-item drag match every realized null row, not just the source row. During live reorder, ItemsRepeater_ElementPrepared calls IsContainerBoundToDraggedItem after ApplyDragAffordance, so with two null items all blank null containers get Opacity = 0 and IsHitTestVisible = false. Use the instance state available there (for example container.Tag is int t && t == _draggedSourceIndex, or ItemsRepeater.GetElementIndex) for blank/null containers instead of value matching on null.

[MauiBot]

[major] Logic and Correctness — Allowing _draggedItem == null into grouped reorder still leaves source-group discovery based on value equality (ReferenceEquals(groupItem, _draggedItem) || Equals(groupItem, _draggedItem)). In grouped data with more than one null item, this picks the first null in the first matching group rather than the row whose container started the drag, so dragging Group B[0] == null can move Group A's null item instead. For null items, map _draggedSourceIndex (the captured flat row index, accounting for group headers/footers) back to source group/item instead of searching by value.

[MauiBot]

[major] Logic and Correctness — The tag validation now accepts Equals(null, null), so a stale Tag on a null-item container is considered valid whenever the old index also contains any null item. If that check does not return, the new linear-search fallback has the same ambiguity (IndexOfItem(null, sourceList) returns the first null). After a reorder or recycled-container handoff, a second drag on a later null row can therefore capture the wrong _draggedSourceIndex and move the wrong data item. Use ItemsRepeaterControl.GetElementIndex(container as ItemContainer) for null/blank containers before any value-equality fallback.

[Copilot]

Pull request overview

This PR addresses several edge cases in the WinUI CollectionView2 (Items2) handler when the bound ItemsSource contains null elements, focusing on selection visual state correctness, grouped Replace consistency, and drag-and-drop reorder robustness.

Changes:

• Adjust selection-state computation to avoid false “Selected” visuals when item data is null.
• Fix grouped INCC Replace handling so null replacement items still get a fresh ItemTemplateContext2 (keeping the flattened mirror in sync).
• Improve drag/drop reorder handling around null items by using _draggedSourceIndex as the drag-active sentinel and updating index/tag handling for null rows.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
src/Controls/src/Core/Handlers/Items2/Windows/MauiItemsView.DragDrop.cs	Updates drag/drop sentinels and container index/tag logic to correctly support null items during reorder.
src/Controls/src/Core/Handlers/Items2/Windows/ItemFactory.cs	Adds a null-item fast path to render blank rows and align row sizing behavior with CV1 expectations.
src/Controls/src/Core/Handlers/Items2/Windows/GroupedItemTemplateCollection2.cs	Ensures grouped Replace always creates a new ItemTemplateContext2 even when the new item is null.
src/Controls/src/Core/Handlers/Items2/CollectionViewHandler2.Windows.cs	Prevents WinUI null selection from being immediately undone by suppressing the SelectedItem round-trip during platform-driven selection updates.

[MauiBot]

Expert Review — 6 findings

See inline comments for details.

[MauiBot]

Expert Review — 15 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[critical] Regression Prevention - This using block breaks the Windows DeviceTests compilation, so neither new test can ever run.

1. using Microsoft.UI.Xaml.Controls; (this line) collides with the pre-existing using Microsoft.Maui.Controls; on line 7. This file already constructs new Grid() on line 170 and new Grid { ... } on line 364, and Grid exists in both namespaces -> CS0104: 'Grid' is an ambiguous reference between 'Microsoft.Maui.Controls.Grid' and 'Microsoft.UI.Xaml.Controls.Grid'. The file's existing convention already solves this for the WinUI types it needs (WItemsView, WSetter aliases on lines 18-19) - do the same for ItemContainer (e.g. using WItemContainer = Microsoft.UI.Xaml.Controls.ItemContainer;) instead of importing the whole namespace.
2. ConfigureMauiHandlers (used on lines 435 and 479) is an extension method in Microsoft.Maui.Hosting (HandlerMauiAppBuilderExtensions). Usings are per-file, and the using Microsoft.Maui.Hosting; the rest of this partial class relies on lives in CollectionViewTests.cs, not here -> CS1061 on builder.ConfigureMauiHandlers(...). Add using Microsoft.Maui.Hosting; to this file.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness / Regression Prevention - This guard is too broad and breaks the most common MAUI selection idiom on Windows CV2.

Element.OnBindablePropertySet deliberately defers UpdateHandlerValue until after property.PropertyChanged fires (see Element.cs:694-703 and the comment at Element.cs:711-719). SelectableItemsView raises the public SelectionChanged event from that PropertyChanged callback. So everything the app does inside its SelectionChanged handler executes inside the _ignoreVirtualSelectionChange window opened in UpdateVirtualSingleSelection, and MapSelectedItem for both the outer and any nested write is suppressed.

Concrete failing scenario - the canonical tap-then-clear pattern:

cv.SelectionChanged += (s, e) => { Navigate(e.CurrentSelection[0]); cv.SelectedItem = null; };

1. User taps Item 1 -> PlatformSelectionChanged -> UpdateVirtualSingleSelection sets _ignoreVirtualSelectionChange = true.
2. ItemsView.SelectedItem = Item 1 -> SelectionChanged -> app sets SelectedItem = null -> nested MapSelectedItem suppressed.
3. Outer MapSelectedItem suppressed too (still inside the try).
4. UpdatePlatformSelection() never runs, so PlatformView.DeselectAll() never runs. The WinUI ItemContainer stays visually selected and PlatformView.SelectedItem stays non-null.
5. Tapping the same row again produces no WinUI SelectionChanged, so the app handler never fires for that item again.

Before this PR both writes reached UpdatePlatformSelection and the platform was correctly deselected.

Suggested fix: make the guard value-scoped rather than time-scoped - record the value being written (e.g. _pendingVirtualSelection) and in MapSelectedItem only skip when ReferenceEquals(itemsView.SelectedItem, pending); otherwise fall through to UpdatePlatformSelection() so re-entrant app writes still propagate. Also save/restore the previous flag value in the finally on line 348 rather than hard-assigning false, so a nested UpdateVirtualSingleSelection cannot clear the outer frame's guard early.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Cross-Platform Behavioral Consistency - The new flag is only honoured for SelectionMode.Single. UpdateVirtualMultipleSelection does not set it, MapSelectedItems has no equivalent guard, and ExtractPlatformSelectedItems() explicitly drops nulls (if (selectedItem is not null) result.Add(...), line 427).

So in SelectionMode.Multiple a null row can now be checked in the WinUI UI (it gets a real, selectable ItemContainer from the new ItemFactory path) but it never lands in SelectedItems, and the first subsequent MapSelectedItems -> UpdatePlatformSelection -> DeselectAll() visually undoes the check. That is exactly the round-trip this PR fixes for Single mode, left in place for Multiple.

Either extend the fix to multi-select or explicitly document/assert that null items are not selectable in Multiple mode, and add a device test pinning the chosen behaviour.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness - Replacing the value-equality search with pure flat-index arithmetic makes grouped reorder depend on assumptions that do not hold for every grouped configuration, and it removes the safety net the flat path still has.

1. The flat source does not always contain header/footer rows. hasHeaders/hasFooters are derived from GroupHeaderTemplate/GroupFooterTemplate, but _draggedSourceIndex indexes the repeater's flat source (GetContainerIndex -> GetSourceList() -> ItemsSource). That flat source only contains header/footer entries when ItemsViewHandler2.CreateCollectionViewSource takes the TemplatedItemSourceFactory2.CreateGrouped branch (ItemsViewHandler2.Windows.cs:347-355). When IsGrouped=true and GroupHeaderTemplate is set but ItemTemplate is null, it falls through to FlattenGroupedItemsSource(itemsSource) (ItemsViewHandler2.Windows.cs:380-385), which emits no header/footer rows. flatSrcPos then over-counts by one per group and this comparison resolves to the wrong (sourceGroupIndex, sourceItemIndex) - the drop moves a different row.
2. String groups desync the walk. GroupedItemTemplateCollection2.RebuildFlatList skips a group when group is string || group is not IEnumerable (GroupedItemTemplateCollection2.cs:129-130). The guard on line 873 only skips not IEnumerable. A string group is IEnumerable<char>, so it contributes 1 header + N chars + 1 footer to flatSrcPos that the real flat list never contained, and every later group maps to the wrong index. The pre-existing target-mapping loop at line 916 has the same gap.
3. No validation or fallback. PerformReorder validates _draggedSourceIndex against the live item and falls back to IndexOfItem when it is stale (line 765-777); this grouped path now trusts the index unconditionally. A collection change between DragStarting and Drop silently moves the wrong item.

Suggest deriving the group/item position from the flat collection that _draggedSourceIndex actually indexes (walk GetSourceList() / the GroupedItemTemplateCollection2) instead of re-deriving it from groupsList + template flags, and validating the resolved item against _draggedItem when _draggedItem is not null before mutating.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Null Safety and Defensive Coding - Swapping the drag-in-progress sentinel from _draggedItem is not null to _draggedSourceIndex >= 0 is right for null rows, but _draggedSourceIndex is now assigned from a fallible lookup: _draggedSourceIndex = GetContainerIndex(itemContainer) (line 453), and GetContainerIndex can return -1 (the new null branch at line 1252 returns -1 when GetElementIndex fails and Tag is not an int; the non-null path can also fall through to allContainers.IndexOf(container) returning -1).

When that happens DragStarting has already returned without setting args.Cancel, so WinUI starts a real drag, but every downstream guard (ItemsRepeater_ElementPrepared lines 326/335, DimNonSourceContainers line 1767, this Drop guard, PerformReorder line 755, PerformGroupedReorder line 854) now treats it as no drag in progress: no dim, no source hiding, and the drop is silently discarded. Previously a non-null _draggedItem kept all of those alive.

Guard at the source - after computing the index in DragStarting, cancel the drag if it is negative:

_draggedSourceIndex = GetContainerIndex(itemContainer);
if (_draggedSourceIndex < 0) { args.Cancel = true; return; }

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention - The Replace-with-null desync fix here is correct (previously oldItems.Count != newItems.Count in the raised NotifyCollectionChangedEventArgs and the stale ItemTemplateContext2 was left in the flat list), but nothing in this PR exercises it.

The grouped path received two behaviour-affecting changes in this PR - this one and the switch of PerformGroupedReorder to positional _draggedSourceIndex matching (MauiItemsView.DragDrop.cs:862-900) - yet every added test (2 device tests, 2 UI tests, 1 HostApp page) uses a flat, non-grouped source. Grouped + null items with group headers/footers is precisely the case the new flat-index arithmetic was written for (see the comment on line 864 about multiple null items across groups), and it is completely uncovered.

Please add at least: (a) a test that replaces a group item with null and asserts the flat collection stays in sync in Items.Count and index positions, and (b) a grouped variant of the HostApp reorder page with a null item in a group so the header/footer offset arithmetic in PerformGroupedReorder is exercised.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention - This test passes with and without the _ignoreVirtualSelectionChange fix, so it does not guard the regression.

collectionView.SelectedItem is null before the tap. UpdateVirtualSingleSelection then assigns ItemsView.SelectedItem = null (the unwrapped null item). BindableObject.SetValueActual short-circuits on sameValue (Equals(null, null) is true and SelectedItemProperty does not use RaiseOnEqual), so no PropertyChanged fires, UpdateHandlerValue is never called, and MapSelectedItem never runs - with or without the new guard. DeselectAll() can therefore never undo the selection in this scenario.

The real failing scenario requires a transition to null: select a real item first, then select the null row, so SelectedItem changes Item 1 -> null and MapSelectedItem -> UpdatePlatformSelection -> DeselectAll() fires. Please add that step:

containers[0].IsSelected = true;              // SelectedItem = Item 1
await Task.Delay(100);
containers[1].IsSelected = true;              // SelectedItem: Item 1 -> null (real change)
await Task.Delay(100);
Assert.True(containers[1].IsSelected);        // fails on main, passes with the fix

Also note Assert.Null(collectionView.SelectedItem) and Assert.False(nullContainer.IsSelected) (line 540, immediately after setting it to false) are non-discriminating and pass trivially.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention - NullItem_RendersBlankRow also passes without the ItemFactory change, so it does not pin the new behaviour.

Before this PR a null item still produced an ItemContainer: GetElement fell through to the normal path, created the ElementWrapper, and set view.BindingContext = templateContext.Item (null). The Label has HeightRequest = 40, so bounds.Height > 0 held then too. The assertion cannot distinguish the old realized-template row from the new blank row.

Two gaps to close:

1. Assert what actually changed - the container is blank: Assert.Null(nullContainer.Child); (the new path returns an ItemContainer with no ElementWrapper), and optionally that its height matches the new MinHeight = 32 rather than the template height.
2. containers[1] is never verified to be the null row. GetChildren<ItemContainer>() is a VisualTreeHelper walk; ItemsRepeater child order is not guaranteed to follow item index once containers are recycled. Unlike the selection test, nothing here fails loudly if the index is wrong - the test silently measures a real item. Resolve the container via ItemsRepeaterControl.GetElementIndex/TryGetElement(1) or by checking Child is null.

Also, per repo test guidance the fixed await Task.Delay(500) on lines 459/510 should be a deterministic wait (AssertEventually/WaitForMainThread) - arbitrary sleeps are the main source of CV flakiness on Windows CI.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention - These two tests are order-dependent and the second one will fail.

UITestBase.ResetAfterEachTest defaults to false (src/TestUtils/src/UITest.NUnit/UITestBase.cs:12) and this class does not override it, so FixtureSetup() -> TryToResetTestState() -> NavigateToIssue(...) runs only once in [OneTimeSetUp]. Both tests therefore share the same live page and the same ObservableCollection instance.

NUnit runs methods in alphabetical order, so DraggingNullItemRowDoesNotCrashAndReorders mutates Items first; this assertion then sees the already-reordered text and fails with e.g. Expected: 'Item A, null, Item B, Item C' But was: 'Item A, Item B, Item C, null'. The derived nullRowY (line 63) is also computed from an ordering that no longer holds.

Fix by either adding protected override bool ResetAfterEachTest => true;, adding a reset button/[SetUp] that restores Items, or merging the two scenarios into a single test that asserts the order after each drag.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention - The assertions cannot discriminate the bug this PR fixes.

The null row has no AutomationId (it renders as a bare ItemContainer with no template content), so the test infers its position from the gap between ReorderItemLabel0 and ReorderItemLabel2 (line 32). That gap also contains Item A's Padding=16 bottom, its Margin=0,0,0,8, and Item B's top padding, so the drag can easily grab Item A instead of the blank row.

With the current checks - Is.Not.EqualTo(initialOrder), Has.Length.EqualTo(4), Does.Contain('null') - dragging the wrong row still changes the order, still yields 4 entries, and still contains null, so the test passes while the null-row drag path is broken. This is the exact failure mode the GetContainerIndex/UpdateAllContainerIndices -> GetElementIndex work exists to prevent.

Assert the exact expected result instead, e.g. Assert.That(reorderedText, Is.EqualTo("Item A, Item B, Item C, null")) here and the corresponding exact order on line 73. Consider also adding a second null entry to the fixture - with a single null, a first null wins fallback bug is indistinguishable from correct behaviour - and giving the blank row a locatable anchor rather than deriving coordinates from neighbouring labels.

[kubaflo]

/review -b improved-reviewer -p windows

[MauiBot]

Expert Review — 12 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[critical] Logic and Correctness — This using Microsoft.UI.Xaml.Controls; breaks compilation of the Windows DeviceTests project. The file already has using Microsoft.Maui.Controls;, and both namespaces export types named SelectionMode (WinUI''s ListBox selection enum, confirmed present in Microsoft.WinUI.dll) and Grid. That makes every pre-existing bare use ambiguous → CS0104:

• SelectionMode at lines 46, 88 (pre-existing) and 498 (new)
• Grid at lines 170, 186, 364 (pre-existing)

Nothing in the file or project aliases these away (no using SelectionMode = ..., no global usings). The rest of the file already uses the fully-qualified form for WinUI types (UI.Xaml.Controls.ListView line 199, UI.Xaml.Controls.TextBlock line 209), so the new ItemContainer references should follow that convention or add an alias (using WItemContainer = Microsoft.UI.Xaml.Controls.ItemContainer;) instead of importing the whole namespace.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Handler Mapper and Property Patterns — This guard is broader than the round-trip it is meant to break, and it swallows re-entrant SelectedItem writes made by application code.

SelectableItemsView.SelectedItemPropertyChanged runs the BindableProperty changed callback synchronously inside SetValue, and that callback executes SelectionChangedCommand and raises the public SelectionChanged event (src/Controls/src/Core/Items/SelectableItemsView.cs, SelectionPropertyChanged). So any user handler runs while _ignoreVirtualSelectionChange == true.

Concrete scenario (very common MAUI pattern — clearing selection so the same row can be tapped twice):

1. SelectedItem is "A"; user taps "B".
2. UpdateVirtualSingleSelection sets the flag and assigns SelectedItem = "B".
3. The synchronous SelectionChanged/SelectionChangedCommand handler does cv.SelectedItem = null.
4. That nested write''s MapSelectedItem hits this early return, and so does the outer one.
5. finally clears the flag but performs no reconciliation.

Result: MAUI SelectedItem is null while the WinUI container for "B" stays visually selected, and nothing re-syncs. Before this change, the nested MapSelectedItem → UpdatePlatformSelection → DeselectAll kept the two in sync. Consider scoping the suppression to the single assignment (e.g. compare the value the handler itself wrote and re-run UpdatePlatformSelection in the finally when ItemsView.SelectedItem no longer matches it), or fixing the root cause noted below.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness — The null-row selection this PR enables is not durable, because SelectedItem == null remains ambiguous between "the null item is selected" and "nothing is selected".

UpdatePlatformSelection (line 464) unconditionally does if (ItemsView.SelectedItem is null) PlatformView.DeselectAll();. The new _ignoreVirtualSelectionChange flag only covers the synchronous window of this one assignment, so the very next call to UpdatePlatformSelection from any other path silently drops the selection:

• MapSelectionMode (line 106),
• MapSelectedItems (line 94),
• UpdateItemsSource (line 226) on any items-source refresh,
• the _selectionDirty re-sync in OnPlatformViewLoaded (line 197) when the page is navigated away from and back.

The device test added in this PR only asserts immediately after the tap, so it cannot catch this. Please either make the null-item selection explicitly representable (e.g. track the selected index on the handler and consult it in UpdatePlatformSelection) or document/limit that a null row''s selection is transient.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Cross-Platform Behavioral Consistency — Short-circuiting here means a null data item no longer inflates the ItemTemplate at all on Windows CV2, which diverges from Android and iOS.

On Android (Items/Android) and iOS (Items2/iOS) a null item still realizes the template with a null BindingContext, so a template containing static/non-bound content (a fixed Label, an icon, a FallbackValue/TargetNullValue binding, or a converter that maps null to placeholder text) renders normally. After this change the same app shows an empty 32px strip on Windows only. The comment justifies the behaviour by matching CV1 Windows, but the Items/ handler is not the cross-platform contract — the other two platforms are.

If the goal is only to avoid the crash, the narrower fix is to keep realizing the template (and to guard DataTemplateSelector.SelectTemplate(null, ...) at line 81, which is the call that actually throws for user selectors), rather than dropping the template entirely.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Layout Measure-Arrange Correctness — Hardcoding MinHeight = 32 / MinWidth = 88 for the blank container is risky for GridItemsLayout.

CreateGridView only sets MinItemWidth/MinItemHeight when the CollectionView has an explicit cross-axis WidthRequest/HeightRequest (ApplyMinItemSizeForSpan returns early otherwise, ItemsViewHandler2.Windows.cs:813). With those left at NaN, WinUI''s UniformGridLayout derives the uniform cell size from the first realized element. A collection whose first item is null therefore sizes every cell in the grid to 88×32, clipping all real items.

Please either exclude blank containers from the layout''s size derivation, or size the blank container from the measured/cached item size (CollectionViewHandler2.GetCachedFirstItemSize) rather than from two magic numbers. At minimum, add a GridItemsLayout + leading-null test — no test in this PR covers a grid layout.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness — Now that every downstream guard was switched from _draggedItem is not null to _draggedSourceIndex >= 0 (lines 326, 335, 672, 755, 854, 1766), DragStarting must also validate the index it captures.

_draggedSourceIndex = GetContainerIndex(itemContainer) (line 453) can return -1: the new null-item branch returns -1 when GetElementIndex fails and Tag is not an int, and the final fallback returns allContainers.IndexOf(container) which is -1 for a container not found in the visual-tree walk. In that case the drag is not cancelled — args.Cancel is left false and the deferred callback at line 470 still sets _sourceContainer.Opacity = 0 — but ScrollViewer_Drop immediately bails on _draggedSourceIndex < 0, so the user sees the row vanish, drags, drops, and nothing reorders. Add if (_draggedSourceIndex < 0) { args.Cancel = true; return; } after the index is captured.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — This rewrites source resolution for all grouped reorders (not just null items) from value-equality to flat-index arithmetic, yet the PR adds no grouped test at all — the sample page, the UI tests and both device tests are flat/ungrouped.

Two behavioural risks introduced by the arithmetic:

1. It now depends on _draggedSourceIndex being in the same flat index space as the repeater. That holds only because GetSourceList() prefers the WinUI-side ItemsSource (the GroupedItemTemplateCollection2); if that ever falls back to _mauiVirtualView.ItemsSource (line 1059), the index is a group index and this loop resolves the wrong row.
2. The skip condition diverges from the flat list builder: GroupedItemTemplateCollection2.RebuildFlatList skips group is string || group is not IEnumerable (line 129), while this loop only skips is not IEnumerable. A string group is not represented in the flat list but is counted here, desynchronising every subsequent index.

Please add a grouped drag-reorder regression test (with and without GroupHeaderTemplate/GroupFooterTemplate) covering both a null item and a normal item, since the previous value-equality path is being removed.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[minor] Complexity Reduction — This branch is unreachable from the only caller and over-matches if it ever becomes reachable.

IsContainerBoundToDraggedItem (line 568) already returns via the container.Child is not ElementWrapper Tag comparison before delegating here, so the "blank container (no ElementWrapper)" case described in the comment can never reach this code. The condition that does reach it is Child is ElementWrapper but VirtualView is not View — a partially realized container — and returning item is null there means every such container is treated as the drag source (opacity 0, hit-testing off) during a null-item drag. Either delete the branch or restrict it to container.Child is null.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention and Test Coverage — These assertions cannot discriminate the scenario under test from a completely different drag, in both test methods (lines 39-44 and 74-79).

The drag origin is derived geometrically (nullRowY = (itemARect.Bottom + itemBRect.Top) / 2f, line 32) from two label rects, so the grab point depends on the HeightRequest="70" grid, the Margin="0,0,0,8", the label font size, and the new hardcoded MinHeight = 32 on the blank container in ItemFactory. If any of those shift and the press lands on "Item A" or "Item B" instead of the blank row, all three assertions still pass: Is.Not.EqualTo(initialOrder) is satisfied by any reorder, Has.Length.EqualTo(4) is satisfied by every non-destructive move, and Does.Contain("null") is satisfied as long as the null entry still exists anywhere. The test would go green while never exercising the null-row drag path this PR fixes.

Assert the exact expected string (e.g. Is.EqualTo("Item A, Item B, Item C, null") for the first test and the corresponding order for the second), so the test fails when the wrong row is dragged or the null row lands in the wrong slot.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — NullItem_RendersBlankRow cannot fail for the reason it is named.

Three problems:

1. containers[1] assumes GetChildren<ItemContainer>() returns the containers in item order. That is visual-tree child order from ItemsRepeater, which realizes and recycles elements in arbitrary order — there is no guarantee index 1 is the null row. ItemsRepeaterControl.GetElementIndex (which this PR itself relies on as "the authoritative repeater position") is the correct way to locate it.
2. Assert.True(bounds.Height > 0) is vacuous: the fix hardcodes MinHeight = 32 on the blank container and every other container here holds a 40px Label, so the assertion passes for any of the three containers.
3. Nothing asserts the row is actually blank (no ElementWrapper/no template content), which is the behaviour being introduced.

Also replace await Task.Delay(500) (line 459) with the deterministic helpers used elsewhere in this project (AssertHelpers / WaitForUIUpdate) — arbitrary delays are a known source of device-test flakiness.

[kubaflo]

Could you please check the failing tests and ai's suggestions?

[MauiBot]

Expert Review — 15 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[critical] Regression Prevention and Test Coverage — Adding using Microsoft.UI.Xaml.Controls; to this file breaks compilation of the Windows Controls.DeviceTests project. The namespace collides with Microsoft.Maui.Controls types already imported here, producing CS0104 ambiguities for SelectionMode, Grid, and ColumnDefinition used by the pre-existing tests in this file, and the file is also missing the namespace required for ConfigureMauiHandlers. Concrete scenario: dotnet build src/Controls/tests/DeviceTests -f net10.0-windows... fails with 14 errors, so none of the CollectionView Windows device tests (pre-existing or the two added here) can run — the new tests provide zero regression signal and the project is left red. Fix by aliasing (e.g. using WItemContainer = Microsoft.UI.Xaml.Controls.ItemContainer;) instead of importing the whole WinUI namespace, and add the missing Microsoft.Maui.Hosting using.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — containers[1] assumes the visual-child enumeration order of the ItemsView/ItemsRepeater matches item order. ItemsRepeater recycles and re-parents realized containers, so GetChildren<ItemContainer>() order is not guaranteed to be index order (and unrealized slots are absent). Concrete scenario: after the first recycle pass, containers[1] is the container for "Item 3", so NullItem_RendersBlankRow asserts non-zero height on a templated row and passes even if the null row is collapsed — the test cannot discriminate the bug it was written for. Resolve the container via ItemsRepeaterControl.TryGetElement(1)/GetElementIndex instead of positional child order. The await Task.Delay(500) above also has no justification — prefer a deterministic wait (AssertEventually/WaitForMainThread) so the test is not timing-dependent on slower Helix agents.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness Verification — NullItem_TapDoesNotCrash_SingleSelection drives selection by setting ItemContainer.IsSelected = true directly, which is not the platform tap path this fix targets: a real tap goes through ItemsView.SelectionChanged → PlatformSelectionChanged → UpdateVirtualSingleSelection. Setting IsSelected on the container may not raise the ItemsView.SelectionChanged the handler subscribes to, in which case _ignoreVirtualSelectionChange/UpdateVirtualSingleSelection is never entered and the assertions (SelectedItem is null, container stays selected) hold trivially — including on the unfixed code. Combined with the containers[1] ordering assumption above, this test does not prove the round-trip guard works. Assert against the handler-level round trip (e.g. raise selection via the platform ItemsView.Select(index) API and verify UpdatePlatformSelection was not re-entered).

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention and Test Coverage — This UI test does not exercise the fixed code path. Gate evidence for this PR shows both tests here fail with the fix applied and leave the order at Item A, null, Item B, Item C; the sibling test DraggingRealItemOntoNullItemRowDoesNotCrashAndReorders, whose drag source is a normal non-null row and therefore does not depend on any null handling, fails identically. That control result indicates App.DragCoordinates does not initiate the WinUI/OLE drag pipeline, so ItemContainer_DragStarting (and hence PerformReorder) is never reached. The PR therefore ships a drag-reorder fix with no executing regression test on any platform. Either drive the reorder through an API-level path that actually raises DragStarting, or replace these with device tests that invoke the handler's drag entry points directly.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — The assertions (Is.Not.EqualTo(initialOrder), length == 4, Does.Contain("null")) cannot distinguish a correct reorder from a wrong-slot reorder. Concrete scenario: if the drag resolves to the wrong source row (the first null instead of the dragged one, or an adjacent item — the exact failure mode the flat-index changes in MauiItemsView.DragDrop.cs can produce), the resulting order is still 4 items, still contains null, and still differs from the initial order, so the test passes. Assert the exact expected final string (e.g. "Item A, Item B, Item C, null"), which is the only assertion that proves the intended slot moved.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Cross-Platform Behavioral Consistency — MinHeight = 32 / MinWidth = 88 are hard-coded and ignore both the ItemsLayout/ItemSizingStrategy and the surrounding rows' measured size. Concrete scenario: the sample page added by this PR uses HeightRequest="70" rows, so the null row renders at 32px — a visibly different row height than every neighbour, and different from CV1, whose blank row inherits the ListViewItem default rather than a fixed 32/88. Derive the blank container's size from the cached first-item measurement (_firstItemMeasuredSize, already tracked for MeasureFirstItem) or leave it unset and let the layout size it, rather than introducing two magic constants.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Handler Mapper and Property Patterns — Blank containers are pooled without resetting the mutable state that ApplyDragAffordance writes to them (Tag, CanDrag, the DragStarting subscription, and the local Background card brush). That reset lives in ItemsRepeater_ElementClearing, which early-returns when _canReorderItems is false. Concrete scenario: user sets CanReorderItems = false (or the handler updates it) while blank rows are realized — those containers are cleared without the reset, land in _nullItemPool, and are handed back by GetElement still carrying the Fluent card Background, a stale int Tag, CanDrag = true, and a live ItemContainer_DragStarting subscription. The stale Tag alone is enough for ItemContainer_DragStarting to treat the container as hasBinding (line 424) and start a drag on a row the user disabled reordering for. Reset the container state on pooling (or on retrieval) rather than relying on ElementClearing.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Handler Mapper and Property Patterns — Suppressing MapSelectedItem for the entire assignment window drops legitimate selection changes that originate during that window. Concrete scenario: SelectedItem is two-way bound to a view model that coerces/rejects the value (a common "don't allow selecting the placeholder row" pattern). UpdateVirtualSingleSelection sets SelectedItem = null, the VM setter writes back the previous item, the resulting mapper call is swallowed by this guard, and the platform is never told — WinUI keeps the blank row highlighted while CollectionView.SelectedItem is the previous item. Prefer detecting the no-op (compare the incoming value against the current platform selection inside UpdatePlatformSelection) over an unconditional early return, so genuine re-entrant changes still propagate.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Cross-Platform Behavioral Consistency — The round-trip guard is applied only to the single-selection path. UpdateVirtualMultipleSelection still writes ItemsView.SelectedItems with no equivalent flag, and MapSelectedItems (line 90) calls UpdatePlatformSelection() unconditionally. Concrete scenario: SelectionMode="Multiple" over a source containing a null entry — tapping the blank row runs the same WinUI-selects-null → virtual-write → MapSelectedItems → UpdatePlatformSelection round trip this PR fixes for single selection, and the selection is undone. Neither device test added here covers multiple selection, so the asymmetry is untested.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — This desync fix (always creating a replacement context so the flat mirror keeps a slot for a null new item) is correct and matches the initial-build path at line 140, which never skipped nulls. It has no test: neither device test in this PR uses IsGrouped, and the UI test page is ungrouped. The specific scenario to cover is a grouped ObservableCollection where an inner group's item is replaced with null (group[i] = null) — before this change the flat list kept the stale ItemTemplateContext2 and every subsequent index-based lookup in MauiItemsView.DragDrop.cs was off. Add a device test asserting the flattened collection count/contents after such a replace; the same test project also exercises the PerformGroupedReorder rewrite flagged at line 884.

[MauiBot]

AI Review Summary

@SuthiYuvaraj — new AI review results are available based on commit f5716e2.

---

🗂️ Review Sessions — click to expand

🚦 Gate — Test Before & After Fix

Gate Result: ❌ FAILED

Platform: WINDOWS · Base: net11.0 · Merge base: 342bf0b1

🩺 The PR's test does not compile — the build error is in one of the PR's own test files, which the gate never reverts, so it fails identically without and with the fix. This is NOT a pre-existing/environment failure — the PR must fix its test (e.g. an ambiguous using / type collision). Investigate the PR's test code.

D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(46,21): error CS0104: 'SelectionMode' is an ambiguous reference between 'Microsoft.Maui.Controls.Selectio...

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
📱 CollectionViewTests (NullItem_RendersBlankRow, NullItem_TapDoesNotCrash_SingleSelection) Category=CollectionView	🛠️ BUILD ERROR	🛠️ BUILD ERROR
🖥️ CollectionViewNullItemDragReorder CollectionViewNullItemDragReorder	✅ FAIL — 331s	❌ FAIL — 834s

🔴 Without fix — 📱 CollectionViewTests (NullItem_RendersBlankRow, NullItem_TapDoesNotCrash_SingleSelection): 🛠️ BUILD ERROR · 317s

Error-relevant lines (filtered from the build log):

D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(174,12): error CS0104: 'ColumnDefinition' is an ambiguous reference between 'Microsoft.Maui.Controls.ColumnDefinition' and 'Microsoft.UI.Xaml.Controls.ColumnDefinition' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(175,12): error CS0104: 'ColumnDefinition' is an ambiguous reference between 'Microsoft.Maui.Controls.ColumnDefinition' and 'Microsoft.UI.Xaml.Controls.ColumnDefinition' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(176,12): error CS0104: 'ColumnDefinition' is an ambiguous reference between 'Microsoft.Maui.Controls.ColumnDefinition' and 'Microsoft.UI.Xaml.Controls.ColumnDefinition' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(177,12): error CS0104: 'ColumnDefinition' is an ambiguous reference between 'Microsoft.Maui.Controls.ColumnDefinition' and 'Microsoft.UI.Xaml.Controls.ColumnDefinition' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(178,12): error CS0104: 'ColumnDefinition' is an ambiguous reference between 'Microsoft.Maui.Controls.ColumnDefinition' and 'Microsoft.UI.Xaml.Controls.ColumnDefinition' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(186,7): error CS0104: 'Grid' is an ambiguous reference between 'Microsoft.Maui.Controls.Grid' and 'Microsoft.UI.Xaml.Controls.Grid' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(364,21): error CS0104: 'Grid' is an ambiguous reference between 'Microsoft.Maui.Controls.Grid' and 'Microsoft.UI.Xaml.Controls.Grid' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(435,13): error CS1061: 'MauiAppBuilder' does not contain a definition for 'ConfigureMauiHandlers' and no accessible extension method 'ConfigureMauiHandlers' accepting a first argument of type 'MauiAppBuilder' could be found (are you missing a using directive or an assembly reference?) [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(479,13): error CS1061: 'MauiAppBuilder' does not contain a definition for 'ConfigureMauiHandlers' and no accessible extension method 'ConfigureMauiHandlers' accepting a first argument of type 'MauiAppBuilder' could be found (are you missing a using directive or an assembly reference?) [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(498,21): error CS0104: 'SelectionMode' is an ambiguous reference between 'Microsoft.Maui.Controls.SelectionMode' and 'Microsoft.UI.Xaml.Controls.SelectionMode' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
Build FAILED.

🟢 With fix — 📱 CollectionViewTests (NullItem_RendersBlankRow, NullItem_TapDoesNotCrash_SingleSelection): 🛠️ BUILD ERROR · 116s

Error-relevant lines (filtered from the build log):

D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(174,12): error CS0104: 'ColumnDefinition' is an ambiguous reference between 'Microsoft.Maui.Controls.ColumnDefinition' and 'Microsoft.UI.Xaml.Controls.ColumnDefinition' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(175,12): error CS0104: 'ColumnDefinition' is an ambiguous reference between 'Microsoft.Maui.Controls.ColumnDefinition' and 'Microsoft.UI.Xaml.Controls.ColumnDefinition' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(176,12): error CS0104: 'ColumnDefinition' is an ambiguous reference between 'Microsoft.Maui.Controls.ColumnDefinition' and 'Microsoft.UI.Xaml.Controls.ColumnDefinition' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(177,12): error CS0104: 'ColumnDefinition' is an ambiguous reference between 'Microsoft.Maui.Controls.ColumnDefinition' and 'Microsoft.UI.Xaml.Controls.ColumnDefinition' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(178,12): error CS0104: 'ColumnDefinition' is an ambiguous reference between 'Microsoft.Maui.Controls.ColumnDefinition' and 'Microsoft.UI.Xaml.Controls.ColumnDefinition' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(186,7): error CS0104: 'Grid' is an ambiguous reference between 'Microsoft.Maui.Controls.Grid' and 'Microsoft.UI.Xaml.Controls.Grid' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(364,21): error CS0104: 'Grid' is an ambiguous reference between 'Microsoft.Maui.Controls.Grid' and 'Microsoft.UI.Xaml.Controls.Grid' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(435,13): error CS1061: 'MauiAppBuilder' does not contain a definition for 'ConfigureMauiHandlers' and no accessible extension method 'ConfigureMauiHandlers' accepting a first argument of type 'MauiAppBuilder' could be found (are you missing a using directive or an assembly reference?) [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(479,13): error CS1061: 'MauiAppBuilder' does not contain a definition for 'ConfigureMauiHandlers' and no accessible extension method 'ConfigureMauiHandlers' accepting a first argument of type 'MauiAppBuilder' could be found (are you missing a using directive or an assembly reference?) [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(498,21): error CS0104: 'SelectionMode' is an ambiguous reference between 'Microsoft.Maui.Controls.SelectionMode' and 'Microsoft.UI.Xaml.Controls.SelectionMode' [D:\a\1\s\src\Controls\tests\DeviceTests\Controls.DeviceTests.csproj::TargetFramework=net11.0-windows10.0.19041.0]
Build FAILED.

🔴 Without fix — 🖥️ CollectionViewNullItemDragReorder: FAIL ✅ · 331s

Error-relevant lines (filtered from the build log):

     at Microsoft.Maui.TestCases.Tests.Issues.CollectionViewNullItemDragReorder.DraggingNullItemRowDoesNotCrashAndReorders() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CollectionViewNullItemDragReorder.cs:line 39
     at Microsoft.Maui.TestCases.Tests.Issues.CollectionViewNullItemDragReorder.DraggingRealItemOntoNullItemRowDoesNotCrashAndReorders() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CollectionViewNullItemDragReorder.cs:line 73

🟢 With fix — 🖥️ CollectionViewNullItemDragReorder: FAIL ❌ · 834s

Error-relevant lines (filtered from the build log):

     at Microsoft.Maui.TestCases.Tests.Issues.CollectionViewNullItemDragReorder.DraggingNullItemRowDoesNotCrashAndReorders() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CollectionViewNullItemDragReorder.cs:line 39
     at Microsoft.Maui.TestCases.Tests.Issues.CollectionViewNullItemDragReorder.DraggingRealItemOntoNullItemRowDoesNotCrashAndReorders() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CollectionViewNullItemDragReorder.cs:line 73

⚠️ Failure Details

• 🛠️ CollectionViewTests (NullItem_RendersBlankRow, NullItem_TapDoesNotCrash_SingleSelection) without fix: build failed before tests could run
  • D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(46,21): error CS0104: 'SelectionMode' is an ambiguous reference between 'Microsoft.Maui.Controls.Selectio...
• 🛠️ CollectionViewTests (NullItem_RendersBlankRow, NullItem_TapDoesNotCrash_SingleSelection) with fix: build failed (fix does not compile)
  • D:\a\1\s\src\Controls\tests\DeviceTests\Elements\CollectionView\CollectionViewTests.Windows.cs(46,21): error CS0104: 'SelectionMode' is an ambiguous reference between 'Microsoft.Maui.Controls.Selectio...
• ❌ CollectionViewNullItemDragReorder FAILED with fix (should pass)
  • DraggingNullItemRowDoesNotCrashAndReorders [4 s]; DraggingRealItemOntoNullItemRowDoesNotCrashAndReorders [3 s]
  • Dragging the null-item row should trigger ReorderCompleted and change the item order. Assert.That(reorderedText, Is.Not.EqualTo(initialOrder)) Expected: not equal to "Item A, null, Item B, Item C"...; Dropping an item onto the null-item row should trigger ReorderCompleted and change the item ord...

📁 Fix files reverted (4 files)

• src/Controls/src/Core/Handlers/Items2/CollectionViewHandler2.Windows.cs
• src/Controls/src/Core/Handlers/Items2/Windows/GroupedItemTemplateCollection2.cs
• src/Controls/src/Core/Handlers/Items2/Windows/ItemFactory.cs
• src/Controls/src/Core/Handlers/Items2/Windows/MauiItemsView.DragDrop.cs

---

📋 Pre-Flight — Context & Validation

PR #36170 Pre-Flight

Scope

• PR: [WinUI][CV2] Fix for Null Item Handling - follow-ups from CollectionView2 Windows handler (#34600)
• Issue: #36068
• Platform: Windows
• Base: net11.0
• Local review commit: df0fab0f78 (squashed PR commit)
• Gate: Failed previously; do not re-run it and do not overwrite gate/content.md.

Problem

The Windows CollectionView2 implementation mishandles null source entries in three paths:

1. A null item can be treated as selected because Equals(null, null) is true.
2. Grouped Replace with a null new item can leave the flattened mirror out of sync.
3. Drag/reorder can use stale or ambiguous identity for blank null-item containers.

Existing PR Approach

The PR changes four production files:

• src/Controls/src/Core/Handlers/Items2/CollectionViewHandler2.Windows.cs
  • Adds a mapper round-trip suppression flag for platform-originated null selection.
  • Assigns IsItemSelected rather than directly changing visual state.
• src/Controls/src/Core/Handlers/Items2/Windows/GroupedItemTemplateCollection2.cs
  • Creates a replacement context even when the new item is null.
• src/Controls/src/Core/Handlers/Items2/Windows/ItemFactory.cs
  • Represents regular null items with blank ItemContainer instances and adds a dedicated recycle pool.
• src/Controls/src/Core/Handlers/Items2/Windows/MauiItemsView.DragDrop.cs
  • Uses _draggedSourceIndex as active-drag and null-slot identity.
  • Adds blank-container Tag handling and ItemsRepeater.GetElementIndex fallback.
  • Reworks grouped source lookup to use flat positions.

The PR also adds two Windows device tests and two Windows UI drag/reorder tests.

Direct Diff Assessment

The candidate must be an alternative root-cause strategy, not a restatement of the PR's blank-container pool plus broad _draggedSourceIndex rewrite. Preserve unrelated behavior and keep the fix localized to the null handling paths.

The repository instruction claiming Items2 has no Windows implementation is stale for this branch: this PR and the active Windows CV2 code are under Handlers/Items2. Follow the actual registered/tested code path in this review commit.

Existing Gate Evidence

The completed gate produced two independent results:

1. The device-test project does not compile because the PR added using Microsoft.UI.Xaml.Controls, creating ambiguities for SelectionMode, Grid, and ColumnDefinition, and it is also missing the namespace needed for ConfigureMauiHandlers. The same build errors occur with and without the production fix.
2. CollectionViewNullItemDragReorder correctly failed without the fix, but also failed with the PR fix. Both tests left the order unchanged:
   • DraggingNullItemRowDoesNotCrashAndReorders
   • DraggingRealItemOntoNullItemRowDoesNotCrashAndReorders

Do not re-run gate verification. A candidate may report the device-test regression check as blocked by these already-proven PR test compilation errors; do not broaden the candidate into unrelated test cleanup.

Bounded Validation

Primary test:

pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform windows -TestFilter "CollectionViewNullItemDragReorder"

Mandatory regression tests (one filtered invocation only):

pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Controls -Platform windows -TestFilter "FullyQualifiedName~NullItem_RendersBlankRow|FullyQualifiedName~NullItem_TapDoesNotCrash_SingleSelection"

Run no full suite. Each candidate gets one implementation/test pass and at most one focused correction/retest.

Candidate Output Contract

Each attempt must:

• Follow the try-fix skill once, including its baseline and inline expert self-review requirements.
• Save standard try-fix artifacts.
• Write its narrative to try-fix-N/content.md.
• Include the approach, complete candidate diff, primary and regression test results, and failure analysis when applicable.
• Restore the production fix baseline through EstablishBrokenBaseline.ps1 -Restore.

The orchestrator updates try-fix/content.md immediately after each candidate.

---

🔬 Code Review — Deep Analysis

PR #36170 — Expert Review (initial evaluation, raw submitted PR)

• PR: [WinUI][CV2] Fix for Null Item Handling — follow-ups from CollectionView2 Windows handler (#34600)
• Issue: #36068 · Platform: Windows · Base: net11.0
• Reviewed commit: df0fab0f78 (squashed PR commit) in D:\a\1\s — raw worktree, unmodified
• Scope of this document: the raw submitted PR only. Try-fix candidates were read afterward for comparison context only and did not alter any finding.
• Artifacts: CustomAgentLogsTmp/PRState/36170/PRAgent/inline-findings.json (15 findings)

---

1. Independent assessment (formed before reading narrative)

Diff shape: 4 production files (all Windows CV2 / Handlers/Items2), 1 device-test file, 1 host-app sample page (+XAML), 1 shared UI test — 491 insertions / 31 deletions.

File	Change	Independent read
CollectionViewHandler2.Windows.cs	new _ignoreVirtualSelectionChange flag; MapSelectedItem early-return; try/finally around ItemsView.SelectedItem =	Breaks the platform→virtual→platform selection round trip that undoes a null-row selection. Directionally right; the guard is broader than the problem (F13/F14).
GroupedItemTemplateCollection2.cs	CreateItemContext(object?); Replace no longer skips null new items	Correct — the old continue left the stale ItemTemplateContext2 in the flat mirror while the source slot changed, desyncing every index-based lookup. Matches the initial-build path (line 140), which never skipped nulls. Untested (F15).
ItemFactory.cs	blank ItemContainer for null non-header/footer items; _nullItemPool; pooled on recycle; cleared in CleanUp	Blank-row rendering matches CV1 intent. Two issues: hard-coded 32/88 sizing (F11) and pooling without state reset (F12).
MauiItemsView.DragDrop.cs	_draggedItem is not null → _draggedSourceIndex >= 0 as the active-drag predicate throughout; Tag-based blank-container drag detection; GetElementIndex-based index resolution; grouped source lookup rewritten from value equality to flat index	Largest blast radius. The active-drag-predicate swap is a clean improvement. The index work introduces two index spaces and makes previously value-based, header-agnostic grouped lookup positional (F6, F7, F8, F9).
Tests	2 device tests, 2 Appium UI tests, sample page	Device-test file does not compile (F1). UI tests are non-discriminating (F5) and, per gate, do not execute the fixed path at all (F4).

Independent verdict before reading any narrative: the underlying defect is real and the general direction (stop conflating "no drag" with "dragging a null payload") is right, but the implementation broadens well past null handling into shared grouped/index logic, and its own test suite is red / non-signaling.

2. Reconciliation with PR narrative / pre-flight

The pre-flight summary matches my independent read on all four production files; nothing in the narrative changed a finding. One divergence worth recording: pre-flight notes the repo instruction "Items2 has no Windows implementation" is stale for this branch — confirmed, Handlers/Items2/** is the active Windows CV2 path here, so CollectionView dimensions #19/#21 plus #24 (Windows) were applied rather than the Items/-Windows routing.

The narrative does not mention that PerformGroupedReorder now resolves non-null items positionally too — this is presented as null-slot identity work, but it changes behavior for existing grouped reorder. That is the most under-declared part of the change.

3. Prior-review reconciliation

• Try-fix Candidate 1 self-review (3 moderate, 1 minor) independently landed on "reliance on the raw-Tag fallback during ElementPrepared" and "grouped null reorder unsupported" — the same weak points I flag as F9 and F6/F15 against the raw PR.
• Try-fix Candidate 2 self-review flagged "captured-index validation can select the wrong null after a concurrent source shift" — identical failure mode to my F8 (IndexOfItem(null, …) returns the first null).
• Both candidates independently reproduced the UI-test non-signal, corroborating F4 rather than being its source.
• No candidate patch content was reviewed or adopted; every finding above was derived from the raw diff plus the full files and callers. No duplicate of an existing posted PR comment was found in the state directory.

4. Blast radius

• Direct: Windows-only. All four production files are under CV2/Items2 Windows. No iOS/Android/Catalyst files, no public API or PublicAPI.*.txt change, no trimming/AOT surface.
• Widest reach: MauiItemsView.DragDrop.cs. The _draggedItem is not null → _draggedSourceIndex >= 0 swap touches 7 call sites including ElementPrepared (runs for every realized container on every layout pass), ScrollViewer_Drop, PerformReorder, PerformGroupedReorder, DimNonSourceContainers. Any container realized during a drag now takes its dim/hide decision from an index comparison rather than an item comparison.
• Regression-sensitive component: CollectionView is on the frequently-regressed list (layout, scroll position, cell alignment, Header/Footer). Grouped drag-reorder and multi-select are reachable from this diff and have no test in it.
• Not affected: CV1 (Handlers/Items/**) untouched; Android/iOS CV paths untouched.

5. Findings (15 written inline)

#	Sev	Dimension	Location
F1	critical	Regression Prevention	CollectionViewTests.Windows.cs:15 — using Microsoft.UI.Xaml.Controls; breaks the Windows device-test project (CS0104 on SelectionMode/Grid/ColumnDefinition, missing ConfigureMauiHandlers namespace)
F2	moderate	Regression Prevention	CollectionViewTests.Windows.cs:468 — containers[1] assumes visual-child order == item order; unjustified Task.Delay(500)
F3	moderate	Logic & Correctness	CollectionViewTests.Windows.cs:520 — setting ItemContainer.IsSelected may bypass the SelectionChanged round trip the fix targets; assertions can hold on unfixed code
F4	major	Regression Prevention	CollectionViewNullItemDragReorder.cs:34 — DragCoordinates does not raise DragStarting; the non-null control test fails identically, so the suite gives no fix signal
F5	moderate	Regression Prevention	CollectionViewNullItemDragReorder.cs:39 — assertions cannot distinguish a correct move from a wrong-slot move; no exact expected order
F6	major	Logic & Correctness	MauiItemsView.DragDrop.cs:884 — grouped source lookup switched from value equality to positional flat index for all items; continue on a non-IEnumerable group skips header/footer accounting → flatSrcPos drift
F7	major	Logic & Correctness	MauiItemsView.DragDrop.cs:1245 — GetContainerIndex returns repeater-index for null rows vs source-list index for others; consumers interpret them interchangeably
F8	major	Logic & Correctness	MauiItemsView.DragDrop.cs:755 — null payload + failed index validation falls back to IndexOfItem(null) = first null → wrong row moved in multi-null sources
F9	moderate	Logic & Correctness	MauiItemsView.DragDrop.cs:570 — stale Tag vs frozen _draggedSourceIndex can hide (Opacity 0 + no hit-test) an unrelated blank row mid-drag
F10	minor	Complexity Reduction	MauiItemsView.DragDrop.cs:582 — new blank-container branch in IsContainerBoundToItem is unreachable (sole caller already returns at 568)
F11	moderate	Cross-Platform Consistency	ItemFactory.cs:71 — hard-coded MinHeight = 32 / MinWidth = 88 ignores ItemSizingStrategy and neighbouring row height
F12	moderate	Handler Mapper Patterns	ItemFactory.cs:272 — blank containers pooled without resetting Tag/CanDrag/DragStarting/Background; the reset lives in ElementClearing, which early-returns when _canReorderItems is false
F13	moderate	Handler Mapper Patterns	CollectionViewHandler2.Windows.cs:86 — guard swallows legitimate re-entrant SelectedItem writes (VM coercion / two-way binding) → platform/virtual desync
F14	moderate	Cross-Platform Consistency	CollectionViewHandler2.Windows.cs:339 — guard applied to single selection only; MapSelectedItems / UpdateVirtualMultipleSelection keep the same undone-selection round trip
F15	moderate	Regression Prevention	GroupedItemTemplateCollection2.cs:325 — correct desync fix, zero grouped coverage in this PR

Counts: 1 critical, 4 major, 9 moderate, 1 minor.

Not flagged (deliberately): inconsistent indentation in the new ItemContainer_DragStarting / IsContainerBoundToDraggedItem blocks (style — dotnet format territory); comment verbosity; var usage.

6. Failure-mode probes

Probe	Result
Flat, ungrouped, single null, no header — drag the null row	Index spaces coincide; path is plausible-correct. This is the only configuration the PR's tests target.
Two or more nulls, index validation fails after a shift	IndexOfItem(null) → first null; wrong row moves silently (F8). Equals(null, null) also makes the index validation itself non-discriminating.
Grouped source with group headers/footers, drag any (non-null) item	Positional resolution replaces value equality; drift from the continue branch or non-uniform header/footer emission moves the wrong item or aborts the reorder (F6). No test.
Grouped source, item inside a group replaced with null	GroupedItemTemplateCollection2 fix keeps the mirror in sync — correct, untested (F15).
Blank container recycled mid-drag / after CanReorderItems=false	Stale Tag survives (ElementClearing early-returns); pooled container reused with CanDrag/DragStarting/card Background intact; the stale Tag alone satisfies the new hasBinding check at line 424 (F12, F9).
SelectionMode=Multiple with a null entry, tap the blank row	Same undone-selection round trip; no guard on the multiple path (F14).
SelectedItem two-way bound to a coercing VM, tap the null row	Mapper suppressed for the whole window → WinUI keeps the blank row selected while the virtual holds the coerced value (F13).
Null row visual size vs templated rows (sample uses HeightRequest=70)	Null row renders at the hard-coded 32px — visible misalignment (F11). The device test only asserts > 0, so it cannot catch this.

7. CI / gate evidence

Trusted caller-provided Gate result: FAILED — accepted as-is, not re-run; gate/content.md untouched.

1. Windows Controls.DeviceTests does not compile with this PR (14 errors: CS0104 ambiguities + missing ConfigureMauiHandlers namespace). The same errors occur with and without the production fix, confirming they are introduced by the PR's own test edit → F1.
2. CollectionViewNullItemDragReorder failed without the fix (good) and with the fix; both tests left the order at Item A, null, Item B, Item C. The non-null control test failing identically points at the Appium gesture, not the fix → F4.

Net: the PR currently has zero executing regression coverage — the device tests cannot build, and the UI tests do not reach DragStarting.

8. Verdict

Request changes.

Must-fix before merge:

1. F1 — restore compilation of Controls.DeviceTests (Windows). A PR that red-builds an existing test project cannot merge.
2. F4/F5 — provide at least one regression test that demonstrably executes the fixed path and asserts the exact resulting order.
3. F6/F7/F8 — either scope the index rework to null rows only (leaving value-equality resolution intact for non-null grouped items), or normalize on a single index space and add grouped coverage.

Should-fix: F9, F11, F12, F13, F14, F15. Optional: F10.

The underlying defect and the GroupedItemTemplateCollection2 change are sound; the objection is to an unscoped rewrite of shared grouped/index logic shipping with a non-building, non-signaling test suite.

9. Calibrated confidence

Claim	Confidence	Basis
F1 device-test build break	Very high (~0.97)	Gate-proven with and without the fix; consistent with the using added at line 15
F4 UI tests do not exercise the fix	High (~0.9)	Gate + two independent candidates reproduced the identical non-null control failure
F6 grouped positional-resolution regression risk	Medium-high (~0.75)	Static trace of the rewritten loop; not runtime-verified (no grouped test exists to run)
F7 / F8 index-space and first-null resolution	Medium-high (~0.75)	Static trace GetContainerIndex → GetSourceList → PerformReorder; corroborated by Candidate 2's independent self-review
F9 / F12 recycled blank-container state	Medium (~0.65)	Requires _canReorderItems toggling while blank rows are realized; reachable but not observed
F13 / F14 selection guard gaps	Medium (~0.6)	Depends on VM coercion / multiple-selection usage; not runtime-verified
F11 blank row sizing	High (~0.85)	Constants are unconditional in the diff; the sample page's row height makes the mismatch concrete
Overall "request changes" verdict	High (~0.9)	Driven by F1 + F4, which are gate-evidenced and independent of the static-analysis findings

Runtime verification of the drag paths was not possible here (Windows device/UI runs are the gate's domain and were not re-run, per instruction), so all MauiItemsView.DragDrop.cs findings are static-trace based and labelled accordingly.

---

🛠️ Try-Fix — Analysis & Comparison

PR #36170 Alternative Fix Candidates

Candidate 1 — Slot-index drag identity

Model: claude-opus-5
Result: Fail (primary) / Blocked (mandatory regression)
Files: src/Controls/src/Core/Handlers/Items2/Windows/MauiItemsView.DragDrop.cs (+64/-45)
Artifacts: try-fix/attempt-1/
Full narrative and diff: ../try-fix-1/content.md

Approach

Use the realized repeater slot as drag identity instead of the item value:

• Ask ItemsRepeater.GetElementIndex first in GetContainerIndex.
• Validate and capture the source slot in ItemContainer_DragStarting.
• Track active drag state with an explicit boolean rather than _draggedItem != null.
• Match the dragged container by slot index.
• Leave null rendering and pooling untouched.

This differs from the PR's four-file blank-container pool and broad null special-casing by addressing value/slot conflation in one drag file.

Test Results

• Primary BuildAndRunHostApp.ps1 -Platform windows -TestFilter "CollectionViewNullItemDragReorder": 2 tests run, 2 failed. Both left the order equal to Item A, null, Item B, Item C.
• Mandatory filtered device regression invocation: Blocked during build by the PR's 14 known test compilation errors (CS0104 type ambiguities and missing ConfigureMauiHandlers). No test file was edited.

Failure Analysis

The real-item-to-null-row test also leaves the order unchanged even though its drag source never uses null identity. Across the baseline, PR fix, and this independent candidate, Appium DragCoordinates does not appear to initiate the WinUI/OLE drag pipeline, so ItemContainer.DragStarting is never reached. The UI test therefore provides no production-fix signal. The unused correction/retest allowance was not spent on a guaranteed-identical run.

Static analysis still confirms a production defect: baseline ItemContainer_DragStarting cancels a legitimate null payload and later _draggedItem is null guards abort reorder. Candidate 1's slot-index strategy addresses that defect, but the required primary test cannot validate it.

Inline Expert Self-Review

Four findings: 0 critical, 0 major, 3 moderate, 1 minor. Main limitations were reliance on the raw-Tag fallback during ElementPrepared, intentionally leaving grouped null-source reorder unsupported, and lack of a candidate-owned viable regression test.

---

Aggregate state after Candidate 1: 1 of at most 2 candidates completed.

Candidate 2 — Null-object drag payload

Model: gpt-5.6-sol
Result: Fail (primary) / Blocked (mandatory regression)
Files: src/Controls/src/Core/Handlers/Items2/Windows/MauiItemsView.DragDrop.cs
Artifacts: try-fix/attempt-2/
Full narrative and diff: ../try-fix-2/content.md

Approach

Represent a legitimate null source item with a private non-null sentinel only inside the existing drag lifecycle:

• Distinguish “source slot resolved to null” from “no source resolved.”
• Store the sentinel in _draggedItem, retaining existing non-null lifecycle guards.
• Resolve the sentinel back to null at flat reorder lookup boundaries.
• Match a null drag only to the original source-container reference.
• Conservatively leave grouped null reorder unsupported.

This differs from both the PR and Candidate 1: it adds no active-drag boolean, no GetElementIndex-first resolution, and no slot-index container matching rewrite.

Test Results

• Primary BuildAndRunHostApp.ps1 -Platform windows -TestFilter "CollectionViewNullItemDragReorder": 2 tests run, 2 failed. Both left the order unchanged.
• Mandatory filtered device regression invocation: Blocked during build by the same 14 PR-owned compilation errors. Neither regression executed, and no test file was edited.

Failure Analysis

The candidate compiled, but the required UI scenarios supplied no evidence that its production path executed. The non-null control test also failed unchanged despite not using the sentinel, corroborating Candidate 1's conclusion that the Appium gesture does not drive the WinUI drag pipeline. No correction/retest was spent because changing gesture recognition/drop routing would be an unbounded new subsystem rather than a focused null fix.

Inline Expert Self-Review

Two moderate findings: captured-index validation can select the wrong null after a concurrent source shift, and grouped null-item reorder remains unsupported.

Aggregate Conclusion

Two materially different production candidates were implemented and tested:

Candidate	Strategy	Primary	Regression
1	Repeater slot as drag identity	Fail (2/2 unchanged)	Blocked by PR test compilation
2	Null-object drag payload	Fail (2/2 unchanged)	Blocked by PR test compilation

Neither candidate is verified. Both independent implementations compiled, but the primary Appium test failed identically for a real-item drag that bypasses null-source identity. The evidence indicates DragCoordinates does not raise the WinUI/OLE DragStarting event, making the primary test non-signaling. The mandatory device tests are independently non-runnable because the PR-added test file does not compile.

Aggregate state: 2 of 2 candidates completed; no further candidate work launched.

---

📝 PR Finalize — Recommended Title & Description

Assessment: ✏️ Recommend updating — the current description is detailed, but its selection explanation omits the blank-container strategy and the title does not name the three corrected null-item paths.

Recommended title

[Windows] CollectionView2: Fix null-item selection, grouped replacement, and drag reorder

Recommended description

### Issue Description

This fixes three null-item handling problems in the Windows CollectionView2 handler.

#### Null-item false selection

**Root cause**

Null data entries could be confused with the absence of a selected item. Selection-state updates compared item values, where `Equals(null, null)` is true, and a platform-originated selection of a null row could round-trip through `SelectedItem = null` and be immediately deselected.

**Fix**

- Represent regular null data entries with blank `ItemContainer` instances and recycle them through a dedicated pool.
- Suppress the immediate platform-selection round trip while `UpdateVirtualSingleSelection` writes the platform-originated value.
- Update MAUI visual selection through `VisualElement.IsItemSelected`.

#### Grouped null-Replace desynchronization

**Root cause**

When a grouped source raised an `INotifyCollectionChanged.Replace` event with a null new item, `HandleGroupItemsReplace` skipped creating a new `ItemTemplateContext2` for that slot. The flattened mirror could then retain stale data and become misaligned with the source.

**Fix**

Always create a fresh `ItemTemplateContext2`, including for a null replacement item, so the flattened collection preserves every source slot.

#### Null drag stale index

**Root cause**

The drag lifecycle used a non-null item value as the active-drag signal. A legitimate null source item was therefore indistinguishable from no drag, and stale or ambiguous container lookup could feed an invalid source index into reorder calculations.

**Fix**

- Use `_draggedSourceIndex` as the active-drag and source-slot identity.
- Track blank containers by their `ItemsRepeater`/`Tag` index.
- Resolve grouped reorder positions from the flattened slot layout.
- Clean up drag state rather than proceeding when the source index cannot be resolved.

### Key implementation areas

- `CollectionViewHandler2.Windows.cs` — platform-to-virtual selection synchronization.
- `GroupedItemTemplateCollection2.cs` — grouped replacement mirror updates.
- `ItemFactory.cs` — blank null-item containers and recycling.
- `MauiItemsView.DragDrop.cs` — null-slot drag identity and reorder lookup.

### Issues Fixed

Fixes #36068

### Tested the behaviour in the following platforms

- [ ] Android
- [x] Windows
- [ ] iOS
- [ ] Mac

### Output Screenshot

Before Issue Fix | After Issue Fix |
|---|---|
| <video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/a4fc0c94-76e0-436e-bdea-6291a36b50c2"> | <video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/c3dd5c3c-e798-4b3b-813c-b4c51e577d07"> |

---

🏁 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Expert evaluation of the submitted PR

The expert reviewer found the underlying null-item defects valid and considered the grouped Replace fix directionally correct, but rejected the submitted fix as merge-ready. The decisive evidence is the trusted failed Gate:

• the PR's Windows device-test file does not compile because Microsoft.UI.Xaml.Controls makes existing MAUI control names ambiguous and ConfigureMauiHandlers lacks its extension namespace;
• both new UI drag tests fail with the PR fix applied and leave the collection unchanged, including the non-null drag control case, so they do not demonstrate that DragStarting executes;
• the drag/index rewrite broadens shared grouped and duplicate/null slot behavior without executing regression coverage.

The raw inline findings are preserved in inline-findings.json; the complete initial assessment is in expert-pr-eval/content.md.

Candidate comparison

Rank	Candidate	Scope and approach	Required evidence	Comparative assessment
1	pr-plus-reviewer	Retains all three PR fixes, reconciles coerced selection, resets pooled state, normalizes realized drag identity to repeater slots, removes first-null fallback, and repairs the focused device tests.	The two required device regressions compiled and individually passed. The overall device runner exited 1 on unrelated Windows RPC/capture failures. The primary UI project did not compile because the refinement introduced an ambiguous PointerInputDevice reference.	Winner, but not merge-ready. It is the only complete candidate with executing, passing focused regressions and it addresses the expert's highest-value production concerns. Its candidate-caused UI compile error leaves drag validation blocked.
2	try-fix-2	Encodes a legitimate null drag payload with a private sentinel while preserving the existing lifecycle. It deliberately leaves grouped null reorder unsupported.	Primary: 2/2 failed unchanged. Device regressions: blocked by the PR-owned test compilation errors.	Smaller than the PR and fewer static concerns than candidate 1, but incomplete for the grouped/selection defects and unverified. Captured-index validation can still choose the wrong null after a concurrent shift.
3	try-fix-1	Uses repeater slot identity plus an explicit active-drag flag in one drag file; deliberately rejects grouped null reorder.	Primary: 2/2 failed unchanged. Device regressions: blocked by the PR-owned test compilation errors.	Sound root-cause direction for flat drag identity, but partial, dependent on a Tag fallback during preparation, and without executing regression evidence.
4	pr	Submitted four-file blank-container, selection-suppression, grouped replacement, and broad flat-index rewrite.	Trusted Gate failed: device tests do not compile; both UI drag tests fail unchanged.	Lowest-ranked because it has concrete PR-owned test breakage, no signaling drag regression, and unresolved expert findings in shared drag/index behavior.

Candidates without a passing focused regression are ranked below pr-plus-reviewer, whose two named device regressions executed and passed. No candidate achieved a fully green required validation set.

Why pr-plus-reviewer wins

pr-plus-reviewer best preserves the submitted fix's complete problem coverage while replacing ambiguous value identity with explicit slot identity and making the two device regressions deterministic. It also handles the reviewer's re-entrant selection and recycled-container concerns rather than omitting those fix areas as both try-fix candidates do.

This is still a REQUEST CHANGES recommendation. The submitted PR does not contain the winning refinements, the trusted Gate forbids approval, and the winning candidate itself needs the PointerInputDevice ambiguity corrected and the primary drag validation rerun by a subsequent PR update.

Winner

pr-plus-reviewer

---

📱 UI Tests — CollectionView

Detected UI test categories: CollectionView

❌ Deep UI tests — 326 passed, 2 failed, 13 skipped across 1 category on platform-pool agent (replaces in-process counts above).

🧪 UI Test Execution Results (deep, platform pool)

Category	Tests	Snapshot diffs
CollectionView	326/341 (2 ❌, 13 skipped)	—

🔍 AI analysis of failures — PR-related vs unrelated

🔍 AI-generated triage (GitHub Copilot CLI) — a heuristic judgement of whether each deep UI test failure is connected to this PR's changes. Verify before relying on it.

Likely PR-related: one or more failures appear connected to this PR's changes.

• ✗ PR-related — Windows CollectionView2 null-item drag-and-drop reorder (2 tests): Both newly added Windows-only tests directly exercise the null-item reorder path changed by this PR, and the unchanged item order is a functional failure rather than a flaky or infrastructure signature.

Strongest signal: both tests reach their assertions successfully but observe that no reorder occurred.

❌ CollectionView — 2 failed tests

DraggingNullItemRowDoesNotCrashAndReorders

Dragging the null-item row should trigger ReorderCompleted and change the item order.
Assert.That(reorderedText, Is.Not.EqualTo(initialOrder))
  Expected: not equal to "Item A, null, Item B, Item C"
  But was:  "Item A, null, Item B, Item C"
at Microsoft.Maui.TestCases.Tests.Issues.CollectionViewNullItemDragReorder.DraggingNullItemRowDoesNotCrashAndReorders() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CollectionViewNullItemDragReorder.cs:line 39

1)    at Microsoft.Maui.TestCases.Tests.Issues.CollectionViewNullItemDragReorder.DraggingNullItemRowDoesNotCrashAndReorders() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CollectionViewNullItemDragReorder.cs:line 39

DraggingRealItemOntoNullItemRowDoesNotCrashAndReorders

Dropping an item onto the null-item row should trigger ReorderCompleted and change the item order.
Assert.That(reorderedText, Is.Not.EqualTo(initialOrder))
  Expected: not equal to "Item A, null, Item B, Item C"
  But was:  "Item A, null, Item B, Item C"
at Microsoft.Maui.TestCases.Tests.Issues.CollectionViewNullItemDragReorder.DraggingRealItemOntoNullItemRowDoesNotCrashAndReorders() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CollectionViewNullItemDragReorder.cs:line 73

1)    at Microsoft.Maui.TestCases.Tests.Issues.CollectionViewNullItemDragReorder.DraggingRealItemOntoNullItemRowDoesNotCrashAndReorders() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CollectionViewNullItemDragReorder.cs:line 73

📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)

---

🧭 Next Steps — reviewer patch required (pr-plus-reviewer)

The reviewer-enhanced candidate won, so the submitted PR still needs those changes.

Why: pr-plus-reviewer preserves all three submitted fix areas, addresses the expert review's highest-value production concerns, and is the only candidate whose two focused device regressions executed and passed. It still requires changes because its primary UI validation was blocked by a candidate-caused compile ambiguity and the submitted PR does not contain these refinements.

Apply PRAgent/pr-plus-reviewer/reviewer.patch from the CopilotLogs
artifact (or follow the report's Required submitted-PR change), push the update, and run
the review again.