[iOS, MacCatalyst] Fix CollectionView2 grouped ScrollTo MakeVisible producing inconsistent positions

[praveenkumarkarunanithi]

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!

Root Cause

CollectionView2 on iOS uses UICollectionViewCompositionalLayout, where the section provider closure defines item layouts using a hardcoded estimated item height of 30px. When cells are measured to their actual size (for example, ~80px for image-based items), the layout invalidates and the section provider is re-invoked. However, the provider recreates items using the original 30px estimate, discarding the measured dimensions.

During animated ScrollTo, this causes content size and item positions to fluctuate between layout passes. For MakeVisible (UICollectionViewScrollPosition.None), the scroll correction logic operates on different layout states across passes, resulting in inconsistent final scroll positions. CollectionView1 does not exhibit this issue because UICollectionViewFlowLayout retains measured sizes across invalidations.

Description of Change

• LayoutFactory2.cs — Added a MeasuredEstimatedItemSize property to CustomUICollectionViewCompositionalLayout. The section providers in CreateListLayout and CreateGridLayout now use this measured value instead of the hardcoded 30px estimate when available. Introduced effective local variables to avoid mutating captured closure parameters.

• TemplatedCell2.cs — Added UpdateLayoutEstimatedItemSize(), invoked after the first non-supplementary cell measurement inside PreferredLayoutAttributesFittingAttributes. This feeds the actual measured height back into the layout object and is guarded to initialize only once per layout lifecycle.

• ItemsViewController2.cs — Added a MeasuredEstimatedItemSize = null reset in ReloadData(), ensuring new datasets fall back to the default estimate until the first item is measured again. This prevents stale height estimates across major data source changes (for example, 80px → 19px).

Issues Fixed

Fixes #34663

Tested the behaviour in the following platforms

• Android
• Windows
• iOS
• Mac

Screenshots

Before Issue Fix	After Issue Fix
BeforeFix.30.mov	AfterFix.37.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 -- 35668

Or

• Run remotely in PowerShell:

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

[kubaflo]

/review -b feature/refactor-copilot-yml

[MauiBot]

Expert Review — 2 findings

See inline comments for details.

[kubaflo]

Could you check the ai's suggestions?

[kubaflo]

/review -b feature/enhanced-reviewer -p ios

[kubaflo]

Could you please check the ai's suggestions?

[kubaflo]

/review rerun

[github-actions]

Tests Failure Analysis

@praveenkumarkarunanithi — test-failure review results are available based on commit 3c207ac.

Maintainers can request a fresh review after new comments, commits, or CI runs by commenting /review tests.

Test Failure Review: Insufficient data - click to expand

Overall verdict: Insufficient data — 17 checks are failing but every backing AzDO build (1498849, 1522588, 1522587, 1473847) returned HTTP 404, so no test names, logs, or failure evidence could be extracted and 0 distinct failures were recovered. No base-branch builds were available to compare against (main, 0 recent base builds sampled), so nothing can be attributed as PR-caused or pre-existing. The deterministic gate caps the verdict at Insufficient data.

• i Uncertain — Inaccessible integration/device-test build legs (~16 legs): 16 failing checks (iOS RunOniOS_* integration legs, maui-pr-devicetests, and the Android CoreCLR Run DeviceTests leg) could not be inspected because their AzDO builds return 404; evidence is unavailable, e.g. maui-pr (Run Integration Tests RunOniOS_MauiRelease x64).
• i Uncertain — Aborted/cancelled UITest legs (~3 legs): the macOS CollectionView/ScrollView/Shell UITest legs are CANCELLED with no clean conclusion, so a PR-induced hang cannot be ruled out.
• i Uncertain — Device-test result unverified (~6 legs): green/failed device-test counts could not be positively confirmed over a complete error-free read.

Coverage: 169 checks · 152 passing · 17 failing · 0 pending · 16 inaccessible · 1 unmapped · 0 unexplained build legs · 0 unaccounted failing checks · 3 aborted failing checks · 0 canceled-build checks · 6 device-test unverified · 0 unattributed · 0 regressed-vs-base. Deterministic ceiling: Insufficient data — 16 failing check(s) could not be inspected (AzDO build/logs inaccessible).

Builds (this PR): 1498849, 1522588, 1522587, 1473847. Base sampling (main, 0 recent builds per definition): none available.

Recommended action

Human investigation is needed: the AzDO builds were inaccessible (404) so failures could not be classified. Re-run /review tests once the builds are reachable, or open the failing iOS integration and device/UI test legs manually to determine whether they are PR-related.

[kubaflo]

Could you please resolve conflicts?

[MauiBot]

Expert Review — 9 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness — The measured estimate is written into the layout, but nothing invalidates the layout afterwards. UICollectionViewCompositionalLayout only re-invokes its section provider closure when the layout is invalidated, and this write happens inside PreferredLayoutAttributesFittingAttributes (i.e. during a layout pass that has already built its sections from the old Estimated(30) dimensions). Concrete scenario: grouped CV, first ScrollTo(..., MakeVisible) — the first cell measures at e.g. 44pt and sets MeasuredEstimatedItemSize = 44, but the section provider for the current and already-built sections is never re-run, so the offsets that ScrollTo computes are still derived from the 30pt estimate. The new estimate is only picked up if some unrelated code path later calls InvalidateLayout(), which makes the fix non-deterministic. This is consistent with the Gate result (both UI tests pass identically with and without the production change). Either invalidate explicitly (deferred to the next runloop turn to avoid re-entrant invalidation from within a layout pass) or explain why the estimate is guaranteed to be consumed.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] CollectionView iOS/MacCatalyst — MeasuredEstimatedItemSize is null makes this a write-once, first-cell-wins global estimate that is shared by every item in every section. Concrete failure scenarios: (1) DataTemplateSelector or heterogeneous grouped data — whichever cell happens to be measured first (which is not necessarily index 0, and for a scrolled-to position is whatever cell UIKit prepares first) poisons the estimate for all other templates; (2) rotation / dynamic-type / container resize — items remeasure to a materially different height but the estimate is pinned to the pre-rotation value forever, because the only reset paths are ReloadData() and a source Reset. Given the bug being fixed is precisely offset drift caused by a wrong estimate, a stale-but-wrong estimate reintroduces the same class of drift. Consider tracking the estimate per section/template, or updating it whenever the measured dimension diverges from the current estimate beyond a tolerance.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] CollectionView iOS/MacCatalyst — The measured estimate is applied to items and groups, but the group header/footer supplementary items created a few lines below still use the original groupWidth/groupHeight (CreateSupplementaryItems(groupingInfo, null, scrollDirection, groupWidth, groupHeight)), i.e. they remain pinned at Estimated(30f). For the grouped CollectionView in issue #34663, section headers are typically the largest contributor to the cumulative offset error that ScrollTo(..., MakeVisible) accumulates, so the dominant source of drift is left unfixed while item estimates change. Either feed effectiveGroupWidth/effectiveGroupHeight (or a separately measured supplementary estimate) into CreateSupplementaryItems, or document why headers are exempt. The same gap exists in CreateGridLayout.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] CollectionView iOS/MacCatalyst — Subscribing only to the top-level ObservableGroupedSource misses the per-group child ObservableItemsSource instances it creates in _groups. When an individual group's inner collection raises a Reset (or any change routed through ObservableItemsSource.Reload()/Update()), that child source calls collectionView.ReloadData() + InvalidateLayout() directly, the section provider re-runs, and it re-reads the now-stale global estimate — exactly the bypass path this PR is trying to close, but for grouped data, which is the reported repro shape. Subscribe to the child sources too (and unsubscribe when _groups is rebuilt), or hook the reset at a level both sources funnel through.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Memory Leak Prevention — ObservableItemsSource deliberately holds its controller as WeakReference<UICollectionViewController> (see its constructor), because the source itself is rooted by the user's collection via ((INotifyCollectionChanged)itemSource).CollectionChanged += CollectionChanged. This += installs a strong managed reference from the source back to the controller, so for as long as the user's ObservableCollection is alive, the controller — and through it the UICollectionView, its cells, and the whole page/ViewModel graph — is kept alive. That is fine only if Dispose/DisposeItemsSource always runs; on iOS the controller can be released without Dispose running deterministically (e.g. Shell tab switch / handler disconnect ordering). Prefer a weak-event or holder-object subscription so the weak-controller design is preserved, and add a leak-detection device test covering a grouped CV whose source collection outlives the page.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Performance-Critical Path — UpdateLayoutEstimatedItemSize() runs on every item-cell (re)measure, not just the first one, and the cheap short-circuit (MeasuredEstimatedItemSize is null) is evaluated last. Each call first walks CollectionViewHandler (which itself re-walks PlatformHandler.VirtualView → view.Parent → itemsView.Handler with three type checks — note the getter is already invoked twice on this path via GetCachedFirstItemSizeFromHandler/SetCachedFirstItemSizeToHandler) and then crosses into Obj-C for Controller.CollectionView.CollectionViewLayout. During fast scrolling of a large CV this is repeated per cell per pass for no benefit. Hoist a cheap managed guard (e.g. a bool _estimateReported on the controller/handler, reset alongside ResetEstimatedItemSize) and return early before the handler walk and the Obj-C property access.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[minor] Performance-Critical Path — NotifyCollectionChangedEventArgs is allocated unconditionally on every Reload(), even though the only consumer of CollectionViewUpdating (CV2's ItemsViewController2) ignores everything except Action == Reset, and there is normally no subscriber at all for CV1. Guard the allocation on the delegate being non-null (if (CollectionViewUpdating is { } handler) handler(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));) — Reload() is called for every Reset on the bound collection.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[critical] Regression Prevention and Test Coverage — This test does not discriminate fixed from unfixed: the Gate ran both tests in this file against a build without the production change and they passed (confirmed over repeat runs), then passed again with it. The 30pt tolerance is on the same order as the very estimate error being corrected (default estimate 30pt vs. an actual item height), so the drift the PR fixes is absorbed by the tolerance. A regression test for #34663 must (a) tighten the tolerance to a few points, and (b) be verified to FAIL on the unpatched tree — per repo policy, a bug fix needs a test that reproduces the original issue. As written, this PR has no evidence that the reported behavior is fixed.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention and Test Coverage — MakeVisibleScrollToGroupedItemMakesItemVisible asserts only that the target item is somewhere within the CollectionView's bounds after a single ScrollTo(..., MakeVisible). That is trivially true on the first scroll even in the buggy build — the reported defect (#34663) is inconsistent landing positions after repeated MakeVisible scrolls, which this test never exercises (no repeat/scroll-away cycle). It therefore cannot fail for the bug it is named after, which matches the Gate result. Either fold the repeat cycle into this test or drop it, so the suite does not give false green coverage for this issue.

[MauiBot]

AI Review Summary

@praveenkumarkarunanithi — new AI review results are available based on this last commit: 3c207ac.

---

🗂️ Review Sessions — click to expand

🚦 Gate — Test Before & After Fix

Gate Result: ❌ FAILED

Platform: IOS · Base: main · Merge base: f27ca83a

🩺 Test does not reproduce the bug — ran the same in both states (PASS without fix, PASS with fix). The repro test is not exercising the issue. Strengthen the test before reviewing the fix.

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🖥️ Issue34663 Issue34663	❌ PASS — 612s	✅ PASS — 131s

🔴 Without fix — 🖥️ Issue34663: PASS ❌ · 612s

(no coded error found; showing last 1200 chars)

   Discovered:  Controls.TestCases.iOS.Tests
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll
   NUnit3TestExecutor discovered 2 of 2 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 8/8/2026 3:28:09 AM FixtureSetup for Issue34663(iOS)
>>>>> 8/8/2026 3:28:15 AM MakeVisibleScrollToGroupedItemMakesItemVisible Start
>>>>> 8/8/2026 3:28:17 AM MakeVisibleScrollToGroupedItemMakesItemVisible Stop
  Passed MakeVisibleScrollToGroupedItemMakesItemVisible [1 s]
>>>>> 8/8/2026 3:28:17 AM MakeVisibleScrollToGroupedItemProducesConsistentPosition Start
>>>>> 8/8/2026 3:28:40 AM MakeVisibleScrollToGroupedItemProducesConsistentPosition Stop
  Passed MakeVisibleScrollToGroupedItemProducesConsistentPosition [22 s]
NUnit Adapter 4.5.0.0: Test execution complete
Results File: /Users/cloudtest/vss/_work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue34663.trx
Test Run Successful.
Total tests: 2
     Passed: 2
 Total time: 1.9278 Minutes
>>> TRX_RESULT_FILE: /Users/cloudtest/vss/_work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue34663.trx

🟢 With fix — 🖥️ Issue34663: PASS ✅ · 131s

(no coded error found; showing last 1200 chars)

  Discovered:  Controls.TestCases.iOS.Tests
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll
   NUnit3TestExecutor discovered 2 of 2 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 8/8/2026 3:34:51 AM FixtureSetup for Issue34663(iOS)
>>>>> 8/8/2026 3:34:56 AM MakeVisibleScrollToGroupedItemMakesItemVisible Start
>>>>> 8/8/2026 3:34:57 AM MakeVisibleScrollToGroupedItemMakesItemVisible Stop
  Passed MakeVisibleScrollToGroupedItemMakesItemVisible [1 s]
>>>>> 8/8/2026 3:34:57 AM MakeVisibleScrollToGroupedItemProducesConsistentPosition Start
>>>>> 8/8/2026 3:35:19 AM MakeVisibleScrollToGroupedItemProducesConsistentPosition Stop
  Passed MakeVisibleScrollToGroupedItemProducesConsistentPosition [22 s]
NUnit Adapter 4.5.0.0: Test execution complete
Results File: /Users/cloudtest/vss/_work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue34663.trx
Test Run Successful.
Total tests: 2
     Passed: 2
 Total time: 43.9695 Seconds
>>> TRX_RESULT_FILE: /Users/cloudtest/vss/_work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue34663.trx

⚠️ Failure Details

• ❌ Issue34663 PASSED without fix (should fail) — tests don't catch the bug

📁 Fix files reverted (4 files)

• src/Controls/src/Core/Handlers/Items/iOS/ObservableGroupedSource.cs
• src/Controls/src/Core/Handlers/Items2/iOS/ItemsViewController2.cs
• src/Controls/src/Core/Handlers/Items2/iOS/LayoutFactory2.cs
• src/Controls/src/Core/Handlers/Items2/iOS/TemplatedCell2.cs

---

📋 Pre-Flight — Context & Validation

Issue: #34663 - [MacOS][CV2] I9_Scrolling - Setting 'Make Visible' results in wrong position after trying multiple times
PR: #35668 - [iOS, MacCatalyst] Fix CollectionView2 grouped ScrollTo MakeVisible producing inconsistent positions
Platforms Affected: iOS, MacCatalyst
Files Changed: 4 implementation, 2 UI test

Key Findings

• The issue reproduces only with the current iOS/MacCatalyst CollectionView2 handler: repeated animated ScrollTo(..., MakeVisible) calls against grouped data can stop at inconsistent positions; CV1 is unaffected.
• The PR replaces the compositional layout's fixed 30-point item/group estimate with a first-item measured estimate shared through a non-layout holder, then resets that estimate on controller reloads and observable-source resets.
• Existing review feedback identifies unresolved edge cases around one write-once global estimate: dynamic remeasurement/rotation can leave it stale, supplementary header/footer estimates remain at 30 points, and child collection resets inside a group are not observed.
• The earlier retain-cycle concern was addressed by EstimatedItemSizeHolder, which lets the section-provider closure avoid capturing the layout.
• Gate failed definitively because both Issue34663 UI tests passed without the production fix (including two confirmation runs) and with it. Candidate test success therefore cannot establish that the regression is fixed.
• The detected primary command is pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform ios -TestFilter "Issue34663". Regression cross-reference is CLEAN, so there are no additional mandatory regression tests.
• Impacted UI test category: CollectionView.

Code Review Summary

Verdict: SKIPPED
Confidence: N/A
Errors: N/A | Warnings: N/A | Suggestions: N/A

Key code review findings:

• Dedicated expert review is deferred to STEP 5b as required by the execution contract; no separate expert-review agent was launched in STEP 5a.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #35668	Feed the first measured item dimension back into compositional-layout section estimates and reset it on data reload/reset	❌ Gate failed: tests passed with and without fix	4 implementation files	Original PR; behavior remains unproven by its UI tests

---

🔬 Code Review — Deep Analysis

Expert PR Evaluation

Verdict: REQUEST CHANGES

Assessment: The submitted fix is not proven to repair the reported grouped ScrollTo(..., MakeVisible) instability. The measured estimate is recorded during PreferredLayoutAttributesFittingAttributes, after the current compositional-layout sections have already been created, but the change does not invalidate the layout so the section provider can consume the new value. This makes the behavior dependent on a later, unrelated invalidation and is consistent with the trusted Gate result: the added tests did not distinguish the patched and unpatched implementations.

Blocking findings

1. TemplatedCell2.cs stores the estimate without scheduling a layout invalidation, leaving the active layout on its original estimate.
2. The estimate is a write-once, layout-wide first-cell value. Heterogeneous templates, grouped sections, rotation, dynamic type, and later remeasurement can therefore retain a stale estimate and reproduce the same offset-drift class.
3. LayoutFactory2.cs applies the measured estimate to item/group dimensions but not to grouped header/footer supplementary items, leaving an important source of cumulative section-offset error on the original estimate.
4. ItemsViewController2.cs observes only the top-level grouped source. Per-group child sources can reload and invalidate directly while retaining the stale global estimate.
5. The regression tests are non-discriminating. The 30-point tolerance masks an error of the same scale, and the MakeVisible test performs only one scroll and checks only that the item is somewhere in bounds rather than reproducing repeated-position inconsistency.

Additional concerns

• The new strong CollectionViewUpdating subscription can undermine the child source's weak-controller ownership and retain a controller if normal disposal is skipped or delayed.
• UpdateLayoutEstimatedItemSize() performs handler traversal and native layout access on every cell remeasure before reaching its write-once guard.
• ObservableGroupedSource.Reload() allocates event arguments even when no listener is registered.

Positive aspects

The holder indirection avoids directly capturing the layout in the section-provider closure, and the new effective-dimension locals eliminate cross-invocation mutation of captured layout parameters. The measured value also replaces only dimensions that were already estimated.

Required direction: Add deterministic deferred invalidation when the estimate changes, handle grouped supplementary and child-source invalidation paths, avoid a stale first-cell-wins estimate, and replace the tests with assertions proven to fail on the unpatched behavior.

---

🛠️ Try-Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	claude-opus-5 / try-fix	After animated grouped MakeVisible scrolling, realign the content offset from settled item layout attributes instead of learning a global estimated item size	⚠️ PASS (2/2 after one correction), but layered on PR fix	3 files	Initial run crashed on a disposed NSIndexPath; the one allowed correction captured section/item values. Baseline establishment was blocked by unrelated dirty files, so isolation was not achieved. Gate tests are non-discriminating.
2	gpt-5.6-sol / try-fix	Invalidate and synchronously flush the compositional layout before grouped MeasureAllItems MakeVisible scrolling	⚠️ PASS (2/2), but layered on PR fix	1 file	One run; no correction/retest. Baseline establishment was blocked by unrelated dirty files. Gate tests are non-discriminating.
PR	PR #35668	Feed the first measured item dimension into compositional-layout estimates and reset it on reload/reset	❌ Gate failed: tests passed with and without fix	4 implementation files	Original PR

Candidate 1 Details

• Approach: Deterministic post-animation offset realignment using settled layout attributes, while removing a duplicate scroll path.
• Files: src/Controls/src/Core/Handlers/Items2/CollectionViewHandler2.iOS.cs, src/Controls/src/Core/Handlers/Items2/ItemsViewHandler2.iOS.cs, src/Controls/src/Core/Handlers/Items2/iOS/GroupableItemsViewController2.cs
• Run 1: Failed with ObjectDisposedException: Foundation.NSIndexPath.
• Correction: Captured section and item indices by value before the callback boundary.
• Run 2: Passed both Issue34663 tests.
• Failure analysis: EstablishBrokenBaseline.ps1 rejected 36 unrelated pre-existing dirty files, so the candidate ran on top of the PR fix. The pass establishes compilation and absence of the observed crash only; it does not prove either this candidate or the PR fixes #34663 because the same tests pass on the broken baseline.
• Inline self-review: 4 findings (0 critical, 0 major, 2 moderate, 2 minor).
• Full narrative and diff: ../try-fix-1/content.md

Candidate 2 Details

• Approach: Flush pending UIKit self-sizing layout state immediately before grouped animated MakeVisible scrolling.
• File: src/Controls/src/Core/Handlers/Items2/CollectionViewHandler2.iOS.cs
• Run 1: Passed both Issue34663 tests; no correction or retest.
• Evidence limitation: Baseline establishment was blocked by the same unrelated dirty .github/** and eng/** files, so the candidate ran on top of the PR fix. The pass proves execution/compatibility only because the tests pass on the broken baseline.
• Inline self-review: Clean (0 findings).
• Full narrative and diff: ../try-fix-2/content.md

Cross-Pollination

Skipped by the STEP 5a hard execution contract.

Exhausted: Yes — candidate 2 of 2 completed.
Selected Fix: None proven; both candidate passes are non-discriminating and lacked broken-baseline isolation.

---

📝 PR Finalize — Recommended Title & Description

Assessment: ✏️ Recommend updating — the title remains directionally accurate, but the current description documents the superseded per-cell write-once implementation rather than the winning pre-scroll layout-preparation and weak reset-contract approach.

Recommended title

[iOS, MacCatalyst] CollectionView2: Stabilize grouped MakeVisible ScrollTo positions

Recommended description

### Root Cause

CollectionView2 on iOS and MacCatalyst uses `UICollectionViewCompositionalLayout`. Its section provider initially constructs self-sizing item, group, and supplementary layouts with a hardcoded 30-point estimate. During an animated grouped `ScrollTo(..., MakeVisible)`, UIKit can calculate the destination while pending self-sizing work still uses provisional section geometry. Later measurements then change the content size and item positions, so repeated requests can settle at inconsistent offsets.

### Description of Change

* `CollectionViewHandler2.iOS.cs` — Before grouped `MeasureAllItems` scrolls that require adjustment (`MakeVisible` or `End`), reads a valid scroll-axis dimension from a currently visible measured cell, updates the custom compositional layout's estimate, invalidates the layout, and calls `LayoutIfNeeded()` before issuing `ScrollToItem`. The non-animated path now issues only one scroll instead of the previous duplicate call.

* `LayoutFactory2.cs` — Uses an `EstimatedItemSizeHolder` so section-provider closures can consume the current measured estimate without capturing the layout. List and grid providers use effective local dimensions for items, groups, and grouped header/footer supplementary items, avoiding both the fixed 30-point estimate and mutation of captured parameters.

* `IItemsViewSource.cs`, `ObservableItemsSource.cs`, `ObservableGroupedSource.cs`, and `ItemsViewController2.cs` — Add an internal layout reset contract and invoke it before controller, top-level source, and nested group reloads. This prevents a prior dataset's estimate from surviving a reload without introducing a strong source-to-controller event subscription.

* `Issue34663.cs` HostApp and UI test — Add a deterministic button that returns the grouped CollectionView to its first item. The test repeats animated `MakeVisible` scrolling four times, verifies the target remains visible, and requires positions to agree within 2 points rather than masking the estimate error with a 30-point tolerance.

### Key Technical Details

- Estimate capture occurs immediately before the affected scroll, not from every cell-measure callback.
- Layout invalidation and flushing happen after the estimate is set, ensuring the section provider can consume it before `ScrollToItem`.
- Reload resets flow through the layout held by the collection view, preserving the existing weak controller ownership used by observable item sources.

### What NOT to Do (for future agents)

- Do not set a write-once global estimate from `PreferredLayoutAttributesFittingAttributes` without invalidating the compositional layout; that update occurs after the active sections have already been created.
- Do not subscribe the controller strongly to observable sources solely to reset layout state; child sources are rooted by user collections and are designed to hold the controller weakly.
- Do not use a tolerance comparable to the 30-point faulty estimate in the regression test, because it allows the broken implementation to pass.

### Issues Fixed

Fixes #34663

### Platforms Tested

- [x] iOS
- [ ] MacCatalyst (shared implementation; not run in this validation)
- [ ] Android (not affected)
- [ ] Windows (not affected)

### Screenshots

| Before Issue Fix | After Issue Fix |
|------------------|-----------------|
| <video width="350" alt="withoutfix" src="https://github.com/user-attachments/assets/2aeb2b5f-b8ff-429e-8ae3-435f35e6efa0" /> | <video width="350" alt="withfix" src="https://github.com/user-attachments/assets/b668f2b6-ea96-4aae-a057-da68a0e2d6d4" /> |

---

🏁 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Winning candidate: pr-plus-reviewer

The submitted pr candidate should not be approved as-is. The trusted Gate failed because its tests passed both without and with the implementation change, and the expert review found that the measured estimate was written during an active layout pass without a deterministic invalidation to make the section provider consume it. The consolidated reviewer refinement removes that timing gap and the associated hot-path, stale-state, nested-reset, supplementary-layout, and weak-ownership concerns.

Comparative ranking

Rank	Candidate	Validation	Assessment
1	pr-plus-reviewer	✅ pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform ios -TestFilter "Issue34663" passed in its single allowed run	Best alignment with the expert findings. It obtains a current visible-cell dimension immediately before the affected grouped MeasureAllItems scroll, rebuilds and flushes the compositional layout before ScrollToItem, resets the estimate through a weak layout contract on controller, top-level, and nested-group reloads, updates grouped supplementary estimates, removes the duplicate non-animated scroll, and replaces the permissive test with deterministic reset cycles and a 2-point consistency tolerance.
2	try-fix-2	⚠️ Final run passed 2/2, but only while layered on the PR and against tests already known to pass on the broken baseline	The smallest alternative and directionally sound: invalidate and flush the layout before grouped MakeVisible. It does not refresh the fixed 30-point estimate from measured geometry, address supplementary estimates or reload staleness, or provide discriminating test evidence.
3	try-fix-1	⚠️ Initial run crashed; the corrected candidate then passed 2/2 while layered on the PR	Settled-layout offset realignment is plausible and avoids a global estimate, but it had an NSIndexPath lifetime crash before correction and retains moderate uncertainty around callbacks that may not fire and layout attributes that may still be estimated. Its passing tests were non-discriminating.
4	pr	❌ Trusted Gate failed	Lowest-ranked candidate. Its tests did not reproduce the defect, and the expert review found no guaranteed section-provider invalidation, a first-cell-wins global estimate, stale rotation/remeasure behavior, untouched supplementary estimates, missed child-group resets, a strong observer retention path, and unnecessary work in the cell-measure hot path.

Candidates with failed validation were ranked below candidates whose final targeted run passed. A pass is not treated as proof where the test or baseline was non-discriminating: both STEP 5a candidates ran on top of the PR, and neither established a broken-baseline failure. pr-plus-reviewer has stronger positive evidence because its one run used a deterministic top reset, repeated animated MakeVisible requests, visibility assertions, and a 2-point position tolerance; however, the Gate was not rerun and the strengthened test was not independently shown to fail on the unpatched tree, so that residual uncertainty remains explicit.

Expert feedback incorporated by pr-plus-reviewer

• Moves estimate selection out of TemplatedCell2.PreferredLayoutAttributesFittingAttributes, avoiding an update made too late for the current section build and eliminating repeated handler/native traversal from the cell-measure hot path.
• Sets the current measured estimate before an explicit InvalidateLayout() and synchronous LayoutIfNeeded(), so the section provider consumes it before the affected scroll.
• Uses effective dimensions for grouped header/footer supplementary items.
• Replaces strong controller event subscriptions with an internal layout reset contract invoked by ObservableItemsSource, ObservableGroupedSource, and ItemsViewController2; nested group resets therefore clear the estimate without retaining the controller.
• Removes the duplicate non-animated grouped adjustment call.
• Consolidates the two permissive tests into one repeated-scroll test with deterministic reset, visibility checks, and a 2-point tolerance.

Decision

Adopt pr-plus-reviewer before merge. The raw PR is not the winner, the trusted Gate does not permit approval, and the expert review contains blocking findings against the submitted implementation; therefore the required recommendation is REQUEST CHANGES.

---

📱 UI Tests — CollectionView

Detected UI test categories: CollectionView

✅ Deep UI tests — 426 passed, 0 failed, 3 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	426/429 (3 skipped) ✓	—
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)

---

🧭 Next Steps — review latest findings

No alternative fix was selected for this run. Review the session findings and CI results before merging.

[kubaflo]

Could you please check the ai's suggestions?