[Android] Shell: Defer tab infrastructure for single-page startup - #37321
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37321Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37321" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR optimizes the handler-based Android Shell startup path by deferring creation of bottom/top tab infrastructure until it’s actually needed (tabs become visible or collections grow beyond a single child), addressing the cold-start regression described in #37282.
Changes:
- Defer bottom-tab (
ShellItemHandler) infrastructure untilShowTabsis true, and recreate the bottom-nav appearance tracker when the activeShellItemchanges. - Defer top-tab (
ShellSectionHandler) infrastructure until aShellSectionhas more than one visibleShellContent, and initialize it on dynamic collection growth while preserving appearance propagation. - Add/extend Android device tests to assert zero unnecessary startup creation and correct tracker recreation/appearance behavior when tabs become needed.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.Android.cs | Adds a virtual hook to adjust the tab-colors test setup per handler variant. |
| src/Controls/tests/DeviceTests/Elements/Shell/ShellHandlerSubclasses.Android.cs | Adds Android Shell device tests and a tracking handler to verify deferred creation and tracker recreation behavior. |
| src/Controls/src/Core/Handlers/Shell/ShellSectionHandler.Android.cs | Defers top tab (TabLayout/TabbedViewManager) creation until multiple visible contents exist; initializes on collection changes and replays appearance when created late. |
| src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Android.cs | Defers bottom tab manager creation until tabs are shown; recreates the bottom-nav appearance tracker when needed and on rebuild/switch. |
Avoid creating bottom and top tab managers and appearance trackers for single-page Shell startup. Create them when tabs become necessary while preserving appearance across dynamic growth and ShellItem switches. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 82dbb6c6-be36-4d2b-8ce5-ae517e3bc684
f955b6f to
413fb9f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Controls/src/Core/Handlers/Shell/ShellSectionHandler.Android.cs:245
- Deferring top-tab infrastructure means
_tabbedViewManageris null for single-content sections, soUpdateTabLayoutVisibility()becomes a no-op and never hides the sharednavigationlayout_toptabscontainer. Sincenavigationlayout.axmldoesn’t set a defaultandroid:visibilityon that container, this can regress the initial single-page/one-content case by leaving the top-tabs slot visible/laid out until something later callsRemoveTopTabs().
SetupTabbedViewManager();
// Update TabLayout visibility based on item count
UpdateTabLayoutVisibility();
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 9 findings
See inline comments for details.
kubaflo
left a comment
There was a problem hiding this comment.
Could you check the ai's suggestions?
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 82dbb6c6-be36-4d2b-8ce5-ae517e3bc684
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Select the test-bearing class when a concrete helper class appears first in a changed device-test file. Add regression coverage for the PR #37321 Shell layout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15d2af20-e4ab-4e88-9011-cfbd83513bc0
This comment has been minimized.
This comment has been minimized.
Replay badge state when deferred bottom tabs are created and tolerate top-tab setup callbacks after Shell context teardown. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 82dbb6c6-be36-4d2b-8ce5-ae517e3bc684
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Android.cs:265
- RemoveBottomNavigationInfrastructure() clears the TabbedViewManager and BottomNavigationView but leaves the existing _appearanceTracker alive. When SwitchToShellItem() switches to an item with ShowTabs == false, this can keep a tracker created for the previous ShellItem around until a later recreation or DisconnectHandler, increasing the chance of holding stale references longer than necessary. Dispose and null the appearance tracker as part of this teardown to fully drop bottom-tab infrastructure when tabs aren’t shown.
void RemoveBottomNavigationInfrastructure()
{
_tabbedViewManager?.SetElement(null);
_tabbedViewManager = null;
_shellItemAdapter = null;
_bottomNavigationView = null;
}
This comment has been minimized.
This comment has been minimized.
|
A new review pass is needed against the current head, since the previous review was generated from an earlier commit. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 2 findings
See inline comments for details.
| section.Items.Add(secondContent); | ||
|
|
||
| Assert.NotNull(itemHandler._tabbedViewManager); | ||
| Assert.NotNull(sectionHandler.ContentTabLayout); |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Regression Prevention and Test Coverage — The growth step only asserts that the deferred top TabLayout object was created (ContentTabLayout non-null + its background color). It never asserts that the shared navigationlayout_toptabs container actually became visible. The negative case is covered twice (line 95 asserts Gone at startup, line 119 asserts Gone after shrinking back to one content), but the positive case — top tabs are placed and shown after deferred creation — is not. Concrete scenario this test cannot catch: SetupTabbedViewManager() creates _contentTabLayout and the manager, but UpdateTabLayoutVisibility() → PlaceTopTabs() fails to commit the fragment transaction / returns early (e.g. the active-section guard parentItem.CurrentItem != VirtualView evaluates false-negative during the collection-changed callback). The user sees no top tabs after adding a second ShellContent, and this test still passes green. Add Assert.Equal(AViewStates.Visible, topTabsContainer.Visibility); after the growth assertions so the assertion pair discriminates placed-vs-created.
| // setup was deferred, replay the appearance for the newly-created TabLayout. | ||
| if (_registeredShell is not null && IsCurrentlyActiveSection() && VirtualView.CurrentItem is ShellContent currentContent) | ||
| { | ||
| var page = ((IShellContentController)currentContent).GetOrCreateContent(); |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Logic and Correctness — The deferred-setup appearance replay uses the section's root content page (CurrentItem → GetOrCreateContent()) as the AppearanceChanged source, not the page currently displayed by this section. IShellController.AppearanceChanged(source, false) calls UpdateToolbarAppearanceFeatures(source, null) with that source directly (Shell.cs), so the toolbar features are recomputed from a page that may not be on screen. Concrete scenario: an active ShellSection with one ShellContent where the app has pushed a detail page onto the section's stack, and then adds a second ShellContent (the deferred-creation trigger). The replay fires with the section root ContentPage while the pushed detail page is displayed, so Shell-level toolbar appearance is resolved against the wrong page. The sibling ShellItemHandler.SetupTabbedViewManager replay avoids this by using _displayedPage; this path should use the section's currently displayed page (navigation-stack top) and skip the replay when it cannot be resolved.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@jonathanpeppers — new AI review results are available based on commit
0d9d391.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ✅ PASSED
Platform: ANDROID · Base: net11.0 · Merge base: 01685127
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
📱 ShellHandlerTests_Shell (SinglePageShellCreatesTabInfrastructureOnlyWhenNeeded, SwitchingShellItemsCreatesBottomTabsOnlyWhenNeeded, SwitchingShellItemsRecreatesBottomNavAppearanceTracker) Category=Shell |
✅ FAIL — 674s | ✅ PASS — 637s |
📱 ShellTests Category=Shell |
✅ PASS — 417s |
🔴 Without fix — 📱 ShellHandlerTests_Shell (SinglePageShellCreatesTabInfrastructureOnlyWhenNeeded, SwitchingShellItemsCreatesBottomTabsOnlyWhenNeeded, SwitchingShellItemsRecreatesBottomNavAppearanceTracker): FAIL ✅ · 674s
(no coded error found; showing last 1200 chars)
13 13:40:59.985 8623 9653 I DOTNET : === TEST EXECUTION SUMMARY ===
08-13 13:40:59.985 8623 9653 I DOTNET : Tests run: 687 Passed: 68 Inconclusive: 0 Failed: 4 Ignored: 615 Skipped: 0
fail: Non-success instrumentation exit code: 1, expected: 0
info: <<XHARNESS_RESULT_START>>
{
"version": 1,
"machineName": "runnervmtroe5",
"exitCode": 1,
"exitCodeName": "TESTS_FAILED",
"platform": "android",
"instrumentationExitCode": 1,
"device": "emulator-5554",
"deviceOsVersion": "API 30",
"architecture": "x86_64",
"files": [
{
"name": "testResults-342a182c249843768e00cb2a8b255ce5.xml",
"type": "test-results"
},
{
"name": "adb-logcat-com.microsoft.maui.controls.devicetests-default.log",
"type": "logcat"
}
]
}
<<XHARNESS_RESULT_END>>
info: Attempting to remove apk 'com.microsoft.maui.controls.devicetests'..
info: Successfully uninstalled com.microsoft.maui.controls.devicetests
XHarness exit code: 1 (TESTS_FAILED)
Passed: 0
Failed: 3
Skipped: 0
Total: 3
Tests completed with exit code: 1
🟢 With fix — 📱 ShellHandlerTests_Shell (SinglePageShellCreatesTabInfrastructureOnlyWhenNeeded, SwitchingShellItemsCreatesBottomTabsOnlyWhenNeeded, SwitchingShellItemsRecreatesBottomNavAppearanceTracker): PASS ✅ · 637s
(no coded error found; showing last 1200 chars)
37.979 17735 18195 I DOTNET : Xml file was written to the provided writer.
08-13 14:06:37.979 17735 18195 I DOTNET : === TEST EXECUTION SUMMARY ===
08-13 14:06:37.979 17735 18195 I DOTNET : Tests run: 687 Passed: 72 Inconclusive: 0 Failed: 0 Ignored: 615 Skipped: 0
info: <<XHARNESS_RESULT_START>>
{
"version": 1,
"machineName": "runnervmtroe5",
"exitCode": 0,
"exitCodeName": "SUCCESS",
"platform": "android",
"instrumentationExitCode": 0,
"device": "emulator-5554",
"deviceOsVersion": "API 30",
"architecture": "x86_64",
"files": [
{
"name": "testResults-21e84ce7897f4bca9f73cb7a6a2fafdf.xml",
"type": "test-results"
},
{
"name": "adb-logcat-com.microsoft.maui.controls.devicetests-default.log",
"type": "logcat"
}
]
}
<<XHARNESS_RESULT_END>>
info: Attempting to remove apk 'com.microsoft.maui.controls.devicetests'..
info: Successfully uninstalled com.microsoft.maui.controls.devicetests
XHarness exit code: 0
Passed: 3
Failed: 0
Skipped: 0
Total: 3
Tests completed successfully
🔴 Without fix — 📱 ShellTests: ⚠️ ENV ERROR · 898s
No log file found
🟢 With fix — 📱 ShellTests: PASS ✅ · 417s
(no coded error found; showing last 1200 chars)
.467 18660 19146 I DOTNET : Xml file was written to the provided writer.
08-13 14:13:36.467 18660 19146 I DOTNET : === TEST EXECUTION SUMMARY ===
08-13 14:13:36.467 18660 19146 I DOTNET : Tests run: 687 Passed: 69 Inconclusive: 0 Failed: 0 Ignored: 618 Skipped: 0
info: <<XHARNESS_RESULT_START>>
{
"version": 1,
"machineName": "runnervmtroe5",
"exitCode": 0,
"exitCodeName": "SUCCESS",
"platform": "android",
"instrumentationExitCode": 0,
"device": "emulator-5554",
"deviceOsVersion": "API 30",
"architecture": "x86_64",
"files": [
{
"name": "testResults-b7cf6b4387644567bdeca2ee27a7f5b1.xml",
"type": "test-results"
},
{
"name": "adb-logcat-com.microsoft.maui.controls.devicetests-default.log",
"type": "logcat"
}
]
}
<<XHARNESS_RESULT_END>>
info: Attempting to remove apk 'com.microsoft.maui.controls.devicetests'..
info: Successfully uninstalled com.microsoft.maui.controls.devicetests
XHarness exit code: 0
Passed: 69
Failed: 0
Skipped: 0
Total: 69
Tests completed successfully
⚠️ Failure Details
⚠️ ShellTests without fix:XHarness did not produce the expected fresh result 'testResults-79635298602344efa9e5453246641224.xml' for requested class(es) 'Microsoft.Maui.DeviceTests.ShellTests' (the target tests did not run).
📁 Fix files reverted (2 files)
src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Android.cssrc/Controls/src/Core/Handlers/Shell/ShellSectionHandler.Android.cs
📋 Pre-Flight — Context & Validation
PR #37321 Pre-Flight
Context
- Title:
[Android] Shell: Defer tab infrastructure for single-page startup - Base:
net11.0 - Materialized review commit:
a3176decf508b79240d68fef4cf3707ec7014ba7 - Issue: #37282, handler-based Android Shell regresses cold startup for the default single-page shape.
- Target platform: Android
- Gate: Passed previously; do not rerun baseline gate verification and do not modify
gate/content.md.
The regression is caused by eagerly creating bottom/top tab managers, native tab controls, adapters, callbacks, and appearance trackers when a Shell item/section has only one visible child. The PR defers bottom-tab creation until IShellItemController.ShowTabs is true and top-tab creation until a section has more than one visible content. It also adds dynamic-growth, reuse, appearance, badge, and Shell-item-switch handling.
Direct Diff Inspection
Production changes:
src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Android.cs- Makes
SetupTabbedViewManager()lazy and idempotent. - Creates/recreates the bottom-nav appearance tracker with the manager.
- Removes bottom-nav infrastructure when the active item does not show tabs.
- Lazily creates or rebuilds infrastructure during item switching, visibility updates, and collection changes.
- Makes
src/Controls/src/Core/Handlers/Shell/ShellSectionHandler.Android.cs- Defers
TabLayout,TabbedViewManager, adapter, callback, and appearance tracker creation until more than one visible content exists. - Creates deferred infrastructure on collection growth.
- Uses
SetValueFromRendererwhen native page selection updatesCurrentItem.
- Defers
Test changes:
src/Controls/tests/DeviceTests/Elements/Shell/ShellHandlerSubclasses.Android.cs- Adds startup tracker counts and tests for lazy creation, dynamic growth, manager reuse, badges/appearance, and Shell-item switching.
src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.Android.cs- Keeps the Android handler variant of the tab-colors test on the tabbed path.
The existing solution's root-cause hypothesis is deferred native tab infrastructure with later lazy promotion. Alternative candidates must use a materially different implementation strategy, not merely move the same guards to another callback. Public review history also identified lifecycle/appearance risks around time-varying ShowTabs, global AppearanceChanged rebroadcasts, tracker recreation, and single-to-multi Shell-item transitions.
Bounded Test Contract
Run only these Android Controls device-test scopes:
-
Primary:
pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Controls -Platform android -TestFilter "Category=Shell" -IncludeClasses "Microsoft.Maui.DeviceTests.ShellHandlerTests_Shell" -IncludeMethods "SinglePageShellCreatesTabInfrastructureOnlyWhenNeeded;SwitchingShellItemsCreatesBottomTabsOnlyWhenNeeded;SwitchingShellItemsRecreatesBottomNavAppearanceTracker"
-
Mandatory regression:
pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Controls -Platform android -TestFilter "Category=Shell" -IncludeClasses "Microsoft.Maui.DeviceTests.ShellTests"
Do not run the gate, a category-wide unscoped suite, or any full test suite.
Repository State
The worktree contains unrelated pre-existing changes under .github/ and eng/. They are outside this PR's production/test files and must not be edited, reverted, included in candidate diffs, or treated as candidate changes. Baseline setup/restoration must use only .github/scripts/EstablishBrokenBaseline.ps1 as required by the try-fix skill.
🔬 Code Review — Deep Analysis
Expert Evaluation — PR #37321
Independent Assessment
The Android handler-based Shell change lazily materializes bottom-tab infrastructure only when a ShellItem shows tabs and top-tab infrastructure only when a ShellSection has multiple visible contents. It adds promotion paths for dynamic collection growth and Shell-item switching, preserves native-manager reuse, and replays badge and appearance state after deferred creation.
The approach is sound and remains confined to Android Shell handlers and device tests. The reviewer traced teardown, reconnect, visibility, item-switch, badge, appearance, and shrink/regrow paths and found the core deferral and reuse model coherent.
Prior Review and CI Reconciliation
All prior error-level findings found across review bodies, inline comments, and issue comments were either fixed at the submitted HEAD or disproved by the current control flow. Required CI for HEAD was successful or intentionally skipped, with no failing or pending check. The trusted gate passed and was not rerun.
Actionable Findings
- Moderate — missing positive placement assertion:
SinglePageShellCreatesTabInfrastructureOnlyWhenNeededproves that the deferred topTabLayoutis created after growth but does not assert that the shared top-tabs container becomes visible. A created-but-unplaced regression could pass. - Moderate — deferred appearance replay uses the wrong page:
ShellSectionHandler.SetupTabbedViewManager()raisesAppearanceChangedwith the section root content rather than the currently displayed page. If a detail page is pushed when a secondShellContentis added, toolbar appearance can be recomputed from a page that is not on screen.
The raw inline findings are persisted in inline-findings.json.
Blast Radius and Failure Modes
The change affects Android Shell startup, dynamic tab growth, Shell-item switching, appearance and badge propagation, and handler reconnect paths. It introduces no public API or shared static state. Existing tests cover zero startup creation, growth, reuse, shrink/regrow, badges, appearance, and Shell-item switching; the first finding identifies the remaining placement assertion gap.
Verdict
LGTM with two focused follow-ups; confidence medium. Neither finding invalidates the deferral architecture, but both are cheap, concrete improvements suitable for a single consolidated pr-plus-reviewer candidate.
🛠️ Try-Fix — Analysis & Comparison
PR #37321 Alternative Fix Candidates
Candidate 1 — Demand-Reconciled Tab Infrastructure
Model: claude-opus-5
Result: Pass — first implementation/test pass; no corrective retest.
Approach
Candidate 1 replaces the PR's promotion calls spread across lifecycle and mutation sites with a computed desired-state model:
BottomTabsRequireddetermines whether the activeShellItemneeds native bottom tabs.ReconcileBottomTabs(bool allowTeardown)is the single owner of bottom-tab create, refresh, hide, and release transitions.EnsureTopTabInfrastructure()creates section tab objects from the existing visibility/placement decision rather than from adapter setup.- Late-created tab appearance is seeded by re-registering only the affected appearance observer, avoiding the PR's global
AppearanceChangedrebroadcast. - The candidate does not include the PR's
SetValueFromRendererchange.
Production diff: ShellItemHandler.Android.cs +120/-33; ShellSectionHandler.Android.cs +76/-34. No tests or public APIs changed. The complete diff and full narrative are persisted in:
CustomAgentLogsTmp/PRState/37321/PRAgent/try-fix-1/content.mdCustomAgentLogsTmp/PRState/37321/PRAgent/try-fix/attempt-1/fix.diff
Test Results
- Primary
ShellHandlerTests_Shellscope: 3 passed, 0 failed; class and method isolation verified. - Mandatory
ShellTestsregression scope: 69 passed, 0 failed; XHarness exit code 0.
Both commands ran on Android emulator API 30. No correction/retest was needed.
Failure Analysis and Self-Review
No failure or environment blocker occurred. Inline expert self-review recorded one minor finding: collection churn deliberately hides and retains existing bottom-tab infrastructure to satisfy the required reuse behavior; teardown remains limited to Shell-item switches and full disconnect. A pre-test review correction moved top-tab creation behind the active-section guard. Baseline restoration completed and the PR production files returned to their original state.
Candidate 2 — Edge-Triggered Tab-Mode Entry
Model: gpt-5.6-sol
Result: Fail — initial implementation/test pass plus the single permitted focused correction/retest.
Approach
Candidate 2 treats tab infrastructure as resources owned by discrete non-tabbed-to-tabbed mode transitions:
- Each handler tracks its previous visible-child count and allocates only on the first edge into a tabbed shape.
- Shrink/regrow within one item retains the infrastructure for reuse.
- Shell-item switches explicitly transition resource ownership.
- Appearance observers remain registered; the latest
ShellAppearanceis cached while controls are absent and applied directly after mode entry. - The top ViewPager callback is treated as navigation-lifecycle state independent from native top-tab chrome.
This avoids both the PR's idempotent setup calls and candidate 1's desired-state reconciliation and observer re-registration. Production diff: ShellItemHandler.Android.cs +104/-27; ShellSectionHandler.Android.cs +73/-30. The complete diff and narrative are persisted in:
CustomAgentLogsTmp/PRState/37321/PRAgent/try-fix-2/content.mdCustomAgentLogsTmp/PRState/37321/PRAgent/try-fix/attempt-2/fix.diff
Test Results
- Initial primary scope: 69 passed, 3 failed out of 72 executed.
- After the one focused correction: 70 passed, 2 failed out of 72 executed.
- Mandatory
ShellTestsregression scope: 69 passed, 0 failed in both passes.
The correction fixed appearance-tracker recreation for one switch scenario, but the primary command still failed:
SwitchingShellItemsCreatesBottomTabsOnlyWhenNeededobserved two tracker creations instead of one because mapper-driven mode entry occurred duringSetVirtualViewbefore the explicit Shell-item transition rebound the tracker.- The runner also executed inherited handler-variant coverage;
LifeCycleEventsFireWhenNavigatingTopTabscontinued to observeOnNavigatedTo == 0instead of 1.
Failure Analysis and Self-Review
The edge-trigger model does not coalesce mapper-driven and explicit Shell-item transitions, and moving ViewPager callback ownership outside tab chrome was insufficient to preserve top-tab navigation lifecycle delivery. A further redesign would exceed the one-correction bound. Inline expert self-review recorded the same two concrete issues as major findings. Baseline restoration completed, the production files returned to their original state, and unrelated working-tree changes were preserved.
🏁 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Winner: pr-plus-reviewer
pr-plus-reviewer preserves the submitted PR's validated lazy-materialization design while resolving both expert findings with a two-line consolidated patch. Its required focused scope passed 3/3, and its mandatory regression scope passed 69/69.
Candidate Comparison
| Rank | Candidate | Validation | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
Pass: 3/3 focused; 69/69 regression | Best risk/reward: retains the PR's established behavior and performance evidence, uses the actually presented page for deferred appearance replay, and closes the top-tab placement assertion gap. |
| 2 | pr |
Pass: trusted gate; submitted Android Shell coverage and CI green | The core design is sound and the expert review found no architectural blocker, but the submitted HEAD still has two moderate, directly actionable findings addressed by the winner. |
| 3 | try-fix-1 |
Pass: 3/3 focused; 69/69 regression | Its demand-reconciliation model and targeted observer re-registration are coherent, but it replaces substantially more production logic, lacks the submitted PR's broader CI/performance evidence, and provides no advantage over the winner sufficient to justify the larger change. |
| 4 | try-fix-2 |
Fail: focused scope remained 70/72; regression scope 69/69 | The edge-trigger model still duplicates bottom appearance-tracker creation during mapper-driven item transitions and misses top-tab navigation lifecycle delivery. Its required focused validation failure ranks it below every passing candidate. |
Why the Submitted PR Needs Changes
The raw PR remains very close to merge-ready, but it can replay top-tab appearance from a section root while a pushed page is displayed, and its new growth test does not prove that the top-tabs container became visible. Applying pr-plus-reviewer/reviewer.patch addresses both issues without adopting a broader alternative architecture.
📱 UI Tests — Shell
Detected UI test categories: Shell
✅ Deep UI tests — 326 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above).
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
Shell |
326/326 ✓ | — |
📎 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: The pr-plus-reviewer candidate preserves the submitted fix while addressing both expert findings with two focused edits. It passed all required validation: 3/3 focused tests and 69/69 mandatory regression tests.
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.
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
The handler-based Android Shell path eagerly created bottom and top tab managers, native tab controls, adapters, callbacks, and appearance trackers for the default single-page Shell shape even though no tabs were visible.
Description of Change
ShellItemactually shows tabs.ShellSectionhas multiple visible contents.ShellItems.Performance
Post-review retest on Pixel 5 using matched in-tree builds of merge-base
1aa158b771and final commitd6437ef8f9, Releaseandroid-arm64, Mono profiled AOT, and a mirrored 80-launch sequence:The absolute values differ from the earlier package-isolated run because this retest uses the current in-tree dependency graph and a minimal C# single-page Shell harness. The relative A/B comparison changes only the two production Shell handler files.
Earlier Preview 7 package-isolated measurements:
ActivityTaskManager: Displayed, 20 launches/buildam start -W TotalTime, 40 launches/buildThe first comparison isolated only
Microsoft.Maui.Controls.Corewhile keeping the remaining Preview 7 package graph, runtime, build tasks, and updated AOT profile identical. The follow-up used matched rebuilt control and patched APKs with mirrored launch ordering.Tests
Issues Fixed
Fixes #37282