fix(#232): repair QuickFiler navigation-key collision on page swap

[drmoisan]

Fix QuickFiler navigation-key collision on page swap (#232)

Summary

• Fixes System.ArgumentException: "Cannot add key because it already exists. Key 2 SourceId Collection" that crashed active QuickFiler sessions during ordinary OK/Skip/pop-out page transitions.
• Root cause: the whole-page swap path did not maintain the keyboard digit-navigation registry, orphaning "Collection"-sourced keys that later collided on the next RegisterNavigation().
• Adds additive folder-confidence probability debug logging (item summary, score, caller context) at the three scoring call sites, to make a separately-reported "subset of items in high-confidence mode" symptom empirically diagnosable.
• Adds four MSTest regression tests for the swap register/unregister ordering and the double-registration guard.
• Scoped deliberately to the crash fix plus logging; the larger dequeue-time high-confidence filtering redesign was split out to a separate feature (issue Feature: quickfiler-high-confidence-dequeue-streaming #233).

Why

In high-confidence mode, popping out the last item on a page triggers an internal Skip that swaps in the next cached page. The swap path QfcCollectionController.LoadControlsAndHandlers_01(TableLayoutPanel, List<QfcItemGroup>) replaced the active _itemGroups without calling UnregisterNavigation() for the outgoing page or RegisterNavigation() for the incoming page. Because _kbdHandler.StringActionsAsync is a single session-lifetime, purely additive/subtractive collection shared across every page, any page abandoned via OK/Skip while it still held items left its "Collection" digit keys ("1".."N") permanently orphaned. The next unconditional RegisterNavigation() (at the end of RemoveSpecificControlGroupAsync) then collided on the first still-occupied slot — reported as "Key 2".

This is a deterministic sequencing/bookkeeping gap, reproducible under purely sequential execution; the earlier "overlapping-removal race" hypothesis was investigated and ruled out as the primary cause. The existing (previously dead) SwapItemGroups method already encodes the correct UnregisterNavigation(); ...; RegisterNavigation(); pattern.

What Changed

Core fix (QuickFiler/Controllers/QfcCollectionController.cs):

• Route the page-swap in LoadControlsAndHandlers_01(TableLayoutPanel, List<QfcItemGroup>) through the existing SwapItemGroups, restoring the UnregisterNavigation()/RegisterNavigation() pairing so the live registry always matches the displayed page.
• Add a double-registration guard in RemoveSpecificControlGroupAsync so the trailing unconditional RegisterNavigation() is skipped when the zero-item branch already registered the swapped-in page, preventing a re-introduced collision.

Additive probability debug logging (no control-flow change):

• QuickFiler/Controllers/QfcDatamodel.cs — one logger.Debug(...) at the master-queue admission scoring point (ScoreRemainingQueueMailItemAsync).
• QuickFiler/Controllers/QfcItemController.FolderHandling.cs — logging at the four display-time scoring assignment points across LoadFolderHandler/LoadFolderHandlerAsync, with FromField/FromArrayOrString caller context.
• QuickFiler/Controllers/QfcHighConfidencePreFilter.cs — new logger field plus one logger.Debug(...) in the FilterAsync scoring lambda.

Tests:

• QuickFiler.Test/Controllers/QfcCollectionControllerTests.cs — reported-repro regression (throws before fix / passes after), swap register/unregister ordering, and double-registration-guard tests.

Architecture / How It Fits Together

Keyboard digit navigation is registered against _kbdHandler.StringActionsAsync (KbdActions<string, KaStringAsync, Func<string,Task>>), a single collection owned by the session's home controller and reused across pages. Page transitions (OK, Skip, single-item pop-out) all converge on the swap path; routing that path through SwapItemGroups makes the registry a strict function of the currently-displayed _itemGroups. The guard in RemoveSpecificControlGroupAsync ensures exactly one registration per incoming key when the drain-to-zero branch swaps a new page in.

Verification

Completed on the branch (evidence under docs/features/active/2026-07-03-quickfiler-navigation-key-collision-232/evidence/):

• CSharpier: clean, no files reformatted.
• .NET analyzers build: 0 errors, no new diagnostics vs baseline.
• Nullable / TreatWarningsAsErrors build: zero new diagnostics (verified by controlled recompile comparison).
• MSTest: 4641 tests, 4640 pass (the single failure is a pre-existing flaky filesystem-adapter test, at the baseline failure count of 1); +4 new regression tests pass.
• Coverage verified from a persisted machine-readable Cobertura artifact: QfcHighConfidencePreFilter.cs changed lines 100% (>= 90% target); repo-wide 76.575% with no regression versus baseline. Repo-wide raw coverage remains under the generic 80% floor but is governed by the ratified COM/VSTO/WinForms testable-denominator exemption (CLAUDE.md; issue Refactor: qfc-item-controller-testability #227) and is not worsened by this change.
• Feature review (2026-07-03T17-25): Policy COMPLIANT, Code Go, Feature READY; AC1–AC10 all PASS; 0 blocking findings after one remediation cycle.

Recommended before merge:

• Manual QuickFiler high-confidence-mode run exercising OK, Skip, and single-item pop-out transitions across multiple pages; confirm no ArgumentException and that StringActionsAsync accumulates no stale entries across a full session.

Backward Compatibility / Migration Notes

• No public API signatures changed. The swap method's public surface is unchanged; the fix is internal routing plus a method-local guard.
• The new logging is additive logger.Debug(...) only, gated by the log4net root level (debug builds), with no behavioral effect.

Risks and Mitigations

• Risk: the double-registration guard could skip a needed registration. Mitigation: covered by the swap-order and guard regression tests asserting exactly one "Collection" entry per incoming key and no ArgumentException.
• Risk: QfcCollectionController is COM/WinForms-bound and coverage-exempt, so the fix is verified through the IQfcCollectionController/reflection-injection test seams rather than a live Outlook host. Mitigation: tests assert the register/unregister interaction directly at that seam; manual verification is recommended above.

Review Guide

1. QuickFiler/Controllers/QfcCollectionController.cs — the swap routing and the RemoveSpecificControlGroupAsync guard (the actual fix).
2. QuickFiler.Test/Controllers/QfcCollectionControllerTests.cs — the four regression tests.
3. The three logging files (mechanical, additive logger.Debug lines).
4. Feature folder audits and coverage evidence for the review trail.

Follow-ups

• Issue Feature: quickfiler-high-confidence-dequeue-streaming #233 — dequeue-time high-confidence filtering/streaming redesign (move filtering to the Queue only, apply at dequeue with streaming backfill, no removal after surfacing). Intentionally out of scope here.
• Recorded candidates (see evidence/other/follow-up-candidates.md): the fixed-batch-without-backfill pattern, the dormant issue Bug: quickfiler-high-confidence-prefilter #171 pre-filter pipeline, and the removespecificcontrolgroupcounter reentrancy-counter hygiene.

GitHub Auto-close

• Closes Bug: quickfiler-navigation-key-collision #232