fix(644): replace the count-bound navigation unregister loop with a key ledger

[drmoisan]

Suggested title

fix(644): replace the count-bound navigation unregister loop with a key ledger

Summary

• QfcCollectionController.UnregisterNavigation no longer derives which navigation keys to remove from the current _itemGroups.Count. It replays a ledger of the exact (SourceId, Key) pairs that RegisterNavigation actually added, then drains that ledger.
• This closes a silent orphaning defect: when a group was removed through a path that mutates _itemGroups without an unregister/register bracket, the count the unregister loop read no longer matched the count in force at registration, so the loop stopped short and left KbdActions registrations behind.
• The divergence was silent because every production call site discards the bool returned by KbdActions.Remove. It surfaced later as an ArgumentException or InvalidOperationException from an unrelated Add or Find.
• The field _registeredDigits introduced by the Bug: qfc-collection-navigation-digits-desync #472 fix is removed entirely; the ledger supersedes it.
• One production file changes (27 lines). A new 361-line test file adds six ledger tests, and three existing test files are amended where the ledger legitimately changes their expected outcome.
• AC-16 is a PARTIAL with accepted residual risk, not a pass. See "Risks and Mitigations".

Why

UnregisterNavigation and RegisterNavigation did not agree on the number of keys in play.

RegisterNavigation added one keyboard action per item group at the width in force at that moment. UnregisterNavigation then re-derived the removal set from a live read of _itemGroups.Count. RemoveSpecificControlGroup(int) mutates _itemGroups with no unregister/register bracket around the mutation, so any removal through that path left the two sides disagreeing, and every registration past the new, lower count was orphaned.

The unbracketed mutation is reachable from RemoveBelowThresholdAsync via the RemoveGroupByEntryId seam, and from the 'R' char action in QfcItemController.EventWiring.cs.

This is a distinct defect from #472, which was already fixed on this code. #472 concerned the format of the keys removed — a width-1 versus width-2 mismatch — and was fixed by recording the registration width in _registeredDigits and replaying it. #644 concerns the count of them. Recording a width is not sufficient, because a width cannot express "a group was removed from the middle of the set". Only recording the key set itself can.

The fix was deliberately kept out of #472's scope under the CLAUDE.md Bugfix Workflow rule that a deeper design problem uncovered mid-fix opens a new issue rather than widening the current one.

What Changed

Core fix — QuickFiler/Controllers/QfcCollectionController.cs (27 lines)

• Replaced the private int _registeredDigits field with private List<(string SourceId, string Key)> _registeredNavigationKeys, exposed through a lazily-initialising RegisteredNavigationKeys property.
• RegisterNavigationAsyncAction now reads SourceId and Key back off the constructed KaStringAsync instance and appends them to the ledger strictly after a successful Add. Ordering is deliberate: a duplicate-key ArgumentException from Add therefore leaves the ledger clean rather than recording a key that was never registered.
• UnregisterNavigation iterates the ledger, removes each recorded pair verbatim, and clears the ledger. It no longer reads _itemGroups at all.
• RegisterNavigation no longer assigns _registeredDigits.

Tests

• New: QuickFiler.Test/Controllers/QfcCollectionControllerNavigationLedgerTests.cs (361 lines) — six tests covering the RemoveBelowThresholdAsync path, the synchronous RemoveSpecificControlGroup(int) path, the width-crossing case (ten groups at width 2 shrinking to nine), repeated register/unregister state transitions, the empty-ledger negative case, and the proof that UnregisterNavigation completes with _itemGroups set to null.
• Amended: QfcCollectionControllerTests.cs, QfcCollectionControllerDefects468Tests.cs, and QfcCollectionControllerNavigationDigitsTests.cs, where the ledger changes the outcome the existing characterisation tests pinned. QfcCollectionControllerTests.cs sits at the 500-line ceiling and its [TestMethod] count is frozen by issue Bug: qfc-collection-controller-unreachable-load-paths #468; both constraints are preserved.
• Build: one <Compile Include> line added to QuickFiler.Test/QuickFiler.Test.csproj for the new file.

Documentation and evidence

All planning, evidence and audit artifacts are under docs/features/active/2026-08-27-qfc-unregister-navigation-count-mismatch-orphan-644/, including the atomic plan, two remediation cycles, and three timestamped audit sets.

Architecture / How It Fits Together

KbdActions.StringActionsAsync is a keyed registry. QfcCollectionController is the only component that registers navigation entries into it for the QuickFiler item collection.

Before: registration wrote N entries keyed by position; unregistration recomputed N from the live collection and removed positions 1..N. The registry and the collection were coupled through a number that either side could change independently.

After: registration is the sole author of the ledger, and unregistration is a pure replay of it. The registry is now coupled to the ledger, which only registration writes, so a mutation of _itemGroups between the two operations cannot desynchronise them. UnregisterNavigation has no remaining read of _itemGroups, which is asserted directly by a test that sets that field to null.

Verification

Completed

Independently re-run at the branch tip in a freshly bootstrapped worktree, in the mandated order, in one clean pass:

Gate	Command	Result
Format	dotnet tool run csharpier check .	exit 0, Checked 1562 files, none unformatted
Analyzers	msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true	exit 0
Nullable	msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true	exit 0
Tests	vstest.console.exe QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /InIsolation /TestCaseFilter:"TestCategory!=LiveOutlook"	exit 0, 1254 of 1254 passing

/t:Rebuild is used rather than /t:Build so CoreCompile runs on every project and the analyzer gate is not vacuous.

Three review passes were run against this branch, plus two remediation cycles. The closing pass returns 0 blocking findings and a GO verdict. Evidence is at docs/features/active/2026-08-27-qfc-unregister-navigation-count-mismatch-orphan-644/evidence/other/resume-toolchain-verification.2026-08-30T13-20.md.

Recommended

• Exercise the 'R' char action and RemoveBelowThresholdAsync against a live Outlook session, since the TestCategory!=LiveOutlook filter excludes host-bound tests from the local gate.

Backward Compatibility / Migration Notes

• No public API changes. RegisterNavigation and UnregisterNavigation keep their signatures.
• _registeredDigits was a private field; its removal is not observable by any caller.
• Behaviour does change for callers that previously relied on the buggy outcome: after a group is removed through an unbracketed path, UnregisterNavigation now removes every registered key rather than a truncated prefix. Three existing characterisation tests encoded the old outcome and are amended accordingly — that amendment is the visible cost of the fix, and is itself the reason this work was split out of Bug: qfc-collection-navigation-digits-desync #472.

Risks and Mitigations

AC-16 (no coverage regression on changed lines) is a PARTIAL and is deliberately left unchecked. It is not satisfied and is not presented as satisfied.

The acceptance clause demands a >= comparison of a repository-wide coverage percentage at two-decimal resolution. That comparison is not decidable at that resolution by this instrument. Two runs over a byte-identical tree, with no intervening change of any kind, produced 54793 and 54811 covered lines against an identical 64221-line denominator — 85.3194% and 85.3475%, straddling the 85.3303% baseline. The instrument's root-level noise is therefore roughly 0.03 percentage points, about three times the 0.01-point shortfall the gate reported, so the observed delta carries no information about whether a regression occurred.

The disposition explicitly does not rest on selecting the favourable run. Both runs are recorded; the basis is the measured indecidability of the clause.

• Accepted residual risk: a real coverage regression smaller than the ~0.03-point noise floor would be indistinguishable from noise by this gate, and this PR does not exclude one.
• Bounding fact 1: the only production file this change touches, QuickFiler/Controllers/QfcCollectionController.cs, carries [ExcludeFromCodeCoverage], and appears in 0 of the 558 class entries of the post-processed coverage document, so it sits in neither the numerator nor the denominator and cannot move the figure mechanically. That attribute was verified to be pre-existing at the branch's base commit and untouched by this change, so the argument is not circular.
• Bounding fact 2: lines-valid is identical across all three runs, confirming no production file changed instrumented size.
• Bounding fact 3: the whole-repository suite reports no test regressed.

Other risks:

• Ledger and registry could drift if a future caller registers navigation keys outside RegisterNavigationAsyncAction. Mitigated by that method being the single write path, and by the record-after-successful-Add ordering.
• Memory growth if UnregisterNavigation is never called. Mitigated by Clear() on every unregister and by the state-transition test that asserts repeated cycles leave the registry empty.

Review Guide

1. QuickFiler/Controllers/QfcCollectionController.cs — the whole fix, 27 lines across three hunks. Read UnregisterNavigation first, then the record-after-Add ordering in RegisterNavigationAsyncAction.
2. QuickFiler.Test/Controllers/QfcCollectionControllerNavigationLedgerTests.cs — the new coverage.
3. The three amended test files — confirm each amendment reflects a genuine behaviour change rather than a weakened assertion.
4. The feature folder — audit trail only; no review needed for correctness.

The feature folder is the bulk of the diff by line count and is entirely documentation.

Follow-ups

• The repository-wide >= coverage comparison misfires on any plan that adopts it, because it demands a decision at 0.01 points from an instrument with a ~0.03-point noise floor. This design defect should be tracked as its own issue rather than left as prose in a feature folder.
• The analyzer HintPath version skew in UtilitiesCS.csproj and VBFunctions.csproj is pre-existing, byte-identical at the merge base, and untouched here. Reported separately for promotion.

GitHub Auto-close

• Closes Bug: qfc-unregister-navigation-count-mismatch-orphan #644