fix(breadcrumb): make suggestions upgrade rebuild atomic to prevent SelectRow race (#398)

[drmoisan]

Suggested title

fix(breadcrumb): make suggestions upgrade rebuild atomic to prevent SelectRow race (#398)

Summary

• Fixes an intermittent ArgumentOutOfRangeException ("Row selection requires -1 or an index in [0, 0]", actual value 1) thrown from BreadcrumbStateModel.SelectRow during QuickFiler high-confidence loads with two or more folder suggestions.
• Root cause: FolderBreadcrumbBridgeRouter.SetSuggestionsAsync cleared the breadcrumb model synchronously before its first await, then re-added rows one at a time on thread-pool continuations. The unawaited UpgradeSuggestionsAsync started by BreadcrumbBridgeCoordinator.SetSuggestions therefore raced the host's subsequent SetFolderSelectedIndex(1) call against a transiently cleared/partial model.
• Fix: resolve all upgraded rows into a local collection with no model mutation while awaiting, then publish them through a new BreadcrumbStateModel.ReplaceRows seam that swaps the backing rows in a single synchronous operation and reconciles the selection before publishing. No empty window is ever observable.
• Adds deterministic MSTest regression coverage: a coordinator-level regression test (fails pre-fix with the exact reported exception signature) and router-level in-flight invariant tests, all gated by TaskCompletionSource fakes — no sleeps, no wall-clock waits, no temp files.
• Distinct from issue Bug: folder-combobox-fallback-index-out-of-range #392 / PR fix(quickfiler): make folder-suggestion fallback selection bounds-safe (#392) #393 (the deterministic single-suggestion index clamp); this defect required FolderArray.Length > 1 plus an asynchronously-resolving hierarchy provider.

Why

QuickFiler's breadcrumb pipeline populates suggestion rows synchronously, then upgrades them to full ancestor-chain rows via a fire-and-forget async rebuild. The rebuild's up-front Clear() violated the documented readback contract ("rows are populated immediately... so the selection contract holds without awaiting the provider"), aborting the entire QuickFiler load whenever the host selected a row before the rebuild finished. The crash was intermittent because a synchronously-resolving (cached) provider leaves no race window.

What Changed

Core fix (production, 2 files):

• UtilitiesCS/OutlookObjects/Folder/FolderBreadcrumbBridgeRouter.cs — SetSuggestionsAsync now builds rows into a local list (no _model mutation while awaiting) and applies them atomically at the end.
• UtilitiesCS/OutlookObjects/Folder/BreadcrumbStateModel.cs — new ReplaceRows operation: single synchronous backing-list swap that preserves a still-valid selection index.

Tests:

• QuickFiler.Test/Viewers/BreadcrumbBridgeCoordinatorTests.cs — regression test SelectRow_WhileSuggestionsUpgradeInFlight_DoesNotThrowAndAppliesSelection (failed pre-fix with the exact reported exception).
• UtilitiesCS.Test/OutlookObjects/Folder/FolderBreadcrumbBridgeRouterTests.cs + new FolderBreadcrumbBridgeRouterInFlightTests.cs — in-flight invariants: row count never drops below the pre-upgrade count; FolderContains/GetSelectedFolder/SelectRow stay consistent mid-upgrade; selection survives the swap.
• UtilitiesCS.Test/OutlookObjects/Folder/BreadcrumbStateModelTests.cs + new BreadcrumbStateModelSequenceTests.cs — ReplaceRows seam coverage; files split as partial classes to satisfy the 500-line file limit (all four now 235–320 lines).
• UtilitiesCS.Test/UtilitiesCS.Test.csproj — explicit <Compile Include> items for the two new test files.

Docs/evidence:

• Feature folder docs/features/active/2026-07-20-breadcrumb-suggestions-upgrade-race-398/ with issue, plan, remediation plan, policy/code/feature audit artifacts (initial review + R4 re-audit PASS), and Phase 0 baseline / QA-gate / regression evidence.

Architecture / How It Fits Together

QfcItemController.AssignFolderComboBox → ItemViewer.SetFolderSuggestions → BreadcrumbBridgeCoordinator.SetSuggestions (immediate plain rows + unawaited upgrade) → FolderBreadcrumbBridgeRouter.SetSuggestionsAsync (now: local build + atomic ReplaceRows publish) → BreadcrumbStateModel (state) → render JSON posted to the WebView2 page. Host selection calls (SetFolderSelectedIndex → SelectRow) can now interleave with the upgrade at any point without observing an empty or partial model.

Verification

Completed (evidence in the feature folder):

• Regression fail-before: exact ArgumentOutOfRangeException signature reproduced pre-fix (EXIT 1), pass-after post-fix (EXIT 0).
• Full C# toolchain green in final pass: csharpier check . (EXIT 0), analyzer build (EXIT 0), nullable build with TreatWarningsAsErrors (EXIT 0), vstest.console.exe /EnableCodeCoverage — 5061/5061 tests passing (5054 baseline + 7 new).
• Coverage (first-party UtilitiesCS + QuickFiler denominator): line 86.54% (>= 85% floor), branch 80.85% (>= 75% floor); new/changed-code line coverage 100% (>= 90% target); no regression on changed lines.
• All five acceptance criteria in issue.md verified and checked off; R4 re-audit verdict PASS with zero blocking findings.

Recommended:

• PR CI pipeline (format, build, analyze, test) against this branch head.
• Manual: run ribbon "QuickFiler High Confidence" repeatedly against multi-suggestion items with a live (asynchronous) hierarchy provider; the exception must not recur.

Backward Compatibility / Migration Notes

• No public API removed or renamed. BreadcrumbStateModel.ReplaceRows is additive. Completed-upgrade semantics are byte-identical (suggestion rows with chains/probabilities; unresolvable scored rows fall back to plain rows; non-scored rows verbatim).
• Test-only restructuring: two test classes became partial classes across four files; no test was removed or weakened.

Risks and Mitigations

• Risk: selection reconciliation in ReplaceRows could alter selection behavior for callers other than the upgrade path. Mitigation: dedicated seam tests plus the full 5061-test suite passing unchanged; completed-upgrade output verified identical.
• Risk: the model is still written from a thread-pool continuation while the UI thread reads (plain reference swap, no memory barrier). Mitigation: the swap is a single reference publish reconciled before visibility; readers can never observe a torn/empty list. Documenting the memory-model assumption is noted as a non-blocking follow-up.
• Rollback: revert the two production-file changes; tests that encode the in-flight invariants will fail again, flagging the regression.

Review Guide

1. UtilitiesCS/OutlookObjects/Folder/FolderBreadcrumbBridgeRouter.cs — the local-build + atomic-publish rewrite of SetSuggestionsAsync (the fix).
2. UtilitiesCS/OutlookObjects/Folder/BreadcrumbStateModel.cs — ReplaceRows seam and selection reconciliation.
3. QuickFiler.Test/Viewers/BreadcrumbBridgeCoordinatorTests.cs — the end-to-end regression test.
4. Router/model test files — in-flight invariant and seam tests (the partial-class splits are mechanical; content moved, not changed).
5. Feature-folder docs/evidence — audit trail; low review priority.

Follow-ups

• Pre-existing duplicate <Compile Include> for PercentageFormatterTests.cs in UtilitiesCS.Test.csproj (CS2002 warning on rebuild) — pre-existing at base, out of scope here, already tracked as a promoted potential entry.
• Non-blocking: document the memory-model assumption on the _rows reference swap.

GitHub Auto-close

• Closes Bug: breadcrumb-suggestions-upgrade-race #398