fix(quickfiler): correct keyboard-action registration desync in KbdActions, collection navigation and item expansion (#444, #472, #482)

[drmoisan]

Summary

Fixes three keyboard-action registration defects in QuickFiler. All three share one root cause: a registration and an unregistration disagreeing about what was registered, with the disagreement swallowed because every production call site discards the bool that KbdActions.Remove returns.

Issue	Defect	Fix
#444	KbdActions(IEnumerable<UClass>) bypassed the duplicate-registration guard that Add enforces, so a seed sequence containing two StoredKeyEquals-equal keys constructed silently and threw later.	The enumerable constructor now applies the same guard, logs via the existing logger.Error, and throws ArgumentException.
#472	UnregisterNavigation re-read the live Digits width instead of the width in force at registration, so growing or shrinking the item count between register and unregister removed keys the page never registered and left the real ones behind.	QfcCollectionController records the registration width in a new private int _registeredDigits field and UnregisterNavigation replays that recorded width, with zero reads of the Digits property.
#482	ToggleExpansion and ToggleExpansionAsync each maintained a different keyboard registry, so a synchronous toggle cleared CharActions (where nothing had been added) while CharActionsAsync kept its entries, and the next expansion threw ArgumentException from Add.	Both overloads now delegate expansion registration to a single owner, private void SyncExpandedRegistrations(bool expanded), which maintains both registries together.

Base branch is epic/quickfiler-bug-family-integration (epic quickfiler-bug-family), not main.

Deliberate behaviour widening

The #482 fix routes all expansion keyboard registration through SyncExpandedRegistrations(bool expanded), which maintains both the synchronous _kbdHandler.CharActions registry and the asynchronous _kbdHandler.CharActionsAsync registry together rather than one per toggle path. That is a deliberate widening of observable behaviour, not an incidental side effect:

• 'B' and 'D' now respond after a synchronous expansion. Previously only an asynchronous expansion populated CharActionsAsync, and the Alt-key path that reads CharActions was left empty after a synchronous toggle.
• Alt+B and Alt+D now respond after an asynchronous expansion. Previously only a synchronous expansion populated CharActions.

The alternative — collapsing onto a single registry — was considered and rejected. Four focus-path methods in QfcItemController.EventWiring.cs conditionally call the expansion register and unregister methods on _expanded. Under a single-registry unification one of those cleanup paths would remove from the registry that no longer holds the entries, re-creating exactly the silent-false divergence these three issues describe. Maintaining both registries makes every one of those four call sites operate on a registry that genuinely holds the entries.

Idempotence is preserved because the two unregister calls are unconditional and KbdActions.Remove returns false rather than throwing when the pair is absent. That is why repeated and interleaved toggles no longer raise ArgumentException from KbdActions.Add, without any change to Add's contract.

Correction to #482's filed trigger and severity

The filed issue's stated trigger is unreachable and its stated severity is overstated. Both corrections are recorded in spec.md under ### #482 — expansion registry divergence and are repeated here so this PR does not restate an unsupported claim.

The filed trigger is dead code with respect to this interleaving. The promoted document names the synchronous ToggleExpansion() call inside ActivateBySelectionAsync as what makes the interleaving reachable in production rather than theoretical. That call is guarded by if (blExpanded), and both asynchronous callers pass a value that is always false: one passes the literal false, and the other passes a value returned from ToggleOffActiveItemAsync, whose expansion branch is commented out so it returns its parameter unchanged. The guarded call therefore never executes with a true argument, and the filed trigger cannot produce the interleaving.

The live trigger is Right, then Down, then Right:

1. Right on an item runs ToggleExpansionAsync(On), setting _expanded = true and adding 'B' and 'D' to CharActionsAsync.
2. Down runs SelectNextItemAsync, which marshals to the synchronous SelectNextItem, and through ChangeByIndex and ToggleOffActiveItem reaches the synchronous ToggleExpansion(). That clears _expanded and removes from CharActions, where nothing was ever added, so Remove returns false silently. CharActionsAsync still holds 'B' and 'D'.
3. Right on the same item again finds _expanded == false, so ToggleExpansionAsync(On) runs and CharActionsAsync.Add is called for an entry that is already present, raising ArgumentException.

The severity is a dead key, not a crash. The exception surfaces through the asynchronous keyboard handler in KeyboardHandler.cs, whose catch block logs it. The user-visible symptom is a 'B' or 'D' key that stops responding for that item — not an unhandled exception and not a crash.

Known follow-up: issue #644

spec.md ### Downstream notes item 3 describes a second, distinct defect in UnregisterNavigation, which this PR deliberately leaves in place: the method bounds its unregister loop with the current _itemGroups.Count, while RemoveSpecificControlGroup(int) mutates _itemGroups with no unregister/register bracket. When a group is removed through that unbracketed path — reachable from RemoveBelowThresholdAsync via the RemoveGroupByEntryId seam, and from the 'R' char action — the count the unregister loop later reads no longer matches the count in force at registration, so the loop stops short and leaves orphaned navigation registrations behind.

That defect is tracked as issue #644 (qfc-unregister-navigation-count-mismatch-orphan, #644), promoted through the feature-promotion lifecycle in commit 12256da4 with potential entry docs/features/potential/promoted/2026-08-27-qfc-unregister-navigation-count-mismatch-orphan.md.

It is out of scope here under CLAUDE.md's Bugfix Workflow step 2 ("If you uncover deeper design problems, open a new issue instead of widening scope"): the fix requires a key-ledger redesign that breaks characterization tests in QfcCollectionControllerTests.cs, a file sitting at exactly 500 lines with a [TestMethod] count frozen by upstream #468. This PR's #472 regression test asserts the residual orphan explicitly — exactly one "10" entry remains — and carries an XML doc comment attributing that residual to issue #644, so the assertion does not silently absorb the second defect.

Changes

Three production files, four test files, one project-file line.

• QuickFiler/Controllers/KbdActions.cs (+36)
• QuickFiler/Controllers/QfcCollectionController.cs (+8 −8)
• QuickFiler/Controllers/QfcItemController.Navigation.cs (+28 −4)
• QuickFiler.Test/Controllers/QfcCollectionControllerNavigationDigitsTests.cs (new, +226)
• QuickFiler.Test/Controllers/QfcItemController.NavigationTests.cs (+107)
• QuickFiler.Test/Controllers/KbdActionsRemainingBranchesTests.cs (+91)
• QuickFiler.Test/Controllers/KbdActionsTests.cs (+37)
• QuickFiler.Test/QuickFiler.Test.csproj (+1, one <Compile Include> in the Controllers\Qfc* slot)

No public API was added, removed, or re-signed; the two added members are both private. QuickFiler/QuickFiler.csproj is untouched. KeyboardHandler.cs, IQfcCollectionController.cs and the nine other QfcItemController partials are unmodified.

Verification

Full C# toolchain, re-run in the mandated order after merging integration tip 13a22ade. All four gates pass in a single loop; no gate rewrote a file, so no restart was triggered.

Gate	Command	Result
Format	dotnet tool run csharpier check .	exit 0 — 1543 files, 0 unformatted
Analyzers	msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true	exit 0 — 0 errors, 5 warnings
Nullable	msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true	exit 0 — 0 errors
Tests	pwsh -NoProfile -File ./scripts/vscode/Invoke-MSTestWithCoverage.ps1	exit 0 — 6719 / 6719 passed

/t:Rebuild was used for both msbuild gates. Non-vacuity is proven by a zero count of Skipping target "CoreCompile" together with 15 and 11 executed CoreCompile: targets respectively — not by a csc-invocation count, which reads 0 at this verbosity even on a real compile. /p:Nullable=enable was not added to any command.

All five analyzer warnings are the pre-existing System.Reactive 7.0.0 packages.config diagnostic, one per consuming project, identical to the Phase 0 baseline.

Coverage

Unfiltered whole-run denominator, the same wrapper and denominator as the Phase 0 baseline.

Figure	Baseline	Final
Repository-wide line coverage	85.04%	85.1326%
Repository-wide branch coverage	79.12%	79.2162%
KbdActions.cs line-rate	0.93976	0.98980
QfcItemController.Navigation.cs line-rate	0.90678	0.92126
SyncExpandedRegistrations (new member)	n/a	100%

Both figures moved up and clear both readings of the repository's two coverage policies — the CLAUDE.md §UT2 floor of >= 80% line, and the .claude/rules/general-unit-test.md / quality-tiers.md floors of >= 85% line and >= 75% branch. No interpretation was needed to declare a pass. That two-policy conflict is pre-existing and is recorded as unresolved rather than silently decided.

Regression tests

Each of the three fixes has a fail-before / pass-after record under evidence/regression-testing/ and evidence/qa-gates/. The guard's throw path and its normal-completion path are both covered.

Acceptance criteria

spec.md is the sole acceptance-criteria source (work mode full-bug; user-story.md is intentionally absent). 57 of 57 criteria are checked. Feature review returned 0 Blocking findings across policy-audit, code-review and feature-audit.

Notes for the epic capstone

• The Closes references below will not fire on this base. GitHub registers closing references only for pull requests targeting the default branch, and this PR targets epic/quickfiler-bug-family-integration. Issues Bug: kbdactions-enumerable-ctor-bypasses-duplicate-guard #444, Bug: qfc-collection-navigation-digits-desync #472 and Bug: qfc-item-controller-expansion-registry-divergence #482 are all still OPEN and must be closed by the epic's integration-to-main PR or by hand. Nothing here should be read as asserting they are closed.
• Two features are barrier-blocked on this merge: Bug: efc-controllers-null-guard-and-async-void-boundary-defects #464 and Bug: itemviewer-ui-thread-marshalling-divergence #489, with Bug: itemviewer-breadcrumb-pipeline-lifecycle #488 waiting behind Bug: itemviewer-ui-thread-marshalling-divergence #489.
• Project-file ownership regions overlap. This feature was assigned Controllers\Qfc*, but Bug: uithread-dispatcher-static-swap-no-restore #493's two added includes (Controllers\QfcItemController.UiThreadDispatcherFixture*.cs) also match that pattern. No conflict arose and nothing was lost, but the declared regions are not disjoint and would benefit from being narrowed to per-file lists in future waves.
• QfcCollectionController.cs is 2437 lines, far over the 500-line cap. It is pre-existing at the base and unchanged in size by this branch (+8 −8); remediation is forbidden to this feature by the epic's ownership rules.

Closes #444
Closes #472
Closes #482

🤖 Generated with Claude Code

https://claude.ai/code/session_01T45irz1DRk6bPnxx5ifqFA